Skip to content

qos-sai:dwrr:cisco-8000:Handle non-multiasic part as well for the dshell-script change. #16315

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.

Sign up for GitHub

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

Merged
yejianquan merged 4 commits into from
Jan 13, 2025
Merged

qos-sai:dwrr:cisco-8000:Handle non-multiasic part as well for the dshell-script change. #16315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bbed02b
Handle non-multiasic part as well.
rraghav-cisco Jan 3, 2025
d2d8f5f
Change the PTF code to handle single or multi asic as well.
rraghav-cisco Jan 6, 2025
d4a4c79
Modified the logic to find if it is a single or multi asic.
rraghav-cisco Jan 7, 2025
fa3a6dc
Change the name of the function, remove '_and_run' part.
rraghav-cisco Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions tests/qos/qos_sai_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2638,7 +2638,7 @@ def change_lag_lacp_timer(self, duthosts, get_src_dst_asic_and_duts, tbinfo, nbr
"Changing lacp timer multiplier to default for %s in %s" % (neighbor_lag_member, peer_device))
vm_host.no_lacp_time_multiplier(neighbor_lag_member)

def copy_and_run_set_cir_script_cisco_8000(self, dut, ports, asic="", speed="10000000"):
def copy_set_cir_script_cisco_8000(self, dut, ports, asic="", speed="10000000"):
if dut.facts['asic_type'] != "cisco-8000":
raise RuntimeError("This function should have been called only for cisco-8000.")
dshell_script = '''
Expand All @@ -2658,8 +2658,12 @@ def set_port_cir(interface, rate):

script_path = "/tmp/set_scheduler.py"
dut.copy(content=dshell_script, dest=script_path)
if dut.sonichost.is_multi_asic:
dest = f"syncd{asic}"
else:
dest = "syncd"
dut.docker_copy_to_all_asics(
container_name=f"syncd{asic}",
container_name=dest,
Copy link
Copy Markdown
Contributor

@XuChen-MSFT XuChen-MSFT Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function copy_and_run_set_cir_script_cisco_8000() just upload dshell script to dut, not really run this script. why do we call it copy_and_run__ ?

Copy link
Copy Markdown
Collaborator

@yxieca yxieca Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is asic value not 'none' on single asic platforms? If the value is none, would it also solve the issue?

Copy link
Copy Markdown
Contributor Author

@rraghav-cisco rraghav-cisco Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is asic value not 'none' on single asic platforms? If the value is none, would it also solve the issue?

@yxieca : Yes, it was "0" for single-asic. That is why the original code failed. If it were "None" the original code would have worked.

src=script_path,
dst="/")

Expand All @@ -2686,7 +2690,7 @@ def set_cir_change(self, get_src_dst_asic_and_duts, dutConfig):
interfaces = match.group(1).split(' ')

# Set scheduler to 5 Gbps.
self.copy_and_run_set_cir_script_cisco_8000(
self.copy_set_cir_script_cisco_8000(
dut=dst_dut,
ports=interfaces,
asic=dst_index,
Expand Down
15 changes: 11 additions & 4 deletions tests/saitests/py3/sai_qos_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3847,17 +3847,24 @@ def runTest(self):
recv_pkt = scapy.Ether(received.packet)

if asic_type == 'cisco-8000':
cmd_opt = ""
if 'dst_asic_index' in self.test_params:
cmd_opt = "-n asic{}".format(self.test_params['dst_asic_index'])
out, err, ret = self.exec_cmd_on_dut(
self.dst_server_ip,
self.test_params['dut_username'],
self.test_params['dut_password'],
"show platform summary | egrep 'ASIC Count' | awk -F: '{print $2}'")
cmd_opt = "-n asic{}".format(self.test_params['dst_asic_index'])
if out[0].strip() == "1":
cmd_opt = ""
Copy link
Copy Markdown
Contributor

@XuChen-MSFT XuChen-MSFT Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we set scheduler to 5Gpps during the wrr test, but why don't we revert scheduler to default value after test complete?

Copy link
Copy Markdown
Contributor Author

@rraghav-cisco rraghav-cisco Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XuChen-MSFT : That was an artifact from earlier logic. Earlier this function used to copy and run, but now only copy. The run is done in the ptf code.

Copy link
Copy Markdown
Contributor Author

@rraghav-cisco rraghav-cisco Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XuChen-MSFT : Its not needed, since tx_enable is taking care of that part. It resets the scheduler as well.

Copy link
Copy Markdown
Contributor

@XuChen-MSFT XuChen-MSFT Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you update the function name of "copy_and_run_set_cir_script_cisco_8000()" to reflect its real behaviors

Copy link
Copy Markdown
Contributor

@sdszhang sdszhang Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XuChen-MSFT this is updated now.

cmd = "sudo show platform npu script {} -s set_scheduler.py".format(cmd_opt)
out, err, ret = self.exec_cmd_on_dut(
self.dst_server_ip,
self.test_params['dut_username'],
self.test_params['dut_password'],
cmd)
if err != "" and out == "":
if err and out == []:
raise RuntimeError("cmd({}) might have failed in the DUT. Error:{}".format(cmd, err))
else:
print("Success in setting scheduler in DUT.", file=sys.stderr)
else:
# Release port
self.sai_thrift_port_tx_enable(
Expand Down