-
Notifications
You must be signed in to change notification settings - Fork 741
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
[Mellanox] Update dscp remapping cases for Nvidia platforms #8317
Conversation
This commit contains the fix for two test cases in DSCP remapping test. The reason that we need this fix is Nvidia implementation for DSCP remapping is different from Community due to the limitation that Nvidia platforms can't do the remap directly on tunnel. For test case test_tunnel_decap_dscp_to_pg_mapping: 1. Add a new mapping file tunnel_qos_map_nvidia.json and use it when testing Nvidia platforms. 2. Use cell size 144 for Nvidia platforms. 3. Use outer DSCP instead of inner DSCP. 4. Add some logs for easier debugging. For the xoff test case: 1. Add test data in qos.yml file for 4600C. 2. Stop packet aging in buffer before test. 3. Skip filling the leak on Nvidia platforms for it is not needed. 4. Use cell size 144 and packet length 300, small packets will cause descriptor exhaustion. Change-Id: If0090bd5bd664222cc7399598f7f46c10acebd1f
for k, v in list(maps['DSCP_TO_TC_MAP'][TUNNEL_MAP_NAME].items()): | ||
ret['inner_dscp_to_pg_map'][int(k)] = int( | ||
maps['TC_TO_PRIORITY_GROUP_MAP'][TUNNEL_MAP_NAME][v]) | ||
if is_nvidia_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.
better to avoid the if-else statements for Nvidia, and do it once at the beginning.
if nvidia:
param1 = A
param2= B
else:
param1 = C
param2 = D
func_a(param1, param2)
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.
Hi @roy-sror, can't do it here.
As you can see the params are named according to the mapping names defined in SONiC, and for Nvidia and other platforms, we are using different maps:
if is_nvidia_platform:
for k, v in maps['DSCP_TO_TC_MAP'][UPLINK_MAP_NAME].items():
ret['inner_dscp_to_pg_map'][int(k)] = int(
maps['TC_TO_PRIORITY_GROUP_MAP'][MAP_NAME][v])
else:
for k, v in maps['DSCP_TO_TC_MAP'][TUNNEL_MAP_NAME].items():
ret['inner_dscp_to_pg_map'][int(k)] = int(
maps['TC_TO_PRIORITY_GROUP_MAP'][TUNNEL_MAP_NAME][v])
It's very hard to unify the map names, and even if we can do that, the names will be long and the code will be hard to understand.
For Nvidia(Mellanox) platforms, packets in buffer will be aged after a timeout. Need to disable this before any buffer tests.
@neethajohn Could you help review the change? Thanks! |
/azp run Azure.sonic-mgmt |
Azure Pipelines successfully started running 1 pipeline(s). |
@congh-nvidia is it needed for other branches? |
Yes, it's needed for 202205, but there will be conflict, I'll open a new PR for it. Thanks. |
@liat-grozovik @bingwang-ms @neethajohn , this is the manual cherry-pick for 202205 #8713, please review, thanks. |
What is the motivation for this PR? Fix the following issue, it only happened in py3 env, not for py2. The error is introduced by #8317 Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
…t#8317) This PR contains the fix for two test cases in DSCP remapping test: test_tunnel_decap_dscp_to_pg_mapping and test_xoff_for_pcbb. The reason that we need this fix: Nvidia's implementation for DSCP remapping is different from Community. We need some additional mapping and the tunnel mode is pipe. For details please check the PR: [Mellanox] Support DSCP remapping in dual ToR topo on T0 switch sonic-buildimage#12605. Nvidia's cell size is 144. Need to stop packet aging in buffer. Need to add qos parameters for pcbb xoff test case.
What is the motivation for this PR? Fix the following issue, it only happened in py3 env, not for py2. The error is introduced by sonic-net#8317 Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
…t#8317) This PR contains the fix for two test cases in DSCP remapping test: test_tunnel_decap_dscp_to_pg_mapping and test_xoff_for_pcbb. The reason that we need this fix: Nvidia's implementation for DSCP remapping is different from Community. We need some additional mapping and the tunnel mode is pipe. For details please check the PR: [Mellanox] Support DSCP remapping in dual ToR topo on T0 switch sonic-buildimage#12605. Nvidia's cell size is 144. Need to stop packet aging in buffer. Need to add qos parameters for pcbb xoff test case.
What is the motivation for this PR? Fix the following issue, it only happened in py3 env, not for py2. The error is introduced by sonic-net#8317 Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
Description of PR
Summary:
This PR contains the fix for two test cases in DSCP remapping test: test_tunnel_decap_dscp_to_pg_mapping and test_xoff_for_pcbb.
The reason that we need this fix:
Type of change
Back port request
Approach
What is the motivation for this PR?
Support dscp remapping cases test_tunnel_decap_dscp_to_pg_mapping and test_xoff_for_pcbb on Nvidia platforms.
How did you do it?
Changes:
For test case test_tunnel_decap_dscp_to_pg_mapping:
For test_xoff_for_pcbb:
How did you verify/test it?
By automation, all cases passed on 4600C dualtor setup.
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation