Skip to content
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

Add extra lossy PG profile for ports between T1 and T2 #11157

Merged

Conversation

bingwang-ms
Copy link
Contributor

@bingwang-ms bingwang-ms commented Jun 16, 2022

Why I did it

This PR brings two changes

  1. Add lossy PG profile for PG2 and PG6 on T1 for ports between T1 and T2.
    After PR Update qos config to clear queues for bounced back traffic #10176 , the DSCP_TO_TC_MAP and TC_TO_PG_MAP is updated when remapping is enable
  • DSCP_TO_TC_MAP
Before After Why do this change
"2" : "1" "2" : "2" Only change for leaf router to map DSCP 2 to TC 2 as TC 2 will be used for lossless TC
"6" : "1" "6" : "6" Only change for leaf router to map DSCP 6 to TC 6 as TC 6 will be used for lossless TC
  • TC_TO_PRIORITY_GROUP_MAP
Before After Why do this change
"2" : "0" "2" : "2" Only change for leaf router to map TC 2 to PG 2 as PG 2 will be used for lossless PG
"6" : "0" "6" : "6" Only change for leaf router to map TC 6 to PG 6 as PG 6 will be used for lossless PG

So, we have two new lossy PGs (2 and 6) for the T2 facing ports on T1, and two new lossless PGs (2 and 6) for the T0 facing port on T1.
However, there is no lossy PG profile for the T2 facing ports on T1. The lossless PGs for ports between T1 and T0 have been handled by buffermgrd .Therefore, We need to add lossy PG profiles for T2 facing ports on T1.

We don't have this issue on T0 because PG 2 and PG 6 are lossless PGs, and there is no lossy traffic mapped to PG 2 and PG 6

  1. Map port level TC7 to PG0
    Before the PCBB change, DSCP48 -> TC 6 -> PG 0.
    After the PCBB change, DSCP48 -> TC 7 -> PG 7
    Actually, we can map TC7 to PG0 to save a lossy PG.

How I did it

Update the qos and buffer template.

How to verify it

Verified by UT.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205

Description for the changelog

  1. Add extra lossy PG profile for ports between T1 and T2
  2. Map TC7->PG0

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: bingwang <wang.bing@microsoft.com>
@bingwang-ms bingwang-ms requested review from a team and lguohan as code owners June 16, 2022 03:27
@bingwang-ms bingwang-ms requested a review from stephenxs June 16, 2022 03:33
@bingwang-ms bingwang-ms requested review from neethajohn, lguohan and a team June 16, 2022 04:49
@stephenxs
Copy link
Collaborator

Hi
Originally, in the HLD, we agreed that there is no traffic of DSCP 6 and 2 except for the bounced back traffic from the standby to the active. So it's not clear to me why there is traffic of DSCP 6 and 2 between t1 and t2.
Am I missing anything?
Thanks

@bingwang-ms
Copy link
Contributor Author

Hi Originally, in the HLD, we agreed that there is no traffic of DSCP 6 and 2 except for the bounced back traffic from the standby to the active. So it's not clear to me why there is traffic of DSCP 6 and 2 between t1 and t2. Am I missing anything? Thanks

Yes, the issue can be triggered if server send traffic with DSCP=2/6 to the ToR. Please consider below scenario
image

For example, server will generate traffic with DSCP = 2 when we run ssh with remote command, like ssh 10.1.1.1 'uptime'. That traffic will be dropped on T1 without the PG profile for PG2.
Please feel free to discuss.

@stephenxs
Copy link
Collaborator

stephenxs commented Jun 16, 2022

Yes, the issue can be triggered if server send traffic with DSCP=2/6 to the ToR. Please consider below scenario

For example, server will generate traffic with DSCP = 2 when we run ssh with remote command, like ssh 10.1.1.1 'uptime'. That traffic will be dropped on T1 without the PG profile for PG2. Please feel free to discuss.

Thanks for the update.
in case servers sent packets with DSCP=2 or 6, it makes sense. but this incurs additional buffer consumption, which is not good. I was thinking whether we can deploy different DSCP_TO_TC or TC_TO_PG maps between downlink and uplink ports on Mellanox devices. By doing so, we do not need the additional lossy PGs on uplink ports.

@stephenxs
Copy link
Collaborator

FYI. Another concern is that the lossy traffic can compete with lossless traffic for shared buffer on ToR uplink egress and T1 downlink ingress, making lossless traffic a bit more likely to get congested, which eventually leads to the T2 downlink lossless queue being congested due to T1 uplink lossy queue being congested.

@bingwang-ms
Copy link
Contributor Author

FYI. Another concern is that the lossy traffic can compete with lossless traffic for shared buffer on ToR uplink egress and T1 downlink ingress, making lossless traffic a bit more likely to get congested, which eventually leads to the T2 downlink lossless queue being congested due to T1 uplink lossy queue being congested.

Yes, that's right. The ingressed traffic from T2 to T1 with DSCP=2/6 will finally egress from T1 to T0 as lossless traffic. We are also discussing internally and try to find out a workaround.

@stephenxs
Copy link
Collaborator

stephenxs commented Jun 21, 2022

For example, server will generate traffic with DSCP = 2 when we run ssh with remote command, like ssh 10.1.1.1 'uptime'. That traffic will be dropped on T1 without the PG profile for PG2.
Please feel free to discuss.

Another scenario that is difficult to handle. assume

  • on t1 downlink, PG 2 xoff is reached, PFC frames are sent with priority 2. PG 2 is used to accommodate traffic with DSCP 2
  • on t0 uplink, dscp 2 can be sent from queue 1/2
    • for queue 2, as it's lossless, the queue is ceased on receiving PFC frames
    • for queue 1, it's lossy and won't react on receiving PFC frames

as a result, headroom on t1 downlink PG 2 can be fully occupied by packets sent from t0 uplink through queue 1, which makes lossless traffic dropped

@neethajohn neethajohn merged commit ac86f71 into sonic-net:master Jun 28, 2022
yxieca pushed a commit that referenced this pull request Jun 30, 2022
Signed-off-by: bingwang <wang.bing@microsoft.com>

Why I did it
This PR brings two changes

Add lossy PG profile for PG2 and PG6 on T1 for ports between T1 and T2.
After PR Update qos config to clear queues for bounced back traffic #10176 , the DSCP_TO_TC_MAP and TC_TO_PG_MAP is updated when remapping is enable

DSCP_TO_TC_MAP
Before	After	Why do this change
"2" : "1"	"2" : "2"	Only change for leaf router to map DSCP 2 to TC 2 as TC 2 will be used for lossless TC
"6" : "1"	"6" : "6"	Only change for leaf router to map DSCP 6 to TC 6 as TC 6 will be used for lossless TC

TC_TO_PRIORITY_GROUP_MAP
Before	After	Why do this change
"2" : "0"	"2" : "2"	Only change for leaf router to map TC 2 to PG 2 as PG 2 will be used for lossless PG
"6" : "0"	"6" : "6"	Only change for leaf router to map TC 6 to PG 6 as PG 6 will be used for lossless PG

So, we have two new lossy PGs (2 and 6) for the T2 facing ports on T1, and two new lossless PGs (2 and 6) for the T0 facing port on T1.
However, there is no lossy PG profile for the T2 facing ports on T1. The lossless PGs for ports between T1 and T0 have been handled by buffermgrd .Therefore, We need to add lossy PG profiles for T2 facing ports on T1.

We don't have this issue on T0 because PG 2 and PG 6 are lossless PGs, and there is no lossy traffic mapped to PG 2 and PG 6

Map port level TC7 to PG0
Before the PCBB change, DSCP48 -> TC 6 -> PG 0.
After the PCBB change, DSCP48 -> TC 7 -> PG 7
Actually, we can map TC7 to PG0 to save a lossy PG.

How I did it
Update the qos and buffer template.

How to verify it
Verified by UT.
neethajohn pushed a commit that referenced this pull request Aug 1, 2022
…nt (#11580)

Why I did it
This PR is to backport #11569 into 202012 branch.
This PR is to apply different DSCP_TO_TC_MAP to downlink and uplink ports on T1 in dualtor deployment.
For T1 downlink ports (To T0)
The DSCP_TO_TC_MAP is not changed. DSCP2 and DSCP6 are mapped to TC2 and TC6 respectively.
For T1 uplink ports (To T1)
A new DSCP_TO_TC_MAP|AZURE_UPLINK is defined and applied. DSCP2 and DSCP6 are mapped to TC1 to avoid mixing up lossy and lossless traffic from T2.
The extra lossy PG2 and PG6 added in PR #11157 is reverted as well because no traffic from T2 is mapped to PG2 or PG6 now.

How I did it
Define a new map DSCP_TO_TC_MAP|AZURE_UPLINK for 7260 T1.

How to verify it
Verified by test case in test_j2files.py.
skbarista pushed a commit to skbarista/sonic-buildimage that referenced this pull request Aug 17, 2022
Signed-off-by: bingwang <wang.bing@microsoft.com>

Why I did it
This PR brings two changes

Add lossy PG profile for PG2 and PG6 on T1 for ports between T1 and T2.
After PR Update qos config to clear queues for bounced back traffic sonic-net#10176 , the DSCP_TO_TC_MAP and TC_TO_PG_MAP is updated when remapping is enable

DSCP_TO_TC_MAP
Before	After	Why do this change
"2" : "1"	"2" : "2"	Only change for leaf router to map DSCP 2 to TC 2 as TC 2 will be used for lossless TC
"6" : "1"	"6" : "6"	Only change for leaf router to map DSCP 6 to TC 6 as TC 6 will be used for lossless TC

TC_TO_PRIORITY_GROUP_MAP
Before	After	Why do this change
"2" : "0"	"2" : "2"	Only change for leaf router to map TC 2 to PG 2 as PG 2 will be used for lossless PG
"6" : "0"	"6" : "6"	Only change for leaf router to map TC 6 to PG 6 as PG 6 will be used for lossless PG

So, we have two new lossy PGs (2 and 6) for the T2 facing ports on T1, and two new lossless PGs (2 and 6) for the T0 facing port on T1.
However, there is no lossy PG profile for the T2 facing ports on T1. The lossless PGs for ports between T1 and T0 have been handled by buffermgrd .Therefore, We need to add lossy PG profiles for T2 facing ports on T1.

We don't have this issue on T0 because PG 2 and PG 6 are lossless PGs, and there is no lossy traffic mapped to PG 2 and PG 6

Map port level TC7 to PG0
Before the PCBB change, DSCP48 -> TC 6 -> PG 0.
After the PCBB change, DSCP48 -> TC 7 -> PG 7
Actually, we can map TC7 to PG0 to save a lossy PG.

How I did it
Update the qos and buffer template.

How to verify it
Verified by UT.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants