Routing-stack warm-reboot feature.

Please refer to the corresponding design document for more details: sonic-net/SONiC#239

The following manual UT plan has been successfully executed. UT automation pending.

Physical topology formed by various BGP peers connecting to the DUT. Both non-ecmp and ecmp prefixes are learned/tested.

Testcase Suite 1: Making use of “pkill -9 bgpd && pkill -9 zebra” as trigger.
=============

IPv4 prefixes
==========

    * Restart zebra/bgpd:
            - Verify that all prefixes are properly stale-marked and that no
    change is pushed to AppDB during reconciliation.
            - Result: PASSED

    * Restart zebra/bgpd and add one new non-ecmp IPv4 prefix.
            - To produce a route-state-inconsistency, add prefix in adjacent
    node before bgp sessions are re-established.
            - Verify that new-prefix msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart zebra/bgpd and withdraw one non-ecmp IPv4 prefix.
            - To produce a route-state-inconsistency, remove prefix in adjacent
    node before bgp sessions are re-established.
            - Verify that deleted-prefix msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart zebra/bgpd and add one new path to an IPv4 ecmp-prefix.
            - To produce a route-state-inconsistency, add prefix-path in
    adjacent node before bgp sessions are re-established.
            - Verify that new prefix-path msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart zebra/bgpd and withdraw one ecmp-path from an existing ecmp IPv4
      prefix.
              - To produce a route-state-inconsistency, remove prefix-path in
      adjacent node before bgp sessions are re-established.
              - Verify that deleted-prefix-path msg is NOT pushed down to AppDB
      till reconciliation takes place.
              - Eventually, during reconciliation, this path will be eliminated
      as it’s not being refreshed by remote-peer.
              - Result: PASSED

IPv6 prefixes
==========

    * Restart zebra/bgpd:
            - Verify that all prefixes are properly stale-marked and that no
    change is pushed to AppDB during reconciliation.
            - Result: PASSED

    * Restart zebra/bgpd and add one new non-ecmp IPv6 prefix.
            - To produce a route-state-inconsistency, add prefix in adjacent
    node before bgp sessions are re-established.
            - Verify that new-prefix msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart zebra/bgpd and withdraw one non-ecmp IPv6 prefix.
            - To produce a route-state-inconsistency, remove prefix in adjacent
    node before bgp sessions are re-established.
            - Verify that deleted-prefix msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart zebra/bgpd and add one new path to an IPv6 ecmp-prefix.
            - To produce a route-state-inconsistency, add prefix-path in
    adjacent node before bgp sessions are re-established.
            - Verify that new prefix-path msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart zebra/bgpd and withdraw one ecmp-path from an existing ecmp IPv6
      prefix.
              - To produce a route-state-inconsistency, remove prefix-path in
      adjacent node before bgp sessions are re-established.
              - Verify that deleted-prefix-path msg is NOT pushed down to AppDB
      till reconciliation takes place.
              - Eventually, during reconciliation, this path will be eliminated
      as it’s not being refreshed by remote-peer.
              - Result: PASSED

Testcase Suite 2: Making use of sudo service bgp restart” as trigger.
=============

IPv4 prefixes
==========

    * Restart bgp service/docker:
            - Verify that all prefixes are properly stale-marked and that no
    change is pushed to AppDB during reconciliation.
            - Result: PASSED

    * Restart bgp service/docker and add one new non-ecmp IPv4 prefix.
            - To produce a route-state-inconsistency, add prefix in adjacent
    node before bgp sessions are re-established.
            - Verify that new-prefix msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart bgp service/docker and withdraw one non-ecmp IPv4 prefix.
            - To produce a route-state-inconsistency, remove prefix in adjacent
    node before bgp sessions are re-established.
            - Verify that deleted-prefix msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart bgp service/docker and add one new path to an IPv4 ecmp-prefix.
            - To produce a route-state-inconsistency, add prefix-path in
    adjacent node before bgp sessions are re-established.
            - Verify that new prefix-path msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart bgp service/docker and withdraw one ecmp-path from an existing
      ecmp IPv4 prefix.
              - To produce a route-state-inconsistency, remove prefix-path in
      adjacent node before bgp sessions are re-established.
              - Verify that deleted-prefix-path msg is NOT pushed down to AppDB
      till reconciliation takes place.
              - Eventually, during reconciliation, this path will be eliminated
      as it’s not being refreshed by remote-peer.
              - Result: PASSED

