[multi-asic fix] fix new change in test_route_perf.py to support multi-asic by wenyiz2021 · Pull Request #15452 · sonic-net/sonic-mgmt

wenyiz2021 · 2024-11-07T23:53:45Z

Description of PR

Summary:
Fixes # (issue)
for multi-asic devices, when asic is broadcom, using "bcmcmd" needs to specify asic id.

TODO:
depending on type of asic, e.g. xgs/dnx, we need to confirm with broadcom if the asic supports "conf show l3_alpm_enable" command, if not, skip for that platform.

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

mssonicbld · 2024-11-07T23:53:47Z

/azp run

azure-pipelines · 2024-11-07T23:53:58Z

Azure Pipelines successfully started running 1 pipeline(s).

arista-nwolfe

LGTM, thanks for fixing this

mssonicbld · 2024-11-08T20:23:26Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...............................................................Passed check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/route/test_route_perf.py:68:121: E501 line too long (122 > 120 characters) flake8...............................................(no files to check)Skipped check conditional mark sort..............................................Passed

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

wenyiz2021 · 2024-11-11T19:11:20Z

@arlakshm @judyjoseph can you help merge?

wenyiz2021 · 2024-11-13T17:58:21Z

@yejianquan can you help backport this PR to 202405? thanks

…i-asic (sonic-net#15452) Description of PR Summary: Fixes # (issue) for multi-asic devices, when asic is broadcom, using "bcmcmd" needs to specify asic id.

mssonicbld · 2024-11-14T06:16:16Z

Cherry-pick PR to 202405: #15557

…i-asic (#15452) Description of PR Summary: Fixes # (issue) for multi-asic devices, when asic is broadcom, using "bcmcmd" needs to specify asic id.

…i-asic (sonic-net#15452) Description of PR Summary: Fixes # (issue) for multi-asic devices, when asic is broadcom, using "bcmcmd" needs to specify asic id.

lolyu · 2024-11-19T05:51:11Z

This is causing regression on dualtor, @wenyiz2021, could you please help check/fix the following failure?

enum_rand_one_frontend_asic_index = None
tbinfo = {'auto_recover': 'True', 'comment': 'v-saidia', 'conf-name': 'vms21-dual-t0-7260', 'duts': ['str2-7260cx3-acs-10', 'str2-7260cx3-acs-11'], ...}

    @pytest.fixture
    def check_config(duthosts, enum_rand_one_per_hwsku_frontend_hostname, enum_rand_one_frontend_asic_index, tbinfo):
        if tbinfo["topo"]["type"] in ["m0", "mx"]:
            return
    
        duthost = duthosts[enum_rand_one_per_hwsku_frontend_hostname]
        asic = duthost.facts["asic_type"]
        asic_id = enum_rand_one_frontend_asic_index
    
        if (asic == "broadcom"):
            broadcom_cmd = "bcmcmd -n " + str(asic_id) if duthost.is_multi_asic else "bcmcmd"
            alpm_cmd = "{} {}".format(broadcom_cmd, "conf show l3_alpm_enable")
            alpm_enable = duthost.command(alpm_cmd)["stdout_lines"][2].strip()
            logger.info("Checking config: {}".format(alpm_enable))
>           pytest_assert(alpm_enable == "l3_alpm_enable=2", "l3_alpm_enable is not set for route scaling")
E           Failed: l3_alpm_enable is not set for route scaling

arista-nwolfe · 2024-11-19T16:54:48Z

This is causing regression on dualtor, @wenyiz2021, could you please help check/fix the following failure?

enum_rand_one_frontend_asic_index = None
tbinfo = {'auto_recover': 'True', 'comment': 'v-saidia', 'conf-name': 'vms21-dual-t0-7260', 'duts': ['str2-7260cx3-acs-10', 'str2-7260cx3-acs-11'], ...}

    @pytest.fixture
    def check_config(duthosts, enum_rand_one_per_hwsku_frontend_hostname, enum_rand_one_frontend_asic_index, tbinfo):
        if tbinfo["topo"]["type"] in ["m0", "mx"]:
            return
    
        duthost = duthosts[enum_rand_one_per_hwsku_frontend_hostname]
        asic = duthost.facts["asic_type"]
        asic_id = enum_rand_one_frontend_asic_index
    
        if (asic == "broadcom"):
            broadcom_cmd = "bcmcmd -n " + str(asic_id) if duthost.is_multi_asic else "bcmcmd"
            alpm_cmd = "{} {}".format(broadcom_cmd, "conf show l3_alpm_enable")
            alpm_enable = duthost.command(alpm_cmd)["stdout_lines"][2].strip()
            logger.info("Checking config: {}".format(alpm_enable))
>           pytest_assert(alpm_enable == "l3_alpm_enable=2", "l3_alpm_enable is not set for route scaling")
E           Failed: l3_alpm_enable is not set for route scaling

I think @vivekverma-arista has a fix for this:
#15620

Description of PR Summary: Fixes #323 Approach What is the motivation for this PR? Regression due to #15452 How did you do it? Added missing quotes to the command. How did you verify/test it? Ran route/test_route_perf.py on Arista 7260CX3 platform with dualtor topology. co-authorized by: jianquanye@microsoft.com

…i-asic (sonic-net#15452) Description of PR Summary: Fixes # (issue) for multi-asic devices, when asic is broadcom, using "bcmcmd" needs to specify asic id.

Description of PR Summary: Fixes sonic-net#323 Approach What is the motivation for this PR? Regression due to sonic-net#15452 How did you do it? Added missing quotes to the command. How did you verify/test it? Ran route/test_route_perf.py on Arista 7260CX3 platform with dualtor topology. co-authorized by: jianquanye@microsoft.com

Description of PR Summary: Fixes #323 Approach What is the motivation for this PR? Regression due to #15452 How did you do it? Added missing quotes to the command. How did you verify/test it? Ran route/test_route_perf.py on Arista 7260CX3 platform with dualtor topology. co-authorized by: jianquanye@microsoft.com Co-authored-by: Vivek Verma <137406113+vivekverma-arista@users.noreply.github.com>

Update test_route_perf.py

f81dd1c

Update pr_test_scripts.yaml

dd802af

wenyiz2021 requested review from arista-nwolfe, arlakshm and kenneth-arista November 8, 2024 00:13

arista-nwolfe approved these changes Nov 8, 2024

View reviewed changes

Update test_route_perf.py

4f9aeea

Update test_route_perf.py

2e86124

wenyiz2021 added the Request for 202405 branch label Nov 11, 2024

wenyiz2021 requested a review from judyjoseph November 11, 2024 18:15

wenyiz2021 self-assigned this Nov 11, 2024

arlakshm approved these changes Nov 11, 2024

View reviewed changes

arlakshm merged commit 91ea307 into sonic-net:master Nov 11, 2024

wenyiz2021 deleted the route_perf branch November 12, 2024 23:26

yejianquan added the Approved for 202405 branch label Nov 14, 2024

mssonicbld added the Created PR to 202405 branch label Nov 14, 2024

mssonicbld mentioned this pull request Nov 14, 2024

[action] [PR:15452] [multi-asic fix] fix new change in test_route_perf.py to support multi-asic #15557

Merged

8 tasks

mssonicbld added Included in 202405 branch and removed Created PR to 202405 branch labels Nov 14, 2024

vivekverma-arista mentioned this pull request Nov 19, 2024

Fix routes/test_route_perf.py #15620

Merged

8 tasks

mssonicbld mentioned this pull request Nov 21, 2024

[action] [PR:15620] Fix routes/test_route_perf.py #15672

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[multi-asic fix] fix new change in test_route_perf.py to support multi-asic#15452

[multi-asic fix] fix new change in test_route_perf.py to support multi-asic#15452
arlakshm merged 4 commits intosonic-net:masterfrom
wenyiz2021:route_perf

wenyiz2021 commented Nov 7, 2024 •

edited

Loading

Uh oh!

mssonicbld commented Nov 7, 2024

Uh oh!

azure-pipelines Bot commented Nov 7, 2024

Uh oh!

arista-nwolfe left a comment

Uh oh!

mssonicbld commented Nov 8, 2024

Uh oh!

wenyiz2021 commented Nov 11, 2024

Uh oh!

wenyiz2021 commented Nov 13, 2024

Uh oh!

mssonicbld commented Nov 14, 2024

Uh oh!

lolyu commented Nov 19, 2024

Uh oh!

arista-nwolfe commented Nov 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

wenyiz2021 commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

mssonicbld commented Nov 7, 2024

Uh oh!

azure-pipelines Bot commented Nov 7, 2024

Uh oh!

arista-nwolfe left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Nov 8, 2024

Uh oh!

wenyiz2021 commented Nov 11, 2024

Uh oh!

wenyiz2021 commented Nov 13, 2024

Uh oh!

mssonicbld commented Nov 14, 2024

Uh oh!

lolyu commented Nov 19, 2024

Uh oh!

arista-nwolfe commented Nov 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wenyiz2021 commented Nov 7, 2024 •

edited

Loading