Fix PFC watchdog not getting lossless TC (sonic-net#876)

wendani · lguohan · commit 9f9a069d87af · 2019-05-20T07:34:42.000-07:00
* Allow PFC watchdog to retry start on port

Signed-off-by: Wenda Ni &lt;wenni@microsoft.com&gt;

* Specify the qos mapping order in doTask() to avoid retry

Signed-off-by: Wenda Ni &lt;wenni@microsoft.com&gt;

* Remove debugging symbols

Signed-off-by: Wenda Ni &lt;wenni@microsoft.com&gt;

* Reduce log level to NOTICE for empty lossless TC on a port

Signed-off-by: Wenda Ni &lt;wenni@microsoft.com&gt;
diff --git a/orchagent/pfcwdorch.cpp b/orchagent/pfcwdorch.cpp
@@ -68,20 +68,38 @@ void PfcWdOrch<DropHandler, ForwardHandler>::doTask(Consumer& consumer)
             string key = kfvKey(t);
             string op = kfvOp(t);
 
+            task_process_status task_status = task_process_status::task_ignore;
             if (op == SET_COMMAND)
             {
-                createEntry(key, kfvFieldsValues(t));
+                task_status = createEntry(key, kfvFieldsValues(t));
             }
             else if (op == DEL_COMMAND)
             {
-                deleteEntry(key);
+                task_status = deleteEntry(key);
             }
             else
             {
+                task_status = task_process_status::task_invalid_entry;
                 SWSS_LOG_ERROR("Unknown operation type %s\n", op.c_str());
             }
-
-            consumer.m_toSync.erase(it++);
+            switch (task_status)
+            {
+                case task_process_status::task_success:
+                    consumer.m_toSync.erase(it++);
+                    break;
+                case task_process_status::task_need_retry:
+                    SWSS_LOG_INFO("Failed to process PFC watchdog %s task, retry it", op.c_str());
+                    ++it;
+                    break;
+                case task_process_status::task_invalid_entry:
+                    SWSS_LOG_ERROR("Failed to process PFC watchdog %s task, invalid entry", op.c_str());
+                    consumer.m_toSync.erase(it++);
+                    break;
+                default:
+                    SWSS_LOG_ERROR("Invalid task status %d", task_status);
+                    consumer.m_toSync.erase(it++);
+                    break;
+            }
         }
 
         if (consumer.m_toSync.empty())
@@ -156,7 +174,7 @@ string PfcWdOrch<DropHandler, ForwardHandler>::serializeAction(const PfcWdAction
 
 
 template <typename DropHandler, typename ForwardHandler>
-void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,
+task_process_status PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,
         const vector<FieldValueTuple>& data)
 {
     SWSS_LOG_ENTER();
@@ -170,13 +188,13 @@ void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,
     if (!gPortsOrch->getPort(key, port))
     {
         SWSS_LOG_ERROR("Invalid port interface %s", key.c_str());
-        return;
+        return task_process_status::task_invalid_entry;
     }
 
     if (port.m_type != Port::PHY)
     {
         SWSS_LOG_ERROR("Interface %s is not physical port", key.c_str());
-        return;
+        return task_process_status::task_invalid_entry;
     }
 
     for (auto i : data)
@@ -205,7 +223,7 @@ void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,
                 if (action == PfcWdAction::PFC_WD_ACTION_UNKNOWN)
                 {
                     SWSS_LOG_ERROR("Invalid PFC Watchdog action %s", value.c_str());
-                    return;
+                    return task_process_status::task_invalid_entry;
                 }
             }
             else
@@ -214,7 +232,7 @@ void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,
                         "Failed to parse PFC Watchdog %s configuration. Unknown attribute %s.\n",
                         key.c_str(),
                         field.c_str());
-                return;
+                return task_process_status::task_invalid_entry;
             }
         }
         catch (const exception& e)
@@ -224,36 +242,37 @@ void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,
                     key.c_str(),
                     field.c_str(),
                     e.what());
-            return;
+            return task_process_status::task_invalid_entry;
         }
         catch (...)
         {
             SWSS_LOG_ERROR(
                     "Failed to parse PFC Watchdog %s attribute %s. Unknown error has been occurred",
                     key.c_str(),
                     field.c_str());
-            return;
+            return task_process_status::task_invalid_entry;
         }
     }
 
     // Validation
     if (detectionTime == 0)
     {
         SWSS_LOG_ERROR("%s missing", PFC_WD_DETECTION_TIME);
-        return;
+        return task_process_status::task_invalid_entry;
     }
 
     if (!startWdOnPort(port, detectionTime, restorationTime, action))
     {
         SWSS_LOG_ERROR("Failed to start PFC Watchdog on port %s", port.m_alias.c_str());
-        return;
+        return task_process_status::task_need_retry;
     }
 
     SWSS_LOG_NOTICE("Started PFC Watchdog on port %s", port.m_alias.c_str());
+    return task_process_status::task_success;
 }
 
 template <typename DropHandler, typename ForwardHandler>
-void PfcWdOrch<DropHandler, ForwardHandler>::deleteEntry(const string& name)
+task_process_status PfcWdOrch<DropHandler, ForwardHandler>::deleteEntry(const string& name)
 {
     SWSS_LOG_ENTER();
 
@@ -263,14 +282,15 @@ void PfcWdOrch<DropHandler, ForwardHandler>::deleteEntry(const string& name)
     if (!stopWdOnPort(port))
     {
         SWSS_LOG_ERROR("Failed to stop PFC Watchdog on port %s", name.c_str());
-        return;
+        return task_process_status::task_failed;
     }
 
     SWSS_LOG_NOTICE("Stopped PFC Watchdog on port %s", name.c_str());
+    return task_process_status::task_success;
 }
 
 template <typename DropHandler, typename ForwardHandler>
-void PfcWdSwOrch<DropHandler, ForwardHandler>::createEntry(const string& key,
+task_process_status PfcWdSwOrch<DropHandler, ForwardHandler>::createEntry(const string& key,
         const vector<FieldValueTuple>& data)
 {
     SWSS_LOG_ENTER();
@@ -297,8 +317,10 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::createEntry(const string& key,
     }
     else
     {
-        PfcWdOrch<DropHandler, ForwardHandler>::createEntry(key, data);
+        return PfcWdOrch<DropHandler, ForwardHandler>::createEntry(key, data);
     }
+
+    return task_process_status::task_success;
 }
 
 template <typename DropHandler, typename ForwardHandler>
@@ -457,7 +479,7 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::enableBigRedSwitchMode()
 }
 
 template <typename DropHandler, typename ForwardHandler>
-void PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,
+bool PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,
         uint32_t detectionTime, uint32_t restorationTime, PfcWdAction action)
 {
     SWSS_LOG_ENTER();
@@ -467,7 +489,7 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,
     if (!gPortsOrch->getPortPfc(port.m_port_id, &pfcMask))
     {
         SWSS_LOG_ERROR("Failed to get PFC mask on port %s", port.m_alias.c_str());
-        return;
+        return false;
     }
 
     set<uint8_t> losslessTc;
@@ -480,6 +502,11 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,
 
         losslessTc.insert(i);
     }
+    if (losslessTc.empty())
+    {
+        SWSS_LOG_NOTICE("No lossless TC found on port %s", port.m_alias.c_str());
+        return false;
+    }
 
     if (!c_portStatIds.empty())
     {
@@ -541,6 +568,8 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,
     sai_object_id_t groupId;
     gPortsOrch->createBindAclTableGroup(port.m_port_id, groupId, ACL_STAGE_INGRESS);
     gPortsOrch->createBindAclTableGroup(port.m_port_id, groupId, ACL_STAGE_EGRESS);
+
+    return true;
 }
 
 template <typename DropHandler, typename ForwardHandler>
@@ -716,9 +745,7 @@ bool PfcWdSwOrch<DropHandler, ForwardHandler>::startWdOnPort(const Port& port,
 {
     SWSS_LOG_ENTER();
 
-    registerInWdDb(port, detectionTime, restorationTime, action);
-
-    return true;
+    return registerInWdDb(port, detectionTime, restorationTime, action);
 }
 
 template <typename DropHandler, typename ForwardHandler>
diff --git a/orchagent/pfcwdorch.h b/orchagent/pfcwdorch.h
@@ -48,8 +48,8 @@ class PfcWdOrch: public Orch
     static PfcWdAction deserializeAction(const string& key);
     static string serializeAction(const PfcWdAction &action); 
 
-    virtual void createEntry(const string& key, const vector<FieldValueTuple>& data);
-    void deleteEntry(const string& name);
+    virtual task_process_status createEntry(const string& key, const vector<FieldValueTuple>& data);
+    task_process_status deleteEntry(const string& name);
 
 protected:
     virtual bool startWdActionOnQueue(const string &event, sai_object_id_t queueId) = 0;
@@ -79,7 +79,7 @@ class PfcWdSwOrch: public PfcWdOrch<DropHandler, ForwardHandler>
             uint32_t detectionTime, uint32_t restorationTime, PfcWdAction action);
     virtual bool stopWdOnPort(const Port& port);
 
-    void createEntry(const string& key, const vector<FieldValueTuple>& data);
+    task_process_status createEntry(const string& key, const vector<FieldValueTuple>& data) override;
     virtual void doTask(SelectableTimer &timer);
     //XXX Add port/queue state change event handlers
 
@@ -106,7 +106,7 @@ class PfcWdSwOrch: public PfcWdOrch<DropHandler, ForwardHandler>
 
     template <typename T>
     static string counterIdsToStr(const vector<T> ids, string (*convert)(T));
-    void registerInWdDb(const Port& port,
+    bool registerInWdDb(const Port& port,
             uint32_t detectionTime, uint32_t restorationTime, PfcWdAction action);
     void unregisterFromWdDb(const Port& port);
     void doTask(swss::NotificationConsumer &wdNotification);
diff --git a/orchagent/qosorch.cpp b/orchagent/qosorch.cpp
@@ -1361,6 +1361,27 @@ task_process_status QosOrch::handlePortQosMapTable(Consumer& consumer)
     return task_process_status::task_success;
 }
 
+void QosOrch::doTask()
+{
+    SWSS_LOG_ENTER();
+
+    auto *port_qos_map_cfg_exec = getExecutor(CFG_PORT_QOS_MAP_TABLE_NAME);
+
+    for (const auto &it : m_consumerMap)
+    {
+        auto *exec = it.second.get();
+
+        if (exec == port_qos_map_cfg_exec)
+        {
+            continue;
+        }
+
+        exec->drain();
+    }
+
+    port_qos_map_cfg_exec->drain();
+}
+
 void QosOrch::doTask(Consumer &consumer)
 {
     SWSS_LOG_ENTER();
diff --git a/orchagent/qosorch.h b/orchagent/qosorch.h
@@ -114,6 +114,7 @@ class QosOrch : public Orch
     static type_map& getTypeMap();
     static type_map m_qos_maps;
 private:
+    void doTask() override;
     virtual void doTask(Consumer& consumer);
 
     typedef task_process_status (QosOrch::*qos_table_handler)(Consumer& consumer);

Original file line number	Diff line number	Diff line change
`@@ -68,20 +68,38 @@ void PfcWdOrch<DropHandler, ForwardHandler>::doTask(Consumer& consumer)`
`68`	`68`	`string key = kfvKey(t);`
`69`	`69`	`string op = kfvOp(t);`
`70`	`70`
	`71`	`+ task_process_status task_status = task_process_status::task_ignore;`
`71`	`72`	`if (op == SET_COMMAND)`
`72`	`73`	`{`
`73`		`- createEntry(key, kfvFieldsValues(t));`
	`74`	`+ task_status = createEntry(key, kfvFieldsValues(t));`
`74`	`75`	`}`
`75`	`76`	`else if (op == DEL_COMMAND)`
`76`	`77`	`{`
`77`		`- deleteEntry(key);`
	`78`	`+ task_status = deleteEntry(key);`
`78`	`79`	`}`
`79`	`80`	`else`
`80`	`81`	`{`
	`82`	`+ task_status = task_process_status::task_invalid_entry;`
`81`	`83`	`SWSS_LOG_ERROR("Unknown operation type %s\n", op.c_str());`
`82`	`84`	`}`
`83`		`-`
`84`		`- consumer.m_toSync.erase(it++);`
	`85`	`+ switch (task_status)`
	`86`	`+ {`
	`87`	`+ case task_process_status::task_success:`
	`88`	`+ consumer.m_toSync.erase(it++);`
	`89`	`+ break;`
	`90`	`+ case task_process_status::task_need_retry:`
	`91`	`+ SWSS_LOG_INFO("Failed to process PFC watchdog %s task, retry it", op.c_str());`
	`92`	`+ ++it;`
	`93`	`+ break;`
	`94`	`+ case task_process_status::task_invalid_entry:`
	`95`	`+ SWSS_LOG_ERROR("Failed to process PFC watchdog %s task, invalid entry", op.c_str());`
	`96`	`+ consumer.m_toSync.erase(it++);`
	`97`	`+ break;`
	`98`	`+ default:`
	`99`	`+ SWSS_LOG_ERROR("Invalid task status %d", task_status);`
	`100`	`+ consumer.m_toSync.erase(it++);`
	`101`	`+ break;`
	`102`	`+ }`
`85`	`103`	`}`
`86`	`104`
`87`	`105`	`if (consumer.m_toSync.empty())`
`@@ -156,7 +174,7 @@ string PfcWdOrch<DropHandler, ForwardHandler>::serializeAction(const PfcWdAction`
`156`	`174`
`157`	`175`
`158`	`176`	`template <typename DropHandler, typename ForwardHandler>`
`159`		`-void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,`
	`177`	`+task_process_status PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,`
`160`	`178`	`const vector<FieldValueTuple>& data)`
`161`	`179`	`{`
`162`	`180`	`SWSS_LOG_ENTER();`
`@@ -170,13 +188,13 @@ void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,`
`170`	`188`	`if (!gPortsOrch->getPort(key, port))`
`171`	`189`	`{`
`172`	`190`	`SWSS_LOG_ERROR("Invalid port interface %s", key.c_str());`
`173`		`- return;`
	`191`	`+ return task_process_status::task_invalid_entry;`
`174`	`192`	`}`
`175`	`193`
`176`	`194`	`if (port.m_type != Port::PHY)`
`177`	`195`	`{`
`178`	`196`	`SWSS_LOG_ERROR("Interface %s is not physical port", key.c_str());`
`179`		`- return;`
	`197`	`+ return task_process_status::task_invalid_entry;`
`180`	`198`	`}`
`181`	`199`
`182`	`200`	`for (auto i : data)`
`@@ -205,7 +223,7 @@ void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,`
`205`	`223`	`if (action == PfcWdAction::PFC_WD_ACTION_UNKNOWN)`
`206`	`224`	`{`
`207`	`225`	`SWSS_LOG_ERROR("Invalid PFC Watchdog action %s", value.c_str());`
`208`		`- return;`
	`226`	`+ return task_process_status::task_invalid_entry;`
`209`	`227`	`}`
`210`	`228`	`}`
`211`	`229`	`else`
`@@ -214,7 +232,7 @@ void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,`
`214`	`232`	`"Failed to parse PFC Watchdog %s configuration. Unknown attribute %s.\n",`
`215`	`233`	`key.c_str(),`
`216`	`234`	`field.c_str());`
`217`		`- return;`
	`235`	`+ return task_process_status::task_invalid_entry;`
`218`	`236`	`}`
`219`	`237`	`}`
`220`	`238`	`catch (const exception& e)`
`@@ -224,36 +242,37 @@ void PfcWdOrch<DropHandler, ForwardHandler>::createEntry(const string& key,`
`224`	`242`	`key.c_str(),`
`225`	`243`	`field.c_str(),`
`226`	`244`	`e.what());`
`227`		`- return;`
	`245`	`+ return task_process_status::task_invalid_entry;`
`228`	`246`	`}`
`229`	`247`	`catch (...)`
`230`	`248`	`{`
`231`	`249`	`SWSS_LOG_ERROR(`
`232`	`250`	`"Failed to parse PFC Watchdog %s attribute %s. Unknown error has been occurred",`
`233`	`251`	`key.c_str(),`
`234`	`252`	`field.c_str());`
`235`		`- return;`
	`253`	`+ return task_process_status::task_invalid_entry;`
`236`	`254`	`}`
`237`	`255`	`}`
`238`	`256`
`239`	`257`	`// Validation`
`240`	`258`	`if (detectionTime == 0)`
`241`	`259`	`{`
`242`	`260`	`SWSS_LOG_ERROR("%s missing", PFC_WD_DETECTION_TIME);`
`243`		`- return;`
	`261`	`+ return task_process_status::task_invalid_entry;`
`244`	`262`	`}`
`245`	`263`
`246`	`264`	`if (!startWdOnPort(port, detectionTime, restorationTime, action))`
`247`	`265`	`{`
`248`	`266`	`SWSS_LOG_ERROR("Failed to start PFC Watchdog on port %s", port.m_alias.c_str());`
`249`		`- return;`
	`267`	`+ return task_process_status::task_need_retry;`
`250`	`268`	`}`
`251`	`269`
`252`	`270`	`SWSS_LOG_NOTICE("Started PFC Watchdog on port %s", port.m_alias.c_str());`
	`271`	`+ return task_process_status::task_success;`
`253`	`272`	`}`
`254`	`273`
`255`	`274`	`template <typename DropHandler, typename ForwardHandler>`
`256`		`-void PfcWdOrch<DropHandler, ForwardHandler>::deleteEntry(const string& name)`
	`275`	`+task_process_status PfcWdOrch<DropHandler, ForwardHandler>::deleteEntry(const string& name)`
`257`	`276`	`{`
`258`	`277`	`SWSS_LOG_ENTER();`
`259`	`278`
`@@ -263,14 +282,15 @@ void PfcWdOrch<DropHandler, ForwardHandler>::deleteEntry(const string& name)`
`263`	`282`	`if (!stopWdOnPort(port))`
`264`	`283`	`{`
`265`	`284`	`SWSS_LOG_ERROR("Failed to stop PFC Watchdog on port %s", name.c_str());`
`266`		`- return;`
	`285`	`+ return task_process_status::task_failed;`
`267`	`286`	`}`
`268`	`287`
`269`	`288`	`SWSS_LOG_NOTICE("Stopped PFC Watchdog on port %s", name.c_str());`
	`289`	`+ return task_process_status::task_success;`
`270`	`290`	`}`
`271`	`291`
`272`	`292`	`template <typename DropHandler, typename ForwardHandler>`
`273`		`-void PfcWdSwOrch<DropHandler, ForwardHandler>::createEntry(const string& key,`
	`293`	`+task_process_status PfcWdSwOrch<DropHandler, ForwardHandler>::createEntry(const string& key,`
`274`	`294`	`const vector<FieldValueTuple>& data)`
`275`	`295`	`{`
`276`	`296`	`SWSS_LOG_ENTER();`
`@@ -297,8 +317,10 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::createEntry(const string& key,`
`297`	`317`	`}`
`298`	`318`	`else`
`299`	`319`	`{`
`300`		`- PfcWdOrch<DropHandler, ForwardHandler>::createEntry(key, data);`
	`320`	`+ return PfcWdOrch<DropHandler, ForwardHandler>::createEntry(key, data);`
`301`	`321`	`}`
	`322`	`+`
	`323`	`+ return task_process_status::task_success;`
`302`	`324`	`}`
`303`	`325`
`304`	`326`	`template <typename DropHandler, typename ForwardHandler>`
`@@ -457,7 +479,7 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::enableBigRedSwitchMode()`
`457`	`479`	`}`
`458`	`480`
`459`	`481`	`template <typename DropHandler, typename ForwardHandler>`
`460`		`-void PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,`
	`482`	`+bool PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,`
`461`	`483`	`uint32_t detectionTime, uint32_t restorationTime, PfcWdAction action)`
`462`	`484`	`{`
`463`	`485`	`SWSS_LOG_ENTER();`
`@@ -467,7 +489,7 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,`
`467`	`489`	`if (!gPortsOrch->getPortPfc(port.m_port_id, &pfcMask))`
`468`	`490`	`{`
`469`	`491`	`SWSS_LOG_ERROR("Failed to get PFC mask on port %s", port.m_alias.c_str());`
`470`		`- return;`
	`492`	`+ return false;`
`471`	`493`	`}`
`472`	`494`
`473`	`495`	`set<uint8_t> losslessTc;`
`@@ -480,6 +502,11 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,`
`480`	`502`
`481`	`503`	`losslessTc.insert(i);`
`482`	`504`	`}`
	`505`	`+ if (losslessTc.empty())`
	`506`	`+ {`
	`507`	`+ SWSS_LOG_NOTICE("No lossless TC found on port %s", port.m_alias.c_str());`
	`508`	`+ return false;`
	`509`	`+ }`
`483`	`510`
`484`	`511`	`if (!c_portStatIds.empty())`
`485`	`512`	`{`
`@@ -541,6 +568,8 @@ void PfcWdSwOrch<DropHandler, ForwardHandler>::registerInWdDb(const Port& port,`
`541`	`568`	`sai_object_id_t groupId;`
`542`	`569`	`gPortsOrch->createBindAclTableGroup(port.m_port_id, groupId, ACL_STAGE_INGRESS);`
`543`	`570`	`gPortsOrch->createBindAclTableGroup(port.m_port_id, groupId, ACL_STAGE_EGRESS);`
	`571`	`+`
	`572`	`+ return true;`
`544`	`573`	`}`
`545`	`574`
`546`	`575`	`template <typename DropHandler, typename ForwardHandler>`
`@@ -716,9 +745,7 @@ bool PfcWdSwOrch<DropHandler, ForwardHandler>::startWdOnPort(const Port& port,`
`716`	`745`	`{`
`717`	`746`	`SWSS_LOG_ENTER();`
`718`	`747`
`719`		`- registerInWdDb(port, detectionTime, restorationTime, action);`
`720`		`-`
`721`		`- return true;`
	`748`	`+ return registerInWdDb(port, detectionTime, restorationTime, action);`
`722`	`749`	`}`
`723`	`750`
`724`	`751`	`template <typename DropHandler, typename ForwardHandler>`