-
Notifications
You must be signed in to change notification settings - Fork 642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable PFCWD only on ports where PFC is enabled #1508
Enable PFCWD only on ports where PFC is enabled #1508
Conversation
This pull request introduces 1 alert when merging 46392e4 into 0de99c3 - view on LGTM.com new alerts:
|
You should also update the "start_default()", and add a unit test for "config pfcwd start_default" |
Thanks for adding the unit tests. For completeness, please also add a unit test for 'multi ASIC' platform. You already updated the multi ASIC config DBs, just need to add test case. |
46392e4
to
f5be9ee
Compare
This pull request introduces 1 alert when merging f5be9ee into fbad274 - view on LGTM.com new alerts:
|
Done |
Done |
retest this please |
Nice!! Wish there was a command to show which ports has PFCWD enabled. Lets wait for tests to pass. |
retest this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What I did Prevent errors in the log when PFCWD starts on ports that PFC was not previously enabled. Example of such errors in the log: Nov 13 17:27:22.878468 arc-switch1038 ERR swss#orchagent: :- createEntry: Failed to start PFC Watchdog on port Ethernet100 Nov 13 17:27:23.878620 arc-switch1038 NOTICE swss#orchagent: :- registerInWdDb: No lossless TC found on port Ethernet100 Add unit test to cover the new flow. - How I did it Before enabling PFCWD on a port, verify if PFC was previously enabled. If not, reply with an error to the user and as a warning in the log. In the case where the command to enable PFCWD is done for all ports, the configuration will be applied for only those that has PCF enabled and for the rest the configuration will be skipped. In this case the output of the requested command will be only with a list of skipped ports. - How to verify it 'pfcwd start EthernetX 600' where EthernetX is a port that has not configured PFC (usually admin state 'down' by default)
…dered (#1937) Signed-off-by: Neetha John <nejo@microsoft.com> Fixes sonic-net/sonic-buildimage#9292 What I did Pfcwd was not getting started after executing load_minigraph after the changes done in #1508. This was because the PORT_QOS_MAP table is not yet present in config db (this gets populated only after the buffer templates are rendered) at the time we try to start pfcwd and hence the 'pfc_enable' field will always be empty and we skip writing PFC_WD table entries to config db. How I did it Delay pfcwd start until the buffer templates are rendered How to verify it Issue "config load_minigraph" with the changes and ensure that pfcwd is started on all active ports
…dered (#1937) Signed-off-by: Neetha John <nejo@microsoft.com> Fixes sonic-net/sonic-buildimage#9292 What I did Pfcwd was not getting started after executing load_minigraph after the changes done in #1508. This was because the PORT_QOS_MAP table is not yet present in config db (this gets populated only after the buffer templates are rendered) at the time we try to start pfcwd and hence the 'pfc_enable' field will always be empty and we skip writing PFC_WD table entries to config db. How I did it Delay pfcwd start until the buffer templates are rendered How to verify it Issue "config load_minigraph" with the changes and ensure that pfcwd is started on all active ports
…dered (#1937) Signed-off-by: Neetha John <nejo@microsoft.com> Fixes sonic-net/sonic-buildimage#9292 What I did Pfcwd was not getting started after executing load_minigraph after the changes done in #1508. This was because the PORT_QOS_MAP table is not yet present in config db (this gets populated only after the buffer templates are rendered) at the time we try to start pfcwd and hence the 'pfc_enable' field will always be empty and we skip writing PFC_WD table entries to config db. How I did it Delay pfcwd start until the buffer templates are rendered How to verify it Issue "config load_minigraph" with the changes and ensure that pfcwd is started on all active ports
What I did
Prevent errors in the log when PFCWD starts on ports that PFC was not previously enabled.
Example of such errors in the log:
Add unit test to cover the new flow.
How I did it
Before enabling PFCWD on a port, verify if PFC was previously enabled. If not, reply with an error to the user and as a warning in the log.
In the case where the command to enable PFCWD is done for all ports, the configuration will be applied for only those that has PCF enabled and for the rest the configuration will be skipped. In this case the output of the requested command will be only with a list of skipped ports.
How to verify it
run:
pfcwd start EthernetX 600
where EthernetX is a port that has not configured PFC (usually admin state 'down' by default)Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)