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 2 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
6 changes: 5 additions & 1 deletion tests/qos/qos_sai_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
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
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 Down
16 changes: 14 additions & 2 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 @@ -3856,8 +3856,20 @@ def runTest(self):
self.test_params['dut_username'],
self.test_params['dut_password'],
cmd)
if err != "" and out == "":
raise RuntimeError("cmd({}) might have failed in the DUT. Error:{}".format(cmd, err))
if err and out == []:
if "Invalid value for " in " ".join(err):
Copy link
Contributor

@sdszhang sdszhang Jan 6, 2025

Choose a reason for hiding this comment

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

Instead of using fallback mechanism, can you check for the hwsku? you may need to add a function is_hwsku_multi_asic() for this.

cmd = "sudo show platform npu script -s set_scheduler.py"
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 == []:
raise RuntimeError("cmd({}) might have failed in the DUT. Error:{}".format(cmd, err))
else:
print("Success in setting scheduler in sAsic DUT.", file=sys.stderr)
else:
print("Success in setting scheduler in mAsic DUT.", file=sys.stderr)
else:
# Release port
self.sai_thrift_port_tx_enable(
Expand Down