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.

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

Jump to bottom

Support multiple IP addresses in LLDPRemManAddrTable #136

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

Support multiple IP addresses in LLDPRemManAddrTable #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5ad2c8e
Support multiple IP addresses in LLDPRemManAddrTable
Jun 18, 2020
e55cf14
Addressed Review comments
Jun 24, 2020
9aa6c94
Addedd Test cases for lldpRemManAddrIfSubtype, lldpRemManAddrIfId, ll…
Jun 24, 2020
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
54 changes: 28 additions & 26 deletions src/sonic_ax_impl/mibs/ieee802_1ab.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,26 +511,28 @@ def update_rem_if_mgmt(self, if_oid, if_name):
return
time_mark = int(lldp_kvs[b'lldp_rem_time_mark'])
remote_index = int(lldp_kvs[b'lldp_rem_index'])
subtype = self.get_subtype(mgmt_ip_str)
ip_hex = self.get_ip_hex(mgmt_ip_str, subtype)
if subtype == ManAddrConst.man_addr_subtype_ipv4:
addr_subtype_sub_oid = 4
mgmt_ip_sub_oid = (addr_subtype_sub_oid, *[int(i) for i in mgmt_ip_str.split('.')])
elif subtype == ManAddrConst.man_addr_subtype_ipv6:
addr_subtype_sub_oid = 6
mgmt_ip_sub_oid = (addr_subtype_sub_oid, *[int(i, 16) if i else 0 for i in mgmt_ip_str.split(':')])
else:
logger.warning("Invalid management IP {}".format(mgmt_ip_str))
return
self.if_range.append((time_mark,
if_oid,
remote_index,
subtype,
*mgmt_ip_sub_oid))

self.mgmt_ips.update({if_name: {"ip_str": mgmt_ip_str,
"addr_subtype": subtype,
"addr_hex": ip_hex}})

for mgmt_ip in mgmt_ip_str.split(','):
ipa = ipaddress.ip_address(mgmt_ip)
subtype = self.get_subtype(mgmt_ip)
ip_hex = self.get_ip_hex(mgmt_ip, subtype)

if subtype == ManAddrConst.man_addr_subtype_ipv4:
addr_subtype_sub_oid = 4
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."

else:
logger.warning("Invalid management IP {}".format(mgmt_ip_str))
return

mgmt_ip_sub_oid = (addr_subtype_sub_oid,) + tuple(i for i in ipa.packed)
sub_id = (if_oid,) + (remote_index,) + (subtype,) + mgmt_ip_sub_oid
Copy link
Contributor

@qiluo-msft qiluo-msft Jun 23, 2020 •
edited
Loading

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 •
edited
Loading

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.

self.if_range.append( (time_mark, *sub_id))

self.mgmt_ips[sub_id] = {if_name: {"ip_str": mgmt_ip,
"addr_subtype": subtype,
"addr_hex": ip_hex}}

except (KeyError, AttributeError) as e:
logger.warning("Error updating remote mgmt addr: {}".format(e))
return
Expand Down Expand Up @@ -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 •
edited
Loading

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

return None
sub_id = sub_id[1]
if sub_id not in self.oid_name_map:
sub_id_part = sub_id[1]
if sub_id_part not in self.oid_name_map:
return None
if_name = self.oid_name_map[sub_id]
if if_name not in self.mgmt_ips:
if_name = self.oid_name_map[sub_id_part]
if if_name not in self.mgmt_ips[sub_id[1:]]:
# no data for this interface
return None
return callable(sub_id, if_name)
Expand Down Expand Up @@ -625,14 +627,14 @@ def get_subtype(self, ip_str):
return None

def man_addr_subtype(self, sub_id, if_name):
return self.mgmt_ips[if_name]['addr_subtype']
return self.mgmt_ips[sub_id[1:]][if_name]['addr_subtype']

def man_addr(self, sub_id, if_name):
"""
:param sub_id:
:return: MGMT IP in HEX
"""
return self.mgmt_ips[if_name]['addr_hex']
return self.mgmt_ips[sub_id[1:]][if_name]['addr_hex']

@staticmethod
def man_addr_if_subtype(sub_id, _): return ManAddrConst.man_addr_if_subtype
Expand Down
49 changes: 47 additions & 2 deletions tests/test_lldp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,56 @@ def test_subtype_lldp_loc_man_addr_table(self):

def test_subtype_lldp_rem_man_addr_table(self):
samaity marked this conversation as resolved.
Show resolved Hide resolved
for entry in range(3, 6):
interface_number = 9
time_mark = 18543
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 1))
ret = mib_entry(sub_id=(time_mark, interface_number, 1, 1, 4, 10, 224, 25, 102, ))
self.assertIsNotNone(ret)
print(ret)

def test_lldp_rem_man_addr_if_subtype(self):
oid = ObjectIdentifier(20, 0, 1, 0, (1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, 3, 18543, 9, 1, 1, 4, 10, 224, 25, 102))
get_pdu = GetNextPDU(
header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0),
oids=[oid]
)

encoded = get_pdu.encode()
response = get_pdu.make_response(self.lut)
value0 = response.values[0]
self.assertEqual(value0.type_, ValueType.INTEGER)
self.assertEqual(str(value0.name), str(ObjectIdentifier(20, 0, 1, 0, (1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, 3, 18543, 9, 1, 1, 4, 10, 224, 25, 102))))
self.assertEqual(str(value0.data), '2')

def test_lldp_rem_man_addr_if_id(self):
oid = ObjectIdentifier(20, 0, 1, 0, (1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, 4, 18543, 9, 1, 1, 4, 10, 224, 25, 102))
get_pdu = GetNextPDU(
header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0),
oids=[oid]
)

encoded = get_pdu.encode()
response = get_pdu.make_response(self.lut)
value0 = response.values[0]
self.assertEqual(value0.type_, ValueType.INTEGER)
self.assertEqual(str(value0.name), str(ObjectIdentifier(20, 0, 1, 0, (1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, 4, 18543, 9, 1, 1, 4, 10, 224, 25, 102))))
self.assertEqual(str(value0.data), '0')

def test_lldp_rem_man_addr_oid(self):
oid = ObjectIdentifier(20, 0, 1, 0, (1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, 5, 18543, 9, 1, 1, 4, 10, 224, 25, 102))
get_pdu = GetNextPDU(
header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0),
oids=[oid]
)

encoded = get_pdu.encode()
response = get_pdu.make_response(self.lut)
value0 = response.values[0]
print(value0)
self.assertEqual(value0.type_, ValueType.OBJECT_IDENTIFIER)
self.assertEqual(str(value0.name), str(ObjectIdentifier(20, 0, 1, 0, (1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, 5, 18543, 9, 1, 1, 4, 10, 224, 25, 102))))
self.assertEqual(str(value0.data), '.1.3.6.1.2.1.2.2.1.1')

def test_local_port_identification(self):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 3, 7, 1, 3)]
ret = mib_entry(sub_id=(1,))
Expand Down Expand Up @@ -218,4 +263,4 @@ def test_getnextpdu_lldpRemSysCapEnabled(self):
value0 = response.values[0]
self.assertEqual(value0.type_, ValueType.OCTET_STRING)
self.assertEqual(str(value0.name), str(ObjectIdentifier(12, 0, 1, 0, (1, 0, 8802, 1, 1, 2, 1, 4, 1, 1, 12, 1, 1))))
self.assertEqual(str(value0.data), "\x28\x00")
self.assertEqual(str(value0.data), "\x28\x00")