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

Updated lldpRemManAddrTable to use all the management ip address associated with interface. #201

Merged
merged 13 commits into from
Mar 15, 2021
58 changes: 23 additions & 35 deletions src/sonic_ax_impl/mibs/ieee802_1ab.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,10 +494,9 @@ def __init__(self):
# establish connection to application database.
Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB)
self.if_range = []
self.mgmt_ips = {}
self.if_with_mgmt_ips = set()
self.oid_name_map = {}
self.mgmt_oid_name_map = {}
self.mgmt_ip_str = None
self.pubsub = [None] * len(self.db_conn)

def update_rem_if_mgmt(self, if_oid, if_name):
Expand All @@ -511,28 +510,26 @@ def update_rem_if_mgmt(self, if_oid, if_name):
if len(mgmt_ip_str) == 0:
# the peer advertise an emtpy mgmt address
return
time_mark = int(lldp_kvs['lldp_rem_time_mark'])
remote_index = int(lldp_kvs['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 set(mgmt_ip_str.split(',')):
Copy link
Contributor

@qiluo-msft qiluo-msft Mar 13, 2021

Choose a reason for hiding this comment

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

set [](start = 27, length = 3)

What is the benefit of set ? Just use splited list? #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.

this is just extra check in case if we have same ip's

Copy link
Contributor

Choose a reason for hiding this comment

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

Please be aware that different ipv6 strings could be the same ipv6 address.


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

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 Updated.

time_mark = int(lldp_kvs['lldp_rem_time_mark'])
remote_index = int(lldp_kvs['lldp_rem_index'])
subtype = self.get_subtype(mgmt_ip)
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.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.split(':')])
Copy link
Contributor

@qiluo-msft qiluo-msft Mar 13, 2021

Choose a reason for hiding this comment

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

[int(i, 16) if i else 0 for i in mgmt_ip.split(':')] [](start = 62, length = 52)

Seems you are trying to parse ipv6

[int(i, 16) if i else 0 for i in mgmt_ip.split(':')]

but this is not working for general cases #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.

@qiluo-msft Updated to use exploded API of ipaddress module.
https://docs.python.org/3/library/ipaddress.html

else:
logger.warning("Invalid management IP {}".format(mgmt_ip))
continue
self.if_range.append((time_mark,
if_oid,
remote_index,
subtype,
*mgmt_ip_sub_oid))
self.if_with_mgmt_ips.add(if_name)

except (KeyError, AttributeError) as e:
logger.warning("Error updating remote mgmt addr: {}".format(e))
return
Expand Down Expand Up @@ -577,7 +574,7 @@ def reinit_data(self):
Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB)

self.if_range = []
self.mgmt_ips = {}
self.if_with_mgmt_ips = set()
for if_oid, if_name in self.oid_name_map.items():
self.update_rem_if_mgmt(if_oid, if_name)

Expand All @@ -594,7 +591,7 @@ def lookup(self, sub_id, callable):
if sub_id 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 if_name not in self.if_with_mgmt_ips:
# no data for this interface
return None
return callable(sub_id, if_name)
Expand Down Expand Up @@ -623,15 +620,6 @@ def get_subtype(self, ip_str):
logger.warning("Invalid mgmt IP {}".format(ip_str))
return None

def man_addr_subtype(self, sub_id, if_name):
return self.mgmt_ips[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']

@staticmethod
def man_addr_if_subtype(sub_id, _): return ManAddrConst.man_addr_if_subtype
Expand Down
8 changes: 4 additions & 4 deletions tests/mock_tables/appl_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"lldp_rem_chassis_id_subtype": "4",
"lldp_rem_sys_name": "switch13",
"lldp_rem_port_id": "Ethernet1",
"lldp_rem_man_addr": "10.224.25.100"
"lldp_rem_man_addr": "10.224.25.100,2603:10e2:290:5016::"
},
"LLDP_ENTRY_TABLE:Ethernet4": {
"lldp_rem_port_id_subtype": "5",
Expand All @@ -25,7 +25,7 @@
"lldp_rem_chassis_id_subtype": "4",
"lldp_rem_sys_name": "switch13",
"lldp_rem_port_id": "Ethernet2",
"lldp_rem_man_addr": "fe80::268a:7ff:fe3f:834c"
"lldp_rem_man_addr": "fe80::268a::7ff:fe3f:834c,10.224.25.101"
},
"LLDP_ENTRY_TABLE:Ethernet8": {
"lldp_rem_port_id_subtype": "5",
Expand Down Expand Up @@ -53,7 +53,7 @@
"lldp_rem_chassis_id_subtype": "4",
"lldp_rem_sys_name": "switch13",
"lldp_rem_port_id": "Ethernet4",
"lldp_rem_man_addr": "10.224.25.103"
"lldp_rem_man_addr": "fe80:268a::7ff:fe3f:834c"
},
"LLDP_ENTRY_TABLE:Ethernet16": {
"lldp_rem_port_id_subtype": "5",
Expand All @@ -67,7 +67,7 @@
"lldp_rem_chassis_id_subtype": "4",
"lldp_rem_sys_name": "switch13",
"lldp_rem_port_id": "Ethernet5",
"lldp_rem_man_addr": "10.224.25.104"
"lldp_rem_man_addr": ""
},
"LLDP_ENTRY_TABLE:Ethernet20": {
"lldp_rem_port_id_subtype": "5",
Expand Down
4 changes: 2 additions & 2 deletions tests/mock_tables/asic0/appl_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"lldp_rem_chassis_id_subtype": "4",
"lldp_rem_sys_name": "switch13",
"lldp_rem_port_id": "Ethernet1",
"lldp_rem_man_addr": "10.224.25.100"
"lldp_rem_man_addr": "10.224.25.101,fe80:268a::7ff:fe3f:834c"
},
"LLDP_ENTRY_TABLE:Ethernet4": {
"lldp_rem_port_id_subtype": "5",
Expand All @@ -25,7 +25,7 @@
"lldp_rem_chassis_id_subtype": "4",
"lldp_rem_sys_name": "switch13",
"lldp_rem_port_id": "Ethernet2",
"lldp_rem_man_addr": "fe80::268a:7ff:fe3f:834c"
"lldp_rem_man_addr": "fe80::268a::7ff:fe3f:834c,10.224.25.102"
},
"LLDP_LOC_CHASSIS": {
"lldp_loc_chassis_id_subtype": "5",
Expand Down
4 changes: 2 additions & 2 deletions tests/mock_tables/asic1/appl_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"lldp_rem_chassis_id_subtype": "4",
"lldp_rem_sys_name": "switch13",
"lldp_rem_port_id": "Ethernet3",
"lldp_rem_man_addr": "fe80::268a:7ff:fe3f:834c"
"lldp_rem_man_addr": "10.224.25.102"
},
"LLDP_ENTRY_TABLE:Ethernet12": {
"lldp_rem_port_id_subtype": "5",
Expand All @@ -25,7 +25,7 @@
"lldp_rem_chassis_id_subtype": "4",
"lldp_rem_sys_name": "switch13",
"lldp_rem_port_id": "Ethernet4",
"lldp_rem_man_addr": "10.224.25.102"
"lldp_rem_man_addr": "fe80::268a:7ff:fe3f:834c"
},
"LLDP_LOC_CHASSIS": {
"lldp_loc_chassis_id_subtype": "5",
Expand Down
19 changes: 19 additions & 0 deletions tests/mock_tables/asic2/appl_db.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
{
"LLDP_ENTRY_TABLE:Ethernet16": {
"lldp_rem_port_id_subtype": "5",
"lldp_rem_sys_cap_supported": "28 00",
"lldp_rem_index": "1",
"lldp_rem_chassis_id": "00:11:22:33:44:55",
"lldp_rem_sys_desc": "I'm a little teapot.",
"lldp_rem_time_mark": "18543",
"lldp_rem_sys_cap_enabled": "28 00",
"lldp_rem_port_desc": " ",
"lldp_rem_chassis_id_subtype": "4",
"lldp_rem_sys_name": "switch13",
"lldp_rem_port_id": "Ethernet5",
"lldp_rem_man_addr": ""
},
"LLDP_LOC_CHASSIS": {
"lldp_loc_chassis_id_subtype": "5",
"lldp_loc_chassis_id": "00:11:22:AB:CD:EF",
Expand All @@ -8,6 +22,11 @@
"lldp_loc_sys_cap_supported": "28 00",
"lldp_loc_man_addr": "fe80::ce37:abff:feec:de9c"
},
"PORT_TABLE:Ethernet16": {
"description": "snowflake",
"alias": "etp5",
"speed": 100000
},
"PORT_TABLE:Ethernet-BP16": {
"description": "backplane",
"alias": "etp9",
Expand Down
25 changes: 25 additions & 0 deletions tests/namespace/test_lldp.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,36 @@ def test_subtype_lldp_loc_man_addr_table(self):


def test_subtype_lldp_rem_man_addr_table(self):
# Verfiy both valid ipv4 and ipv6 address exit
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 1))
self.assertIsNotNone(ret)
print(ret)
# Verfiy both valid ipv4 and invalid ipv6 address exit
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 5))
self.assertIsNotNone(ret)
Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi Mar 12, 2021

Choose a reason for hiding this comment

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

How is the return value checked if both ipv4 and ipv6 exits?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

intention of testcase is to check when both ip address are present we can get the data from first key (can be either v4 or v6) for given interface. Basically to check we are not ignoring management ip information in such case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SuvarnaMeenakshi Updated test case for this. Fixed lookup() API.

print(ret)
# Verfiy only valid ipv4 address exit
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 9))
self.assertIsNotNone(ret)
print(ret)
# Verfiy only valid ipv6 address exit
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 13))
self.assertIsNotNone(ret)
print(ret)
# Verfiy no mgmt address exit
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 17))
self.assertIsNone(ret)
print(ret)

def test_local_port_identification(self):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 3, 7, 1, 3)]
Expand Down
25 changes: 25 additions & 0 deletions tests/test_lldp.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,36 @@ def test_subtype_lldp_loc_man_addr_table(self):


def test_subtype_lldp_rem_man_addr_table(self):
# Verfiy both valid ipv4 and ipv6 address exist
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 1))
self.assertIsNotNone(ret)
print(ret)
# Verfiy valid ipv4 and invalid ipv6 address exist
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 5))
self.assertIsNotNone(ret)
abdosi marked this conversation as resolved.
Show resolved Hide resolved
print(ret)
# Verfiy only valid ipv4 address exist
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 9))
self.assertIsNotNone(ret)
print(ret)
# Verfiy only valid ipv6 address exist
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 13))
self.assertIsNotNone(ret)
print(ret)
# Verfiy no mgmt address exist
for entry in range(3, 6):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 4, 2, 1, entry)]
ret = mib_entry(sub_id=(1, 17))
self.assertIsNone(ret)
print(ret)

def test_local_port_identification(self):
mib_entry = self.lut[(1, 0, 8802, 1, 1, 2, 1, 3, 7, 1, 3)]
Expand Down