-
Notifications
You must be signed in to change notification settings - Fork 717
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
QoSHdrmPoolsize _tx_disable_enable for all destination ports in lag #11008
Conversation
@vmittal-msft , kindly verify & approve |
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:
|
tests/saitests/py3/sai_qos_tests.py
Outdated
# Collect destination ports that may be in a lag | ||
pkt_dst_mac = self.router_mac if self.router_mac != '' else self.dst_port_mac | ||
dst_port_ids = [] | ||
for i in range(len(self.src_port_ids)): |
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.
Can you please clarify why dst ports and dependent on src_port_id ? Should we check if destination port is part of the lag then disable all ports under that lag ?
tests/saitests/py3/sai_qos_tests.py
Outdated
@@ -2733,7 +2739,9 @@ def runTest(self): | |||
pkts_num_egr_mem = int(self.test_params['pkts_num_egr_mem']) | |||
|
|||
# Pause egress of dut xmit port | |||
self.sai_thrift_port_tx_disable(self.dst_client, self.asic_type, [self.dst_port_id]) | |||
# Disable all dst ports |
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.
Also, Can we have this specific to dnx chassis ?
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.
done
assert (recv_counters[sidx_dscp_pg_tuples[i][2]] > | ||
recv_counters_bases[sidx_dscp_pg_tuples[i][0]][sidx_dscp_pg_tuples[i][2]]) | ||
self.sai_thrift_port_tx_enable(self.dst_client, self.asic_type, uniq_dst_ports) | ||
sys.exit("Too many pkts needed to trigger pfc: %d" % (pkt_cnt)) |
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.
same as above
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.
done
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:
|
add @kenneth-arista, @ysmanman for viz.. |
Hi @kenneth-arista, @ysmanman can you please take a look at this PR and check if this changes works on single asic LCs are well |
hi @ansrajpu-git @vmittal-msft the PR, #8775, which breaks the testQosSaiHeadroomPoolWatermark for box devices due to wrong intent in line 2386. And in current PR it seems changed this part, could you pls verify the change on ToR / Leaf before merge? |
…onic-net#11008) * QoSHdrmPoolsize_collecting dst ports in lag for tx_disable_enable
Cherry-pick PR to 202311: #12024 |
…onic-net#11008) * QoSHdrmPoolsize_collecting dst ports in lag for tx_disable_enable
Cherry-pick PR to 202305: #12025 |
…11008) * QoSHdrmPoolsize_collecting dst ports in lag for tx_disable_enable
…11008) * QoSHdrmPoolsize_collecting dst ports in lag for tx_disable_enable
@vmittal-msft, please help this merge to 202205 branch. |
…onic-net#11008) * QoSHdrmPoolsize_collecting dst ports in lag for tx_disable_enable
Description of PR
Summary:
Since testQosHeadroomPoolsize test takes more than one source port as input, even after tx disable, the packets could still hash from the other ports. Hence tx_disable has to be applied on all ports in the destination lag to block all the traffic during tx_disable.
Fixes # (issue)
Type of change
Back port request
Approach
Since testQosHeadroomPoolsize test takes more than one source port as input, even after tx disable, the packets could still hash from the other ports. Hence tx_disable has to be applied on all ports in the destination lag to block all the traffic during tx_disable.
This was achieved by
-Collecting all the dst_port_ids ports associated with the src_port_ids
-Sending all the dst_port_ids in a list, instead of sending a single dst_port_id in tx_disable
What is the motivation for this PR?
Intermitted failure of testQosHdrmPoolsize test
How did you do it?
Executed Qos test suite and verify the results
How did you verify/test it?
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation