[vnetorch] Change logic for adding VNet interface (sonic-net#761)

* [vnetorch] Change logic for adding VNet interface

Signed-off-by: Marian Pritsak <marianp@mellanox.com>

* Insert ipprefix only if exists

Signed-off-by: Marian Pritsak <marianp@mellanox.com>

Author: marian-pritsak

orchagent/intfsorch.cpp

@@ -100,6 +100,73 @@ set<IpPrefix> IntfsOrch:: getSubnetRoutes()
     return subnet_routes;
 }
 
+bool IntfsOrch::setIntf(const string& alias, sai_object_id_t vrf_id, const IpPrefix *ip_prefix)
+{
+    SWSS_LOG_ENTER();
+
+    Port port;
+    gPortsOrch->getPort(alias, port);
+
+    auto it_intfs = m_syncdIntfses.find(alias);
+    if (it_intfs == m_syncdIntfses.end())
+    {
+        if (addRouterIntfs(vrf_id, port))
+        {
+            IntfsEntry intfs_entry;
+            intfs_entry.ref_count = 0;
+            m_syncdIntfses[alias] = intfs_entry;
+        }
+        else
+        {
+            return false;
+        }
+    }
+
+    if (!ip_prefix || m_syncdIntfses[alias].ip_addresses.count(*ip_prefix))
+    {
+        /* Request to create router interface, no prefix present or Duplicate entry */
+        return true;
+    }
+
+    /* NOTE: Overlap checking is required to handle ifconfig weird behavior.
+     * When set IP address using ifconfig command it applies it in two stages.
+     * On stage one it sets IP address with netmask /8. On stage two it
+     * changes netmask to specified in command. As DB is async event to
+     * add IP address with original netmask may come before event to
+     * delete IP with netmask /8. To handle this we in case of overlap
+     * we should wait until entry with /8 netmask will be removed.
+     * Time frame between those event is quite small.*/
+    bool overlaps = false;
+    for (const auto &prefixIt: m_syncdIntfses[alias].ip_addresses)
+    {
+        if (prefixIt.isAddressInSubnet(ip_prefix->getIp()) ||
+                ip_prefix->isAddressInSubnet(prefixIt.getIp()))
+        {
+            overlaps = true;
+            SWSS_LOG_NOTICE("Router interface %s IP %s overlaps with %s.", port.m_alias.c_str(),
+                    prefixIt.to_string().c_str(), ip_prefix->to_string().c_str());
+            break;
+        }
+    }
+
+    if (overlaps)
+    {
+        /* Overlap of IP address network */
+        return false;
+    }
+
+    addSubnetRoute(port, *ip_prefix);
+    addIp2MeRoute(vrf_id, *ip_prefix);
+
+    if (port.m_type == Port::VLAN && ip_prefix->isV4())
+    {
+        addDirectedBroadcast(port, ip_prefix->getBroadcastIp());
+    }
+
+    m_syncdIntfses[alias].ip_addresses.insert(*ip_prefix);
+    return true;
+}
+
 void IntfsOrch::doTask(Consumer &consumer)
 {
     SWSS_LOG_ENTER();
@@ -149,17 +216,7 @@ void IntfsOrch::doTask(Consumer &consumer)
         }
 
         sai_object_id_t vrf_id = gVirtualRouterId;
-        if (!vnet_name.empty())
-        {
-            VNetOrch* vnet_orch = gDirectory.get<VNetOrch*>();
-            if (!vnet_orch->isVnetExists(vnet_name))
-            {
-                it++;
-                continue;
-            }
-            vrf_id = vnet_orch->getVRid(vnet_name);
-        }
-        else if (!vrf_name.empty())
+        if (!vrf_name.empty())
         {
             if (m_vrfOrch->isVRFexists(vrf_name))
             {
@@ -225,67 +282,29 @@ void IntfsOrch::doTask(Consumer &consumer)
                 continue;
             }
 
-            auto it_intfs = m_syncdIntfses.find(alias);
-            if (it_intfs == m_syncdIntfses.end())
+            if (!vnet_name.empty())
             {
-                if (addRouterIntfs(vrf_id, port))
+                VNetOrch* vnet_orch = gDirectory.get<VNetOrch*>();
+                if (!vnet_orch->isVnetExists(vnet_name))
                 {
-                    IntfsEntry intfs_entry;
-                    intfs_entry.ref_count = 0;
-                    m_syncdIntfses[alias] = intfs_entry;
+                    it++;
+                    continue;
                 }
-                else
+                if (!vnet_orch->setIntf(alias, vnet_name, ip_prefix_in_key ? &ip_prefix : nullptr))
                 {
                     it++;
                     continue;
                 }
             }
-
-            vrf_id = port.m_vr_id;
-            if (!ip_prefix_in_key || m_syncdIntfses[alias].ip_addresses.count(ip_prefix))
-            {
-                /* Request to create router interface, no prefix present or Duplicate entry */
-                it = consumer.m_toSync.erase(it);
-                continue;
-            }
-
-            /* NOTE: Overlap checking is required to handle ifconfig weird behavior.
-             * When set IP address using ifconfig command it applies it in two stages.
-             * On stage one it sets IP address with netmask /8. On stage two it
-             * changes netmask to specified in command. As DB is async event to
-             * add IP address with original netmask may come before event to
-             * delete IP with netmask /8. To handle this we in case of overlap
-             * we should wait until entry with /8 netmask will be removed.
-             * Time frame between those event is quite small.*/
-            bool overlaps = false;
-            for (const auto &prefixIt: m_syncdIntfses[alias].ip_addresses)
+            else
             {
-                if (prefixIt.isAddressInSubnet(ip_prefix.getIp()) ||
-                        ip_prefix.isAddressInSubnet(prefixIt.getIp()))
+                if (!setIntf(alias, vrf_id, ip_prefix_in_key ? &ip_prefix : nullptr))
                 {
-                    overlaps = true;
-                    SWSS_LOG_NOTICE("Router interface %s IP %s overlaps with %s.", port.m_alias.c_str(),
-                            prefixIt.to_string().c_str(), ip_prefix.to_string().c_str());
-                    break;
+                    it++;
+                    continue;
                 }
             }
 
-            if (overlaps)
-            {
-                /* Overlap of IP address network */
-                ++it;
-                continue;
-            }
-
-            addSubnetRoute(port, ip_prefix);
-            addIp2MeRoute(vrf_id, ip_prefix);
-
-            if (port.m_type == Port::VLAN && ip_prefix.isV4())
-            {
-                addDirectedBroadcast(port, ip_prefix.getBroadcastIp());
-            }
-
-            m_syncdIntfses[alias].ip_addresses.insert(ip_prefix);
             it = consumer.m_toSync.erase(it);
         }
         else if (op == DEL_COMMAND)

orchagent/intfsorch.h

@@ -35,6 +35,7 @@ class IntfsOrch : public Orch
 
     bool setRouterIntfsMtu(Port &port);
     std::set<IpPrefix> getSubnetRoutes();
+    bool setIntf(const string& alias, sai_object_id_t vrf_id = gVirtualRouterId, const IpPrefix *ip_prefix = nullptr);
 private:
     VRFOrch *m_vrfOrch;
     IntfsTable m_syncdIntfses;

orchagent/vnetorch.cpp

@@ -14,12 +14,14 @@
 #include "vxlanorch.h"
 #include "directory.h"
 #include "swssnet.h"
+#include "intfsorch.h"
 
 extern sai_virtual_router_api_t* sai_virtual_router_api;
 extern sai_route_api_t* sai_route_api;
 extern sai_object_id_t gSwitchId;
 extern Directory<Orch*> gDirectory;
 extern PortsOrch *gPortsOrch;
+extern IntfsOrch *gIntfsOrch;
 
 /*
  * VRF Modeling and VNetVrf class definitions
@@ -163,6 +165,26 @@ VNetOrch::VNetOrch(DBConnector *db, const std::string& tableName, VNET_EXEC op)
     }
 }
 
+bool VNetOrch::setIntf(const string& alias, const string vnet_name, const IpPrefix *prefix)
+{
+    SWSS_LOG_ENTER();
+
+    if (isVnetExecVrf())
+    {
+        if (!isVnetExists(vnet_name))
+        {
+            SWSS_LOG_WARN("VNET %s doesn't exist", vnet_name.c_str());
+            return false;
+        }
+
+        auto *vnet_obj = getTypePtr<VNetVrfObject>(vnet_name);
+        sai_object_id_t vrf_id = vnet_obj->getVRidIngress();
+
+        return gIntfsOrch->setIntf(alias, vrf_id, prefix);
+    }
+
+    return false;
+}
 bool VNetOrch::addOperation(const Request& request)
 {
     SWSS_LOG_ENTER();
@@ -226,8 +248,9 @@ bool VNetOrch::addOperation(const Request& request)
                 create = true;
             }
 
+            VNetVrfObject *vrfObj = dynamic_cast<VNetVrfObject*>(obj.get());
             if (!vxlan_orch->createVxlanTunnelMap(tunnel, TUNNEL_MAP_T_VIRTUAL_ROUTER, vni,
-                                                  obj->getEncapMapId(), obj->getDecapMapId()))
+                                                  vrfObj->getEncapMapId(), vrfObj->getDecapMapId()))
             {
                 SWSS_LOG_ERROR("VNET '%s', tunnel '%s', map create failed",
                                 vnet_name.c_str(), tunnel.c_str());

orchagent/vnetorch.h

@@ -49,19 +49,13 @@ class VNetObject
 public:
     VNetObject(string& tunName, set<string>& peer) : tunnel_(tunName), peer_list_(peer) { }
 
-    virtual sai_object_id_t getEncapMapId() const = 0;
-
-    virtual sai_object_id_t getDecapMapId() const = 0;
-
     virtual bool updateObj(vector<sai_attribute_t>&) = 0;
 
     void setPeerList(set<string>& p_list)
     {
         peer_list_ = p_list;
     }
 
-    virtual sai_object_id_t getVRid() const = 0;
-
     const set<string>& getPeerList() const
     {
         return peer_list_;
@@ -90,17 +84,17 @@ class VNetVrfObject : public VNetObject
 
     set<sai_object_id_t> getVRids() const;
 
-    virtual sai_object_id_t getEncapMapId() const
+    sai_object_id_t getEncapMapId() const
     {
         return getVRidIngress();
     }
 
-    virtual sai_object_id_t getDecapMapId() const
+    sai_object_id_t getDecapMapId() const
     {
         return getVRidEgress();
     }
 
-    virtual sai_object_id_t getVRid() const
+    sai_object_id_t getVRid() const
     {
         return getVRidIngress();
     }
@@ -123,6 +117,7 @@ class VNetOrch : public Orch2
 {
 public:
     VNetOrch(DBConnector *db, const std::string&, VNET_EXEC op = VNET_EXEC::VNET_EXEC_VRF);
+    bool setIntf(const string& alias, const string vnet_name, const IpPrefix *prefix = nullptr);
 
     bool isVnetExists(const std::string& name) const
     {
@@ -135,26 +130,11 @@ class VNetOrch : public Orch2
         return static_cast<T *>(vnet_table_.at(name).get());
     }
 
-    sai_object_id_t getEncapMapId(const std::string& name) const
-    {
-        return vnet_table_.at(name)->getEncapMapId();
-    }
-
-    sai_object_id_t getDecapMapId(const std::string& name) const
-    {
-        return vnet_table_.at(name)->getDecapMapId();
-    }
-
     const set<string>& getPeerList(const std::string& name) const
     {
         return vnet_table_.at(name)->getPeerList();
     }
 
-    sai_object_id_t getVRid(const std::string& name) const
-    {
-        return vnet_table_.at(name)->getVRid();
-    }
-
     string getTunnelName(const std::string& name) const
     {
         return vnet_table_.at(name)->getTunnelName();