-
Notifications
You must be signed in to change notification settings - Fork 661
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
[fdbshow] Adding more options for fdbshow and show mac #1982
Conversation
Signed-off-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
Signed-off-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
Signed-off-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
/azp run Azure.sonic-utilities |
Azure Pipelines successfully started running 1 pipeline(s). |
Total number of entries 2 | ||
``` | ||
``` | ||
admin@sonic:~$ show mac -c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Qi,
I wrote a script and this would save a good amount of time in scaled configuration
Script is as below
date
fdbshow -c
date
fdbshow | grep Total
date
Output: -c takes 8 seconds whereas with grep the time taken is 14 seconds.
Wed 12 Jan 2022 05:30:23 AM UTC
Total number of entries 30747
Wed 12 Jan 2022 05:30:31 AM UTC
Total number of entries 30747
Wed 12 Jan 2022 05:30:45 AM UTC
In some scenarios it almost halves the time taken. On another note most of the options could be replaced by simple grep. The point of having them is performance as well as ease of use to the user. Most other operating systems do have these options and hence it makes beneficial to add this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering why it is so slow to grep. The only difference is collecting and printing a table. Is tabulate
the slowest procedure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Tabulate appears to be the bottleneck especially with bulk configurations. I prefer having the count option as is.
Added validation for different options Added new tests to validate error inputs
This pull request introduces 1 alert when merging 8834555 into dd8c8fe - view on LGTM.com new alerts:
|
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azpw run Azure.sonic-utilities |
/AzurePipelines run Azure.sonic-utilities |
Azure Pipelines successfully started running 1 pipeline(s). |
scripts/fdbshow
Outdated
|
||
if not count: | ||
for fdb in self.bridge_mac_list: | ||
self.FDB_COUNT += 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed.
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azpw run Azure.sonic-utilities |
/AzurePipelines run Azure.sonic-utilities |
Azure Pipelines successfully started running 1 pipeline(s). |
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azpw run Azure.sonic-utilities |
/AzurePipelines run Azure.sonic-utilities |
Azure Pipelines successfully started running 1 pipeline(s). |
/azpw run Azure.sonic-utilities |
/AzurePipelines run Azure.sonic-utilities |
Azure Pipelines successfully started running 1 pipeline(s). |
#### What I did Added more options to filter output in show mac and fdbshow command. Introduced options for filter by address and filter by type. Added one more option to display only count. Introduced show command to display fdb aging time in the switch. #### How I did it Modifying fdbshow and show scripts to include the above-mentioned options #### How to verify it Added UT for all the newly introduced options and commands #### Previous command output (if the output of a command-line utility has changed) ``` show mac -h Usage: show mac [OPTIONS] Show MAC (FDB) entries Options: -v, --vlan TEXT -p, --port TEXT --verbose Enable verbose output -h, -?, --help Show this message and exit. ``` #### New command output (if the output of a command-line utility has changed) ``` show mac -h Usage: show mac [OPTIONS] COMMAND [ARGS]... Show MAC (FDB) entries Options: -v, --vlan TEXT -p, --port TEXT -a, --address TEXT -t, --type TEXT -c, --count --verbose Enable verbose output -h, -?, --help Show this message and exit. Commands: aging-time show mac No. Vlan MacAddress Port Type ----- ------ ----------------- ----------- ------- 1 10 98:03:9B:82:BB:5B Ethernet60 Dynamic 2 10 EC:0D:9A:CD:91:72 Ethernet64 Dynamic 3 10 EC:0D:9A:CD:91:73 Ethernet124 Dynamic Total number of entries 3 show mac --address EC:0D:9A:CD:91:72 No. Vlan MacAddress Port Type ----- ------ ----------------- ---------- ------- 1 10 EC:0D:9A:CD:91:72 Ethernet64 Dynamic show mac --count Total number of entries 3 show mac --type Dynamic No. Vlan MacAddress Port Type ----- ------ ----------------- ----------- ------- 1 10 98:03:9B:82:BB:5B Ethernet60 Dynamic 2 10 EC:0D:9A:CD:91:72 Ethernet64 Dynamic 3 10 EC:0D:9A:CD:91:73 Ethernet124 Dynamic Total number of entries 3 show mac aging-time Aging time for switch is 600 seconds ```
What I did
Added more options to filter output in show mac and fdbshow command.
Introduced options for filter by address and filter by type.
Added one more option to display only count.
Introduced show command to display fdb aging time in the switch.
How I did it
Modifying fdbshow and show scripts to include the above-mentioned options
How to verify it
Added UT for all the newly introduced options and commands
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)