[202605][intfsorch]: Fence new RIF dependencies during interface removal

orchagent/intfsorch.h

@@ -26,6 +26,8 @@ struct IntfsEntry
     int                 ref_count;
     sai_object_id_t     vrf_id;
     bool                proxy_arp;
+    MacAddress          mac;
+    std::string         loopback_action;
 };
 
 typedef map<string, IntfsEntry> IntfsTable;
@@ -36,6 +38,8 @@ class IntfsOrch : public Orch
     IntfsOrch(DBConnector *db, string tableName, VRFOrch *vrf_orch, DBConnector *chassisAppDb);
 
     sai_object_id_t getRouterIntfsId(const string&);
+    bool isIntfRemovalPending(const string&) const;
+    bool isIntfVrfUpdatePending(const string&) const;
     bool isPrefixSubnet(const IpPrefix&, const string&);
     bool isInbandIntfInMgmtVrf(const string& alias);
     string getRouterIntfsAlias(const IpAddress &ip, const string &vrf_name = "");
@@ -57,7 +61,7 @@ class IntfsOrch : public Orch
 
     bool setIntfLoopbackAction(const Port &port, string actionStr);
     bool getSaiLoopbackAction(const string &actionStr, sai_packet_action_t &action);
-    bool setIntf(const string& alias, sai_object_id_t vrf_id = gVirtualRouterId, const IpPrefix *ip_prefix = nullptr, const bool adminUp = true, const uint32_t mtu = 0, string loopbackAction = "");
+    bool setIntf(const string& alias, sai_object_id_t vrf_id = gVirtualRouterId, const IpPrefix *ip_prefix = nullptr, const bool adminUp = true, const uint32_t mtu = 0, string loopbackAction = "", const bool vrfIdIsExplicit = false);
     bool removeIntf(const string& alias, sai_object_id_t vrf_id = gVirtualRouterId, const IpPrefix *ip_prefix = nullptr);
 
     void addIp2MeRoute(sai_object_id_t vrf_id, const IpPrefix &ip_prefix);
@@ -92,6 +96,7 @@ class IntfsOrch : public Orch
     unique_ptr<Table> m_vidToRidTable;
 
     std::set<std::string> m_removingIntfses;
+    std::set<std::string> m_pendingVrfUpdates;
 
     std::string getRifFlexCounterTableKey(std::string s);
 

orchagent/mplsrouteorch.cpp

@@ -16,6 +16,7 @@ extern sai_object_id_t gSwitchId;
 extern CrmOrch *gCrmOrch;
 extern NhgOrch *gNhgOrch;
 extern CbfNhgOrch *gCbfNhgOrch;
+extern PortsOrch *gPortsOrch;
 
 void RouteOrch::doLabelTask(ConsumerBase& consumer)
 {
@@ -876,6 +877,28 @@ bool RouteOrch::removeLabelRoute(LabelRouteBulkContext& ctx)
         return true;
     }
 
+    if (it_route->second.nhg_index.empty())
+    {
+        const auto& nexthops = it_route->second.nhg_key.getNextHops();
+        const auto remote_mpls_nexthop = find_if(nexthops.begin(), nexthops.end(),
+            [this](const auto& nexthop)
+            {
+                return nexthop.isMplsNextHop() &&
+                       m_intfsOrch->isRemoteSystemPortIntf(nexthop.alias);
+            });
+
+        if (remote_mpls_nexthop != nexthops.end())
+        {
+            Port inbp;
+            if (!gPortsOrch->getInbandPort(inbp))
+            {
+                SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s",
+                              remote_mpls_nexthop->to_string().c_str());
+                return false;
+            }
+        }
+    }
+
     auto& object_statuses = ctx.object_statuses;
 
     object_statuses.emplace_back();

orchagent/neighorch.cpp

