Skip to content

[SNMP]: Add SNMP test case to test over Loopback IP#2400

Merged
SuvarnaMeenakshi merged 8 commits intosonic-net:masterfrom
SuvarnaMeenakshi:snmp_loopback
Dec 7, 2020
Merged

[SNMP]: Add SNMP test case to test over Loopback IP#2400
SuvarnaMeenakshi merged 8 commits intosonic-net:masterfrom
SuvarnaMeenakshi:snmp_loopback

Conversation

@SuvarnaMeenakshi
Copy link
Contributor

Send SNMP query to DUT from one of the device neighbor
over LoopbackIP.

Signed-off-by: SuvarnaMeenakshi [email protected]

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Approach

What is the motivation for this PR?

Current SNMP test case executes SNMP query over management IP.
This testcase is to ensure SNMP query is reachable over Loopback IP.

How did you do it?

Execute SNMP command to sysDescr from a neighbor VM so that the SNMP query goes to front-panel interface.

How did you verify/test it?

Execute the newly added testcase test_snmp_loopback.py and ensure that it passes in single and multi-asic platforms.

Any platform specific information?

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

Documentation

Send SNMP query to DUT from one of the device neighbor
over LoopbackIP.

Signed-off-by: SuvarnaMeenakshi <[email protected]>
@SuvarnaMeenakshi
Copy link
Contributor Author

retest vsimage please

@SuvarnaMeenakshi SuvarnaMeenakshi marked this pull request as ready for review November 4, 2020 18:04
@SuvarnaMeenakshi
Copy link
Contributor Author

retest vsimage please

@lguohan
Copy link
Contributor

lguohan commented Nov 5, 2020

can you add this test into kvmtest.sh?

Signed-off-by: Ubuntu <sumeenak@sumeenak-ubuntu-vm0.q3q211rtov2etb23rce1vewych.cx.internal.cloudapp.net>
@SuvarnaMeenakshi
Copy link
Contributor Author

can you add this test into kvmtest.sh?

Added test into kvmtest.sh .

# Get first neighbor VM information
nbr = nbrhosts[list(nbrhosts.keys())[0]]

# Perform SNMP walk rom neighbor
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 6, 2020

Choose a reason for hiding this comment

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

rom [](start = 24, length = 3)

typo #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

nbr = nbrhosts[list(nbrhosts.keys())[0]]

# Perform SNMP walk rom neighbor
for ip in config_facts[u'LOOPBACK_INTERFACE'][u'Loopback0']:
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 6, 2020

Choose a reason for hiding this comment

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

Loopback0 [](start = 52, length = 9)

Do we have test case for management port?
Both cases can share the code, and in future we may have either one plus ipv4 or ipv6. Reuse case will make future extension easier. #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other SNMP test cases use management IP to query. The other SNMP test cases uses ansible snmp_facts module which will retrieve results over management IP. So in this test case, we specifically use IPv4 and IPv6 Loopback addresses present in config_db to query sysDescr OID. We can query any OID here, but chose one of the OIDs and compare it with information from snmp_facts.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see loopback ipv6 is covered here. But mgmt ipv6 is not covered by any test cases. Could you cover it in this PR?


In reply to: 518449100 [](ancestors = 518449100)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@qiluo-msft I have updated the test case to use only IPv4 Loopback IP.
The query over Ipv6 Loopback Interface / Management Interface does not work.
I have raised a github issue to look into this: https://github.com/Azure/sonic-snmpagent/issues/180

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comments

Signed-off-by: Ubuntu <sumeenak@sumeenak-ubuntu-vm0.q3q211rtov2etb23rce1vewych.cx.internal.cloudapp.net>
@lguohan
Copy link
Contributor

lguohan commented Nov 8, 2020

looks like new test failure. @SuvarnaMeenakshi , can you investigate?

@SuvarnaMeenakshi
Copy link
Contributor Author

looks like new test failure. @SuvarnaMeenakshi , can you investigate?

Made changes, we had to add ACL to allow SNMP query over Loopback Interface.