IPv6 prefixes
==========

    * Restart bgp service/docker:
            - Verify that all prefixes are properly stale-marked and that no
    change is pushed to AppDB during reconciliation.
            - Result: PASSED

    * Restart bgp service/docker and add one new non-ecmp IPv6 prefix.
            - To produce a route-state-inconsistency, add prefix in adjacent
    node before bgp sessions are re-established.
            - Verify that new-prefix msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart bgp service/docker and withdraw one non-ecmp IPv6 prefix.
            - To produce a route-state-inconsistency, remove prefix in adjacent
    node before bgp sessions are re-established.
            - Verify that deleted-prefix msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart bgp service/docker and add one new path to an IPv6 ecmp-prefix.
            - To produce a route-state-inconsistency, add prefix-path in
    adjacent node before bgp sessions are re-established.
            - Verify that new prefix-path msg is NOT pushed down to AppDB till
    reconciliation takes place.
            - Result: PASSED

    * Restart bgp service/docker and withdraw one ecmp-path from an existing
      ecmp IPv6 prefix.
              - To produce a route-state-inconsistency, remove prefix-path in
      adjacent node before bgp sessions are re-established.
              - Verify that deleted-prefix-path msg is NOT pushed down to AppDB
      till reconciliation takes place.
              - Eventually, during reconciliation, this path will be eliminated
      as it’s not being refreshed by remote-peer.
              - Result: PASSED

RB=
G=lnos-reviewers
R=pchaudhary,pmao,rmolina,samaity,sfardeen,zxu
A=

Signed-off-by: Rodny Molina <[email protected]>

Author: Rodny Molina

fpmsyncd/Makefile.am

@@ -1,4 +1,4 @@
-INCLUDES = -I $(top_srcdir) -I $(FPM_PATH)
+INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/warmrestart -I $(FPM_PATH)
 
 bin_PROGRAMS = fpmsyncd
 
@@ -8,9 +8,8 @@ else
 DBGFLAGS = -g
 endif
 
-fpmsyncd_SOURCES = fpmsyncd.cpp fpmlink.cpp routesync.cpp
+fpmsyncd_SOURCES = fpmsyncd.cpp fpmlink.cpp routesync.cpp $(top_srcdir)/warmrestart/warmRestartHelper.cpp $(top_srcdir)/warmrestart/warmRestartHelper.h $(top_srcdir)/warmrestart/warm_restart.cpp $(top_srcdir)/warmrestart/warm_restart.h
 
 fpmsyncd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON)
 fpmsyncd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON)
 fpmsyncd_LDADD = -lnl-3 -lnl-route-3 -lswsscommon
-

fpmsyncd/fpmsyncd.cpp

@@ -1,13 +1,24 @@
 #include <iostream>
 #include "logger.h"
 #include "select.h"
+#include "selectabletimer.h"
 #include "netdispatcher.h"
+#include "warmRestartHelper.h"
 #include "fpmsyncd/fpmlink.h"
 #include "fpmsyncd/routesync.h"
 
+
 using namespace std;
 using namespace swss;
 
+
+/*
+ * Default warm-restart timer interval for routing-stack app. To be used only if
+ * no explicit value has been defined in configuration.
+ */
+const uint32_t DEFAULT_ROUTING_RESTART_INTERVAL = 120;
+
+
 int main(int argc, char **argv)
 {
     swss::Logger::linkToDbNative("fpmsyncd");
@@ -18,25 +29,70 @@ int main(int argc, char **argv)
     NetDispatcher::getInstance().registerMessageHandler(RTM_NEWROUTE, &sync);
     NetDispatcher::getInstance().registerMessageHandler(RTM_DELROUTE, &sync);
 
-    while (1)
+    while (true)
     {
         try
         {
             FpmLink fpm;
             Select s;
 
-            cout << "Waiting for connection..." << endl;
+            SelectableTimer warmStartTimer(timespec{0, 0});
+
+            cout << "Waiting for fpm-client connection..." << endl;
             fpm.accept();
             cout << "Connected!" << endl;
 
             s.addSelectable(&fpm);
+
+            /* Initialize warm-restart logic if this one is enabled */
+            bool warmStartEnabled = sync.m_warmStartHelper.isEnabled();
+            if (warmStartEnabled)
+            {
+                /* Obtain warm-restart timer defined for routing application */
+                uint32_t warmRestartIval = sync.m_warmStartHelper.getRestartTimer();
+                if (!warmRestartIval)
+                {
+                    warmStartTimer.setInterval(timespec{DEFAULT_ROUTING_RESTART_INTERVAL, 0});
+                }
+                else
+                {
+                    warmStartTimer.setInterval(timespec{warmRestartIval, 0});
+                }
+
+                /* Execute recovery instruction and kick off warm-restart timer */
+                if (sync.m_warmStartHelper.runRecovery())
+                {
+                    warmStartTimer.start();
+                    s.addSelectable(&warmStartTimer);
+                }
+            }
+
             while (true)
             {
                 Selectable *temps;
-                /* Reading FPM messages forever (and calling "readData" to read them) */
+
+                /* Reading FPM messages forever (and calling "readMe" to read them) */
                 s.select(&temps);
-                pipeline.flush();
-                SWSS_LOG_DEBUG("Pipeline flushed");
+
+                /*
+                 * Upon expiration of the warm-restart timer, proceed to run the
+                 * reconciliation process and remove warm-restart timer from
+                 * select() loop.
+                 */
+                if (warmStartEnabled && temps == &warmStartTimer)
+                {
+                    SWSS_LOG_NOTICE("Warm-Restart timer expired.");
+                    sync.m_warmStartHelper.reconciliate();
+                    s.removeSelectable(&warmStartTimer);
+
+                    pipeline.flush();
+                    SWSS_LOG_NOTICE("Pipeline flushed");
+                }
+                else if (!warmStartEnabled || sync.m_warmStartHelper.isReconciled())
+                {
+                    pipeline.flush();
+                    SWSS_LOG_NOTICE("Pipeline flushed");
+                }
             }
         }
         catch (FpmLink::FpmConnectionClosedException &e)

fpmsyncd/routesync.cpp

@@ -14,7 +14,8 @@ using namespace std;
 using namespace swss;
 
 RouteSync::RouteSync(RedisPipeline *pipeline) :
-    m_routeTable(pipeline, APP_ROUTE_TABLE_NAME, true)
+    m_routeTable(pipeline, APP_ROUTE_TABLE_NAME, true),
+    m_warmStartHelper(pipeline, &m_routeTable, "bgp", "bgp")
 {
     m_nl_sock = nl_socket_alloc();
     nl_connect(m_nl_sock, NETLINK_ROUTE);
@@ -38,10 +39,30 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj)
         return;
     }
 
+    /*
+     * Upon arrival of a delete msg we could either push the change right away,
+     * or we could opt to defer it if we are in the middle of a warm-reboot
+     * process. The goal here is to avoid unnecessary churn in swss/syncd layers.
+     */
+    auto warmState = m_warmStartHelper.getState();
+
     if (nlmsg_type == RTM_DELROUTE)
     {
-        m_routeTable.del(destipprefix);
-        return;
+        if (warmState == WarmStart::INITIALIZED ||
+            warmState == WarmStart::RECONCILED)
+        {
+            m_routeTable.del(destipprefix);
+            return;
+        }
+        else
+        {
+            SWSS_LOG_INFO("Warm-Restart: Receiving delete msg: %s\n", destipprefix);
+
+            vector<FieldValueTuple> fvVector;
+            const KeyOpFieldsValuesTuple kfv = std::make_tuple(destipprefix, "", fvVector);
+            m_warmStartHelper.removeRecoveryMap(kfv, WarmStartHelper::DELETE);
+            return;
+        }
     }
     else if (nlmsg_type != RTM_NEWROUTE)
     {
@@ -118,8 +139,28 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj)
     vector<FieldValueTuple> fvVector;
     FieldValueTuple nh("nexthop", nexthops);
     FieldValueTuple idx("ifname", ifnames);
+
     fvVector.push_back(nh);
     fvVector.push_back(idx);
-    m_routeTable.set(destipprefix, fvVector);
-    SWSS_LOG_DEBUG("RoutTable set: %s %s %s\n", destipprefix, nexthops.c_str(), ifnames.c_str());
+
+    if (warmState == WarmStart::INITIALIZED ||
+        warmState == WarmStart::RECONCILED)
+    {
+        m_routeTable.set(destipprefix, fvVector);
+        SWSS_LOG_DEBUG("RouteTable set msg: %s %s %s\n",
+                       destipprefix, nexthops.c_str(), ifnames.c_str());
+    }
+
+    /*
+     * During routing-stack restarting scenarios route-updates will be temporarily
+     * put on hold by warm-reboot logic.
+     */
+    else
+    {
+        SWSS_LOG_INFO("Warm-Restart: RouteTable set msg: %s %s %s\n",
+                      destipprefix, nexthops.c_str(), ifnames.c_str());
+
+        const KeyOpFieldsValuesTuple kfv = std::make_tuple(destipprefix, "", fvVector);
+        m_warmStartHelper.insertRecoveryMap(kfv, WarmStartHelper::CLEAN);
+    }
 }

fpmsyncd/routesync.h

@@ -4,6 +4,8 @@
 #include "dbconnector.h"
 #include "producerstatetable.h"
 #include "netmsg.h"
+#include "warmRestartHelper.h"
+
 
 namespace swss {
 
@@ -16,10 +18,12 @@ class RouteSync : public NetMsg
 
     virtual void onMsg(int nlmsg_type, struct nl_object *obj);
 
+    WarmStartHelper  m_warmStartHelper;
+
 private:
-    ProducerStateTable m_routeTable;
-    struct nl_cache *m_link_cache;
-    struct nl_sock *m_nl_sock;
+    ProducerStateTable  m_routeTable;
+    struct nl_cache    *m_link_cache;
+    struct nl_sock     *m_nl_sock;
 };
 
 }