[pfcwd]: support BIG_RED_SWITCH mode by sihuihan88 · Pull Request #467 · sonic-net/sonic-swss

sihuihan88 · 2018-04-10T21:21:45Z

Signed-off-by: Sihui Han sihan@microsoft.com

What I did
Enable support for BIG_RED_SWITCH mode:
The Expect behavior:

When BIG_RED_SWITCH mode is enabled:

Regardless of PFCWD configuration or PFC storm status, PFCWD starts to drop all lossless TC packets on all ports. It's a big red button to drop all lossless packets "blindly".
PFCWD will not do storm check/restore once BIG_RED_SWITCH is enabled. If there is some active action handler handling ongoing PFC storm when enabling BIG_RED_SWITCH, stop these handlers and rely on BIG_RED_SWITCH to drop all the packets.
Lossy traffic should not be affected.

When BIG_RED_SWITCH mode is disabled:
PFCWD goes back to normal, and start to check/restore pfc storm.
BIG_RED_SWITCH mode can be enabled and disabled anytime even if PFCWD was not enabled on any port before.

Why I did it
Per request, BIG_RED_SWITCH mode provides a button to drop all lossless packets on a switch.
How I verified it
Tested on DUT
Details if related

Signed-off-by: Sihui Han <sihan@microsoft.com>

sihuihan88 · 2018-04-10T22:08:21Z

cli sonic-net/sonic-utilities#237

sihuihan88 · 2018-04-10T23:53:43Z

retest this please

marian-pritsak · 2018-04-11T09:53:32Z

orchagent/pfcwdorch.cpp

    {
        if (entry->second.handler != nullptr)
        {
+            SWSS_LOG_NOTICE(


Why move logs from handler to this orch?

Since in big-red-switch mode, we don't want to have log relates to storm detect/restore. Moving log out of handler class provides flexibility for users to input various logs depending on the use case when they create action handler

marian-pritsak · 2018-04-11T09:54:25Z

orchagent/pfcwdorch.cpp

+    for (auto & it: allPorts)
+    {
+        Port port = it.second;
+        if (port.m_alias != "Ethernet8")


What's this magic ifname?

marian-pritsak · 2018-04-11T09:58:03Z

orchagent/pfcwdorch.h

+    void setBigRedSwitchMode(string value);
+
    map<sai_object_id_t, PfcWdQueueEntry> m_entryMap;
+    map<sai_object_id_t, PfcWdQueueEntry> m_brsEntryMap;


Why separate map for BRS?

After disable big-red-switch, we need to keep the PFC WD original configuration. This information actually is stored in m_entryMap. It would be better if we don't mix this information with brs.

In line 398 in .cpp file entry is removed from the regular map when BRS is enabled, but it is not added back when BRS is disabled.
In case we had some handler, and BRS is flipped on/off, will we end up with no handler after BRS off?

That’s true. The previous active handlers mark the storm happens when BRS was enabled. After BRS runs for a while, the previous state is out of date so it’s unecessary to recreate those handlers. Instead we just need to recover the PWCWD configuration and let it detect storm again. And the configuration comes from entrymap

marian-pritsak · 2018-04-11T10:01:05Z

orchagent/pfcwdorch.cpp

+            string queueIdStr = sai_serialize_object_id(queueId);
+
+            vector<FieldValueTuple> countersFieldValues;
+            countersFieldValues.emplace_back("BIG_RED_SWITCH_MODE", "enable");


I don't think that plugins should know about BRS. Since you chose to keep them running, you might as well ignore notifications from plugins by checking a flag in orch.

I would prefer to let plugins aware of BRS. Otherwise, syncd will runplugin and keeps sending notification every 100ms. Even though swss can ingore, it still consumes a lot unnecessary resources.

Maybe then it is better to unregister plugins or have some field in DB along with poll interval that will globally turn plugins on/off?

I think both method are mostly identical, and the only difference is how to skip plugins. The benefit to achieve this in lua script is it's lightweight and only affect pfcwd, also it don't need modification of syncd and makes syncd logic simpler. If you would prefer to change the logic to syncd, I can add it to backlog and change it later.

Have added it to backlog. sonic-net/sonic-sairedis#314

marian-pritsak · 2018-04-11T10:02:38Z

orchagent/pfcwdorch.cpp

+            return;
+        }
+
+        uint8_t pfcMask = attr.value.u8;


This is not aligned with asymmetric PFC

This need to be updated once asym PFC is supported. There was one comment above this part of the code to remind us to update this part once asy is enabled.

Oh, I thought it already was. My bad.

* [pfcwd]: enable BIG_RED_SWITCH mode Signed-off-by: Sihui Han <sihan@microsoft.com> * update as comments

* Option '-d' means read the EEPROM information from DB directly * CLI "show platform syseeprom" will invoke decode-syseeprom with '-d' option Signed-off-by: Kevin Wang <kevinw@mellanox.com>

* Rename to fix typo Signed-off-by: Wenda Ni <wenni@microsoft.com> * Correct shared_ptr creation parameters Signed-off-by: Wenda Ni <wenni@microsoft.com>

* [pfcwd]: enable BIG_RED_SWITCH mode Signed-off-by: Sihui Han <sihan@microsoft.com> * update as comments

[pfcwd]: enable BIG_RED_SWITCH mode

a510acf

Signed-off-by: Sihui Han <sihan@microsoft.com>

sihuihan88 mentioned this pull request Apr 10, 2018

[pfcwd]: add cli to enable/disable BIG_RED_SWITCH mode sonic-net/sonic-utilities#237

Merged

sihuihan88 changed the title ~~[pfcwd]: enable BIG_RED_SWITCH mode~~ [pfcwd]: support BIG_RED_SWITCH mode Apr 11, 2018

marian-pritsak requested changes Apr 11, 2018

View reviewed changes

update as comments

afbc4cf

marian-pritsak approved these changes Apr 13, 2018

View reviewed changes

sihuihan88 merged commit 046628b into sonic-net:master Apr 13, 2018

sihuihan88 deleted the dev/sihan/brs branch April 13, 2018 17:49

lguohan pushed a commit that referenced this pull request Apr 27, 2018

[pfcwd]: support BIG_RED_SWITCH mode (#467)

e34104e

* [pfcwd]: enable BIG_RED_SWITCH mode Signed-off-by: Sihui Han <sihan@microsoft.com> * update as comments

Janetxxx pushed a commit to Janetxxx/sonic-swss that referenced this pull request Nov 10, 2025

[pfcwd]: support BIG_RED_SWITCH mode (sonic-net#467)

20fbe02

* [pfcwd]: enable BIG_RED_SWITCH mode Signed-off-by: Sihui Han <sihan@microsoft.com> * update as comments

Janetxxx pushed a commit to Janetxxx/sonic-swss that referenced this pull request Nov 10, 2025

[pfcwd]: support BIG_RED_SWITCH mode (sonic-net#467)

10cf4fb

* [pfcwd]: enable BIG_RED_SWITCH mode Signed-off-by: Sihui Han <sihan@microsoft.com> * update as comments

Conversation

sihuihan88 commented Apr 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sihuihan88 commented Apr 10, 2018

Uh oh!

sihuihan88 commented Apr 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sihuihan88 Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sihuihan88 Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sihuihan88 Apr 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sihuihan88 commented Apr 10, 2018 •

edited

Loading

sihuihan88 Apr 12, 2018 •

edited

Loading

sihuihan88 Apr 12, 2018 •

edited

Loading

sihuihan88 Apr 11, 2018 •

edited

Loading