Skip to content

Commit

Permalink
change swapsyncd to module level. (#15501)
Browse files Browse the repository at this point in the history
Description of PR
Summary:
change swapsyncd and disable_ipv6 to module level.
Running time on T2 reduced by around 140 minutes (from 685.08 minutes to 543.95 minutes).

Existing code will do swapSyncd for selected dut only in each iteration. In a T2 full test, it was around 8 LC which needs to do swapsyncd, including setup and teardown for each iteration.

select_src_dst_dut_and_asic	selected DUT to do swapsyncd (Existing)
single_asic	one downstream LC
single_dut_multi_asic	one downstream LC
multi_dut_longlink_to_shortlink	one upstream LC, one downstream LC
multi_dut_shortlink_to_shortlink	two downstream LC
multi_dut_shortlink_to_longlink	one upstream LC, one downstream LC
After the fix, swapsyncd will be done for all LCs at the beginning of the test. Saved around 5 LC swapsyncd time

select_src_dst_dut_and_asic	selected DUT to do swapsyncd (after fix)
Setup	one upstream LC, two downstream LC
single_asic	none
single_dut_multi_asic	none
multi_dut_longlink_to_shortlink	none
multi_dut_shortlink_to_shortlink	none
multi_dut_shortlink_to_longlink	none
Teardown	one upstream LC, two downstream LC
Type of change
 Bug fix
 Testbed and Framework(new/improvement)
 Test case(new/improvement)
Back port request
 202012
 202205
 202305
 202311
 202405
Approach
What is the motivation for this PR?
Reduce the run time for test_qos_sai module.

How did you do it?
change the swapsyncd fixture to module level. replace all dut with rpcsyncd container, instead of replacing selected dut multiple times for each iteration.

How did you verify/test it?
verified the physical testbed.

co-authorized by: jianquanye@microsoft.com
  • Loading branch information
sdszhang authored and mssonicbld committed Jan 2, 2025
1 parent 0c32259 commit cd861c2
Showing 1 changed file with 18 additions and 10 deletions.
28 changes: 18 additions & 10 deletions tests/qos/qos_sai_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,8 +563,8 @@ def __assignTestPortIps(self, mgFacts, topo):

return dutPortIps

@pytest.fixture(scope='class')
def swapSyncd_on_selected_duts(self, request, duthosts, get_src_dst_asic_and_duts, creds, tbinfo, lower_tor_host): # noqa F811
@pytest.fixture(scope='module')
def swapSyncd_on_selected_duts(self, request, duthosts, creds, tbinfo, lower_tor_host): # noqa F811
"""
Swap syncd on DUT host
Expand All @@ -575,6 +575,10 @@ def swapSyncd_on_selected_duts(self, request, duthosts, get_src_dst_asic_and_dut
Returns:
None
"""
if 'dualtor' in tbinfo['topo']['name']:
dut_list = [lower_tor_host]
else:
dut_list = duthosts.frontend_nodes
swapSyncd = request.config.getoption("--qos_swap_syncd")
public_docker_reg = request.config.getoption("--public_docker_registry")
try:
Expand All @@ -586,12 +590,12 @@ def swapSyncd_on_selected_duts(self, request, duthosts, get_src_dst_asic_and_dut
new_creds['docker_registry_password'] = ''
else:
new_creds = creds
for duthost in get_src_dst_asic_and_duts["all_duts"]:
for duthost in dut_list:
docker.swap_syncd(duthost, new_creds)
yield
finally:
if swapSyncd:
for duthost in get_src_dst_asic_and_duts["all_duts"]:
for duthost in dut_list:
docker.restore_default_syncd(duthost, new_creds)

@pytest.fixture(scope='class', name="select_src_dst_dut_and_asic",
Expand Down Expand Up @@ -1872,11 +1876,15 @@ def populateArpEntries(
yield
return

@pytest.fixture(scope='class', autouse=True)
def dut_disable_ipv6(self, duthosts, get_src_dst_asic_and_duts, tbinfo, lower_tor_host, # noqa F811
swapSyncd_on_selected_duts):
@pytest.fixture(scope='module', autouse=True)
def dut_disable_ipv6(self, duthosts, tbinfo, lower_tor_host, swapSyncd_on_selected_duts): # noqa F811
if 'dualtor' in tbinfo['topo']['name']:
dut_list = [lower_tor_host]
else:
dut_list = duthosts.frontend_nodes

all_docker0_ipv6_addrs = {}
for duthost in get_src_dst_asic_and_duts['all_duts']:
for duthost in dut_list:
try:
all_docker0_ipv6_addrs[duthost.hostname] = \
duthost.shell("sudo ip -6 addr show dev docker0 | grep global" + " | awk '{print $2}'")[
Expand All @@ -1891,14 +1899,14 @@ def dut_disable_ipv6(self, duthosts, get_src_dst_asic_and_duts, tbinfo, lower_to

yield

for duthost in get_src_dst_asic_and_duts['all_duts']:
for duthost in dut_list:
duthost.shell("sysctl -w net.ipv6.conf.all.disable_ipv6=0")
if all_docker0_ipv6_addrs[duthost.hostname] is not None:
logger.info("Adding docker0's IPv6 address since it was removed when disabing IPv6")
duthost.shell("ip -6 addr add {} dev docker0".format(all_docker0_ipv6_addrs[duthost.hostname]))

# TODO: parallelize this step.. Do we really need this ?
for duthost in get_src_dst_asic_and_duts['all_duts']:
for duthost in dut_list:
config_reload(duthost, config_source='config_db', safe_reload=True, check_intf_up_ports=True)

@pytest.fixture(scope='class', autouse=True)
Expand Down

0 comments on commit cd861c2

Please sign in to comment.