CLI support for Layer 2 MAC/FDB show #106

prsunny · 2017-09-25T16:56:24Z

Support to show Layer 2 MAC entries learnt in Hardware. The entries can also be shown per Port/Vlan

usage: fdbshow [-p PORT] [-v VLAN]
optional arguments:
  -p,  --port              FDB learned on specific port: Ethernet0
  -v,  --vlan              FDB learned on specific Vlan: 1000

Example of the output:
admin@str~$ fdbshow
  No.    Vlan  MacAddress         Port
-----  ------  -----------------  ----------
    1    1000  7C:FE:90:80:9F:05  Ethernet20
    2    1000  7C:FE:90:80:9F:10  Ethernet40
    3    1000  7C:FE:90:80:9F:01  Ethernet4
    4    1000  7C:FE:90:80:9F:02  Ethernet8
Total number of entries 4

qiluo-msft · 2017-09-25T17:44:58Z

scripts/fdbshow

+
+    def __init__(self):
+        super(FdbShow,self).__init__()
+        self.db = swsssdk.SonicV2Connector(host="127.0.0.1")


host [](start = 43, length = 4)

We prefer using unix socket. #Closed

As discussed, this format is used by other standalone scripts which also handles the permission issue.

qiluo-msft · 2017-09-25T17:48:30Z

scripts/fdbshow

+            self.if_br_oid_map[br_port_id] = port_id 
+
+
+    def fetch_fdb_data(self):


fetch_fdb_data [](start = 8, length = 14)

I notice code duplication with sonic-snmpagent. Could you reuse code?

I checked this in detail to see if we can reuse some functionality but there is tight coupling with MIB and SNMP functionality. Since this is a standalone functionality, thought of keeping it separate.

Modified to use common library API

qiluo-msft · 2017-09-25T17:52:10Z

scripts/fdbshow

+        print "Total number of entries {0} ".format(self.FDB_COUNT)
+
+
+    def display_summary(self):


display_summary [](start = 8, length = 15)

I guess you don't need one more function for no filters. Just reuse display_specific(). #Closed

qiluo-msft · 2017-09-25T17:52:51Z

scripts/fdbshow

+            vlan = int(vlan)
+            s_index = self.get_iter_index(vlan)
+            fdb_list = [fdb for fdb in self.bridge_mac_list[s_index:]
+                        if fdb[0] == vlan and fdb[2] == port]


Simplify the conditions into one? #Closed

It is not simplified, and become more complex...

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

I think it now handles the three conditions -
1 - All arguments None
2 - Some arguments None
3 - None of the arguments are None.
Could you suggest a different way?

Agree its slightly complex since I accounted for adding future options. Revisited the logic and made it simple. Thanks for pointing out.

qiluo-msft · 2017-09-25T17:53:33Z

scripts/fdbshow

+        else:
+            fdb.display_specific(args.port, args.vlan)
+
+    except Exception as e:


Exception [](start = 11, length = 9)

No need to catch all exceptions.

As we discussed, I removed this and checked. But observed that in case of invalid inputs, in addition to displaying the error message, it also gives the trace back which is not expected. For e.g:

admin@str-s6000-acs-7:~$ ./fdbshow -v 1001
Traceback (most recent call last):
File "./fdbshow", line 182, in
main()
File "./fdbshow", line 179, in main
fdb.display(args.vlan, args.port)
File "./fdbshow", line 158, in display
s_index = self.get_iter_index(arg_list[m_index], m_index)
File "./fdbshow", line 127, in get_iter_index
return keys.index(key_value)
ValueError: 1001 is not in list

Whereas, with the exception is would be:

admin@str-s6000-acs-7:~$ ./fdbshow -v 1001
1001 is not in list

If you are arguing about this case, the solution should be catch specific exception, instead of catch all.

qiluo-msft · 2017-09-26T21:01:55Z