@@ -261,14 +261,30 @@ bool NeighOrch::addNextHop(NeighborContext& ctx)
     {
         //For remote system ports kernel nexthops are always on inband. Change the key
         Port inbp;
-        gPortsOrch->getInbandPort(inbp);
-        assert(inbp.m_alias.length());
+        if (!gPortsOrch->getInbandPort(inbp))
+        {
+            SWSS_LOG_INFO("Inband port is not available for remote next hop %s", nh.to_string().c_str());
+            return false;
+        }
 
         nexthop.alias = inbp.m_alias;
     }
 
     assert(!hasNextHop(nexthop));
-    sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(nh.alias);
+    if (!m_intfsOrch->isRemoteSystemPortIntf(nh.alias) &&
+        (m_intfsOrch->isIntfRemovalPending(nh.alias) ||
+         m_intfsOrch->isIntfVrfUpdatePending(nh.alias)))
+    {
+        SWSS_LOG_INFO("Interface %s is pending removal or VRF update", nh.alias.c_str());
+        return false;
+    }
+
+    sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(nexthop.alias);
+    if (rif_id == SAI_NULL_OBJECT_ID)
+    {
+        SWSS_LOG_INFO("Failed to get rif_id for %s", nexthop.alias.c_str());
+        return false;
+    }
 
     vector<sai_attribute_t> next_hop_attrs;
 
@@ -346,7 +362,7 @@ bool NeighOrch::addNextHop(NeighborContext& ctx)
     next_hop_entry.nh_flags = 0;
     m_syncdNextHops[nexthop] = next_hop_entry;
 
-    m_intfsOrch->increaseRouterIntfsRefCount(nh.alias);
+    m_intfsOrch->increaseRouterIntfsRefCount(nexthop.alias);
 
     if (nexthop.isMplsNextHop())
     {
@@ -405,6 +421,18 @@ bool NeighOrch::processBulkAddNextHop(NeighborContext& ctx)
     }
 
     NextHopKey nexthop(nh);
+    if (m_intfsOrch->isRemoteSystemPortIntf(nh.alias))
+    {
+        Port inbp;
+        if (!gPortsOrch->getInbandPort(inbp))
+        {
+            SWSS_LOG_INFO("Inband port is not available for remote next hop %s", nh.to_string().c_str());
+            return false;
+        }
+
+        nexthop.alias = inbp.m_alias;
+    }
+
     if (ctx.next_hop_id == SAI_NULL_OBJECT_ID)
     {
         sai_status_t bulker_status = gNextHopBulker.create_status(ctx.next_hop_id);
@@ -438,7 +466,7 @@ bool NeighOrch::processBulkAddNextHop(NeighborContext& ctx)
     next_hop_entry.nh_flags = 0;
     m_syncdNextHops[nexthop] = next_hop_entry;
 
-    m_intfsOrch->increaseRouterIntfsRefCount(nh.alias);
+    m_intfsOrch->increaseRouterIntfsRefCount(nexthop.alias);
 
     if (nexthop.isMplsNextHop())
     {
@@ -655,8 +683,11 @@ bool NeighOrch::removeNextHop(const IpAddress &ipAddress, const string &alias)
     {
         //For remote system ports kernel nexthops are always on inband. Change the key
         Port inbp;
-        gPortsOrch->getInbandPort(inbp);
-        assert(inbp.m_alias.length());
+        if (!gPortsOrch->getInbandPort(inbp))
+        {
+            SWSS_LOG_INFO("Inband port is not available for remote next hop %s", nexthop.to_string().c_str());
+            return false;
+        }
 
         nexthop.alias = inbp.m_alias;
     }
@@ -667,13 +698,13 @@ bool NeighOrch::removeNextHop(const IpAddress &ipAddress, const string &alias)
 
     if (m_syncdNextHops[nexthop].ref_count > 0)
     {
-        SWSS_LOG_ERROR("Failed to remove still referenced next hop %s on %s",
-                       ipAddress.to_string().c_str(), alias.c_str());
+        SWSS_LOG_ERROR("Failed to remove still referenced next hop %s requested on %s, tracked on %s",
+                       ipAddress.to_string().c_str(), alias.c_str(), nexthop.alias.c_str());
         return false;
     }
 
     m_syncdNextHops.erase(nexthop);
-    m_intfsOrch->decreaseRouterIntfsRefCount(alias);
+    m_intfsOrch->decreaseRouterIntfsRefCount(nexthop.alias);
     return true;
 }
 
@@ -686,8 +717,12 @@ bool NeighOrch::removeMplsNextHop(const NextHopKey& nh)
     {
         //For remote system ports kernel nexthops are always on inband. Change the key
         Port inbp;
-        gPortsOrch->getInbandPort(inbp);
-        assert(inbp.m_alias.length());
+        if (!gPortsOrch->getInbandPort(inbp))
+        {
+            SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s",
+                          nexthop.to_string().c_str());
+            return false;
+        }
 
         nexthop.alias = inbp.m_alias;
     }
