[dualtor] Fix standby neighbor inconsistency after warm reboot (sonic…

…-net#3356) What I did Let's enable mux orch to cache the neighbor updates during warm-reboot restoration, and do the updates after everything else being settled (mux configs/peer switch config are applied), so the neighbors could be reprogrammed based on the mux state. Microsoft ADO (number only): 30148066 Signed-off-by: Longxiang Lyu [email protected]
bradh352 · Nov 27, 2024 · 3da2e67 · 3da2e67
1 parent 8bf38af
commit 3da2e67
Show file tree

Hide file tree

Showing 4 changed files with 147 additions and 8 deletions.
diff --git a/orchagent/muxorch.cpp b/orchagent/muxorch.cpp
@@ -1625,7 +1625,14 @@ void MuxOrch::update(SubjectType type, void *cntx)
         case SUBJECT_TYPE_NEIGH_CHANGE:
         {
             NeighborUpdate *update = static_cast<NeighborUpdate *>(cntx);
-            updateNeighbor(*update);
+            if (enable_cache_neigh_updates_)
+            {
+                cached_neigh_updates_.push_back(*update);
+            }
+            else
+            {
+                updateNeighbor(*update);
+            }
             break;
         }
         case SUBJECT_TYPE_FDB_CHANGE:
@@ -1862,6 +1869,30 @@ bool MuxOrch::isStandaloneTunnelRouteInstalled(const IpAddress& neighborIp)
     return standalone_tunnel_neighbors_.find(neighborIp) != standalone_tunnel_neighbors_.end();
 }
 
+void MuxOrch::updateCachedNeighbors()
+{
+    if (!enable_cache_neigh_updates_)
+    {
+        SWSS_LOG_NOTICE("Skip process cached neighbor updates");
+        return;
+    }
+    if (mux_peer_switch_.isZero())
+    {
+        SWSS_LOG_NOTICE("Skip process cached neighbor updates, no peer switch addr is configured");
+        return;
+    }
+
+    while (!cached_neigh_updates_.empty())
+    {
+        const NeighborUpdate &update = cached_neigh_updates_.back();
+        SWSS_LOG_NOTICE("Update cached neighbor %s, add %d",
+                        update.entry.ip_address.to_string().c_str(),
+                        update.add);
+        updateNeighbor(update);
+        cached_neigh_updates_.pop_back();
+    }
+}
+
 MuxCableOrch::MuxCableOrch(DBConnector *db, DBConnector *sdb, const std::string& tableName):
               Orch2(db, tableName, request_),
               app_tunnel_route_table_(db, APP_TUNNEL_ROUTE_TABLE_NAME),

diff --git a/orchagent/muxorch.h b/orchagent/muxorch.h
@@ -237,6 +237,16 @@ class MuxOrch : public Orch2, public Observer, public Subject
     void updateRoute(const IpPrefix &pfx, bool add);
     bool isStandaloneTunnelRouteInstalled(const IpAddress& neighborIp);
 
+    void enableCachingNeighborUpdate()
+    {
+        enable_cache_neigh_updates_ = true;
+    }
+    void disableCachingNeighborUpdate()
+    {
+        enable_cache_neigh_updates_ = false;
+    }
+    void updateCachedNeighbors();
+
 private:
     virtual bool addOperation(const Request& request);
     virtual bool delOperation(const Request& request);
@@ -285,6 +295,9 @@ class MuxOrch : public Orch2, public Observer, public Subject
     MuxCfgRequest request_;
     std::set<IpAddress> standalone_tunnel_neighbors_;
     std::set<IpAddress> skip_neighbors_;
+
+    bool enable_cache_neigh_updates_ = false;
+    std::vector<NeighborUpdate> cached_neigh_updates_;
 };
 
 const request_description_t mux_cable_request_description = {

diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp
@@ -64,6 +64,7 @@ FlowCounterRouteOrch *gFlowCounterRouteOrch;
 DebugCounterOrch *gDebugCounterOrch;
 MonitorOrch *gMonitorOrch;
 TunnelDecapOrch *gTunneldecapOrch;
+MuxOrch *gMuxOrch;
 
 bool gIsNatSupported = false;
 event_handle_t g_events_handle;
@@ -390,8 +391,8 @@ bool OrchDaemon::init()
         CFG_MUX_CABLE_TABLE_NAME,
         CFG_PEER_SWITCH_TABLE_NAME
     };
