-
Notifications
You must be signed in to change notification settings - Fork 544
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
[pfc] Introduce pfc config bitmask to track user-configured pfc enabled TCs on physical port #1612
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
…, uint8_t pfc_bitmask_status) to PortsOrch Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
in config is set Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
/Azurepipelines run |
Commenter does not have sufficient privileges for PR 1612 in repo Azure/sonic-swss |
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
can you add a test case for this? |
others look good to me. |
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
This pull request fixes 2 alerts when merging f6d6e6d into 89d0728 - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging e1bd125 into 15818ad - view on LGTM.com fixed alerts:
|
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
This pull request fixes 2 alerts when merging 92a0e35 into 15818ad - view on LGTM.com fixed alerts:
|
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
This pull request fixes 2 alerts when merging cbb2410 into 691bd30 - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging 6673a9c into cba6576 - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging c491f08 into 872b5cb - view on LGTM.com fixed alerts:
|
Done |
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>
This pull request fixes 2 alerts when merging 8248910 into 872b5cb - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging 1d4f9bc into 8bc7aee - view on LGTM.com fixed alerts:
|
* Filter out VNET routes, Fix errors related to VNET routes printed by route_check script Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>
What I did
Introduce PFC config bitmask to track user-configured PFC enabled TCs on physical port
This allows separate tracking of user-configured PFC bitmask (
pfc_enable
field inCONFIG_DB
) from PFC bitmask status in ASIC (i.e., pfcwd config). As when pfc wd is enabled and PFC storm occurs on a lossless TC, PFC bit on that TC is cleared in ASIC during the storm byPfcWdLossyHandler
, making PFC bitmask status in ASIC to be different from that configured by user.Such a difference causes the following misleading log message in syslog when a lossless TC is in PFC storm and pfc wd drop action is in place:
Why I did it
Fix sonic-net/sonic-buildimage#5910
How I verified it
On brcm
Details if related
Depends on
When user wants to change PFC enable bits on TCs, if pfc wd is enabled, user still needs to toggle pfc wd stop and start to ensure pfc wd state machine is stopped on a PFC-disabled TC and is running on a PFC-enabled TC (#842). However, before this change, command sequence matters: pfcwd must be stopped before PFC bits change on TCs, followed by
pfcwd start
. With this change, restriction on command sequence is no longer needed, i.e., PFC bits change on TC can be issued either before or afterpfcwd stop
, followed bypfcwd start
.Removing manual intervention described above (pfc wd stop and start toggle in changing PFC bits on TCs) is out of the scope of this PR, and will be in follow-up ones.
Covers fix to allow disabling last lossless TC(s) in pending PR #1055
vs tests
test case 1: Clear PFC enable bits on a physical port
This is to verify fix to #1055
SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
is set 0Before the change, test fails: port
SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
remains 24test case 2: Separate user and wd PFC enable bits on a physical port
Test is designed around the aforementioned step restriction on changing PFC enable bits on TCs, if pfc wd is enabled---Without this change, pfcwd must be stopped before PFC bits change on TCs, followed by pfcwd start.
SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
8'b00001000 == 8
DEBUG_STORM
. PortSAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
changes to8'b00000000 == 0
SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
remains8'b00000000 == 0
. W/o this change, PortSAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
changes to8'b00001000 == 8
. Test fail point 1SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
changes to8'b00010000 == 16
SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
remains8'b00010000 == 16
. W/o this change, portSAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
changes to8'b00011000 == 24
. Test fail point 2Test fail point 1
Test fail point 2
Variation: install PFC WD drop action using big red switch mode in place of faking PFC storm.
SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
8'b00010000 == 16
SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
changes to8'b00000000 == 0
SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
remains8'b00000000 == 0
. W/o this change, PortSAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
changes to8'b00010000 == 16
. Test fail point 1SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
changes to8'b00001000 == 8
SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
remains8'b00001000 == 8
. W/o this change, portSAI_PORT_ATTR_PRIORITY_FLOW_CONTROL
changes to8'b00011000 == 24
. Test fail point 2Test fail point 1
Test fail point 2