Enable PFCWD only on ports where PFC is enabled #1508
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request introduces 1 alert when merging 46392e4 into 0de99c3 - view on LGTM.com new alerts:
|
smaheshm
reviewed
Mar 16, 2021
smaheshm
reviewed
Mar 16, 2021
|
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. |
ayurkiv-nvda
force-pushed
the
pfcwd_run_if_pfc_enabled_external
branch
from
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 |
liat-grozovik
changed the title
liat-grozovik
approved these changes
Mar 24, 2021
yxieca
pushed a commit
that referenced
this pull request
Mar 26, 2021
- 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)
neethajohn
added a commit
that referenced
this pull request
Nov 24, 2021
…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
qiluo-msft
pushed a commit
that referenced
this pull request
Nov 30, 2021
…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
abdosi
pushed a commit
that referenced
this pull request
Dec 8, 2021
…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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 600where 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)