install_requires=[

Should we add swsssdk as a dependency here? #Closed

Refers to: setup.py:45 in 25f704d. [](commit_id = 25f704d, deletion_comment = False)

prsunny · 2017-09-26T21:25:00Z

I don't think swsssdk is required in setup.py. It says it's mainly used to install dependencies when the project is installed using 'pip'. Also swsssdk is used by other scripts files in the same repo.

jleveque · 2017-09-26T21:46:52Z

I don't think it would hurt to add swsssdk in the list of dependencies. I think it was overlooked in the past.

jleveque · 2017-09-26T21:48:08Z

scripts/fdbshow

+import json
+import sys
+
+from swsssdk import port_util


This line is redundant with the import swsssdk line above. Remove whichever is less appropriate.

jleveque · 2017-09-26T22:11:02Z

scripts/fdbshow

+
+from swsssdk import port_util
+from tabulate import tabulate
+from natsort import natsorted


Please reorder so that import ... statements are ordered alphabetically followed by from ... import ... statements ordered alphabetically. This makes it easier to locate used packages/modules.

stcheng · 2017-09-27T06:12:40Z

scripts/fdbshow

+
+            ent = self.db.get_all('ASIC_DB', s, blocking=True)
+            br_port_id = ent[b"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID"][oid_pfx:]
+            try:


use if instead of try/except?

lguohan · 2017-09-27T07:08:40Z

somehow, I did not see my previous comments.

can we add hook to show command

show macs
show macs [-v vlan]
show macs [-p port]

lguohan

add hook to show commands

lguohan

add hook to show commands.

prsunny · 2017-09-27T17:29:33Z

Added hooks to Show CLI command.

qiluo-msft · 2017-09-27T19:12:33Z

show/main.py

@@ -226,6 +226,25 @@ def sfp(interfacename):

    run_command(cmd)

+#
+# 'mac' command ("show mac ...")


mac [](start = 23, length = 3)

I am not sure about other vendors' CLI. Is it more accurate to call it 'show fdb'? #Closed

@lguohan , can you suggest?

* msft_github/master: Enhancement of 'show' commands and addition of 'debug', and 'undebug'… (sonic-net#113) CLI support for Layer 2 MAC/FDB show (sonic-net#106) [show]: Add 'show interfaces alias' command to display port name/alias mapping (sonic-net#107) Add 'ipv6' group along with 'bgp' and 'route' subcommands; Remove duplicate commands ('bgp,' 'route') from under root group (sonic-net#102) [generate_dump]: Skip the sparse file /var/log/lastlog (sonic-net#104) Added syncd SAI dump to sysdump script (sonic-net#89) Adapt to new minigraph_parser schema (sonic-net#103) [core dump] remove number of parameter assumption from script coredump-compress [FastReboot]: Update FR to make it working with 1.0.3 (sonic-net#95)

…ies in setup.py (sonic-net#106) Remove dependence on the 'enum' package, as we are currently transitioning from Python 2 to Python 3 and there are installation conflict issues between the `enum` package and the `enum34` package. Add 'sonic-py-common' as dependencies in setup.py for xcvrd, also add spaces around "equals" signs.

…onic-net#106) To support the component firmware automatic update, add auto_update_firmware() to platform component api This is to support fwutil auto_update command to update the platform component firmware automatically. Fwutil HLD to support auto_update interfaces/commands. sonic-net/SONiC#648

prsunny added 2 commits September 25, 2017 16:40

CLI support to show MAC/FDB entries learnt in Hardware

83a6a85

Minor fix

0626d7d

msftclas added the cla-not-required label Sep 25, 2017

prsunny requested review from lguohan, stcheng and qiluo-msft September 25, 2017 16:56

qiluo-msft reviewed Sep 25, 2017

View reviewed changes

prsunny added 3 commits September 25, 2017 23:39

Review comments, some restructuring

76b493d

Exception is caught in main function

7e32eee

Fix for a case where -p and -v are same

9ec32a2

msftgits removed the cla-not-required label Sep 26, 2017

Updated to use common library API to get bridge port mapping

25f704d

Add install dependency for swsssdk

3bf4b8a

jleveque suggested changes Sep 26, 2017

View reviewed changes

Rearranged imports

592e17b

sonic-net deleted a comment from msftclas Sep 27, 2017

stcheng reviewed Sep 27, 2017

View reviewed changes

lguohan reviewed Sep 27, 2017

View reviewed changes

lguohan requested changes Sep 27, 2017

View reviewed changes

prsunny added 2 commits September 27, 2017 16:33

Addressed review comments, check-logic modified

67b52a2

Merge remote-tracking branch 'upstream/master'

96356c5

Show command hookup added

8edf4e7

Added check for None type and remove try-catch

508fb78

qiluo-msft reviewed Sep 27, 2017

View reviewed changes

lguohan approved these changes Sep 27, 2017

View reviewed changes

jleveque approved these changes Sep 27, 2017

View reviewed changes

prsunny merged commit 206aabe into sonic-net:master Sep 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI support for Layer 2 MAC/FDB show #106

CLI support for Layer 2 MAC/FDB show #106

prsunny commented Sep 25, 2017

qiluo-msft Sep 25, 2017 •

edited

Loading

prsunny Sep 25, 2017

qiluo-msft Sep 25, 2017

prsunny Sep 25, 2017

prsunny Sep 26, 2017

qiluo-msft Sep 25, 2017 •

edited

Loading

prsunny Sep 25, 2017

qiluo-msft Sep 25, 2017 •

edited

Loading

prsunny Sep 25, 2017

qiluo-msft Sep 26, 2017

prsunny Sep 26, 2017

prsunny Sep 27, 2017

qiluo-msft Sep 25, 2017

prsunny Sep 25, 2017

qiluo-msft Sep 27, 2017

qiluo-msft commented Sep 26, 2017 •

edited

Loading

prsunny commented Sep 26, 2017

jleveque commented Sep 26, 2017

jleveque Sep 26, 2017 •

edited

Loading

prsunny Sep 26, 2017

jleveque Sep 26, 2017

prsunny Sep 26, 2017

stcheng Sep 27, 2017

lguohan commented Sep 27, 2017

lguohan left a comment

lguohan left a comment

prsunny commented Sep 27, 2017 •

edited

Loading

qiluo-msft Sep 27, 2017 •

edited

Loading

prsunny Sep 27, 2017

		self.if_br_oid_map[br_port_id] = port_id


		def fetch_fdb_data(self):

		print "Total number of entries {0} ".format(self.FDB_COUNT)


		def display_summary(self):

CLI support for Layer 2 MAC/FDB show #106

CLI support for Layer 2 MAC/FDB show #106

Conversation

prsunny commented Sep 25, 2017

qiluo-msft Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft commented Sep 26, 2017 • edited Loading

prsunny commented Sep 26, 2017

jleveque commented Sep 26, 2017

jleveque Sep 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan commented Sep 27, 2017

lguohan left a comment

Choose a reason for hiding this comment

lguohan left a comment

Choose a reason for hiding this comment

prsunny commented Sep 27, 2017 • edited Loading

qiluo-msft Sep 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Sep 25, 2017 •

edited

Loading

qiluo-msft Sep 25, 2017 •

edited

Loading

qiluo-msft Sep 25, 2017 •

edited

Loading

qiluo-msft commented Sep 26, 2017 •

edited

Loading

jleveque Sep 26, 2017 •

edited

Loading

prsunny commented Sep 27, 2017 •

edited

Loading

qiluo-msft Sep 27, 2017 •

edited

Loading