@qiluo-msft qiluo-msft requested a review from jleveque December 2, 2020 22:56
# and clean up after snmp query over the IP address is executed.
shell_cmd = "nohup /tmp/loopback_snmp_acls.sh " + str(ip) + " /dev/null > /dev/null 2>&1 &"
duthost.shell(shell_cmd)
eos_snmpwalk = 'bash snmpget -v2c -c ' + creds['snmp_rocommunity'] + ' ' + str(loip) + ' 1.3.6.1.2.1.1.1.0'
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 2, 2020

Choose a reason for hiding this comment

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

eos_snmpwalk [](start = 8, length = 12)

Variable name is 'walk', but actually 'get'. Could you make them no conflicting? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated variable name

# and clean up after snmp query over the IP address is executed.
shell_cmd = "nohup /tmp/loopback_snmp_acls.sh " + str(ip) + " /dev/null > /dev/null 2>&1 &"
duthost.shell(shell_cmd)
eos_snmpwalk = 'bash snmpget -v2c -c ' + creds['snmp_rocommunity'] + ' ' + str(loip) + ' 1.3.6.1.2.1.1.1.0'
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 2, 2020

Choose a reason for hiding this comment

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

creds['snmp_rocommunity'] [](start = 49, length = 25)

Better to quote. Suggest https://docs.python.org/3/library/shlex.html in python3 #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

Still applicable.


In reply to: 534552080 [](ancestors = 534552080)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

shell_cmd = "nohup /tmp/loopback_snmp_acls.sh " + str(ip) + " /dev/null > /dev/null 2>&1 &"
duthost.shell(shell_cmd)
eos_snmpwalk = 'bash snmpget -v2c -c ' + creds['snmp_rocommunity'] + ' ' + str(loip) + ' 1.3.6.1.2.1.1.1.0'
out = nbr['host'].eos_command(
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 2, 2020

Choose a reason for hiding this comment

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

( [](start = 37, length = 1)

Better not to wrap line. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated as per comment.

result = out[u'stdout_lines']
assert len(out[u'stdout_lines']) > 0, 'No result from snmpwalk'
for line in out[u'stdout_lines'][0]:
assert snmp_facts['ansible_sysdescr'] in line, 'Sysdescr not found in SNMP result from loopbackIP.'
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 2, 2020

Choose a reason for hiding this comment

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

loopbackIP [](start = 99, length = 10)

Better print the IP, it will also work if you reuse code for mgmt IPs. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated as per comment.

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comments

to allow SNMP query over loopback IP.
Update as per review comments.

Signed-off-by: Suvarna Meenakshi <[email protected]>
Signed-off-by: Suvarna Meenakshi <[email protected]>
Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

LGTM. Please check with other reviewers.

@SuvarnaMeenakshi
Copy link
Contributor Author

retest vsimage please


pytestmark = [
pytest.mark.topology('any'),
pytest.mark.device_type('vs')
Copy link
Contributor

Choose a reason for hiding this comment

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

does it mean it will run only on vs ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is marked to run on vs test, but when running tests, if we specify the name of the test file, that specific test case will be executed on physical DUT or on vs, and marker will not be used. If pytest is triggered to execute all vs marked tests, then this test will be a part of that. Added this as other SNMP test cases have this marker.

iptables_cmd = "ip6tables"
return None

ip_tbl_rule_add = "sudo {} -I INPUT 1 -p udp --dport 161 -d {} -j ACCEPT".format(iptables_cmd, ip)
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this iptable rule needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By default, packets to loopback interface are dropped. So to accept snmp packets over loopback IP, we have to add this rule above the default rule.
DROP all anywhere fc00:1::32 /
DROP all -- anywhere 10.1.0.32

@SuvarnaMeenakshi
Copy link
Contributor Author

retest vsimage please

@SuvarnaMeenakshi
Copy link
Contributor Author

retest vsimage please

@xumia
Copy link
Collaborator

xumia commented Dec 5, 2020

retest vsimage please

@SuvarnaMeenakshi SuvarnaMeenakshi merged commit 0ca933d into sonic-net:master Dec 7, 2020
@SuvarnaMeenakshi SuvarnaMeenakshi deleted the snmp_loopback branch December 7, 2020 20:32
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
…submodule head (sonic-net#11705)

Kernel:
* 86c4b66 2022-07-28 | [Mellanox] Add new kernel patches from HW-MGMT package V.7.0020.3005 (sonic-net#287) (HEAD -> 202205) [Kebo Liu]
* 3a8416a 2022-07-05 | [patch] mlxsw: i2c: Prevent transaction execution for special chip (sonic-net#279) [Stepan Blyshchak]

swss:
* 3f69944 2022-08-10 | Set internal class state to reflect the actual state (sonic-net#2410) (HEAD -> 202205, tag: foo) [Prince Sunny]
* 87e98eb 2022-08-09 | [portsorch] Expose supported FEC modes to STABE_DB and check whether FEC mode is supported before setting it (sonic-net#2400) [Stephen Sun]
* e71ab99 2022-07-29 | portsorch: initial support for link-training (sonic-net#2359) [Dante (Kuo-Jung) Su]
* ed5e5be 2022-07-08 | Port configuration incremental update support (sonic-net#2305) [Junchao-Mellanox]

utilities:
* 0df3ba8 2022-08-12 | Revert "Improve the way to check port type of RJ45 port (sonic-net#2249)" (HEAD -> 202205) [Ying Xie]
* 9b21903 2022-08-12 | Fix test failure in dump table test in 202205 (sonic-net#2307) (HEAD -> 202205, github/202205) [Stephen Sun]
* 750d1db 2022-08-11 | Convert IPv6 addresses to lowercase in apply-patch (sonic-net#2299) (HEAD -> 202205) [dbarashinvd]
* 555947e 2022-08-09 | [config][muxcable] add support to enable/disable ycable telemetry (sonic-net#2297) [vdahiya12]
* 978f416 2022-08-09 | Fix GCU bug when backend service modifying config (sonic-net#2295) [jingwenxie]
* 8fed381 2022-08-02 | [intfutil] Check whether the FEC mode is supported on the platform before configuring it to CONFIG_DB (sonic-net#2223) (github/202205) [Stephen Sun]
* a1a09e4 2022-07-29 | Improve the way to check port type of RJ45 port (sonic-net#2249) [Stephen Sun]
* 9bdbfb8 2022-05-19 | sonic-utils: initial support for link-training (sonic-net#2071) [Dante (Kuo-Jung) Su]
* c088ec4 2022-08-10 | Support to enable fips for the command sonic_installer (sonic-net#2154) (sonic-net#2303) [xumia]

platform-daemon:
* 767cfb6 2022-08-09 | [ycabled] add capability to enable/disable telemetry (sonic-net#279) (HEAD -> 202205) [vdahiya12]

linkmgrd:
* cf1ba2b 2022-08-12 | wait for handler to be completed (sonic-net#114) (HEAD -> 202205, github/202205) [Jing Zhang]
* e99026c 2022-08-11 | [lgtm]: add uuid-dev to lgtm prepare (sonic-net#112) (HEAD -> 202205) [Jing Zhang]
* bd1b7f0 2022-08-11 | Adjust `DbInterfaceRaceConditionCheck` to Wait Longer for Handlers to be executed (sonic-net#111) (HEAD -> 202205, github/202205) [Jing Zhang]
* e9dc6b2 2022-08-11 | Backoff mux probing for server down scenario (sonic-net#106) [Jing Zhang]
* 0d61171 2022-08-09 | Fix race condition caused by strand `wrap` method (sonic-net#104) [Jing Zhang]
* e9ede7d 2022-07-02 | Enforce switch after config mux to active (sonic-net#95) [Longxiang Lyu]
* 15dbc30 2022-06-30 | Add unittest to verify mux toggle active (sonic-net#94) [Longxiang Lyu]

Signed-off-by: Ying Xie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants