Update qos sai cases for ipv6 only topo (#20798)#879
Conversation
|
Hi @r12f , Can you please review it |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| self.__deleteTmpSwitchConfig(duthost) | ||
|
|
||
| @pytest.fixture(scope='class', autouse=True) | ||
| def update_delay_first_probe_time_for_v6_top(self, get_src_dst_asic_and_duts, tbinfo, dutConfig): |
There was a problem hiding this comment.
can we rename the function to "update_delay_first_probe_time_for_v6_topo" or "update_delay_first_probe_time_for_v6_topology" ?
| ecn=ecn, | ||
| ttl=ttl) | ||
| if ip_type == 'ipv6': | ||
| pkt = construct_ipv6_pkt(packet_length, |
There was a problem hiding this comment.
there are many repetitive conditional checks like this.
This pattern appears throughout the code, even though the IPv4 and IPv6 functions are very similar. The challenge is to use a unified API by redirecting function pointers at runtime based on the ip_type parameter.
can we Leverage Python's dynamic nature to bind different function implementations to the same attribute name during test initialization. This eliminates repetitive conditionals in the test logic.
for example:
def setUp(self):
"""Initialize and setup IP version specific function bindings"""
super().setUp()
# Get IP type from test parameters
ip_type = self.test_params.get('ip_type', 'ipv4')
# Dynamically bind functions based on IP type
# This creates instance attributes that point to different implementations
if ip_type == 'ipv6':
# Redirect: IPv4 API name points to IPv6 implementation
self.construct_ip_pkt = construct_ipv6_pkt
self._get_rx_port_func = get_rx_port_ipv6
else:
# IPv4: use original implementations
self.construct_ip_pkt = construct_ip_pkt
self._get_rx_port_func = get_rx_port
def runTest(self):
# ... parameter initialization ...
# Unified API call - no conditional logic needed
# Automatically routes to correct implementation
pkt = self.construct_ip_pkt(
packet_length,
pkt_dst_mac,
src_port_mac,
src_port_ip,
dst_port_ip,
dscp,
src_port_vlan,
ecn=ecn,
ttl=ttl
)
# Unified API call - no conditional logic needed
dst_port_id = self._get_rx_port_func(
self, 0, src_port_id, pkt_dst_mac,
dst_port_ip, src_port_ip, src_port_vlan
)
There was a problem hiding this comment.
Since the original PR was merged to master branch. Can we keep it as it?
If need, can we open a ticket and then optimize it?
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
* Update qos sai case for ipv6 only topo 1. Update the method to get the test port and ip for v6 2. Update the method to generate the arp for v6 3. Update the method to get the dst port for v6 4. Update the method to send v6 packet
JibinBao
force-pushed
the
cp_qos_sai_v6_2412
branch
from
7e3491a to
4b93eba
Compare
|
Hi @r12f, |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
Description of PR
Update qos sai cases for ipv6 only topo
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
Update qos sai case for ipv6 only topo
How did you do it?
How did you verify/test it?
Run qos sai tests on v6 topo
Any platform specific information?
SPC5
Supported testbed topology if it's a new test case?
V6 topo
Documentation