-
Notifications
You must be signed in to change notification settings - Fork 744
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]Increasing LACP timer for lag ports for broadcom-dnx neighbor EOS host #14469
Conversation
The pre-commit check detected issues in the files touched by this pull request. Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
@vmittal-msft .Please review. |
9d88827
to
2b8339d
Compare
The pre-commit check detected issues in the files touched by this pull request. Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
can we run this test on dest being non portchanne ? |
@judyjoseph ,@arlakshm , in prior discussion we concluded not to skip portchannel. Please let me know your view. |
/Azp run Azure.sonic-mgmt |
Azure Pipelines successfully started running 1 pipeline(s). |
The pre-commit check detected issues in the files touched by this pull request. Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
@ansrajpu-git, can you fix the pre-commit failures? |
2b8339d
to
e556d78
Compare
@arlakshm, please review all checked passed now. |
@ansrajpu-git Looks like this change won't work if fanout is non EOS ? |
@vmittal-msft , setting Lacp timer is not supported for vsonic neighbors |
@ansrajpu-git how about any other HW vendor fanout switches? |
@vmittal-msft , For EOS based VMs, these changes should work. For vsonic based VMs, we are skipping the test. These changes are only for broadcom-dnx platform. |
/Azp Azure.sonic-mgmt |
Command 'Azure.sonic-mgmt' is not supported by Azure Pipelines. Supported commands
See additional documentation. |
@yejianquan can you please approve this change for 202405. This changes is for DNX platforms only |
dutTestParams["basicParams"]["platform_asic"] == "broadcom-dnx"): | ||
src_dut = get_src_dst_asic_and_duts['src_dut'] | ||
dst_dut = get_src_dst_asic_and_duts['dst_dut'] | ||
if src_dut.sonichost.is_multi_asic and dst_dut.sonichost.is_multi_asic: |
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.
What's the reason we only want to do this increase on multi-asic systems? The LACP timeout issue is a problem on single-asic systems as well right?
Pending on #15778 |
@arlakshm, please add tag for 202205 as well. |
Description of PR
Intermittently testQosSaiLossyQueue tests fails due to Port-channel flap on broadcom-dnx T2 Voq chassis.
The reason the port-channel goes down is because this test requires disabling TX on the egress port (which is a member of a port-channel)
With the huge buffer-size, it takes a longer time to send packets . This will result in the TX LACP packets to stop egressing, so after 3 LACP packets are missed (~90s) on the server side the LAG is torn down.
Issue # #11682
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
Intermittently testQosSaiLossyQueue tests fails due to Port-channel flap
How did you do it?
The lacp timer multiplier on the EOS host is configurable.
By default, timeout is 30 secs with a failure tolerance of 3.
We changed the multiplier to an increased value to hold the connectivity for some time until all packets are sent.
And revert the changes after test case execution.
How did you verify/test it?
Executed qos test cases and verfiy the results.
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation