[muxorch] Skip programming SoC IP kernel tunnel route (#2557)

What I did
Let mux orch skip adding kernel tunnel routes for SoC IPs.

Signed-off-by: Longxiang Lyu [email protected]

Why I did it
Please refer to sonic-net/SONiC#1132 for more details.

How I verified it
Add unittest.

Signed-off-by: Longxiang Lyu <[email protected]>

Author: lolyu

orchagent/muxorch.cpp

@@ -359,8 +359,8 @@ static bool remove_nh_tunnel(sai_object_id_t nh_id, IpAddress& ipAddr)
     return true;
 }
 
-MuxCable::MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip, std::set<IpAddress> skip_neighbors)
-         :mux_name_(name), srv_ip4_(srv_ip4), srv_ip6_(srv_ip6), peer_ip4_(peer_ip), skip_neighbors_(skip_neighbors)
+MuxCable::MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip)
+         :mux_name_(name), srv_ip4_(srv_ip4), srv_ip6_(srv_ip6), peer_ip4_(peer_ip)
 {
     mux_orch_ = gDirectory.get<MuxOrch*>();
     mux_cb_orch_ = gDirectory.get<MuxCableOrch*>();
@@ -549,11 +549,6 @@ bool MuxCable::nbrHandler(bool enable, bool update_rt)
 void MuxCable::updateNeighbor(NextHopKey nh, bool add)
 {
     sai_object_id_t tnh = mux_orch_->getNextHopTunnelId(MUX_TUNNEL, peer_ip4_);
-    if (add && skip_neighbors_.find(nh.ip_address) != skip_neighbors_.end())
-    {
-        SWSS_LOG_INFO("Skip update neighbor %s on %s", nh.ip_address.to_string().c_str(), nh.alias.c_str());
-        return;
-    }
     nbr_handler_->update(nh, tnh, add, state_);
     if (add)
     {
@@ -570,7 +565,6 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu
     SWSS_LOG_INFO("Neigh %s on %s, add %d, state %d",
                    nh.ip_address.to_string().c_str(), nh.alias.c_str(), add, state);
 
-    MuxCableOrch* mux_cb_orch = gDirectory.get<MuxCableOrch*>();
     IpPrefix pfx = nh.ip_address.to_string();
 
     if (add)
@@ -597,7 +591,7 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu
         case MuxState::MUX_STATE_STANDBY:
             neighbors_[nh.ip_address] = tunnelId;
             gNeighOrch->disableNeighbor(nh);
-            mux_cb_orch->addTunnelRoute(nh);
+            updateTunnelRoute(nh, true);
             create_route(pfx, tunnelId);
             break;
         default:
@@ -612,7 +606,7 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu
         if (state == MuxState::MUX_STATE_STANDBY)
         {
             remove_route(pfx);
-            mux_cb_orch->removeTunnelRoute(nh);
+            updateTunnelRoute(nh, false);
         }
         neighbors_.erase(nh.ip_address);
     }
@@ -621,7 +615,6 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu
 bool MuxNbrHandler::enable(bool update_rt)
 {
     NeighborEntry neigh;
-    MuxCableOrch* mux_cb_orch = gDirectory.get<MuxCableOrch*>();
 
     auto it = neighbors_.begin();
     while (it != neighbors_.end())
@@ -677,7 +670,7 @@ bool MuxNbrHandler::enable(bool update_rt)
             {
                 return false;
             }
-            mux_cb_orch->removeTunnelRoute(nh_key);
+            updateTunnelRoute(nh_key, false);
         }
 
         it++;
@@ -689,7 +682,6 @@ bool MuxNbrHandler::enable(bool update_rt)
 bool MuxNbrHandler::disable(sai_object_id_t tnh)
 {
     NeighborEntry neigh;
-    MuxCableOrch* mux_cb_orch = gDirectory.get<MuxCableOrch*>();
 
     auto it = neighbors_.begin();
     while (it != neighbors_.end())
@@ -735,7 +727,7 @@ bool MuxNbrHandler::disable(sai_object_id_t tnh)
             return false;
         }
 
-        mux_cb_orch->addTunnelRoute(nh_key);
+        updateTunnelRoute(nh_key, true);
 
         IpPrefix pfx = it->first.to_string();
         if (create_route(pfx, it->second) != SAI_STATUS_SUCCESS)
@@ -760,6 +752,27 @@ sai_object_id_t MuxNbrHandler::getNextHopId(const NextHopKey nhKey)
     return SAI_NULL_OBJECT_ID;
 }
 
+void MuxNbrHandler::updateTunnelRoute(NextHopKey nh, bool add)
+{
+    MuxOrch* mux_orch = gDirectory.get<MuxOrch*>();
+    MuxCableOrch* mux_cb_orch = gDirectory.get<MuxCableOrch*>();
+
+    if (mux_orch->isSkipNeighbor(nh.ip_address))
+    {
+        SWSS_LOG_INFO("Skip updating neighbor %s, add %d", nh.ip_address.to_string().c_str(), add);
+        return;
+    }
+
+    if (add)
+    {
+        mux_cb_orch->addTunnelRoute(nh);
+    }
+    else
+    {
+        mux_cb_orch->removeTunnelRoute(nh);
+    }
+}
+
 std::map<std::string, AclTable> MuxAclHandler::acl_table_;
 
 MuxAclHandler::MuxAclHandler(sai_object_id_t port, string alias)
@@ -977,7 +990,7 @@ bool MuxOrch::isNeighborActive(const IpAddress& nbr, const MacAddress& mac, stri
 
     if (ptr)
     {
-        return (ptr->isActive() || ptr->isSkipNeighbor(nbr));
+        return ptr->isActive();
     }
 
     string port;
@@ -991,15 +1004,15 @@ bool MuxOrch::isNeighborActive(const IpAddress& nbr, const MacAddress& mac, stri
     if (!port.empty() && isMuxExists(port))
     {
         MuxCable* ptr = getMuxCable(port);
-        return (ptr->isActive() || ptr->isSkipNeighbor(nbr));
+        return ptr->isActive();
     }
 
     NextHopKey nh_key = NextHopKey(nbr, alias);
     string curr_port = getNexthopMuxName(nh_key);
     if (port.empty() && !curr_port.empty() && isMuxExists(curr_port))
     {
         MuxCable* ptr = getMuxCable(curr_port);
-        return (ptr->isActive() || ptr->isSkipNeighbor(nbr));
+        return ptr->isActive();
     }
 
     return true;
@@ -1295,7 +1308,8 @@ bool MuxOrch::handleMuxCfg(const Request& request)
         }
 
         mux_cable_tb_[port_name] = std::make_unique<MuxCable>
-                                   (MuxCable(port_name, srv_ip, srv_ip6, mux_peer_switch_, skip_neighbors));
+                                   (MuxCable(port_name, srv_ip, srv_ip6, mux_peer_switch_));
+        addSkipNeighbors(skip_neighbors);
 
         SWSS_LOG_NOTICE("Mux entry for port '%s' was added", port_name.c_str());
     }
@@ -1307,6 +1321,7 @@ bool MuxOrch::handleMuxCfg(const Request& request)
             return true;
         }
 
+        removeSkipNeighbors(skip_neighbors);
         mux_cable_tb_.erase(port_name);
 
         SWSS_LOG_NOTICE("Mux cable for port '%s' was removed", port_name.c_str());

orchagent/muxorch.h

@@ -67,6 +67,9 @@ class MuxNbrHandler
 
     sai_object_id_t getNextHopId(const NextHopKey);
 
+private:
+    inline void updateTunnelRoute(NextHopKey, bool = true);
+
 private:
     MuxNeighbor neighbors_;
     string alias_;
@@ -76,7 +79,7 @@ class MuxNbrHandler
 class MuxCable
 {
 public:
-    MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip, std::set<IpAddress> skip_neighbors);
+    MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip);
 
     bool isActive() const
     {
@@ -97,10 +100,6 @@ class MuxCable
     {
         return nbr_handler_->getNextHopId(nh);
     }
-    bool isSkipNeighbor(const IpAddress& nbr)
-    {
-        return (skip_neighbors_.find(nbr) != skip_neighbors_.end());
-    }
 
 private:
     bool stateActive();
@@ -119,8 +118,6 @@ class MuxCable
     IpPrefix srv_ip4_, srv_ip6_;
     IpAddress peer_ip4_;
 
-    std::set<IpAddress> skip_neighbors_;
-
     MuxOrch *mux_orch_;
     MuxCableOrch *mux_cb_orch_;
     MuxStateOrch *mux_state_orch_;
@@ -180,6 +177,11 @@ class MuxOrch : public Orch2, public Observer, public Subject
         return mux_cable_tb_.at(portName).get();
     }
 
+    bool isSkipNeighbor(const IpAddress& nbr)
+    {
+        return (skip_neighbors_.find(nbr) != skip_neighbors_.end());
+    }
+
     MuxCable* findMuxCableInSubnet(IpAddress);
     bool isNeighborActive(const IpAddress&, const MacAddress&, string&);
     void update(SubjectType, void *);
@@ -212,6 +214,19 @@ class MuxOrch : public Orch2, public Observer, public Subject
     void createStandaloneTunnelRoute(IpAddress neighborIp);
     void removeStandaloneTunnelRoute(IpAddress neighborIp);
 
+    void addSkipNeighbors(const std::set<IpAddress> &neighbors)
+    {
+        skip_neighbors_.insert(neighbors.begin(), neighbors.end());
+    }
+
+    void removeSkipNeighbors(const std::set<IpAddress> &neighbors)
+    {
+        for (const IpAddress &neighbor : neighbors)
+        {
+            skip_neighbors_.erase(neighbor);
+        }
+    }
+
     IpAddress mux_peer_switch_ = 0x0;
     sai_object_id_t mux_tunnel_id_ = SAI_NULL_OBJECT_ID;
 
@@ -227,6 +242,7 @@ class MuxOrch : public Orch2, public Observer, public Subject
 
     MuxCfgRequest request_;
     std::set<IpAddress> standalone_tunnel_neighbors_;
+    std::set<IpAddress> skip_neighbors_;
 };
 
 const request_description_t mux_cable_request_description = {

tests/test_mux.py

@@ -16,6 +16,7 @@ class TestMuxTunnelBase():
     APP_MUX_CABLE               = "MUX_CABLE_TABLE"
     APP_NEIGH_TABLE             = "NEIGH_TABLE"
     APP_TUNNEL_DECAP_TABLE_NAME = "TUNNEL_DECAP_TABLE"
+    APP_TUNNEL_ROUTE_TABLE_NAME = "TUNNEL_ROUTE_TABLE"
     ASIC_TUNNEL_TABLE           = "ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL"
     ASIC_TUNNEL_TERM_ENTRIES    = "ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL_TERM_TABLE_ENTRY"
     ASIC_RIF_TABLE              = "ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE"
@@ -166,6 +167,14 @@ def get_vlan_rif_oid(self, asicdb):
 
         return vlan_oid
 
+    def check_tunnel_route_in_app_db(self, dvs, destinations, expected=True):
+        appdb = dvs.get_app_db()
+
+        if expected:
+            appdb.wait_for_matching_keys(self.APP_TUNNEL_ROUTE_TABLE_NAME, destinations)
+        else:
+            appdb.wait_for_deleted_keys(self.APP_TUNNEL_ROUTE_TABLE_NAME, destinations)
+
     def check_neigh_in_asic_db(self, asicdb, ip, expected=True):
         rif_oid = self.get_vlan_rif_oid(asicdb)
         switch_oid = self.get_switch_oid(asicdb)
@@ -275,9 +284,6 @@ def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route):
         self.add_neighbor(dvs, self.SERV1_IPV6, "00:00:00:00:00:01")
         srv1_v6 = self.check_neigh_in_asic_db(asicdb, self.SERV1_IPV6)
 
-        self.add_neighbor(dvs, self.SERV1_SOC_IPV4, "00:00:00:00:00:01")
-        self.check_neigh_in_asic_db(asicdb, self.SERV1_SOC_IPV4)
-
         existing_keys = asicdb.get_keys(self.ASIC_NEIGH_TABLE)
 
         self.add_neighbor(dvs, self.SERV2_IPV4, "00:00:00:00:00:02")
@@ -291,7 +297,7 @@ def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route):
         )
 
         # The first standby route also creates as tunnel Nexthop
-        self.check_tnl_nexthop_in_asic_db(asicdb, 4)
+        self.check_tnl_nexthop_in_asic_db(asicdb, 3)
 
         # Change state to Standby. This will delete Neigh and add Route
         self.set_mux_state(appdb, "Ethernet0", "standby")
@@ -301,8 +307,6 @@ def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route):
         dvs_route.check_asicdb_route_entries(
             [self.SERV1_IPV4+self.IPV4_MASK, self.SERV1_IPV6+self.IPV6_MASK]
         )
-        self.check_neigh_in_asic_db(asicdb, self.SERV1_SOC_IPV4)
-        dvs_route.check_asicdb_deleted_route_entries([self.SERV1_SOC_IPV4+self.IPV4_MASK])
 
         # Change state to Active. This will add Neigh and delete Route
         self.set_mux_state(appdb, "Ethernet4", "active")
@@ -313,6 +317,26 @@ def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route):
         self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV4)
         self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV6)
 
+    def create_and_test_soc(self, appdb, asicdb, dvs, dvs_route):
+
+        self.set_mux_state(appdb, "Ethernet0", "active")
+
+        self.add_fdb(dvs, "Ethernet0", "00-00-00-00-00-01")
+        self.add_neighbor(dvs, self.SERV1_SOC_IPV4, "00:00:00:00:00:01")
+
+        time.sleep(1)
+
+        srv1_soc_v4 = self.check_neigh_in_asic_db(asicdb, self.SERV1_SOC_IPV4)
+        self.check_tunnel_route_in_app_db(dvs, [self.SERV1_SOC_IPV4+self.IPV4_MASK], expected=False)
+
+        self.set_mux_state(appdb, "Ethernet0", "standby")
+
+        asicdb.wait_for_deleted_entry(self.ASIC_NEIGH_TABLE, srv1_soc_v4)
+        dvs_route.check_asicdb_route_entries(
+            [self.SERV1_SOC_IPV4+self.IPV4_MASK]
+        )
+        self.check_tunnel_route_in_app_db(dvs, [self.SERV1_SOC_IPV4+self.IPV4_MASK], expected=False)
+
         marker = dvs.add_log_marker()
 
         self.set_mux_state(appdb, "Ethernet0", "active")
@@ -1122,6 +1146,11 @@ def test_neighbor_miss_no_peer(
         for ip in test_ips:
             self.check_neighbor_state(dvs, dvs_route, ip, expect_route=False)
 
+    def test_soc_ip(self, dvs, dvs_route, setup_vlan, setup_mux_cable, testlog):
+        appdb = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
+        asicdb = dvs.get_asic_db()
+
+        self.create_and_test_soc(appdb, asicdb, dvs, dvs_route)
 
 # Add Dummy always-pass test at end as workaroud
 # for issue when Flaky fail on final test it invokes module tear-down before retrying