@@ -1201,6 +1236,14 @@ bool NeighOrch::addNeighbor(NeighborContext& ctx)
     string alias = neighborEntry.alias;
     bool bulk_op = ctx.bulk_op;
 
+    if (!m_intfsOrch->isRemoteSystemPortIntf(alias) &&
+        (m_intfsOrch->isIntfRemovalPending(alias) ||
+         m_intfsOrch->isIntfVrfUpdatePending(alias)))
+    {
+        SWSS_LOG_INFO("Interface %s is pending removal or VRF update", alias.c_str());
+        return false;
+    }
+
     sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(alias);
     if (rif_id == SAI_NULL_OBJECT_ID)
     {
@@ -1324,9 +1367,12 @@ bool NeighOrch::addNeighbor(NeighborContext& ctx)
         if (bulk_op)
         {
             SWSS_LOG_INFO("Adding neighbor entry %s on %s to bulker.", ip_address.to_string().c_str(), alias.c_str());
+            if (!addNextHop(ctx))
+            {
+                return false;
+            }
             object_statuses.emplace_back();
             gNeighBulker.create_entry(&object_statuses.back(), &neighbor_entry, (uint32_t)neighbor_attrs.size(), neighbor_attrs.data());
-            addNextHop(ctx);
             return true;
         }
 
@@ -1452,15 +1498,24 @@ bool NeighOrch::removeNeighbor(NeighborContext& ctx, bool disable)
     bool bulk_op = ctx.bulk_op;
 
     NextHopKey nexthop = { ip_address, alias };
+    auto neighborIt = m_syncdNeighbors.find(neighborEntry);
+    if (neighborIt == m_syncdNeighbors.end())
+    {
+        return true;
+    }
+
     sai_object_id_t port_vrf_id;
     port_vrf_id = gVirtualRouterId;
 
     if(m_intfsOrch->isRemoteSystemPortIntf(alias))
     {
         //For remote system ports kernel nexthops are always on inband. Change the key
         Port inbp;
-        gPortsOrch->getInbandPort(inbp);
-        assert(inbp.m_alias.length());
+        if (!gPortsOrch->getInbandPort(inbp))
+        {
+            SWSS_LOG_INFO("Inband port is not available for remote neighbor %s", nexthop.to_string().c_str());
+            return false;
+        }
 
         nexthop.alias = inbp.m_alias;
     }
@@ -1475,6 +1530,9 @@ bool NeighOrch::removeNeighbor(NeighborContext& ctx, bool disable)
         SWSS_LOG_ERROR("Port does not exist for %s!", alias.c_str());
     }
 
+    SWSS_LOG_INFO("Try to remove neighbor %s on %s",
+                  ip_address.to_string().c_str(), alias.c_str());
+
     if (m_syncdNeighbors.find(neighborEntry) == m_syncdNeighbors.end())
     {
         return true;
@@ -1579,7 +1637,10 @@ bool NeighOrch::removeNeighbor(NeighborContext& ctx, bool disable)
                 gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEIGHBOR);
             }
 
-            removeNextHop(ip_address, alias);
+            if (!removeNextHop(ip_address, nexthop.alias))
+            {
+                return false;
+            }
             m_intfsOrch->decreaseRouterIntfsRefCount(alias);
             SWSS_LOG_NOTICE("Removed neighbor %s on %s",
                     m_syncdNeighbors[neighborEntry].mac.to_string().c_str(), alias.c_str());
@@ -1728,12 +1789,25 @@ bool NeighOrch::processBulkDisableNeighbor(NeighborContext& ctx)
     const NeighborEntry neighborEntry = ctx.neighborEntry;
     string alias = neighborEntry.alias;
     IpAddress ip_address = neighborEntry.ip_address;
+    NextHopKey nexthop = { ip_address, alias };
 
     if (m_syncdNeighbors.find(neighborEntry) == m_syncdNeighbors.end())
     {
         return true;
     }
 
+    if (m_intfsOrch->isRemoteSystemPortIntf(alias))
+    {
+        Port inbp;
+        if (!gPortsOrch->getInbandPort(inbp))
+        {
+            SWSS_LOG_INFO("Inband port is not available for remote neighbor %s", nexthop.to_string().c_str());
+            return false;
+        }
+
+        nexthop.alias = inbp.m_alias;
+    }
+
     SWSS_LOG_INFO("Checking neighbor remove entry status %s on %s.", ip_address.to_string().c_str(), m_syncdNeighbors[neighborEntry].mac.to_string().c_str());
 
     if (isHwConfigured(neighborEntry))
@@ -1809,7 +1883,10 @@ bool NeighOrch::processBulkDisableNeighbor(NeighborContext& ctx)
                 gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEIGHBOR);
             }
 
-            removeNextHop(ip_address, alias);
+            if (!removeNextHop(ip_address, nexthop.alias))
+            {
+                return false;
+            }
             m_intfsOrch->decreaseRouterIntfsRefCount(alias);
             SWSS_LOG_NOTICE("Removed neighbor %s on %s",
                     m_syncdNeighbors[neighborEntry].mac.to_string().c_str(), alias.c_str());
@@ -1883,13 +1960,13 @@ bool NeighOrch::enableNeighbors(std::list<NeighborContext>& bulk_ctx_list)
     for (auto ctx = bulk_ctx_list.begin(); ctx != bulk_ctx_list.end(); ctx++)
     {
         const NeighborEntry& neighborEntry = ctx->neighborEntry;
-        ctx->mac = m_syncdNeighbors[neighborEntry].mac;
-
-        if (m_syncdNeighbors.find(neighborEntry) == m_syncdNeighbors.end())
+        auto neighborIt = m_syncdNeighbors.find(neighborEntry);
+        if (neighborIt == m_syncdNeighbors.end())
         {
             SWSS_LOG_INFO("Neighbor %s not found", neighborEntry.ip_address.to_string().c_str());
             continue;
         }
+        ctx->mac = neighborIt->second.mac;
 
         if (isHwConfigured(neighborEntry))
         {

orchagent/nhgorch.cpp

@@ -1,3 +1,4 @@
+#include <algorithm>
 #include <random>
 #include "nhgorch.h"
 #include "neighorch.h"
@@ -878,6 +879,26 @@ bool NextHopGroup::remove()
     {
         return true;
     }
+
+    const auto remote_mpls_member = find_if(m_members.begin(), m_members.end(),
+        [](const auto& member)
+        {
+            const auto& nh_key = member.first;
+            return nh_key.isMplsNextHop() &&
+                   gIntfsOrch->isRemoteSystemPortIntf(nh_key.alias);
+        });
+
+    if (remote_mpls_member != m_members.end())
+    {
+        Port inbp;
+        if (!gPortsOrch->getInbandPort(inbp))
+        {
+            SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s",
+                          remote_mpls_member->first.to_string().c_str());
+            return false;
+        }
+    }
+
     //  If the group is temporary or non-recursive, update the neigh or rif ref-count and reset the ID.
     if (m_is_temp ||
         (!isRecursive() && m_members.size() == 1))