[voq/inbandif] Voq inbandif port (sonic-net#1602)

What I did

Changes to support inband interface configuration

Why I did it

The VOQ chassis needs inband interface in the kernel for cpu to cpu communication across different asic instance and remote neighbor programming in the kernel. This PR adds support for configuring inband interface so that kernel interface can be created via sonic configuration.

How I verified it
swss vs test (test_virtual_chassis.py) to verify the remote neigh programming.

cfgmgr/nbrmgr.cpp

@@ -348,7 +348,8 @@ void NbrMgr::doStateSystemNeighTask(Consumer &consumer)
     //Get the name of the device on which the neigh and route are
     //going to be programmed.
     string nbr_odev;
-    if(!getVoqInbandInterfaceName(nbr_odev))
+    string ibif_type;
+    if(!getVoqInbandInterfaceName(nbr_odev, ibif_type))
     {
         //The inband interface is not available yet
         return;
@@ -380,16 +381,19 @@ void NbrMgr::doStateSystemNeighTask(Consumer &consumer)
                     mac_address = MacAddress(fvValue(*i));
             }
 
-            if (!isIntfStateOk(nbr_odev))
+            if (ibif_type == "port" && !isIntfOperUp(nbr_odev))
             {
-                SWSS_LOG_DEBUG("Interface %s is not ready, skipping system neigh %s'", nbr_odev.c_str(), kfvKey(t).c_str());
+                SWSS_LOG_DEBUG("Device %s is not oper up, skipping system neigh %s'", nbr_odev.c_str(), kfvKey(t).c_str());
                 it++;
                 continue;
             }
 
             if (!addKernelNeigh(nbr_odev, ip_address, mac_address))
             {
                 SWSS_LOG_ERROR("Neigh entry add on dev %s failed for '%s'", nbr_odev.c_str(), kfvKey(t).c_str());
+                // Delete neigh to take care of deletion of exiting nbr for mac change. This makes sure that
+                // re-try will be successful and route addtion (below) will be attempted and be successful
+                delKernelNeigh(nbr_odev, ip_address);
                 it++;
                 continue;
             }
@@ -402,6 +406,8 @@ void NbrMgr::doStateSystemNeighTask(Consumer &consumer)
             {
                 SWSS_LOG_ERROR("Route entry add on dev %s failed for '%s'", nbr_odev.c_str(), kfvKey(t).c_str());
                 delKernelNeigh(nbr_odev, ip_address);
+                // Delete route to take care of deletion of exiting route of nbr for mac change.
+                delKernelRoute(ip_address);
                 it++;
                 continue;
             }
@@ -437,9 +443,24 @@ void NbrMgr::doStateSystemNeighTask(Consumer &consumer)
     }
 }
 
-bool NbrMgr::getVoqInbandInterfaceName(string &ibif)
+bool NbrMgr::isIntfOperUp(const string &alias)
 {
+    string oper;
+
+    if (m_statePortTable.hget(alias, "netdev_oper_status", oper))
+    {
+        if (oper == "up")
+        {
+            SWSS_LOG_DEBUG("NetDev %s is oper up", alias.c_str());
+            return true;
+        }
+    }
 
+    return false;
+}
+
+bool NbrMgr::getVoqInbandInterfaceName(string &ibif, string &type)
+{
     vector<string> keys;
     m_cfgVoqInbandInterfaceTable->getKeys(keys);
 
@@ -448,9 +469,21 @@ bool NbrMgr::getVoqInbandInterfaceName(string &ibif)
         SWSS_LOG_NOTICE("Voq Inband interface is not configured!");
         return false;
     }
-    //key:"alias" = inband interface name
+
+    // key:"alias" = inband interface name
+
     vector<string> if_keys = tokenize(keys[0], config_db_key_delimiter);
+
     ibif = if_keys[0];
+
+    // Get the type of the inband interface
+
+    if (!m_cfgVoqInbandInterfaceTable->hget(ibif, "inband_type", type))
+    {
+        SWSS_LOG_ERROR("Getting Voq Inband interface type failed for %s", ibif.c_str());
+        return false;
+    }
+
     return true;
 }
 

cfgmgr/nbrmgr.h

@@ -32,11 +32,12 @@ class NbrMgr : public Orch
     void doSetNeighTask(Consumer &consumer);
     void doTask(Consumer &consumer);
     void doStateSystemNeighTask(Consumer &consumer);
-    bool getVoqInbandInterfaceName(string &nbr_odev);
+    bool getVoqInbandInterfaceName(string &nbr_odev, string &ibiftype);
     bool addKernelRoute(string odev, IpAddress ip_addr);
     bool delKernelRoute(IpAddress ip_addr);
     bool addKernelNeigh(string odev, IpAddress ip_addr, MacAddress mac_addr);
     bool delKernelNeigh(string odev, IpAddress ip_addr);
+    bool isIntfOperUp(const std::string &alias);
     unique_ptr<Table> m_cfgVoqInbandInterfaceTable;
 
     Table m_statePortTable, m_stateLagTable, m_stateVlanTable, m_stateIntfTable, m_stateNeighRestoreTable;

orchagent/neighorch.cpp

@@ -1028,6 +1028,16 @@ void NeighOrch::doVoqSystemNeighTask(Consumer &consumer)
         return;
     }
 
+    // For "port" type inband interface, wait till the Inband interface is both admin up and oper up
+    if (ibif.m_type != Port::VLAN)
+    {
+        if (ibif.m_admin_state_up != true || ibif.m_oper_status != SAI_PORT_OPER_STATUS_UP)
+        {
+            // Inband port is not operational yet
+            return;
+        }
+    }
+
     auto it = consumer.m_toSync.begin();
     while (it != consumer.m_toSync.end())
     {
@@ -1161,20 +1171,126 @@ void NeighOrch::doVoqSystemNeighTask(Consumer &consumer)
 
 bool NeighOrch::addInbandNeighbor(string alias, IpAddress ip_address)
 {
-    //For "port" type inband, the inband reachability info syncing can be done through static
-    //configureation or CHASSIS_APP_DB sync (this function)
+    //Add neighbor record in SAI without adding host route for local inband to avoid route
+    //looping for packets destined to the Inband interface if the Inband is port type
+
+    if(gIntfsOrch->isRemoteSystemPortIntf(alias))
+    {
+        //Remote Inband interface. Skip
+        return true;
+    }
+
+    sai_status_t status;
+    MacAddress inband_mac = gMacAddress;
+
+    sai_object_id_t rif_id = gIntfsOrch->getRouterIntfsId(alias);
+    if (rif_id == SAI_NULL_OBJECT_ID)
+    {
+        SWSS_LOG_INFO("Failed to get rif_id for %s", alias.c_str());
+        return false;
+    }
+
+    //Make the object key
+    sai_neighbor_entry_t neighbor_entry;
+    neighbor_entry.rif_id = rif_id;
+    neighbor_entry.switch_id = gSwitchId;
+    copy(neighbor_entry.ip_address, ip_address);
+
+    vector<sai_attribute_t> neighbor_attrs;
+    sai_attribute_t attr;
+    attr.id = SAI_NEIGHBOR_ENTRY_ATTR_DST_MAC_ADDRESS;
+    memcpy(attr.value.mac, inband_mac.getMac(), 6);
+    neighbor_attrs.push_back(attr);
+
+    //No host route for neighbor of the Inband IP address
+    attr.id = SAI_NEIGHBOR_ENTRY_ATTR_NO_HOST_ROUTE;
+    attr.value.booldata = true;
+    neighbor_attrs.push_back(attr);
+
+    status = sai_neighbor_api->create_neighbor_entry(&neighbor_entry, static_cast<uint32_t>(neighbor_attrs.size()), neighbor_attrs.data());
+    if (status != SAI_STATUS_SUCCESS)
+    {
+        if (status == SAI_STATUS_ITEM_ALREADY_EXISTS)
+        {
+            SWSS_LOG_ERROR("Entry exists: neighbor %s on %s, rv:%d", inband_mac.to_string().c_str(), alias.c_str(), status);
+            return true;
+        }
+        else
+        {
+            SWSS_LOG_ERROR("Failed to create neighbor %s on %s, rv:%d", inband_mac.to_string().c_str(), alias.c_str(), status);
+            return false;
+        }
+    }
+
+    SWSS_LOG_NOTICE("Created inband neighbor %s on %s", inband_mac.to_string().c_str(), alias.c_str());
 
-    //For "vlan" type inband, the inband reachability info syncinng can be ARP learning of other
-    //asics inband or static configuration or through CHASSIS_APP_DB sync (this function)
+    gIntfsOrch->increaseRouterIntfsRefCount(alias);
 
-    //May implement inband rechability info syncing through CHASSIS_APP_DB sync here
+    if (neighbor_entry.ip_address.addr_family == SAI_IP_ADDR_FAMILY_IPV4)
+    {
+        gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV4_NEIGHBOR);
+    }
+    else
+    {
+        gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEIGHBOR);
+    }
+
+    //Sync the neighbor to add to the CHASSIS_APP_DB
+    voqSyncAddNeigh(alias, ip_address, inband_mac, neighbor_entry);
 
     return true;
 }
 
 bool NeighOrch::delInbandNeighbor(string alias, IpAddress ip_address)
 {
-    //Remove inband rechability info sync
+    // Remove local inband neighbor from SAI
+
+    if(gIntfsOrch->isRemoteSystemPortIntf(alias))
+    {
+        //Remote Inband interface. Skip
+        return true;
+    }
+
+    MacAddress inband_mac = gMacAddress;
+
+    sai_object_id_t rif_id = gIntfsOrch->getRouterIntfsId(alias);
+
+    sai_neighbor_entry_t neighbor_entry;
+    neighbor_entry.rif_id = rif_id;
+    neighbor_entry.switch_id = gSwitchId;
+    copy(neighbor_entry.ip_address, ip_address);
+
+    sai_status_t status;
+    status = sai_neighbor_api->remove_neighbor_entry(&neighbor_entry);
+    if (status != SAI_STATUS_SUCCESS)
+    {
+        if (status == SAI_STATUS_ITEM_NOT_FOUND)
+        {
+            SWSS_LOG_ERROR("Failed to locate neigbor %s on %s, rv:%d", inband_mac.to_string().c_str(), alias.c_str(), status);
+            return true;
+        }
+        else
+        {
+            SWSS_LOG_ERROR("Failed to remove neighbor %s on %s, rv:%d", inband_mac.to_string().c_str(), alias.c_str(), status);
+            return false;
+        }
+    }
+
+    if (neighbor_entry.ip_address.addr_family == SAI_IP_ADDR_FAMILY_IPV4)
+    {
+        gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV4_NEIGHBOR);
+    }
+    else
+    {
+        gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEIGHBOR);
+    }
+
+    SWSS_LOG_NOTICE("Removed neighbor %s on %s", inband_mac.to_string().c_str(), alias.c_str());
+
+    gIntfsOrch->decreaseRouterIntfsRefCount(alias);
+
+    //Sync the neighbor to delete from the CHASSIS_APP_DB
+    voqSyncDelNeigh(alias, ip_address);
 
     return true;
 }

orchagent/portsorch.cpp

@@ -52,6 +52,7 @@ extern int32_t gVoqMySwitchId;
 extern string gMyHostName;
 extern string gMyAsicName;
 
+#define DEFAULT_SYSTEM_PORT_MTU 9100
 #define VLAN_PREFIX         "Vlan"
 #define DEFAULT_VLAN_ID     1
 #define MAX_VALID_VLAN_ID   4094
@@ -5462,6 +5463,7 @@ bool PortsOrch::addSystemPorts()
             port.m_admin_state_up = true;
             port.m_oper_status = SAI_PORT_OPER_STATUS_UP;
             port.m_speed = attrs[1].value.sysportconfig.speed;
+            port.m_mtu = DEFAULT_SYSTEM_PORT_MTU;
             if (attrs[0].value.s32 == SAI_SYSTEM_PORT_TYPE_LOCAL)
             {
                 //Get the local port oid
@@ -5543,21 +5545,20 @@ bool PortsOrch::setVoqInbandIntf(string &alias, string &type)
         return true;
     }
 
+    //Make sure port and host if exists for the configured inband interface
     Port port;
-    if(type == "port")
+    if (!getPort(alias, port))
     {
-        if (!getPort(alias, port))
-        {
-            SWSS_LOG_NOTICE("Port configured for inband intf %s is not ready!", alias.c_str());
-            return false;
-        }
+        SWSS_LOG_ERROR("Port/Vlan configured for inband intf %s is not ready!", alias.c_str());
+        return false;
     }
 
-    // Check for existence of host interface. If does not exist, may create
-    // host if for the inband here
+    if(type == "port" && !port.m_hif_id)
+    {
+        SWSS_LOG_ERROR("Host interface is not available for port %s", alias.c_str());
+        return false;
+    }
 
-    // May do the processing for other inband type like type=vlan here
-
     //Store the name of the local inband port
     m_inbandPortName = alias;
 

orchagent/routeorch.cpp

@@ -1392,6 +1392,15 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops)
 
         if (nexthop.ip_address.isZero())
         {
+            if(gPortsOrch->isInbandPort(nexthop.alias))
+            {
+                //This routes is the static route added for the remote system neighbors
+                //We do not need this route in the ASIC since the static neighbor creation
+                //in ASIC adds the same full mask route (host route) in ASIC automatically
+                //So skip.
+                return true;
+            }
+
             next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias);
             /* rif is not created yet */
             if (next_hop_id == SAI_NULL_OBJECT_ID)

portsyncd/linksync.cpp

@@ -253,8 +253,10 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
         FieldValueTuple tuple("state", "ok");
         vector<FieldValueTuple> vector;
         vector.push_back(tuple);
+        FieldValueTuple op("netdev_oper_status", oper ? "up" : "down");
+        vector.push_back(op);
         m_statePortTable.set(key, vector);
-        SWSS_LOG_NOTICE("Publish %s(ok) to state db", key.c_str());
+        SWSS_LOG_NOTICE("Publish %s(ok:%s) to state db", key.c_str(), oper ? "up" : "down");
     }
     else
     {

tests/conftest.py

@@ -1446,7 +1446,7 @@ def handle_neighconn(self):
     def get_chassis_instance_port_statuses(self):
         instance_to_port_status_map = {}
         if "neighbor_connections" not in self.virt_topo:
-            return instance_to_neighbor_map
+            return instance_to_port_status_map
 
         working_dir = os.getcwd()
         for conn, endpoints in self.virt_topo["neighbor_connections"].items():