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

Layer 2 Forwarding Enhancements #885

Closed
wants to merge 37 commits into from

Conversation

anilkpandey
Copy link

@anilkpandey anilkpandey commented May 11, 2019

Added code changes related to Layer 2 Forwarding Enhancements:

Changes are as mentioned in the HLD.

Added support for per port, per vlan and per port-vlan fdb flush.

Added fdb reference count in port and vlan in portsorch to keep track of number of fdb entries learnt on the port or vlan and delete the bridge port or vlan when the fdb count in 0.

Added new data structure in portsorch for mapping between OID and vlan/port/bridge port.

Moved SAI Redis fdb handling to fdborch to have both the fdborch and sai redis reference count in sync

Changed default vlan state in kernel to Down and make it Up only when first port/lag in the vlan is oper up. Make vlan Down when last vlan member goes oper down. Orchagent sends notification to vlanmgr when first/last vlan member goes up/down.

Added support for add/remove vlan 1 from config. Also, if port is member of vlan 1, will not remove it from vlan 1 after adding port to bridge in kernel.

Added support for static fdb config and fdb aging time config.

When a port is removed from VLAN, static fdb are flushed in HW and in fdborch, static fdb are added back to savedFdbEntries.

Added port channel port oid to name mapping in COUNTERS_PORT_NAME_MAP table so
that a given portchannel port oid will give port channel name

Made fix to add add portchannel port oid to name mapping when addLag is done
from portsorch. similarly removed the mapping when removeLag is done

Issue:

fdbshow does not show the portchannel name

Fix:

1.Added port channel port oid to name mapping in COUNTERS_PORT_NAME_MAP table so
that a given portchannel port oid will give port channel name

2. made fix to add add portchannel port oid to name mapping when addLag is done
from portsorch. similarly removed the mapping when removeLag is done
@qiluo-msft
Copy link
Contributor

Please exclude the file .DS_Store

@qiluo-msft qiluo-msft requested a review from stcheng May 11, 2019 00:28
orchagent/portsorch.cpp Outdated Show resolved Hide resolved
orchagent/portsorch.cpp Outdated Show resolved Hide resolved
orchagent/portsorch.cpp Outdated Show resolved Hide resolved
@lguohan
Copy link
Contributor

lguohan commented Jun 19, 2019

retest this please

@stcheng
Copy link
Contributor

stcheng commented Aug 21, 2019

@anilkpandey any updates?

@anilkpandey
Copy link
Author

With the change there were compile errors related to some snmp scripts. I am yet to fix those. Will send an update by next week.

@stcheng
Copy link
Contributor

stcheng commented Aug 26, 2019

thanks!

@anilkpandey anilkpandey changed the title [PortsOrch]: add LAG name map to counter table Layer 2 Forwarding Enhancements Sep 8, 2019
@anilkpandey
Copy link
Author

Added code changes related to Layer 2 Forwarding Enhancements

Layer 2 Forwarding Enhancements
Layer 2 Forwarding Enhancements
Updated to reflect latest code changes.
moved sai redis fdb event processing back to notification thread
@anilkpandey
Copy link
Author

thanks!

Hi, I have added a new table LAG_NAME_MAP)TABLE in COUNTERS_DB. Here are the other pull requests related to this. Please check.

sonic-net/sonic-swss-common#303
sonic-net/sonic-snmpagent#114
sonic-net/sonic-py-swsssdk#51

Also, please review new changes added to this pull request related to layer 2 Forwarding Enhancements.

@lgtm-com
Copy link

lgtm-com bot commented Apr 25, 2020

This pull request introduces 46 alerts when merging bcaeb98 into 3829053 - view on LGTM.com

new alerts:

  • 45 for Unused import
  • 1 for Resource not released in destructor

@prsunny
Copy link
Collaborator

prsunny commented May 5, 2020

@anilkpandey , this PR is pretty big and few other PRs are dependent on parts of your changes. Is it possible to split the PR based on functionality to make reviewing easy and merge those to unblock dependent PRs?

@anilkpandey
Copy link
Author

anilkpandey commented May 6, 2020 via email

jainp1979 added a commit to jainp1979/sonic-swss that referenced this pull request Jun 23, 2020
1. Review comments incorporated
2. Dependency for PR sonic-net#885 is resolved
3. Added test_evpn_fdb.py pytest script to verify EVPN-VXLAN-FDB
4. compiled and verified the FDB functionality with EVPN
5. Changes to handle the static mac and aging from CLI
jainp1979 added a commit to jainp1979/sonic-swss that referenced this pull request Jun 23, 2020
1. Review comments incorporated
2. Dependency for PR sonic-net#885 is resolved
3. Added test_evpn_fdb.py pytest script to verify EVPN-VXLAN-FDB
4. compiled and verified the FDB functionality with EVPN
5. Changes to handle the static mac and aging from CLI
jainp1979 added a commit to jainp1979/sonic-swss that referenced this pull request Sep 27, 2020
1. Review comments incorporated
2. Dependency for PR sonic-net#885 is resolved
3. Added test_evpn_fdb.py pytest script to verify EVPN-VXLAN-FDB
4. compiled and verified the FDB functionality with EVPN
5. Changes to handle the static mac and aging from CLI
@lgtm-com
Copy link

lgtm-com bot commented Dec 4, 2020

This pull request introduces 1 alert when merging 56e330e into c0f951c - view on LGTM.com

new alerts:

  • 1 for Resource not released in destructor

@lguohan
Copy link
Contributor

lguohan commented Dec 23, 2020

retest this please

EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
…g DB (sonic-net#885)

Fix is_mgmt_vrf_enabled for the case where MGMT_VRF_CONFIG is not at all in the config DB.
This is the case where mgmt vrf is never configured. The function throws error at
File "/usr/lib/python2.7/dist-packages/show/main.py", line 651, in is_mgmt_vrf_enabled
mvrf_dict = json.loads(p.stdout.read())
Two show commands uses is_mgmt_vrf_enabled. "show mgmt-vrf" and "show ntp"
Both commands throw error if mgmt vrf is never configured

Co-authored-by: Bing Sun <Bing_Sun@dell.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants