-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[QoS] Add tunnel pipe mode support for IPIP Decap mode to use SAI_TUNNEL_ATTR_DECAP_QOS_DSCP_TO_TC_MAP #21868
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
[QoS] Add tunnel pipe mode support for IPIP Decap mode to use SAI_TUNNEL_ATTR_DECAP_QOS_DSCP_TO_TC_MAP #21868
Conversation
/azp run Azure.sonic-buildimage |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -65,7 +65,11 @@ | |||
{ | |||
"TUNNEL_DECAP_TABLE:IPINIP_SUBNET" : { | |||
"tunnel_type":"IPINIP", | |||
{% if "mlnx" in DEVICE_METADATA.localhost.platform or "cisco" in DEVICE_METADATA.localhost.platform %} |
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.
do we need to add arista or bcm here? @abhishekd-brcm
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.
we can't @r12f - will provide info async
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.
Why do we need platform check here? Also please note that Nvidia devices include both mlnx and nvidia in platform name. If you can clarify why this check is needed we need to consider adding nvidia in platform check.
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.
Some broadcom SKUs do not support pipe mode @dgsudharsan
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.
We should then have a check excluding brcm. Do we know if other vendors too have the same restriction?
@@ -65,7 +65,11 @@ | |||
{ | |||
"TUNNEL_DECAP_TABLE:IPINIP_SUBNET" : { | |||
"tunnel_type":"IPINIP", | |||
{% if "mlnx" in DEVICE_METADATA.localhost.platform or "cisco" in DEVICE_METADATA.localhost.platform %} |
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.
Why do we need platform check here? Also please note that Nvidia devices include both mlnx and nvidia in platform name. If you can clarify why this check is needed we need to consider adding nvidia in platform check.
Will modify buildimage tests shortly. Also testing if |
/azp run Azure.sonic-buildimage |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run Azure.sonic-buildimage |
Azure Pipelines successfully started running 1 pipeline(s). |
@dgsudharsan / @Yakiv-Huryk has Nvidia pushed their pipe mode sdk change into master branch? I ran pipe mode tests on the latest master build Steps to reproduce:
I noticed that all packets were getting routed based on the outer dscp value i.e. uniform mode. |
Commenter does not have sufficient privileges for PR 21868 in repo sonic-net/sonic-buildimage |
Waiting till 5/1 on Nvidia's given timeline for proposed changes to master branch. @stephenxs can you pls signoff in the interim. |
Pipe mode test passes after #22532
|
Please update sonic-mgmt test to description |
/azp run Azure.sonic-buildimage |
Azure Pipelines successfully started running 1 pipeline(s). |
added |
@kperumalbfn can you check why Test kvmtest-dualtor-t0 by Elastictest is not running again? |
@developfast 4hr max for each test and will be cancelled by the pipeline. I have restarted |
@dgsudharsan your requested change review is blocking PR merge, would you mind approving? |
@kperumalbfn the Test kvmtest-t0 by Elastictest keeps timing out - can we increase the timeout or bypass the check? Nothing is failing |
@prsunny @developfast @kperumalbfn Are we planning to take this change to 202505? If yes can we add label? |
…NEL_ATTR_DECAP_QOS_DSCP_TO_TC_MAP (sonic-net#21868) [QoS] Add tunnel pipe mode support for IPIP Decap mode to use SAI_TUNNEL_ATTR_DECAP_QOS_DSCP_TO_TC_MAP
Cherry-pick PR to 202505: #22715 |
Why I did it
To be ready for 202505 cutoff
Support for pipe mode in IPIP packets when decapped - request from NetScan. Current support is for all platforms except broadcom SKUs. SAI attribute
SAI_TUNNEL_ATTR_DECAP_QOS_DSCP_TO_TC_MAP
will be used since thedecap_dscp_to_tc_map
key is set in the template.Work item tracking
How I did it
How to verify it
Ran template change on a lab device.
Related sonic-mgmt test change - #17552
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Description for the changelog
Migrate dscp decap mode to pipe for all platforms except broadcom skus
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)