Skip to content

Commit

Permalink
Fix PFC watchdog not getting lossless TC (sonic-net#876)
Browse files Browse the repository at this point in the history 
* Allow PFC watchdog to retry start on port

Signed-off-by: Wenda Ni <[email protected]>

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

Signed-off-by: Wenda Ni <[email protected]>

* Remove debugging symbols

Signed-off-by: Wenda Ni <[email protected]>

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

Signed-off-by: Wenda Ni <[email protected]>
  • Loading branch information
@wendani @lguohan
wendani authored and lguohan committed May 20, 2019
1 parent b0ad3eb commit 9f9a069
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 26 deletions.
71 changes: 49 additions & 22 deletions orchagent/pfcwdorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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();
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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>
Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand All @@ -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())
{
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
8 changes: 4 additions & 4 deletions orchagent/pfcwdorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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

Expand All @@ -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);
Expand Down
21 changes: 21 additions & 0 deletions orchagent/qosorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions orchagent/qosorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 9f9a069

Please sign in to comment.