Skip to content

Commit

Permalink
Routing-stack warm-reboot feature.
Browse files Browse the repository at this point in the history 
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 <rmolina@linkedin.com>
  • Loading branch information
Rodny Molina committed Oct 24, 2018
1 parent 6e76e9b commit 96af911
Show file tree
Hide file tree
Showing 6 changed files with 716 additions and 16 deletions.
5 changes: 2 additions & 3 deletions fpmsyncd/Makefile.am
View file
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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

66 changes: 61 additions & 5 deletions fpmsyncd/fpmsyncd.cpp
View file
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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)
Expand Down
51 changes: 46 additions & 5 deletions fpmsyncd/routesync.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
{
Expand Down Expand Up @@ -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);
}
}
10 changes: 7 additions & 3 deletions fpmsyncd/routesync.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include "dbconnector.h"
#include "producerstatetable.h"
#include "netmsg.h"
#include "warmRestartHelper.h"


namespace swss {

Expand All @@ -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;
};

}
Expand Down
Loading

0 comments on commit 96af911

Please sign in to comment.