[Vxlanorch] Create encap tunnel map entry for VXLAN tunnel

[msosyak]

What I did Added code to create encap tunnel map entry

Why I did it The encap entry was not created after the configuration of VLAN to VNI map in

How I verified it Configure L2 VXLAN tunnel. check sairedis.rec. two entries should be created. One for encap another for decap. Details if related

[msosyak]

@srj102 @kperumalbfn @mickeyspiegel Please review

[prsunny]

@dgsudharsan to review. Please add VS test and check coverage

[dgsudharsan]

@msosyak There is an intentional comment in the code stating only decap is allowed https://github.com/Azure/sonic-swss/blob/50d5be2b399e9bcc43973f99d72d76e4effd8cc0/orchagent/vxlanorch.cpp#L2014

I am not sure what the intention is but @prsunny @srj102 Can you please clarify it?

[lgtm-com[bot]]

This pull request introduces 1 alert when merging fbcef49a44172b23248593b0ee8a7b83fed2e818 into 1fd1dbfe7eaaa52f1d7edbdd2bb88483f031fd5e - view on LGTM.com

new alerts:

• 1 for Unused local variable

[prsunny]

@msosyak There is an intentional comment in the code stating only decap is allowed

https://github.com/Azure/sonic-swss/blob/50d5be2b399e9bcc43973f99d72d76e4effd8cc0/orchagent/vxlanorch.cpp#L2014

I am not sure what the intention is but @prsunny @srj102 Can you please clarify it?

@msosyak , VXLAN_TUNNEL_MAP is intended only for decap case and is a legacy feature. For the regular VLAN-VNI encap/decap, it is handled differently and not part of this Config_DB entry. This change breaks the previous feature. Kindly check the EVPN HLD for further details.

[mickeyspiegel]

@prsunny What do you mean by "legacy feature"? How are switches supposed to learn VLAN <-> VNI mappings? Which SAI attributes are used for this?

[prsunny]

@prsunny What do you mean by "legacy feature"? How are switches supposed to learn VLAN <-> VNI mappings? Which SAI attributes are used for this?

As I said, it is part of the HLD. Let me clarify, when does user configures VXLAN_TUNNEL_MAP? AFAIK, it is not part of CLI or any other user config except created by neighbor_advertiser which is a decap only feature.

[mickeyspiegel]

@prsunny When I read the EVPN HLD, I don't see these points clarified. In fact there is very little discussion of what actually goes down to the switch through SAI. Looking at the SAI definitions, neither fdb_entry nor vlan_member have any attributes saying anything about vni, so vni has to come down to the switch some other way. I thought the normal method for this would be to create SAI_TUNNEL_MAP_ENTRY with SAI_TUNNEL_MAP_TYPE_VLAN_ID_TO_VNI. When we ran the SONiC code, we saw no such entry being generated, which is why we suggested this code change.

Is there some other command required that triggers SAI to tell the switch about the vlan to vni mappings?

When I look at RemoteVnip2pOrch, I don't see any code generating any such SAI entries. I see the call to addVlanMember based on the vlan, which has nothing to do with the vni. I see a call to tunnel_orch->addTunnelUser, but it looks like that ignores the vlan parameter completely.

[mickeyspiegel]

@prsunny Some quotes from the EVPN VXLAN HLD:

1. From Configuration and Management Requirements: Support configuration of a global VLAN-VNI map
2. From Functional Description, L2 forwarding over VXLAN: Traffic received from the access ports are L2 forwarded over VXLAN tunnels if the DMAC lookup result does not match the switch MAC. ... The VNI is based on the configured global VLAN-VNI map.
3. From Per Tunnel Mapper Handling: there is a need to support a Global VLAN VNI map common to all the Tunnels. This is achieved by reusing the same encap and decap mapper ids associated with the P2MP tunnel object for all the dynamic P2P tunnels.

None of this makes it sound like the vlan to vni mapping depends on what is advertised by remote vteps. Moreover looking at the type 3 routes, I thought the ethernet tag was always 0 in which case vni is all that we have to go on, so vlan has to be derived from local configuration.

I just looked at the sonic-swss tests to see whether they can shed any light. I see checks for proper addition of fdb entries and vlan members. That is all good. However, neither fdb entry nor vlan member tells the switch anything about what vni to use to forward vlan traffic to the remote vtep.

The extension that we are proposing does not cause any problems for fdb entries and vlan members, it just adds a tunnel mapper entry on top that we can use to determine the vni to be used.

[srj102]

Global VLAN-VNI map => mapping is the same in all the nodes of the network. Downstream Assigned VNI => VNI to be used is dictated by each downstream VTEP.

For a Global VLAN-VNI map, VXLAN_TUNNEL_MAP cfg table is used for EVPN as well. There is no new table defined for VLAN VNI mapping.

Without this change, SAI implementations need to program encap as well as decap mappings on getting a decap entry create call from OA.

The vni fields in the VXLAN_FDB_TABLE and VXLAN_REMOTE_VNI table were put in as a place holder for use with Downstream assigned VNI case and not for the Global map case.

In the current implementation if there is a mismatch between the locally configured vni and the vni received from the remote then the EVPN route will not be installed.

If we don't see the decap only vxlan tunnel and EVPN Tunnels co-existing then one suggestion that a knob can be introduced to use the VXLAN_TUNNEL_MAP entry for decap_only or for both encap_decap. This knob can be at the VXLAN_TUNNEL cfg db.