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

[vxlan]add L2-vxlan [#376] #867

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

shine4chen
Copy link
Contributor

What I did
Add L2 vxlan feature

Why I did it
Current vxlan implementation only support VNET scenario. We add vxlan support on L2 scenario.

How I verified it
Tested it in nephos lab

Details if related
[vxlanorch]

  • When creating vxlan tunnel, create its bridge port. If VXLAN tunnel
    without bridge port, FDB MAC can not be learnt from or set to VXLAN
    tunnel in ASIC.
  • Add the tunnel name map to counter table, so that the ‘show mac’
    command can display the FDB learnt from VXLAN tunnel.
  • Send command to Linux kernel to create L2 VXLAN tunnel interface.

[fdborch]

  • FDB MAC can be learnt from or set to VXLAN tunnel in ASIC

@shine4chen
Copy link
Contributor Author

@jianjundong

[vxlanorch]

* Adding the following logic:
- When creating vxlan tunnel, create its bridge port. If VXLAN tunnel
without bridge port, FDB MAC can not be learnt from or set to VXLAN
tunnel in ASIC.
- Add the tunnel name map to counter table, so that the ‘show mac’
command can display the FDB learnt from VXLAN tunnel.
- Send command to Linux kernel to create L2 VXLAN tunnel interface.

[fdborch]
* FDB MAC can be learnt from or set to VXLAN tunnel in ASIC

Signed-off-by: jianjun.dong <jianjun.dong@nephosinc.com>
Copy link
Collaborator

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

Please see comments

orchagent/vxlanorch.cpp Outdated Show resolved Hide resolved
orchagent/vxlanorch.cpp Show resolved Hide resolved
orchagent/vxlanorch.cpp Outdated Show resolved Hide resolved
orchagent/fdborch.cpp Outdated Show resolved Hide resolved
orchagent/fdborch.cpp Outdated Show resolved Hide resolved
sai_fdb_entry_t fdb_entry;

fdb_entry.switch_id = gSwitchId;
memcpy(fdb_entry.mac_address, entry.mac.getMac(), sizeof(sai_mac_t));
fdb_entry.bv_id = entry.bv_id;

Port port;
/* Retry until port is created */
if (!m_portsOrch->getPort(port_name, port))
sai_object_id_t bridge_port_id = SAI_NULL_OBJECT_ID;
Copy link
Contributor

Choose a reason for hiding this comment

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

The Sonic VxLAN HLD refers to a separate VXLAN_FDB_TABLE for MACs pointing to remote VTEP. addFdbEntry is to handle entry additions in APP_FDB_TABLE. The current code changes seem to indicate that the VxLAN MACs are populated in the APP_FDB_TABLE. Is there a change in the design ?

Either way It is better to handle VxLAN FDB entry add/remove in a different function to keep it clean instead of checking for port_name against VTT in multiple places.

Choose a reason for hiding this comment

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

Yes, vxlan tunnel MAC is set in APP_FDB_TABLE currently. Packet forwarding is correctly in lab test.
In Sonic VxLAN HLD, I don't see any description about VXLAN_FDB_TABLE, only 'Add VxlanOrch as a member of FDBOrch. For FDB entries learnt on remote VTEP, app-fdb-table shall be updated and programmed to SAI by getting the BridgeIf/RemoteVTEP mapping from VxlanOrch. (TBD)'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Thanks. I think that VXLAN_FDB_TABLE is used for L3 vxlan scenario, such as EVPN, entries in the table are exchanged by MBGP. The modification here is only for L2 vxlan scenario, entries are learned by ASIC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The existing fdborch can't listen to VXLAN_FDB_TABLE. We can refine the code after fdbOrch support listening to vxlan_fdb_table event.

orchagent/fdborch.cpp Outdated Show resolved Hide resolved
leoli-nps and others added 3 commits May 31, 2019 03:35
Signed-off-by: leo.li <leo.li@nephosinc.com>
Signed-off-by: shine.chen <shine.chen@nephosinc.com>
Signed-off-by: shine.chen <shine.chen@nephosinc.com>
@shine4chen
Copy link
Contributor Author

retest this please

EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
- Fix Python warnings flagged by "LGTM" plugin.
- Also make all 'except' statements Python3-compliant
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants