Skip to content
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.

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

Support multiple IP addresses in LLDPRemManAddrTable #136

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

samaity
Copy link
Contributor

@samaity samaity commented Jun 18, 2020

Signed-off-by: Sangita Maity samaity@linkedin.com

LLDPRemManAddrTable now supports multiple IP address just like lldpLocManAddrTable.

- What I did
Currently, Error occurs when "lldp_rem_man_addr" field has a list of IP strings like below.

19) "lldp_rem_man_addr"
20) "10.0.1.1,2000:0:1::1"

Before the fix
was getting log message like below.
"May 20 21:34:42.691758 lnos-x1-a-csw05 INFO snmp#snmp-subagent [ax_interface] ERROR: Invalid mgmt IP 10.0.1.1,2000:0:1::1 ---- multiple IP Addresses

LLDPRemManAddrTable now supports multiple IP address just like lldpLocManAddrTable.

- How I did it
Used split method to separate multiple mgmt_ip, and write a string to redis database.
- How to verify it

no error log in Syslog.

successfully able to snmpwalk 1.0.8802.1.1.2.1.4.2 oid. However,

root@lnos-x1-a-csw05:/# snmpwalk -v2c -c public 127.0.0.1 1.0.8802.1.1.2.1.4.2
iso.0.8802.1.1.2.1.4.2.1.1.796102.1.8.1.4.10.0.1.1 = INTEGER: 1
iso.0.8802.1.1.2.1.4.2.1.1.796102.1.8.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.1 = INTEGER: 2
iso.0.8802.1.1.2.1.4.2.1.1.1148938.5.6.1.4.10.0.1.2 = INTEGER: 1
iso.0.8802.1.1.2.1.4.2.1.1.1148938.5.6.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.2 = INTEGER: 2
iso.0.8802.1.1.2.1.4.2.1.2.796102.1.8.1.4.10.0.1.1 = STRING: "0A 00 01 01"
iso.0.8802.1.1.2.1.4.2.1.2.796102.1.8.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.1 = STRING: "2000 0 1 0 1"
iso.0.8802.1.1.2.1.4.2.1.2.1148938.5.6.1.4.10.0.1.2 = STRING: "0A 00 01 02"
iso.0.8802.1.1.2.1.4.2.1.2.1148938.5.6.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.2 = STRING: "2000 0 1 0 2"
iso.0.8802.1.1.2.1.4.2.1.3.796102.1.8.1.4.10.0.1.1 = INTEGER: 2
iso.0.8802.1.1.2.1.4.2.1.3.796102.1.8.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.1 = INTEGER: 2
iso.0.8802.1.1.2.1.4.2.1.3.1148938.5.6.1.4.10.0.1.2 = INTEGER: 2
iso.0.8802.1.1.2.1.4.2.1.3.1148938.5.6.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.2 = INTEGER: 2
iso.0.8802.1.1.2.1.4.2.1.4.796102.1.8.1.4.10.0.1.1 = INTEGER: 0
iso.0.8802.1.1.2.1.4.2.1.4.796102.1.8.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.1 = INTEGER: 0
iso.0.8802.1.1.2.1.4.2.1.4.1148938.5.6.1.4.10.0.1.2 = INTEGER: 0
iso.0.8802.1.1.2.1.4.2.1.4.1148938.5.6.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.2 = INTEGER: 0
iso.0.8802.1.1.2.1.4.2.1.5.796102.1.8.1.4.10.0.1.1 = OID: iso.3.6.1.2.1.2.2.1.1
iso.0.8802.1.1.2.1.4.2.1.5.796102.1.8.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.1 = OID: iso.3.6.1.2.1.2.2.1.1
iso.0.8802.1.1.2.1.4.2.1.5.1148938.5.6.1.4.10.0.1.2 = OID: iso.3.6.1.2.1.2.2.1.1
iso.0.8802.1.1.2.1.4.2.1.5.1148938.5.6.2.16.32.0.0.0.0.1.0.0.0.0.0.0.0.0.0.2 = OID: iso.3.6.1.2.1.2.2.1.1

- Description for the changelog

Signed-off-by: Sangita Maity <samaity@linkedin.com>
logger.warning("Invalid management IP {}".format(mgmt_ip_str))
return

sub_id = (if_oid,) + (remote_index,) + (subtype,) + mgmt_ip_sub_oid
Copy link
Contributor

@qiluo-msft qiluo-msft Jun 23, 2020

Choose a reason for hiding this comment

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

(remote_index,) + (subtype,) + mgmt_ip_sub_oid [](start = 37, length = 46)

You don't need first element in other places, why not just remove? #Closed

Copy link
Contributor Author

@samaity samaity Jun 24, 2020

Choose a reason for hiding this comment

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

Yeah, I tried to do it. But to maintain the uniformity with other OS, I kept it this way. Also, during testing, I faced issues to pass some test cases which is currently not part of the current repo like test_ipv6_rem_man_addr, test_ipv4_rem_man_addr etc. as those test cases were removed from repo by #112 PR.

mgmt_ip_sub_oid = (addr_subtype_sub_oid,) + tuple(i for i in ipa.packed)
elif subtype == ManAddrConst.man_addr_subtype_ipv6:

addr_subtype_sub_oid = 16
Copy link
Contributor

Choose a reason for hiding this comment

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

16 [](start = 43, length = 2)

Could you double check whether it is 6?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I checked other OS like cisco and did the snmpwalk for the same oid i.e. 1.0.8802.1.1.2.1.4.2. It's 16.

OBJECT  peerAddressType
     SYNTAX  InetAddressType { ipv4(1), ipv6(2) }
     DESCRIPTION
         "An implementation is only required to support IPv4
          and IPv6 addresses."

     OBJECT  peerAddress
     SYNTAX  InetAddress (SIZE(4|16))
     DESCRIPTION
         "An implementation is only required to support IPv4
          and globally unique IPv6 addresses."

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: Sangita Maity <samaity@linkedin.com>
@@ -591,11 +593,11 @@ def get_next(self, sub_id):
def lookup(self, sub_id, callable):
if len(sub_id) == 0:
Copy link
Contributor

@qiluo-msft qiluo-msft Jun 25, 2020

Choose a reason for hiding this comment

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

if len(sub_id) == 0 [](start = 8, length = 19)

What happened if len(sub_id) == 1

…dpRemManAddrOID.

Signed-off-by: Sangita Maity <samaity@linkedin.com>
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.

2 participants