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

Update qos config to clear queues for bounced back traffic #10176

Merged

Conversation

bingwang-ms
Copy link
Contributor

@bingwang-ms bingwang-ms commented Mar 8, 2022

Signed-off-by: bingwang bingwang@microsoft.com

Why I did it

This PR is to redefine DSCP_TO_DC_MAP , TC_TO_QUEUE_MAP and TC_TO_PRIORITY_GROUP_MAP table to clear queue 2 and queue 6 for bounced back traffic.
To support the extra lossless queue 2 and 6, the table pfc_enable, pfcwd_sw_enable and SCHEDULER are also updated in the template.

HLD sonic-net/SONiC#950

SKU includes

  • Arista-7050CX3-32S-C32
  • Arista-7050CX3-32S-D48C8
  • Arista-7260CX3-D108C8
  • Arista-7260CX3-C64
  • Arista-7260CX3-Q64

Changed table:

  • DSCP_TO_TC_MAP
Original Leaf Dual-ToR Why do this change
"2" : "1" "2" : "2" "2" : "1" Only change for leaf router to map DSCP 2 to TC 2 as TC 2 will be used for lossless TC
"5" : "2" "5" : "1" "5" : "1" To clear TC 2 for bounced back traffic
"6" : "1" "6" : "6" "6" : "1" Only change for leaf router to map DSCP 6 to TC 6 as TC 6 will be used for lossless TC
"33" : "1" "33" : "1" "33" : "2" Only change for Tor router.
To map DSCP33->TC 2 as we need to conserve DSCP 33 for bounced back traffic.
The tunnel level map will map TC 2 -> DSCP 33
"48" : "6" "48" : "7" "48" : "7" To clear TC 6 for bounced back traffic
  • TC_TO_PRIORITY_GROUP_MAP
Original Leaf Dual-ToR Why do this change
"2" : "0" "2" : "2" "2" : "0" 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" "6" : "0" Only change for leaf router to map TC 6 to PG 6 as PG 6 will be used for lossless PG
  • TC_TO_QUEUE_MAP
Original Leaf Dual-ToR Why do this change
"2" : "2" "2" : "2" "2" : "1" Only change for ToR router to map TC 2 to Queue 1.
As we mapped DSCP33->TC2 on ToR router, we need to remap TC2 to a lossy queue

How I did it

Define a macro for SKUs that requires remapping.

How to verify it

  1. Verified by rendering the J2 template with sonic-cfggen.
  2. Verified by UT.

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

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

Description for the changelog

Update qos config to clear queues for bounced back traffic.

Link to config_db schema for YANG module changes

YANG model is not changed.

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

Signed-off-by: bingwang <bingwang@microsoft.com>
@neethajohn
Copy link
Contributor

neethajohn commented Mar 8, 2022

@bingwang-ms, we also need to add unit tests for the template rendering. refer to src/sonic-config-engine/tests/test_j2files.py

Signed-off-by: bingwang <bingwang@microsoft.com>
@bingwang-ms bingwang-ms force-pushed the clear_q_for_tunnel_remapping branch from f5fcedb to 6565c2e Compare March 9, 2022 11:40
Signed-off-by: bingwang <bingwang@microsoft.com>
Signed-off-by: bingwang <bingwang@microsoft.com>
@bingwang-ms
Copy link
Contributor Author

@neethajohn All comments are addressed, and UT is also updated and passing. Please help take a look. Thanks so much!

@bingwang-ms
Copy link
Contributor Author

The new QoS map for tunnel DSCP mapping is now covered in this PR. Will raise another PR to add that after this one is merged.

@bingwang-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@neethajohn
Copy link
Contributor

@bingwang-ms, can you also update the PR description to include all the changes done in this PR

Signed-off-by: bingwang <bingwang@microsoft.com>
Signed-off-by: bingwang <bingwang@microsoft.com>
Signed-off-by: bingwang <bingwang@microsoft.com>
@bingwang-ms
Copy link
Contributor Author

All comments addressed. Please take a look one more time. Thanks

Signed-off-by: bingwang <bingwang@microsoft.com>
Signed-off-by: bingwang <bingwang@microsoft.com>
Signed-off-by: bingwang <bingwang@microsoft.com>
@bingwang-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@bingwang-ms
Copy link
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bingwang-ms
Copy link
Contributor Author

YANG model update at #10444
PR testing will pass after #10444 is merged.

@bingwang-ms bingwang-ms merged commit b9dd1df into sonic-net:master Apr 5, 2022
@qiluo-msft
Copy link
Collaborator

This commit could not be cleanly cherry-picked to 202012. Please submit another PR.

neethajohn pushed a commit that referenced this pull request Jun 28, 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.
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.
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants