Skip to content

[202205 | Nvidia] Fix qos sai test for supporting LAG port (#9587)#10121

Merged
yxieca merged 1 commit intosonic-net:202205from
JibinBao:cherry_pick_qos_lag_202205
Sep 27, 2023
Merged

[202205 | Nvidia] Fix qos sai test for supporting LAG port (#9587)#10121
yxieca merged 1 commit intosonic-net:202205from
JibinBao:cherry_pick_qos_lag_202205

Conversation

@JibinBao
Copy link
Contributor

@JibinBao JibinBao commented Sep 25, 2023

Description of PR

Cherry-pick PR #9587 from master to 202205
Previously, on Nvidia devices, to make port congestion, we disable the port. So, if a LAG port is used as the tx port in the test, the LAG port will go down in 90s because the lacp pdus are also blocked, which will fail the tests. Therefore, we skip the test on the LAG port for QoS tests. Currently, to make the LAG port also support QoS test, we block the data plane queue instead of disabling the port. This change will work for all topo on Nvidia devices.

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012
  • 202205

Approach

What is the motivation for this PR?

Make Qos sai test support LAG port

How did you do it?

we block the data plane queue instead of disabling the port. So the control plane still work normally.

How did you verify/test it?

Run qos sai test on t1-lag-64 topo

Any platform specific information?

Any

Supported testbed topology if it's a new test case?

Any

Documentation

Description of PR
Previously, on Nvidia devices, to make port congestion, we disable the port. So, if a LAG port is used as the tx port in the test, the LAG port will go down in 90s because the lacp pdus are also blocked, which will fail the tests. Therefore, we skip the test on the LAG port for QoS tests. Currently, to make the LAG port also support QoS test, we block the data plane queue instead of disabling the port. This change will work for all topo on Nvidia devices.

Change-Id: I6580398b6038e6a850915c57dc6112cdb628ed99
@JibinBao
Copy link
Contributor Author

@bingwang-ms , Could you help review it?

@bingwang-ms
Copy link
Collaborator

Thanks for the fix. LGTM.

@bingwang-ms
Copy link
Collaborator

@yxieca Can you please help merge this PR to 202205? It's to fill a test gap on SN4600 T1. Thanks

@yxieca yxieca merged commit 205710f into sonic-net:202205 Sep 27, 2023
@bingwang-ms
Copy link
Collaborator

@JibinBao This PR cause test regression in 202205 branch. The issue has been fixed by PR #10205. Please ensure the change is always verified before raising a PR. Thanks

@JibinBao
Copy link
Contributor Author

JibinBao commented Oct 9, 2023

@JibinBao This PR cause test regression in 202205 branch. The issue has been fixed by PR #10205. Please ensure the change is always verified before raising a PR. Thanks

Hi @bingwang-ms, Thank you for your reminder kindly.
Actually, I have verified the fix on branch 202205 before raising PR.
My PR does not cause this issue. It is caused another PR.

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.

5 participants