[EVPN]Modified tunnel creation logic when creating tunnel in VRF-VNI map creation flow (#2404)

Same as PR #2387

- What I did
To fix issue sonic-net/sonic-buildimage#11428
Modified the logic of tunnel map creation to create tunnel with tunnel map for vlan-vni map in addition to vrf-vni map when tunnel is created first time in the VRF-VNI map processing flow.
Modified the tunnel stats interval to 10 sec
Modified the logic to create bridge port for p2mp tunnel only when p2p tunnel is not supported

- Why I did it
During the configuration phase when VRF-VNI map arrives before VLAN-VNI map, the tunnel is created without a tunnel map for vlan-vni membership. This is problematic when VLAN to VNI map arrives later, tunnel map entry cannot be created since the tunnel map doesn't exist and its a create only attribute in SAI.

- How I verified it
Modified UT to add VRF-VNI map first and VLAN-VLAN map later

Author: dgsudharsan

orchagent/vxlanorch.cpp

@@ -494,67 +494,6 @@ VxlanTunnel::~VxlanTunnel()
                                           src_creation_, false);
 }
 
-bool VxlanTunnel::createTunnel(MAP_T encap, MAP_T decap, uint8_t encap_ttl)
-{
-    try
-    {
-        VxlanTunnelOrch* tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();
-        sai_ip_address_t ips, ipd, *ip=nullptr;
-        uint8_t mapper_list = 0;
-        swss::copy(ips, src_ip_);
-
-        // Only a single mapper type is created
-
-        if (decap == MAP_T::VNI_TO_BRIDGE)
-        {
-            TUNNELMAP_SET_BRIDGE(mapper_list);
-        }
-        else if (decap == MAP_T::VNI_TO_VLAN_ID)
-        {
-            TUNNELMAP_SET_VLAN(mapper_list);
-        }
-        else
-        {
-            TUNNELMAP_SET_VRF(mapper_list);
-        }
-        
-        createMapperHw(mapper_list, (encap == MAP_T::MAP_TO_INVALID) ? 
-                       TUNNEL_MAP_USE_DECAP_ONLY: TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP);
-
-        if (encap != MAP_T::MAP_TO_INVALID)
-        {
-            ip = &ips;
-        }
-
-        ids_.tunnel_id = create_tunnel(&ids_, ip, NULL, gUnderlayIfId, false, encap_ttl);
-
-        if (ids_.tunnel_id != SAI_NULL_OBJECT_ID)
-        {
-            tunnel_orch->addTunnelToFlexCounter(ids_.tunnel_id, tunnel_name_);
-        }
-
-        ip = nullptr;
-        if (!dst_ip_.isZero())
-        {
-            swss::copy(ipd, dst_ip_);
-            ip = &ipd;
-        }
-
-        ids_.tunnel_term_id = create_tunnel_termination(ids_.tunnel_id, ips, ip, gVirtualRouterId);
-        active_ = true;
-        tunnel_map_ = { encap, decap };
-    }
-    catch (const std::runtime_error& error)
-    {
-        SWSS_LOG_ERROR("Error creating tunnel %s: %s", tunnel_name_.c_str(), error.what());
-        // FIXME: add code to remove already created objects
-        return false;
-    }
-
-    SWSS_LOG_NOTICE("Vxlan tunnel '%s' was created", tunnel_name_.c_str());
-    return true;
-}
-
 sai_object_id_t VxlanTunnel::addEncapMapperEntry(sai_object_id_t obj, uint32_t vni, tunnel_map_type_t type)
 {
     const auto encap_id = getEncapMapId(type);
@@ -1924,20 +1863,18 @@ bool VxlanTunnel::isTunnelReferenced()
     Port tunnelPort;
     bool dip_tunnels_used = tunnel_orch->isDipTunnelsSupported();
 
-    ret = gPortsOrch->getPort(port_tunnel_name, tunnelPort);
-    if (!ret)
-    {
-        SWSS_LOG_ERROR("Get port failed for source vtep %s", port_tunnel_name.c_str());
-        return false;
-    }
-
-
     if (dip_tunnels_used)
     {
         return (getDipTunnelCnt() != 0);
     }
     else
     {
+        ret = gPortsOrch->getPort(port_tunnel_name, tunnelPort);
+        if (!ret)
+        {
+            SWSS_LOG_ERROR("Get port failed for source vtep %s", port_tunnel_name.c_str());
+            return false;
+        }
         if (tunnelPort.m_fdb_count != 0)
         {
 	    return true;
@@ -2013,19 +1950,21 @@ bool VxlanTunnelMapOrch::addOperation(const Request& request)
     if (!tunnel_obj->isActive())
     {
         //@Todo, currently only decap mapper is allowed
-        //tunnel_obj->createTunnel(MAP_T::MAP_TO_INVALID, MAP_T::VNI_TO_VLAN_ID);
         uint8_t mapper_list = 0;
         TUNNELMAP_SET_VLAN(mapper_list);
         TUNNELMAP_SET_VRF(mapper_list);
         tunnel_obj->createTunnelHw(mapper_list,TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP);
-        Port tunPort;
-        auto src_vtep = tunnel_obj->getSrcIP().to_string();
-        if (!tunnel_orch->getTunnelPort(src_vtep, tunPort, true))
+        if (!tunnel_orch->isDipTunnelsSupported())
         {
-            auto port_tunnel_name = tunnel_orch->getTunnelPortName(src_vtep, true);
-            gPortsOrch->addTunnel(port_tunnel_name, tunnel_obj->getTunnelId(), false);
-            gPortsOrch->getPort(port_tunnel_name,tunPort);
-            gPortsOrch->addBridgePort(tunPort);
+            Port tunPort;
+            auto src_vtep = tunnel_obj->getSrcIP().to_string();
+            if (!tunnel_orch->getTunnelPort(src_vtep, tunPort, true))
+            {
+                auto port_tunnel_name = tunnel_orch->getTunnelPortName(src_vtep, true);
+                gPortsOrch->addTunnel(port_tunnel_name, tunnel_obj->getTunnelId(), false);
+                gPortsOrch->getPort(port_tunnel_name,tunPort);
+                gPortsOrch->addBridgePort(tunPort);
+            }
         }
     }
 
@@ -2117,26 +2056,27 @@ bool VxlanTunnelMapOrch::delOperation(const Request& request)
       auto port_tunnel_name = tunnel_orch->getTunnelPortName(src_vtep, true);
       bool ret;
 
-      ret = gPortsOrch->getPort(port_tunnel_name, tunnelPort);
       // If there are Dynamic DIP Tunnels referring to this SIP Tunnel 
       // then mark it as pending for delete. 
       if (!tunnel_obj->isTunnelReferenced())
       {
-          if (!ret)
+          if (!tunnel_orch->isDipTunnelsSupported())
           {
-              SWSS_LOG_ERROR("Get port failed for source vtep %s", port_tunnel_name.c_str());
-              return true;
+              ret = gPortsOrch->getPort(port_tunnel_name, tunnelPort);
+              if (!ret)
+              {
+                  SWSS_LOG_ERROR("Get port failed for source vtep %s", port_tunnel_name.c_str());
+                  return true;
+              }
+              ret = gPortsOrch->removeBridgePort(tunnelPort);
+              if (!ret)
+              {
+                  SWSS_LOG_ERROR("Remove Bridge port failed for source vtep = %s fdbcount = %d",
+                                 port_tunnel_name.c_str(), tunnelPort.m_fdb_count);
+                  return true;
+              }
+              gPortsOrch->removeTunnel(tunnelPort);
           }
-          ret = gPortsOrch->removeBridgePort(tunnelPort);
-          if (!ret)
-          {
-              SWSS_LOG_ERROR("Remove Bridge port failed for source vtep = %s fdbcount = %d",
-                             port_tunnel_name.c_str(), tunnelPort.m_fdb_count);
-              return true;
-          }
-
-          gPortsOrch->removeTunnel(tunnelPort);
-
           uint8_t mapper_list=0;
           TUNNELMAP_SET_VLAN(mapper_list);
           TUNNELMAP_SET_VRF(mapper_list);
@@ -2152,6 +2092,7 @@ bool VxlanTunnelMapOrch::delOperation(const Request& request)
           }
           else
           {
+              gPortsOrch->getPort(port_tunnel_name, tunnelPort);
               SWSS_LOG_WARN("Postponing the SIP Tunnel HW deletion Remote reference count = %d",
                             gPortsOrch->getBridgePortReferenceCount(tunnelPort));
           }
@@ -2218,7 +2159,22 @@ bool VxlanVrfMapOrch::addOperation(const Request& request)
     {
         if (!tunnel_obj->isActive()) 
         {
-            tunnel_obj->createTunnel(MAP_T::VRID_TO_VNI, MAP_T::VNI_TO_VRID);
+            uint8_t mapper_list = 0;
+            TUNNELMAP_SET_VLAN(mapper_list);
+            TUNNELMAP_SET_VRF(mapper_list);
+            tunnel_obj->createTunnelHw(mapper_list,TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP);
+            if (!tunnel_orch->isDipTunnelsSupported())
+            {
+                Port tunPort;
+                auto src_vtep = tunnel_obj->getSrcIP().to_string();
+                if (!tunnel_orch->getTunnelPort(src_vtep, tunPort, true))
+                {
+                    auto port_tunnel_name = tunnel_orch->getTunnelPortName(src_vtep, true);
+                    gPortsOrch->addTunnel(port_tunnel_name, tunnel_obj->getTunnelId(), false);
+                    gPortsOrch->getPort(port_tunnel_name,tunPort);
+                    gPortsOrch->addBridgePort(tunPort);
+                }
+            }
         }
         vrf_id = vrf_orch->getVRFid(vrf_name);
     }

orchagent/vxlanorch.h

@@ -37,7 +37,7 @@ typedef enum
 #define IS_TUNNELMAP_SET_BRIDGE(x) ((x)& (1<<TUNNEL_MAP_T_BRIDGE))
 
 #define TUNNEL_STAT_COUNTER_FLEX_COUNTER_GROUP "TUNNEL_STAT_COUNTER"
-#define TUNNEL_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS 1000
+#define TUNNEL_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS 10000
 #define LOCAL_TUNNEL_PORT_PREFIX "Port_SRC_VTEP_"
 #define EVPN_TUNNEL_PORT_PREFIX  "Port_EVPN_"
 #define EVPN_TUNNEL_NAME_PREFIX  "EVPN_"
@@ -149,7 +149,6 @@ class VxlanTunnel
         return active_;
     }
 
-    bool createTunnel(MAP_T encap, MAP_T decap, uint8_t encap_ttl=0);
     sai_object_id_t addEncapMapperEntry(sai_object_id_t obj, uint32_t vni, 
                                         tunnel_map_type_t type=TUNNEL_MAP_T_VIRTUAL_ROUTER);
     sai_object_id_t addDecapMapperEntry(sai_object_id_t obj, uint32_t vni,

tests/evpn_tunnel.py

@@ -485,7 +485,7 @@ def check_vxlan_tunnel_map_entry(self, dvs, tunnel_name, vidlist, vnidlist):
             (exitcode, out) = dvs.runcmd(iplinkcmd)
             assert exitcode == 0, "Kernel device not created"
 
-    def check_vxlan_sip_tunnel_delete(self, dvs, tunnel_name, sip):
+    def check_vxlan_sip_tunnel_delete(self, dvs, tunnel_name, sip, ignore_bp = True):
         asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
         app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
 
@@ -511,36 +511,35 @@ def check_vxlan_sip_tunnel_delete(self, dvs, tunnel_name, sip):
         status, fvs = tbl.get(self.tunnel_map_map[tunnel_name][3])
         assert status == False, "SIP Tunnel mapper3 not deleted from ASIC_DB"
 
-        tbl = swsscommon.Table(asic_db, self.ASIC_BRIDGE_PORT)
-        status, fvs = tbl.get(self.bridgeport_map[sip])
-        assert status == False, "Tunnel bridgeport entry not deleted"
+        if not ignore_bp:
+            tbl = swsscommon.Table(asic_db, self.ASIC_BRIDGE_PORT)
+            status, fvs = tbl.get(self.bridgeport_map[sip])
+            assert status == False, "Tunnel bridgeport entry not deleted"
 
-    def check_vxlan_sip_tunnel(self, dvs, tunnel_name, src_ip, vidlist, vnidlist, dst_ip = '0.0.0.0', skip_dst_ip = 'True'):
+    def check_vxlan_sip_tunnel(self, dvs, tunnel_name, src_ip, vidlist, vnidlist, 
+                               dst_ip = '0.0.0.0', skip_dst_ip = 'True', ignore_bp = True):
         asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
         app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
 
         tunnel_map_id  = self.helper.get_created_entries(asic_db, self.ASIC_TUNNEL_MAP, self.tunnel_map_ids, 4)
         tunnel_id      = self.helper.get_created_entry(asic_db, self.ASIC_TUNNEL_TABLE, self.tunnel_ids)
         tunnel_term_id = self.helper.get_created_entry(asic_db, self.ASIC_TUNNEL_TERM_ENTRY, self.tunnel_term_ids)
-        tunnel_map_entry_id = self.helper.get_created_entries(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, self.tunnel_map_entry_ids, 3)
 
         # check that the vxlan tunnel termination are there
         assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_MAP) == (len(self.tunnel_map_ids) + 4), "The TUNNEL_MAP wasn't created"
         assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_MAP_ENTRY) == (len(self.tunnel_map_entry_ids) + 3), "The TUNNEL_MAP_ENTRY is created"
         assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_TABLE) == (len(self.tunnel_ids) + 1), "The TUNNEL wasn't created"
         assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_TERM_ENTRY) == (len(self.tunnel_term_ids) + 1), "The TUNNEL_TERM_TABLE_ENTRY wasm't created"
 
-        self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP, tunnel_map_id[2],
-                        {
-                            'SAI_TUNNEL_MAP_ATTR_TYPE': 'SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID',
-                        }
-                )
+        expected_attributes_1 = {}
+        expected_attributes_1['SAI_TUNNEL_MAP_ATTR_TYPE'] = 'SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID'
+        ret = self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP, expected_attributes_1)
+        assert len(ret) == 1, "Unexpected number of tunnel maps created for type SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID"
+
+        expected_attributes_1['SAI_TUNNEL_MAP_ATTR_TYPE'] = 'SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI'
+        ret = self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP, expected_attributes_1)
+        assert len(ret) == 1, "Unexpected number of tunnel maps created for type SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI"
 
-        self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP, tunnel_map_id[3],
-                        {
-                            'SAI_TUNNEL_MAP_ATTR_TYPE': 'SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI',
-                        }
-                )
 
         decapstr = '2:' + tunnel_map_id[0] + ',' + tunnel_map_id[2]
         encapstr = '2:' + tunnel_map_id[1] + ',' + tunnel_map_id[3]
@@ -571,15 +570,15 @@ def check_vxlan_sip_tunnel(self, dvs, tunnel_name, src_ip, vidlist, vnidlist, ds
 
         expected_attributes_1 = {
         'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP_TYPE': 'SAI_TUNNEL_MAP_TYPE_VNI_TO_VLAN_ID',
-        'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP': tunnel_map_id[0],
         'SAI_TUNNEL_MAP_ENTRY_ATTR_VLAN_ID_VALUE': vidlist[0],
         'SAI_TUNNEL_MAP_ENTRY_ATTR_VNI_ID_KEY': vnidlist[0],
         }
 
         for x in range(len(vidlist)):
             expected_attributes_1['SAI_TUNNEL_MAP_ENTRY_ATTR_VLAN_ID_VALUE'] = vidlist[x]
             expected_attributes_1['SAI_TUNNEL_MAP_ENTRY_ATTR_VNI_ID_KEY'] = vnidlist[x]
-            self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, tunnel_map_entry_id[x], expected_attributes_1)
+            self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, expected_attributes_1)
+            assert len(ret) == 1, "Unexpected number of tunnel map entries created for VLAN to VNI mapping"
 
         expected_siptnl_attributes = {
             'src_ip': src_ip,
@@ -593,16 +592,18 @@ def check_vxlan_sip_tunnel(self, dvs, tunnel_name, src_ip, vidlist, vnidlist, ds
         assert len(ret) == 1, "More than 1 Tunn statetable entry created"
         self.tunnel_appdb[tunnel_name] = ret[0]
 
-        expected_bridgeport_attributes = {
-        'SAI_BRIDGE_PORT_ATTR_TYPE': 'SAI_BRIDGE_PORT_TYPE_TUNNEL',
-        'SAI_BRIDGE_PORT_ATTR_TUNNEL_ID': tunnel_id,
-        'SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE': 'SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE',
-        'SAI_BRIDGE_PORT_ATTR_ADMIN_STATE': 'true',
-        }
-        ret = self.helper.get_key_with_attr(asic_db, self.ASIC_BRIDGE_PORT, expected_bridgeport_attributes)
-        assert len(ret) > 0, "Bridgeport entry not created"
-        assert len(ret) == 1, "More than 1 bridgeport entry created"
-        self.bridgeport_map[src_ip] = ret[0]
+        if not ignore_bp:
+            expected_bridgeport_attributes = {
+            'SAI_BRIDGE_PORT_ATTR_TYPE': 'SAI_BRIDGE_PORT_TYPE_TUNNEL',
+            'SAI_BRIDGE_PORT_ATTR_TUNNEL_ID': tunnel_id,
+            'SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE': 'SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE',
+            'SAI_BRIDGE_PORT_ATTR_ADMIN_STATE': 'true',
+            }
+            ret = self.helper.get_key_with_attr(asic_db, self.ASIC_BRIDGE_PORT, expected_bridgeport_attributes)
+            assert len(ret) > 0, "Bridgeport entry not created"
+            assert len(ret) == 1, "More than 1 bridgeport entry created"
+            self.bridgeport_map[src_ip] = ret[0]
+
         self.tunnel_map_ids.update(tunnel_map_id)
         self.tunnel_ids.add(tunnel_id)
         self.tunnel_term_ids.add(tunnel_term_id)
@@ -797,37 +798,33 @@ def check_vxlan_tunnel_entry(self, dvs, tunnel_name, vnet_name, vni_id):
     def check_vxlan_tunnel_vrf_map_entry(self, dvs, tunnel_name, vrf_name, vni_id):
         asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
 
-        if (self.tunnel_map_map.get(tunnel_name) is None):
-            tunnel_map_id = self.helper.get_created_entries(asic_db, self.ASIC_TUNNEL_MAP, self.tunnel_map_ids, 4)
-        else:
-            tunnel_map_id = self.tunnel_map_map[tunnel_name]
-
         tunnel_map_entry_id = self.helper.get_created_entries(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, self.tunnel_map_entry_ids, 3)
 
         # check that the vxlan tunnel termination are there
         assert self.helper.how_many_entries_exist(asic_db, self.ASIC_TUNNEL_MAP_ENTRY) == (len(self.tunnel_map_entry_ids) + 3), "The TUNNEL_MAP_ENTRY is created too early"
 
-        self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, tunnel_map_entry_id[1],
+        ret = self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP_ENTRY,
             {
                 'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP_TYPE': 'SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI',
-                'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP': tunnel_map_id[3],
                 'SAI_TUNNEL_MAP_ENTRY_ATTR_VIRTUAL_ROUTER_ID_KEY': self.vr_map[vrf_name].get('ing'),
                 'SAI_TUNNEL_MAP_ENTRY_ATTR_VNI_ID_VALUE': vni_id,
             }
         )
 
-        self.tunnel_map_vrf_entry_ids.update(tunnel_map_entry_id[1])
+        assert len(ret) == 1, "Invalid number of tunnel map entries for SAI_TUNNEL_MAP_TYPE_VIRTUAL_ROUTER_ID_TO_VNI"
+
+        self.tunnel_map_vrf_entry_ids.update(ret[0])
 
-        self.helper.check_object(asic_db, self.ASIC_TUNNEL_MAP_ENTRY, tunnel_map_entry_id[2],
+        ret = self.helper.get_key_with_attr(asic_db, self.ASIC_TUNNEL_MAP_ENTRY,
             {
                 'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP_TYPE': 'SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID',
-                'SAI_TUNNEL_MAP_ENTRY_ATTR_TUNNEL_MAP': tunnel_map_id[2],
                 'SAI_TUNNEL_MAP_ENTRY_ATTR_VNI_ID_KEY': vni_id,
                 'SAI_TUNNEL_MAP_ENTRY_ATTR_VIRTUAL_ROUTER_ID_VALUE': self.vr_map[vrf_name].get('egr'),
             }
         )
 
-        self.tunnel_map_vrf_entry_ids.update(tunnel_map_entry_id[2])
+        assert len(ret) == 1, "Invalid number of tunnel map entries for SAI_TUNNEL_MAP_TYPE_VNI_TO_VIRTUAL_ROUTER_ID"
+        self.tunnel_map_vrf_entry_ids.update(ret[0])
         self.tunnel_map_entry_ids.update(tunnel_map_entry_id)
 
     def check_vxlan_tunnel_vrf_map_entry_remove(self, dvs, tunnel_name, vrf_name, vni_id):