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 #510

Merged
merged 22 commits into from
Nov 27, 2019
Merged

Conversation

anilkpandey
Copy link
Contributor

@anilkpandey anilkpandey commented Sep 8, 2019

Added code changes related to Layer 2 Forwarding Enhancement:

As part of L2 enhancements, per port, per vlan and per port-vlan fdb flush support has been added. The fdb flush response from SAI comes as individual mac delete as mentioned in the HLD. The change here is to follow the same way for VS as well.

Regarding SAI_FDB_EVENT_FLUSHED, it is handled in sync only for 'flush all' case and handling SAI_FDB_EVENT_FLUSHED for per port and per port-vlan will require hgetall on all fdb keys. So, individual mac delete response is preferred.

Another change is to move fdb handling to fdborch. As the fdb reference count is now maintained in fdborch as well, need to move the sai redis fdb handling also there to avoid the reference counts to go out of sync between the two.

@lguohan
Copy link
Contributor

lguohan commented Sep 9, 2019

@anilkpandey , thanks for the enhancement. can you provide full description of the change?

lib/src/sai_redis_notifications.cpp Outdated Show resolved Hide resolved
vslib/src/sai_vs_fdb.cpp Outdated Show resolved Hide resolved
moved sai redis fdb event processing back to notification thread
vslib/src/sai_vs_fdb.cpp Outdated Show resolved Hide resolved
uncomment  meta_sai_on_fdb_event() to update local SAIDB
vslib/src/sai_vs_fdb.cpp Outdated Show resolved Hide resolved
Added support for handling per port, per vlan and per port-vlan flush event from SAI. Also, doing ASIC_DB flsh in lua script
@kcudnik
Copy link
Collaborator

kcudnik commented Oct 9, 2019

of course this needs to be build/checked again after swss-common lua fix

@anilkpandey
Copy link
Contributor Author

retest this please

@anilkpandey
Copy link
Contributor Author

build passed. please merge.

@lguohan
Copy link
Contributor

lguohan commented Nov 26, 2019

please remove .DS_Store

@anilkpandey
Copy link
Contributor Author

please remove .DS_Store

Done.

@lguohan lguohan merged commit 34a4202 into sonic-net:master Nov 27, 2019
@kcudnik
Copy link
Collaborator

kcudnik commented Dec 6, 2019

@anilkpandey Hey, since you worked on this PR i want to ask about special case, since we are moving forward with supporting syncd multiple switch instances in the same process, then for example there can be possibility that we do a flush with no attributes, just for a single switch, then we want all fdb entries to be removed from that single switch, this is the case when port=0 and bv_id = 0, but your script (the one in swss-common fdb_flush.lua) will remove all the fdb entries from possible all switches.
To make this work, an extra param needs to be pushed and checked which is switchId, and cehcking each fdb_entry.switch_id in each entry, before actually making remove.
This switchId must be passed to the lua script and inside checked if fdb entry to be removed comes from that particular switch.
Ideally this swtich Id would need to be checked in other cases too (for example when bv_id is specified and non zero). Since we won't cross pass objects between switches (for example case when passing one bv_id from one switch to another is forbidden) to be extra sure, always switch Id should be checked.
Can you make an update to your script and submit PR ?

@anilkpandey
Copy link
Contributor Author

Yes, I will create a PR for this change.

@kcudnik
Copy link
Collaborator

kcudnik commented Dec 6, 2019

thanks !

zhenggen-xu pushed a commit to zhenggen-xu/sonic-sairedis that referenced this pull request Feb 19, 2020
Added code changes related to Layer 2 Forwarding Enhancement:

As part of L2 enhancements, per port, per vlan and per port-vlan fdb flush support has been added. The fdb flush response from SAI comes as individual mac delete as mentioned in the HLD. The change here is to follow the same way for VS as well.

Regarding SAI_FDB_EVENT_FLUSHED, it is handled in sync only for 'flush all' case and handling SAI_FDB_EVENT_FLUSHED for per port and per port-vlan will require hgetall on all fdb keys. So, individual mac delete response is preferred.

Another change is to move fdb handling to fdborch. As the fdb reference count is now maintained in fdborch as well, need to move the sai redis fdb handling also there to avoid the reference counts to go out of sync between the two.
pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022
Added code changes related to Layer 2 Forwarding Enhancement:

As part of L2 enhancements, per port, per vlan and per port-vlan fdb flush support has been added. The fdb flush response from SAI comes as individual mac delete as mentioned in the HLD. The change here is to follow the same way for VS as well.

Regarding SAI_FDB_EVENT_FLUSHED, it is handled in sync only for 'flush all' case and handling SAI_FDB_EVENT_FLUSHED for per port and per port-vlan will require hgetall on all fdb keys. So, individual mac delete response is preferred.

Another change is to move fdb handling to fdborch. As the fdb reference count is now maintained in fdborch as well, need to move the sai redis fdb handling also there to avoid the reference counts to go out of sync between the two.
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.

4 participants