Multi-asic support for snmp_queue_counters test by sanjair-git · Pull Request #13637 · sonic-net/sonic-mgmt

sanjair-git · 2024-07-11T18:25:38Z

Description of PR

Summary:
Fixes # (issue)

Added multi-asic support for the test 'test_snmp_queue_counters' under snmp

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?

Currently, 'test_snmp_queue_counters' test doesn't have multi-asic support and the test is failing for multi-asic chassis due to the lack of support.

How did you do it?

Added multi-asic support for test 'test_snmp_queue_counters' and made sure the current test logic is not affected.

How did you verify/test it?

Ran the above-mentioned test case on a T2 chassis and verified.

Any platform specific information?

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

Documentation

SuvarnaMeenakshi · 2024-07-29T06:50:38Z

tests/snmp/test_snmp_queue_counters.py

Instead of getting the configuration only of asic0 always, should we run the test for randomly selected one asic_index using enum_rand_one_asic_index

Modified the test to select random asic_index based on enum_rand_one_asic_index. Please check.

SuvarnaMeenakshi · 2024-07-29T06:55:23Z

tests/snmp/test_snmp_queue_counters.py

def load_new_cfg(duthost, data):
duthost.copy(content=json.dumps(data, indent=4), dest=CFG_DB_PATH) -- This is only copying to /etc/sonic/config_db.json file, whereas the configuration change should be applied to the asic namespace config_db
config_reload(duthost, config_source='config_db', safe_reload=True)

Modified the test to do configuration change based on specific asic namespace in case of multi-asic. Please check

SuvarnaMeenakshi · 2024-07-29T06:56:35Z

tests/snmp/test_snmp_queue_counters.py

Does the test always expect to have queue configuration for Ethernet0 and delete that?

Ethernet0 was chosen as a random arbitrary port when this test was added initially by Aryeh. Similarly, the current changes for multi-asic, Ethernet0 is being chosen for asic0 and Ethernet144 is being chosen for asic1.

Can we find the asic interface dynamically?
Not all HwSKU might have Ethernet144, for example the multi-asic VS does not have Ethernet144 interface https://github.com/sonic-net/sonic-buildimage/blob/master/device/virtual/x86_64-kvm_x86_64_4_asic-r0/msft_four_asic_vs/1/port_config.ini

Added code to select interface dynamically per asic on the dut. Please take a look. Tested the changes on T2 multi-asic dut and it works fine.

SuvarnaMeenakshi · 2024-07-29T06:58:06Z

tests/snmp/test_snmp_queue_counters.py

can this test be added to multi-asic VS test file so that it run on multi-asic VS testbed?

sonic-mgmt/.azure-pipelines/pr_test_scripts.yaml

Line 273 in 12ed07a

multi-asic-t1-lag:

Added this test to 'multi-asic-t1-lag' section.

sanjair-git · 2024-08-01T20:58:55Z

Ran the above test on multi-asic T2 chassis with all the above changes. Results attached.

arlakshm · 2024-09-03T22:33:24Z

@SuvarnaMeenakshi, can you sign-off on this PR?

SuvarnaMeenakshi · 2024-09-04T17:01:04Z

.azure-pipelines/pr_test_scripts.yaml

This is currently getting skipped in the PR checker for multi-asic VS

seems like this is broken for multi-aisc and is being skipped. #14007

seems like this is broken for multi-aisc and is being skipped. #14007

Yes, we got the same error as mentioned in #14007 once this test case was added to T2. This PR will fix this issue.

snmp - Typo fix

SuvarnaMeenakshi

lgtm

Approach What is the motivation for this PR? Currently, 'test_snmp_queue_counters' test doesn't have multi-asic support and the test is failing for multi-asic chassis due to the lack of support. How did you do it? Added multi-asic support for test 'test_snmp_queue_counters' and made sure the current test logic is not affected. How did you verify/test it? Ran the above-mentioned test case on a T2 chassis and verified.

rajendrat · 2024-10-21T22:00:09Z

viz @kevinwangsk . Please merge this to 202405 branch.

Approach What is the motivation for this PR? Currently, 'test_snmp_queue_counters' test doesn't have multi-asic support and the test is failing for multi-asic chassis due to the lack of support. How did you do it? Added multi-asic support for test 'test_snmp_queue_counters' and made sure the current test logic is not affected. How did you verify/test it? Ran the above-mentioned test case on a T2 chassis and verified.

mssonicbld · 2024-10-29T01:18:42Z

Cherry-pick PR to 202405: #15220

StormLiangMS · 2024-11-21T04:13:22Z

hi @sanjair-git could you help to take a look on the cherry pick PR, which has a PR test failure.

sanjair-git · 2024-11-21T18:58:18Z

hi @sanjair-git could you help to take a look on the cherry pick PR, which has a PR test failure.

Sure @StormLiangMS, I will take a look.

Approach What is the motivation for this PR? Currently, 'test_snmp_queue_counters' test doesn't have multi-asic support and the test is failing for multi-asic chassis due to the lack of support. How did you do it? Added multi-asic support for test 'test_snmp_queue_counters' and made sure the current test logic is not affected. How did you verify/test it? Ran the above-mentioned test case on a T2 chassis and verified.

sanjair-git · 2024-11-27T18:00:10Z

hi @sanjair-git could you help to take a look on the cherry pick PR, which has a PR test failure.

Hi @StormLiangMS, for 202405 branch, the changes with the fix have been merged as part of #15735. Taken care.

sanjair-git mentioned this pull request Jul 11, 2024

T2 Chassis - SNMP Queue counters value is not as expected after buffer queue deletion sonic-net/sonic-buildimage#19542

Closed

SuvarnaMeenakshi reviewed Jul 29, 2024

View reviewed changes

sanjair-git force-pushed the snmp-multiasic branch from 6cb39e1 to 3cae14f Compare August 1, 2024 19:48

sanjair-git force-pushed the snmp-multiasic branch from 3cae14f to 784dbdf Compare August 28, 2024 13:52

arlakshm previously approved these changes Sep 3, 2024

View reviewed changes

SuvarnaMeenakshi reviewed Sep 4, 2024

View reviewed changes

sanjair-git added 4 commits September 6, 2024 16:07

snmp-queue-counters: multi-asic support with fix

052c22c

snmp-queue: more changes for brcm-dnx voq chassis

1fc4523

Adding snmp_queue_counters test to multi-asic-t1-lag

b1eb5eb

snmp-queue: choose interface dynamically

4790e0d

snmp - Typo fix

sanjair-git dismissed arlakshm’s stale review via 4790e0d September 6, 2024 20:39

sanjair-git force-pushed the snmp-multiasic branch from 784dbdf to 4790e0d Compare September 6, 2024 20:39

SuvarnaMeenakshi approved these changes Sep 11, 2024

View reviewed changes

arlakshm approved these changes Sep 11, 2024

View reviewed changes

arlakshm merged commit b772899 into sonic-net:master Sep 11, 2024

arlakshm added the Request for 202405 branch label Sep 11, 2024

wsycqyz added the Approved for 202405 branch label Oct 29, 2024

mssonicbld added the Created PR to 202405 branch label Oct 29, 2024

mssonicbld mentioned this pull request Oct 29, 2024

[action] [PR:13637] Multi-asic support for snmp_queue_counters test #15220

Closed

8 tasks

sanjair-git mentioned this pull request Nov 26, 2024

[Cherry-pick] PR [13637]- Multi-asic support for snmp_queue_counters test #15751

Closed

8 tasks

Conversation

sanjair-git commented Jul 11, 2024

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanjair-git commented Aug 1, 2024

Uh oh!

arlakshm commented Sep 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SuvarnaMeenakshi left a comment

Choose a reason for hiding this comment

Uh oh!

rajendrat commented Oct 21, 2024

Uh oh!

mssonicbld commented Oct 29, 2024

Uh oh!

StormLiangMS commented Nov 21, 2024

Uh oh!

sanjair-git commented Nov 21, 2024

Uh oh!

sanjair-git commented Nov 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants