[sub intf] Fix kernel side processing to enslave sub interface to non-default vrf

cfgmgr/intfmgr.cpp

@@ -428,16 +428,15 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
 
     string alias(keys[0]);
     string vlanId;
-    string subIntfAlias;
+    string parentAlias;
     size_t found = alias.find(VLAN_SUB_INTERFACE_SEPARATOR);
     if (found != string::npos)
     {
         // This is a sub interface
-        // subIntfAlias holds the complete sub interface name
-        // while alias becomes the parent interface
-        subIntfAlias = alias;
+        // alias holds the complete sub interface name
+        // while parentAlias holds the parent port name
         vlanId = alias.substr(found + 1);
-        alias = alias.substr(0, found);
+        parentAlias = alias.substr(0, found);
     }
     bool is_lo = !alias.compare(0, strlen(LOOPBACK_PREFIX), LOOPBACK_PREFIX);
     string mac = "";
@@ -482,7 +481,7 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
 
     if (op == SET_COMMAND)
     {
-        if (!isIntfStateOk(alias))
+        if (!isIntfStateOk(parentAlias.empty() ? alias : parentAlias))
         {
             SWSS_LOG_DEBUG("Interface is not ready, skipping %s", alias.c_str());
             return false;
@@ -520,74 +519,28 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
             }
         }
 
-        if (!vrf_name.empty())
-        {
-            setIntfVrf(alias, vrf_name);
-        }
-
-        /*Set the mac of interface*/
-        if (!mac.empty())
-        {
-            setIntfMac(alias, mac);
-        }
-        else
-        {
-            FieldValueTuple fvTuple("mac_addr", MacAddress().to_string());
-            data.push_back(fvTuple);
-        }
-
-        if (!proxy_arp.empty())
+        if (!parentAlias.empty())
         {
-            if (!setIntfProxyArp(alias, proxy_arp))
-            {
-                SWSS_LOG_ERROR("Failed to set proxy ARP to \"%s\" state for the \"%s\" interface", proxy_arp.c_str(), alias.c_str());
-                return false;
-            }
-
-            if (!alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX))
-            {
-                FieldValueTuple fvTuple("proxy_arp", proxy_arp);
-                data.push_back(fvTuple);
-            }
-        }
-
-        if (!grat_arp.empty())
-        {
-            if (!setIntfGratArp(alias, grat_arp))
-            {
-                SWSS_LOG_ERROR("Failed to set ARP accept to \"%s\" state for the \"%s\" interface", grat_arp.c_str(), alias.c_str());
-                return false;
-            }
-
-            if (!alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX))
-            {
-                FieldValueTuple fvTuple("grat_arp", grat_arp);
-                data.push_back(fvTuple);
-            }
-        }
-
-        if (!subIntfAlias.empty())
-        {
-            if (m_subIntfList.find(subIntfAlias) == m_subIntfList.end())
+            if (m_subIntfList.find(alias) == m_subIntfList.end())
             {
                 try
                 {
-                    addHostSubIntf(alias, subIntfAlias, vlanId);
+                    addHostSubIntf(parentAlias, alias, vlanId);
                 }
                 catch (const std::runtime_error &e)
                 {
                     SWSS_LOG_NOTICE("Sub interface ip link add failure. Runtime error: %s", e.what());
                     return false;
                 }
 
-                m_subIntfList.insert(subIntfAlias);
+                m_subIntfList.insert(alias);
             }
 
             if (!mtu.empty())
             {
                 try
                 {
-                    setHostSubIntfMtu(subIntfAlias, mtu);
+                    setHostSubIntfMtu(alias, mtu);
                 }
                 catch (const std::runtime_error &e)
                 {
@@ -609,7 +562,7 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
             }
             try
             {
-                setHostSubIntfAdminStatus(subIntfAlias, adminStatus);
+                setHostSubIntfAdminStatus(alias, adminStatus);
             }
             catch (const std::runtime_error &e)
             {
@@ -618,10 +571,57 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
             }
 
             // set STATE_DB port state
-            setSubIntfStateOk(subIntfAlias);
+            setSubIntfStateOk(alias);
         }
-        m_appIntfTableProducer.set(subIntfAlias.empty() ? alias : subIntfAlias, data);
-        m_stateIntfTable.hset(subIntfAlias.empty() ? alias : subIntfAlias, "vrf", vrf_name);
+
+        if (!vrf_name.empty())
+        {
+            setIntfVrf(alias, vrf_name);
+        }
+
+        /*Set the mac of interface*/
+        if (!mac.empty())
+        {
+            setIntfMac(alias, mac);
+        }
+        else
+        {
+            FieldValueTuple fvTuple("mac_addr", MacAddress().to_string());
+            data.push_back(fvTuple);
+        }
+
+        if (!proxy_arp.empty())
+        {
+            if (!setIntfProxyArp(alias, proxy_arp))
+            {
+                SWSS_LOG_ERROR("Failed to set proxy ARP to \"%s\" state for the \"%s\" interface", proxy_arp.c_str(), alias.c_str());
+                return false;
+            }
+
+            if (!alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX))
+            {
+                FieldValueTuple fvTuple("proxy_arp", proxy_arp);
+                data.push_back(fvTuple);
+            }
+        }
+
+        if (!grat_arp.empty())
+        {
+            if (!setIntfGratArp(alias, grat_arp))
+            {
+                SWSS_LOG_ERROR("Failed to set ARP accept to \"%s\" state for the \"%s\" interface", grat_arp.c_str(), alias.c_str());
+                return false;
+            }
+
+            if (!alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX))
+            {
+                FieldValueTuple fvTuple("grat_arp", grat_arp);
+                data.push_back(fvTuple);
+            }
+        }
+
+        m_appIntfTableProducer.set(alias, data);
+        m_stateIntfTable.hset(alias, "vrf", vrf_name);
     }
     else if (op == DEL_COMMAND)
     {
@@ -640,16 +640,16 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
             m_loopbackIntfList.erase(alias);
         }
 
-        if (!subIntfAlias.empty())
+        if (!parentAlias.empty())
         {
-            removeHostSubIntf(subIntfAlias);
-            m_subIntfList.erase(subIntfAlias);
+            removeHostSubIntf(alias);
+            m_subIntfList.erase(alias);
 
-            removeSubIntfState(subIntfAlias);
+            removeSubIntfState(alias);
         }
 
-        m_appIntfTableProducer.del(subIntfAlias.empty() ? alias : subIntfAlias);
-        m_stateIntfTable.del(subIntfAlias.empty() ? alias : subIntfAlias);
+        m_appIntfTableProducer.del(alias);
+        m_stateIntfTable.del(alias);
     }
     else
     {

orchagent/intfsorch.cpp

@@ -731,6 +731,7 @@ void IntfsOrch::doTask(Consumer &consumer)
                     {
                         IntfsEntry intfs_entry;
                         intfs_entry.ref_count = 0;
+                        intfs_entry.proxy_arp = false;
                         intfs_entry.vrf_id = vrf_id;
                         m_syncdIntfses[alias] = intfs_entry;
                         m_vrfOrch->increaseVrfRefCount(vrf_id);

tests/conftest.py

@@ -980,16 +980,19 @@ def set_interface_status(self, interface, admin_status):
         tbl.set(interface, fvs)
         time.sleep(1)
 
-    # deps: acl, fdb_update, fdb, mirror_port_erspan, vlan
-    def add_ip_address(self, interface, ip):
+    # deps: acl, fdb_update, fdb, mirror_port_erspan, vlan, sub port intf
+    def add_ip_address(self, interface, ip, vrf_name=None):
         if interface.startswith("PortChannel"):
             tbl_name = "PORTCHANNEL_INTERFACE"
         elif interface.startswith("Vlan"):
             tbl_name = "VLAN_INTERFACE"
         else:
             tbl_name = "INTERFACE"
         tbl = swsscommon.Table(self.cdb, tbl_name)
-        fvs = swsscommon.FieldValuePairs([("NULL", "NULL")])
+        pairs = [("NULL", "NULL")]
+        if vrf_name:
+            pairs = [("vrf_name", vrf_name)]
+        fvs = swsscommon.FieldValuePairs(pairs)
         tbl.set(interface, fvs)
         tbl.set(interface + "|" + ip, fvs)
         time.sleep(1)