Skip to content

Layer 2 Forwarding Enhancements#529

Closed
anilkpandey wants to merge 59 commits intosonic-net:masterfrom
anilkpandey:master
Closed

Layer 2 Forwarding Enhancements#529
anilkpandey wants to merge 59 commits intosonic-net:masterfrom
anilkpandey:master

Conversation

@anilkpandey
Copy link
Contributor

@anilkpandey anilkpandey commented May 10, 2019 •
edited
Loading

- What I did

  1. Added code changes related to Layer 2 Forwarding Enhancements:
    Added the following commands:

VLAN range add/del
VLAN member range add/del
Static FDB add/del
FDB aging time configuration
show fdb aging time
show mac count
show vlan count
added port and vlan option in 'sonic-clear fdb'

anilkpandey added 4 commits May 9, 2019 16:22
@anilkpandey
Update main.py
eee827e
@anilkpandey
Update main.py
d4e5346
@anilkpandey
Update main.py
b63e795
@anilkpandey
added configuration validations
a4beefc 
- validate vlan id for vlan creation
- verify that port is not part of vlan when assigning IP address and vice versa
- verify that portchannel has no member and portchannel is not member of vlan when deleting the portchannel
- verify that port is not already untagged member of vlan when adding the port as untagged member of a vlan
-
@prsunny
Copy link
Contributor

prsunny commented May 10, 2019

I'm not sure if these validations must be here. There is an design discussion for overall Config validator and I don't think we need to scatter these checks in multiple places.

@jleveque jleveque requested review from prsunny and stcheng May 31, 2019 19:53
@anilkpandey
'show mac' performance improvement
1564302 
'show mac' was taking a lot of time to display the fdb entries when a large number of entries are present.
Using redis pipeline, reduced the time significantly.
Earlier it used to take about 10 minutes to show 32K MAC. After the fix it takes about 17-20 seconds.
@anilkpandey
revert changes
f567fbb 
revert changes
@anilkpandey anilkpandey changed the title added validation checks for VLAN, IP and Portchannel configurations 'show mac' performance improvements Jun 5, 2019
@anilkpandey
Copy link
Contributor Author

anilkpandey commented Jun 5, 2019

Hi @stcheng and @prsunny,

I just reverted the previous changes for validations and added a new file fdbshow for 'show mac' performance enhancements. Please review the changes.

In case you want to consider the validations changes, let me know, I will readd the changes in that case.

Thanks.

stcheng
stcheng suggested changes Jun 5, 2019
View reviewed changes
Copy link
Contributor

@stcheng stcheng left a comment

Choose a reason for hiding this comment

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

could you resolve the conflicts? and also remove the .DS_Store file in the pull request?

stcheng
stcheng reviewed Jun 6, 2019
View reviewed changes
@anilkpandey
revert validation related changes
d544203 
reverted changes made as part of validations
prsunny
prsunny reviewed Jun 11, 2019
View reviewed changes
@prsunny prsunny mentioned this pull request Jun 13, 2019
Open
@prsunny
Copy link
Contributor

prsunny commented Jul 9, 2019

retest this please

prsunny
prsunny previously approved these changes Jul 9, 2019
View reviewed changes
stcheng
stcheng previously approved these changes Jul 10, 2019
View reviewed changes
@jleveque
Copy link
Contributor

jleveque commented Jul 16, 2019

Retest this please

1 similar comment
@jleveque
Copy link
Contributor

jleveque commented Jul 24, 2019

Retest this please

@anilkpandey anilkpandey changed the title 'show mac' performance improvements Layer 2 Forwarding Enhancements Sep 8, 2019
@anilkpandey
Copy link
Contributor Author

anilkpandey commented Sep 8, 2019

Added code changes related to Layer 2 Forwarding Enhancements

@prsunny prsunny self-requested a review September 9, 2019 19:13
@lgtm-com
Copy link

lgtm-com bot commented Dec 9, 2020

This pull request introduces 1 alert when merging 2a9ab88 into d414970 - view on LGTM.com

new alerts:

  • 1 for Unnecessary pass

@anilkpandey
added more tests for vlan range and fdb config commands
a8c651e 
added more tests for vlan range and fdb config commands
@lgtm-com
Copy link

lgtm-com bot commented Dec 18, 2020

This pull request introduces 2 alerts when merging a8c651e into 9419627 - view on LGTM.com

new alerts:

  • 1 for Unnecessary pass
  • 1 for Unused local variable

@anilkpan
Copy link
Contributor

anilkpan commented Dec 19, 2020

retest this please

1 similar comment
@anilkpan
Copy link
Contributor

anilkpan commented Dec 21, 2020

retest this please

lguohan
lguohan reviewed May 12, 2021
View reviewed changes
scripts/fdbshow Outdated
from natsort import natsorted
from swsssdk import port_util
from swsscommon.swsscommon import SonicV2Connector
from swsssdk import SonicV2Connector, port_util
Copy link
Contributor

@lguohan lguohan May 12, 2021

Choose a reason for hiding this comment

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

we should not use swsssdk, it is deprecating.

@anilkpandey anilkpandey mentioned this pull request Oct 22, 2021
Merged
@lgtm-com
Copy link

lgtm-com bot commented Nov 3, 2021

This pull request introduces 2 alerts when merging 2fcd83d into 474bdbf - view on LGTM.com

new alerts:

  • 1 for Unused local variable
  • 1 for Wrong number of arguments in a call

@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2021

This pull request introduces 2 alerts when merging b037b7d into 8a8577b - view on LGTM.com

new alerts:

  • 1 for Unreachable code
  • 1 for Wrong number of arguments in a call

@anilkpandey
Copy link
Contributor Author

anilkpandey commented Nov 9, 2021

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 9, 2021

Commenter does not have sufficient privileges for PR 529 in repo Azure/sonic-utilities

@anilkpandey
Copy link
Contributor Author

anilkpandey commented Nov 9, 2021

retest this please

@anilkpandey
Copy link
Contributor Author

anilkpandey commented Nov 9, 2021

/azpw run

@mssonicbld
Copy link
Collaborator

mssonicbld commented Nov 9, 2021

/AzurePipelines run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 9, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@anilkpandey
Copy link
Contributor Author

anilkpandey commented Nov 10, 2021

/azpw run

@mssonicbld
Copy link
Collaborator

mssonicbld commented Nov 10, 2021

/AzurePipelines run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 10, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@raphaelt-nvidia raphaelt-nvidia mentioned this pull request Jan 20, 2022
Open
@ljyfree
Copy link

ljyfree commented Apr 12, 2022

Hi all
I notice that "A Static FDB entry should be configurable in hardware from CLI" was mentioned in SONiC Layer 2 Forwarding Enhancements HLD.md.
Wonder why this pull was not be merged which contain SONiC-CLI to config static FDB? Without this pull,how could I add a static FDB from CLI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lguohan lguohan lguohan left review comments

@prsunny prsunny prsunny left review comments

+3 more reviewers

@jainp1979 jainp1979 jainp1979 left review comments

@tahmed-dev tahmed-dev tahmed-dev left review comments

@stcheng stcheng stcheng left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

brcm-upstream Enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.