-    MuxOrch *mux_orch = new MuxOrch(m_configDb, mux_tables, gTunneldecapOrch, gNeighOrch, gFdbOrch);
-    gDirectory.set(mux_orch);
+    gMuxOrch = new MuxOrch(m_configDb, mux_tables, gTunneldecapOrch, gNeighOrch, gFdbOrch);
+    gDirectory.set(gMuxOrch);
 
     MuxCableOrch *mux_cb_orch = new MuxCableOrch(m_applDb, m_stateDb, APP_MUX_CABLE_TABLE_NAME);
     gDirectory.set(mux_cb_orch);
@@ -419,7 +420,7 @@ bool OrchDaemon::init()
      * when iterating ConsumerMap. This is ensured implicitly by the order of keys in ordered map.
      * For cases when Orch has to process tables in specific order, like PortsOrch during warm start, it has to override Orch::doTask()
      */
-    m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, gFlowCounterRouteOrch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, gQosOrch, wm_orch, gPolicerOrch, gTunneldecapOrch, sflow_orch, gDebugCounterOrch, gMacsecOrch, bgp_global_state_orch, gBfdOrch, gSrv6Orch, mux_orch, mux_cb_orch, gMonitorOrch};
+    m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, gFlowCounterRouteOrch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, gQosOrch, wm_orch, gPolicerOrch, gTunneldecapOrch, sflow_orch, gDebugCounterOrch, gMacsecOrch, bgp_global_state_orch, gBfdOrch, gSrv6Orch, gMuxOrch, mux_cb_orch, gMonitorOrch};
 
     bool initialize_dtel = false;
     if (platform == BFN_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING)
@@ -934,6 +935,10 @@ bool OrchDaemon::warmRestoreAndSyncUp()
         o->bake();
     }
 
+    // let's cache the neighbor updates in mux orch and
+    // process them after everything being settled.
+    gMuxOrch->enableCachingNeighborUpdate();
+
     /*
      * Three iterations are needed.
      *
@@ -960,6 +965,9 @@ bool OrchDaemon::warmRestoreAndSyncUp()
         }
     }
 
+    gMuxOrch->updateCachedNeighbors();
+    gMuxOrch->disableCachingNeighborUpdate();
+
     // MirrorOrch depends on everything else being settled before it can run,
     // and mirror ACL rules depend on MirrorOrch, so run these two at the end
     // after the rest of the data has been processed.

diff --git a/tests/test_mux.py b/tests/test_mux.py
@@ -1350,8 +1350,8 @@ def setup_peer_switch(self, dvs):
             self.DEFAULT_PEER_SWITCH_PARAMS
         )
 
-    @pytest.fixture
-    def remove_peer_switch(self, dvs):
+        yield
+
         config_db = dvs.get_config_db()
         config_db.delete_entry(self.CONFIG_PEER_SWITCH, self.PEER_SWITCH_HOST)
 
@@ -1612,7 +1612,7 @@ def test_neighbor_miss_no_mux(
 
     def test_neighbor_miss_no_peer(
             self, dvs, dvs_route, setup_vlan, setup_mux_cable, setup_tunnel,
-            remove_peer_switch, neighbor_cleanup, testlog
+            neighbor_cleanup, testlog
     ):
         """
         test neighbor miss with no peer switch configured
@@ -1634,7 +1634,7 @@ def test_soc_ip(self, dvs, dvs_route, setup_vlan, setup_mux_cable, testlog):
 
     def test_warm_boot_mux_state(
             self, dvs, dvs_route, setup_vlan, setup_mux_cable, setup_tunnel,
-            remove_peer_switch, neighbor_cleanup, testlog
+            setup_peer_switch, neighbor_cleanup, testlog
     ):
         """
         test mux initialization during warm boot.
@@ -1650,6 +1650,7 @@ def test_warm_boot_mux_state(
         dvs.runcmd("config warm_restart enable swss")
         dvs.stop_swss()
         dvs.start_swss()
+        dvs.runcmd(['sh', '-c', 'supervisorctl start restore_neighbors'])
 
         time.sleep(5)
 
@@ -1668,6 +1669,92 @@ def test_warm_boot_mux_state(
             if key == "state":
                 assert fvs[key] == "standby", "Ethernet8 Mux state is not standby after warm boot, state: {}".format(fvs[key])
 
+    def test_warm_boot_neighbor_restore(
+        self, dvs, dvs_route, setup, setup_vlan, setup_mux_cable, setup_tunnel,
+        setup_peer_switch, neighbor_cleanup, testlog
+    ):
+        """Test neighbors could be restored to correct state based on mux state after warm boot."""
+        appdb = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
+        apdb = dvs.get_app_db()
+        asicdb = dvs.get_asic_db()
+
+        self.set_mux_state(appdb, "Ethernet0", "active")
+        self.set_mux_state(appdb, "Ethernet4", "active")
+        self.set_mux_state(appdb, "Ethernet8", "standby")
+
+        self.add_fdb(dvs, "Ethernet0", "00-00-00-00-00-01")
+        self.add_fdb(dvs, "Ethernet4", "00-00-00-00-00-02")
+        self.add_fdb(dvs, "Ethernet8", "00-00-00-00-00-03")
+
+        self.add_neighbor(dvs, self.SERV1_IPV4, "00:00:00:00:00:01")
+        self.add_neighbor(dvs, self.SERV1_IPV6, "00:00:00:00:00:01")
+        self.add_neighbor(dvs, self.NEIGH1_IPV4, "00:00:00:00:00:01")
+        self.add_neighbor(dvs, self.NEIGH1_IPV6, "00:00:00:00:00:01")
+        self.add_neighbor(dvs, self.SERV2_IPV4, "00:00:00:00:00:02")
+        self.add_neighbor(dvs, self.SERV2_IPV6, "00:00:00:00:00:02")
+        self.add_neighbor(dvs, self.NEIGH2_IPV4, "00:00:00:00:00:02")
+        self.add_neighbor(dvs, self.NEIGH2_IPV6, "00:00:00:00:00:02")
+        self.add_neighbor(dvs, self.SERV3_IPV4, "00:00:00:00:00:03")
+        self.add_neighbor(dvs, self.SERV3_IPV6, "00:00:00:00:00:03")
+        self.add_neighbor(dvs, self.NEIGH3_IPV4, "00:00:00:00:00:03")
+        self.add_neighbor(dvs, self.NEIGH3_IPV6, "00:00:00:00:00:03")
+
+        time.sleep(5)
+
+        self.check_neigh_in_asic_db(asicdb, self.SERV1_IPV4)
+        self.check_neigh_in_asic_db(asicdb, self.SERV1_IPV6)
+        self.check_neigh_in_asic_db(asicdb, self.NEIGH1_IPV4)
+        self.check_neigh_in_asic_db(asicdb, self.NEIGH1_IPV6)
+        self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV4)
+        self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV6)
+        self.check_neigh_in_asic_db(asicdb, self.NEIGH2_IPV4)
+        self.check_neigh_in_asic_db(asicdb, self.NEIGH2_IPV6)
+        dvs_route.check_asicdb_route_entries(
+            [
+                self.SERV3_IPV4 + self.IPV4_MASK,
+                self.SERV3_IPV6 + self.IPV6_MASK,
+                self.NEIGH3_IPV4 + self.IPV4_MASK,
+                self.NEIGH3_IPV6 + self.IPV6_MASK
+            ]
+        )
+        # Execute the warm reboot
+        dvs.runcmd("config warm_restart enable system")
+        dvs.stop_swss()
+        dvs.start_swss()
+
+        time.sleep(5)
+
+        fvs = apdb.get_entry(self.APP_MUX_CABLE, "Ethernet0")
+        for key in fvs:
+            if key == "state":
+                assert fvs[key] == "active", "Ethernet0 Mux state is not active after warm boot, state: {}".format(fvs[key])
+
+        fvs = apdb.get_entry(self.APP_MUX_CABLE, "Ethernet4")
+        for key in fvs:
+            if key == "state":
+                assert fvs[key] == "active", "Ethernet4 Mux state is not active after warm boot, state: {}".format(fvs[key])
+
+        fvs = apdb.get_entry(self.APP_MUX_CABLE, "Ethernet8")
+        for key in fvs:
+            if key == "state":
+                assert fvs[key] == "standby", "Ethernet8 Mux state is not standby after warm boot, state: {}".format(fvs[key])
+
+        self.check_neigh_in_asic_db(asicdb, self.SERV1_IPV4)
+        self.check_neigh_in_asic_db(asicdb, self.SERV1_IPV6)
+        self.check_neigh_in_asic_db(asicdb, self.NEIGH1_IPV4)
+        self.check_neigh_in_asic_db(asicdb, self.NEIGH1_IPV6)
+        self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV4)
+        self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV6)
+        self.check_neigh_in_asic_db(asicdb, self.NEIGH2_IPV4)
+        self.check_neigh_in_asic_db(asicdb, self.NEIGH2_IPV6)
+        dvs_route.check_asicdb_route_entries(
+            [
+                self.SERV3_IPV4 + self.IPV4_MASK,
+                self.SERV3_IPV6 + self.IPV6_MASK,
+                self.NEIGH3_IPV4 + self.IPV4_MASK,
+                self.NEIGH3_IPV6 + self.IPV6_MASK
+            ]
+        )
 
 # Add Dummy always-pass test at end as workaroud
 # for issue when Flaky fail on final test it invokes module tear-down before retrying