[LLDP] Fix lldpshow script to enable display multiple MAC addresses on the same remote physical interface (sonic-net#1657)

Scenario:
1- remote interface has 2 MACs on the same physical interface.
2- "show lldp table" command displays one entry for only one MAC address

Root cause:
"show lldp table" command uses lldpshow script to get, parse and display data from the lldp open source package
 (lldpctl script). lldpctl script returns a proper info about the 2 MACs but the issue is with the lldpshow script parser 
where it built a dictionary which its key is the local physical interface. Therefore when having 2 MACs, lldpctl will 
return 2 entries but the lldpshow parser will overwrite the first enrty.

Fix:
Change the key to be a string of "interface#MAC".
This will enable having 2 entries for 2 different MAC addresses.

In addition:
- update display_sum()-->get_summary_output() to return a string instead of printing it directly.
this to allow checking the returned value inside the new unit test.
- add a new unit test for this scenario.

Signed-off-by: Basim Shalata <[email protected]>

Author: BasimShalata

scripts/lldpshow

@@ -127,53 +127,60 @@ class Lldpshow(object):
                 l_intf = intf.attrib['name']
                 if l_intf.startswith(BACKEND_ASIC_INTERFACE_NAME_PREFIX):
                     continue
-                self.lldpsum[l_intf] = {}
+                remote_port = intf.find('port')
+                r_portid = remote_port.find('id').text
+                key = l_intf + "#" + r_portid
+                self.lldpsum[key] = {}
+                self.lldpsum[key]['l_intf'] = l_intf
+                self.lldpsum[key]['r_portid'] = r_portid
                 chassis = intf.find('chassis')
                 capabs = chassis.findall('capability')
                 capab = self.parse_cap(capabs)
                 rmt_name = chassis.find('name')
                 if rmt_name is not None:
-                    self.lldpsum[l_intf]['r_name'] = rmt_name.text
+                    self.lldpsum[key]['r_name'] = rmt_name.text
                 else:
-                    self.lldpsum[l_intf]['r_name'] = ''
-                remote_port = intf.find('port')
-                self.lldpsum[l_intf]['r_portid'] = remote_port.find('id').text
+                    self.lldpsum[key]['r_name'] = ''
                 rmt_desc = remote_port.find('descr')
                 if rmt_desc is not None:
-                    self.lldpsum[l_intf]['r_portname'] = rmt_desc.text
+                    self.lldpsum[key]['r_portname'] = rmt_desc.text
                 else:
-                    self.lldpsum[l_intf]['r_portname'] = ''
-                self.lldpsum[l_intf]['capability'] = capab
+                    self.lldpsum[key]['r_portname'] = ''
+                self.lldpsum[key]['capability'] = capab
 
     def sort_sum(self, summary):
         """ Sort the summary information in the way that is expected(natural string)."""
-        def alphanum_key(key): return [re.findall('[A-Za-z]+', key) + [int(port_num)
-                                                                       for port_num in re.findall('\d+', key)]]
+        def alphanum_key(key):
+            key = key.split("#")[0]
+            return [re.findall('[A-Za-z]+', key) + [int(port_num)
+                                                    for port_num in re.findall('\d+', key)]]
         return sorted(summary, key=alphanum_key)
 
-    def display_sum(self, lldp_detail_info):
+    def get_summary_output(self, lldp_detail_info):
         """
-        print out summary result of lldp neighbors
+        returns summary result of lldp neighbors
         """
+        output_summary = ''
         # In detail mode output is plain text
         if self.lldpraw and lldp_detail_info:
             lldp_output = ''
             for lldp_detail_output in self.lldpraw:
                 lldp_output += lldp_detail_output
-            print(lldp_output)
+            output_summary += lldp_output + "\n"
         elif self.lldpraw:
             lldpstatus = []
-            print('Capability codes: (R) Router, (B) Bridge, (O) Other')
+            output_summary += "Capability codes: (R) Router, (B) Bridge, (O) Other\n"
             header = ['LocalPort', 'RemoteDevice', 'RemotePortID', 'Capability', 'RemotePortDescr']
             sortedsum = self.sort_sum(self.lldpsum)
             for key in sortedsum:
-                lldpstatus.append([key, self.lldpsum[key]['r_name'], self.lldpsum[key]['r_portid'],
+                lldpstatus.append([self.lldpsum[key]['l_intf'], self.lldpsum[key]['r_name'], self.lldpsum[key]['r_portid'],
                                    self.lldpsum[key]['capability'], self.lldpsum[key]['r_portname']])
-            print(tabulate(lldpstatus, header))
-            print('-'.rjust(50, '-'))
-            print('Total entries displayed: ', len(self.lldpsum))
+            output_summary += tabulate(lldpstatus, header) + "\n"
+            output_summary += ('-'.rjust(50, '-')) + "\n"
+            output_summary += "Total entries displayed:  {}".format(len(self.lldpsum))
         elif self.err is not None:
-            print('Error:', self.err)
+            output_summary += "Error: {}".format(self.err)
+        return output_summary
 
 
 def main():
@@ -202,7 +209,8 @@ def main():
         lldp = Lldpshow()
         lldp.get_info(lldp_detail_info, lldp_port)
         lldp.parse_info(lldp_detail_info)
-        lldp.display_sum(lldp_detail_info)
+        output_summary = lldp.get_summary_output(lldp_detail_info)
+        print(output_summary)
     except Exception as e:
         print(str(e), file=sys.stderr)
         sys.exit(1)

tests/lldp_test.py

@@ -0,0 +1,79 @@
+import os
+
+from click.testing import CliRunner
+from utilities_common.general import load_module_from_source
+
+test_path = os.path.dirname(os.path.abspath(__file__))
+modules_path = os.path.dirname(test_path)
+scripts_path = os.path.join(modules_path, "scripts")
+
+# Load the file under test
+lldpshow_path = os.path.join(scripts_path, 'lldpshow')
+lldpshow = load_module_from_source('lldpshow', lldpshow_path)
+
+# Expected output for 2 remote MACs on same physical interface
+expected_2MACs_Ethernet0_output = \
+('Capability codes: (R) Router, (B) Bridge, (O) Other\n'
+ 'LocalPort    RemoteDevice    RemotePortID       Capability    '
+ 'RemotePortDescr\n'
+ '-----------  --------------  -----------------  ------------  '
+ '-----------------\n'
+ 'Ethernet0    dummy           00:00:00:00:00:01  BR            First MAC\n'
+ 'Ethernet0    dummy           00:00:00:00:00:02  R             Second MAC\n'
+ '--------------------------------------------------\n'
+ 'Total entries displayed:  2')
+
+expected_lldpctl_xml_output = \
+['<?xml version="1.0" encoding="UTF-8"?>\n\
+        <lldp label="LLDP neighbors">\n\
+        <interface label="Interface" name="Ethernet0" via="LLDP" rid="2" age="7 days, 22:11:33">\n\
+            <chassis label="Chassis">\n\
+                <id label="ChassisID" type="mac">00:00:00:00:00:01</id>\n\
+                <name label="SysName">dummy</name>\n\
+                <descr label="SysDescr">NA</descr>\n\
+                <mgmt-ip label="MgmtIP">00:00:00:00:00:00</mgmt-ip>\n\
+                <capability label="Capability" type="Bridge" enabled="on"/>\n\
+                <capability label="Capability" type="Router" enabled="on"/>\n\
+                <capability label="Capability" type="Wlan" enabled="off"/>\n\
+                <capability label="Capability" type="Station" enabled="off"/>\n\
+            </chassis>\n\
+            <port label="Port">\n\
+                <id label="PortID" type="mac">00:00:00:00:00:01</id>\n\
+                <descr label="PortDescr">First MAC</descr>\n\
+                <ttl label="TTL">120</ttl>\n\
+            </port>\n\
+        </interface>\n\
+        <interface label="Interface" name="Ethernet0" via="LLDP" rid="4" age="7 days, 22:11:34">\n\
+            <chassis label="Chassis">\n\
+                <id label="ChassisID" type="mac">00:00:00:00:00:02</id>\n\
+                <name label="SysName">dummy</name>\n\
+                <descr label="SysDescr">NA</descr>\n\
+                <mgmt-ip label="MgmtIP">00:00:00:00:00:00</mgmt-ip>\n\
+                <capability label="Capability" type="Router" enabled="on"/>\n\
+            </chassis>\n\
+            <port label="Port">\n\
+            <id label="PortID" type="mac">00:00:00:00:00:02</id>\n\
+                <descr label="PortDescr">Second MAC</descr>\n\
+                <ttl label="TTL">120</ttl>\n\
+            </port>\n\
+        </interface>\n\
+        </lldp>\n']
+
+class TestLldp(object):
+    @classmethod
+    def setup_class(cls):
+        print("SETUP")
+
+    def test_show_lldp_2_macs_same_phy_interface(self):
+        runner = CliRunner()
+        # Create lldpshow instance
+        lldp = lldpshow.Lldpshow()
+        # Mock lldpraw to check new functionality in parse_info()
+        lldp.lldpraw = expected_lldpctl_xml_output
+        lldp.parse_info(lldp_detail_info=False)
+        output_summary = lldp.get_summary_output(lldp_detail_info=False)
+        assert output_summary == expected_2MACs_Ethernet0_output
+
+    @classmethod
+    def teardown_class(cls):
+        print("TEARDOWN")