Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Create a separate VPP VXLAN tunnel per VNI instead of only the first mapper entry. Follows the L3 tunnel_encap_nexthop_action pattern: iterate all decap mapper entries and create/remove a VPP tunnel for each VNI-VLAN mapping. Signed-off-by: Samuel Benko <samuel.benko@pantheon.tech>
1382584 to
ead225f
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Samuel Benko <samuel.benko@pantheon.tech>
When a VNI-to-VLAN mapper entry is added after a P2P tunnel already exists (BGP IMET arrived before all maps were configured), the VPP tunnel for that VNI was never created. Fix by hooking into SAI_OBJECT_TYPE_TUNNEL_MAP_ENTRY creation in SwitchVpp::create() to check for existing P2P tunnels and create the missing VPP tunnel. Signed-off-by: Samuel Benko <samuel.benko@pantheon.tech>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hey @yue-fred-gao, this is a patch to support multi L2 vxlan tunnel with a small refactor of tunnel creation as well to allow late map entry additions. Can someone review it? Thank you. |
| // Only handle VNI_TO_VLAN_ID mapper type (L2 VXLAN) | ||
| auto mapper_obj = m_switch_db->get_sai_object(SAI_OBJECT_TYPE_TUNNEL_MAP, | ||
| sai_serialize_object_id(tunnel_map_oid)); | ||
| if (!mapper_obj) { |
There was a problem hiding this comment.
This is an error if db is in inconsistent state. We should log an error message.
| SWSS_LOG_NOTICE("Tunnel %s not in L2 tunnel map, skipping removal", | ||
| sai_serialize_object_id(tunnel_oid).c_str()); | ||
| return SAI_STATUS_SUCCESS; | ||
| uint32_t vni = 0; |
There was a problem hiding this comment.
According to SAI definition, tunnel map entry has tunnel map type: https://github.com/opencomputeproject/SAI/blob/9ca8312f2f78c35051344fc36c5ab0ab3e1e8d62/inc/saitunnel.h#L107. I think we should check the type first and skip if it is not SAI_TUNNEL_MAP_TYPE_VNI_TO_VLAN_ID. The code will be more readable.
| sai_serialize_object_id(tunnel_oid).c_str(), | ||
| tunnel_data.sw_if_index, tunnel_data.vni, tunnel_data.vlan_id); | ||
| // Find P2P tunnels that reference this mapper as a decap mapper | ||
| auto& tunnel_hash = m_switch_db->m_objectHash.at(SAI_OBJECT_TYPE_TUNNEL); |
There was a problem hiding this comment.
you can call get_child_objs from tunnel_mapper object with type SAI_OBJECT_TYPE_TUNNEL to find all the tunnel objects that are referring to this tunnel_mapper.
| tunnel_data.dst_ip = dst_ip; | ||
| tunnel_data.vlan_id = vlan_id; // Store for cleanup | ||
| sai_status_t | ||
| TunnelManager::remove_l2_vxlan_tunnel( |
There was a problem hiding this comment.
You added handle_l2_vxlan_tunnel_map_entry to process new map entry after tunnel is created. I think we should handle map entry deleted before tunnel by removing the corresponding vpp tunnels instead of cleaning up tunnels when SAI tunnel is deleted. Removing a tunnel map entry alone without deleting tunnel might be a valid use case.
Following PR #1761, this one aims to add functionality of creating/deleting multiple L2 VXLAN tunnels based on VLAN VNI pairs, something which didn't work in the aforementioned base L2 VXLAN support PR.
Root cause: The code that creates L2 VXLAN tunnels looped through VNI-to-VLAN mapper entries but had a break after the first one. Only one VPP tunnel was ever created per remote VTEP, regardless of how many VNIs were configured.
This patch introduces 3 fixes: