*: Add and use option for graceful (re)start

Add a new start option "-K" to libfrr to denote a graceful start,
and use it in zebra and bgpd.

zebra will use this option to denote a planned FRR graceful restart
(supporting only bgpd currently) to wait for a route sync completion
from bgpd before cleaning up old stale routes from the FIB. An optional
timer provides an upper-bounds for this cleanup.

bgpd will use this option to denote either a planned FRR graceful
restart or a bgpd-only graceful restart, and this will drive the BGP
GR restarting router procedures.

Signed-off-by: Vivek Venkatraman <[email protected]>

Author: vivek-cumulus

bgpd/bgp_main.c

@@ -516,7 +516,9 @@ int main(int argc, char **argv)
 		bgp_option_set(BGP_OPT_NO_FIB);
 	if (no_zebra_flag)
 		bgp_option_set(BGP_OPT_NO_ZEBRA);
-	SET_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART);
+	if (bgpd_di.graceful_restart) {
+		SET_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART);
+	}
 	bgp_error_init();
 	/* Initializations. */
 	libagentx_init();

lib/libfrr.c

@@ -102,37 +102,41 @@ static void opt_extend(const struct optspec *os)
 #define OPTION_SCRIPTDIR 1009
 
 static const struct option lo_always[] = {
-	{"help", no_argument, NULL, 'h'},
-	{"version", no_argument, NULL, 'v'},
-	{"daemon", no_argument, NULL, 'd'},
-	{"module", no_argument, NULL, 'M'},
-	{"profile", required_argument, NULL, 'F'},
-	{"pathspace", required_argument, NULL, 'N'},
-	{"vrfdefaultname", required_argument, NULL, 'o'},
-	{"vty_socket", required_argument, NULL, OPTION_VTYSOCK},
-	{"moduledir", required_argument, NULL, OPTION_MODULEDIR},
-	{"scriptdir", required_argument, NULL, OPTION_SCRIPTDIR},
-	{"log", required_argument, NULL, OPTION_LOG},
-	{"log-level", required_argument, NULL, OPTION_LOGLEVEL},
-	{"command-log-always", no_argument, NULL, OPTION_LOGGING},
-	{"limit-fds", required_argument, NULL, OPTION_LIMIT_FDS},
-	{NULL}};
+	{ "help", no_argument, NULL, 'h' },
+	{ "version", no_argument, NULL, 'v' },
+	{ "daemon", no_argument, NULL, 'd' },
+	{ "module", no_argument, NULL, 'M' },
+	{ "profile", required_argument, NULL, 'F' },
+	{ "pathspace", required_argument, NULL, 'N' },
+	{ "vrfdefaultname", required_argument, NULL, 'o' },
+	{ "graceful_restart", optional_argument, NULL, 'K' },
+	{ "vty_socket", required_argument, NULL, OPTION_VTYSOCK },
+	{ "moduledir", required_argument, NULL, OPTION_MODULEDIR },
+	{ "scriptdir", required_argument, NULL, OPTION_SCRIPTDIR },
+	{ "log", required_argument, NULL, OPTION_LOG },
+	{ "log-level", required_argument, NULL, OPTION_LOGLEVEL },
+	{ "command-log-always", no_argument, NULL, OPTION_LOGGING },
+	{ "limit-fds", required_argument, NULL, OPTION_LIMIT_FDS },
+	{ NULL }
+};
 static const struct optspec os_always = {
-	"hvdM:F:N:o:",
+	"hvdM:F:N:o:K::",
 	"  -h, --help         Display this help and exit\n"
 	"  -v, --version      Print program version\n"
 	"  -d, --daemon       Runs in daemon mode\n"
 	"  -M, --module       Load specified module\n"
 	"  -F, --profile      Use specified configuration profile\n"
 	"  -N, --pathspace    Insert prefix into config & socket paths\n"
 	"  -o, --vrfdefaultname     Set default VRF name.\n"
+	"  -K, --graceful_restart   FRR starting in Graceful Restart mode, with optional route-cleanup timer\n"
 	"      --vty_socket   Override vty socket path\n"
 	"      --moduledir    Override modules directory\n"
 	"      --scriptdir    Override scripts directory\n"
 	"      --log          Set Logging to stdout, syslog, or file:<name>\n"
 	"      --log-level    Set Logging Level to use, debug, info, warn, etc\n"
 	"      --limit-fds    Limit number of fds supported\n",
-	lo_always};
+	lo_always
+};
 
 static bool logging_to_stdout = false; /* set when --log stdout specified */
 
@@ -358,6 +362,8 @@ void frr_preinit(struct frr_daemon_info *daemon, int argc, char **argv)
 	strlcpy(frr_protonameinst, di->logname, sizeof(frr_protonameinst));
 
 	di->cli_mode = FRR_CLI_CLASSIC;
+	di->graceful_restart = false;
+	di->gr_cleanup_time = 0;
 
 	/* we may be starting with extra FDs open for whatever purpose,
 	 * e.g. logging, some module, etc.  Recording them here allows later
@@ -520,6 +526,11 @@ static int frr_opt(int opt)
 		di->db_file = optarg;
 		break;
 #endif
+	case 'K':
+		di->graceful_restart = true;
+		if (optarg)
+			di->gr_cleanup_time = atoi(optarg);
+		break;
 	case 'C':
 		if (di->flags & FRR_NO_SPLIT_CONFIG)
 			return 1;

lib/libfrr.h

@@ -118,6 +118,8 @@ struct frr_daemon_info {
 	bool dryrun;
 	bool daemon_mode;
 	bool terminal;
+	bool graceful_restart;
+	int gr_cleanup_time;
 	enum frr_cli_mode cli_mode;
 
 	struct event *read_in;

tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-1.py

@@ -1047,8 +1047,9 @@ def test_BGP_GR_TC_4_p0(request):
             tc_name, result
         )
 
+        #R-bit must not be set if the helper node R2 restarts
         result = verify_r_bit(tgen, topo, addr_type, input_dict, dut="r1", peer="r2")
-        assert result is True, "Testcase {} : Failed \n Error {}".format(
+        assert result is not True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result
         )
 
@@ -1209,7 +1210,11 @@ def test_BGP_GR_TC_5_1_2_p1(request):
     logger.info("[Phase 2] : Restart BGPd on router R2.  ")
     kill_router_daemons(tgen, "r2", ["bgpd"])
 
+    #Start BGP with -K option to start BGP gracefully
+    tgen.net["r2"].daemons_options["bgpd"] = "-K "
     start_router_daemons(tgen, "r2", ["bgpd"])
+    #Unset -K after starting BGP
+    tgen.net["r2"].daemons_options["bgpd"] = ""
 
     logger.info("[Phase 4] : R2 is UP now, so time to collect GR stats  ")
 

tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-2.py

@@ -361,7 +361,12 @@ def test_BGP_GR_TC_8_p1(request):
     kill_router_daemons(tgen, "r1", ["bgpd"])
 
     logger.info("[Phase 3] : R1 is about to come up now  ")
+    #Set -K option to start BGP gracefully
+    tgen.net["r1"].daemons_options["bgpd"] = "-K "
+
     start_router_daemons(tgen, "r1", ["bgpd"])
+    #Unset -K after starting BGP
+    tgen.net["r1"].daemons_options["bgpd"] = ""
 
     logger.info("[Phase 4] : R2 is UP now, so time to collect GR stats  ")
 

tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-3.py

@@ -1577,7 +1577,12 @@ def test_BGP_GR_TC_9_p1(request):
         )
 
     logger.info("[Phase 5] : R2 is about to come up now  ")
+    #Set -K option to start BGP gracefully
+    tgen.net["r2"].daemons_options["bgpd"] = "-K "
+
     start_router_daemons(tgen, "r2", ["bgpd"])
+    #Unset -K after starting BGP
+    tgen.net["r2"].daemons_options["bgpd"] = ""
 
     logger.info("[Phase 4] : R2 is UP now, so time to collect GR stats  ")
 

tests/topotests/bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2-2.py

@@ -548,7 +548,12 @@ def test_BGP_GR_chaos_28_p1(request):
     logger.info("[Step 3] : Start BGPd daemon on R1..")
 
     # Start BGPd daemon on R1
+    #Set -K option to start BGP gracefully
+    tgen.net["r1"].daemons_options["bgpd"] = "-K "
     start_router_daemons(tgen, "r1", ["bgpd"])
+    #Unset -K after starting BGP
+    tgen.net["r1"].daemons_options["bgpd"] = ""
+
 
     logger.info("[Step 4] : Start BGPd daemon on R3..")
 

tests/topotests/bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2-3.py

@@ -725,7 +725,11 @@ def test_BGP_GR_chaos_37_p1(request):
     logger.info("[Step 4] : Start BGPd daemon on R1..")
 
     # Start BGPd daemon on R1
+    #Set -K option to start BGP gracefully
+    tgen.net["r1"].daemons_options["bgpd"] = "-K "
     start_router_daemons(tgen, "r1", ["bgpd"])
+    #Unset -K after starting BGP
+    tgen.net["r1"].daemons_options["bgpd"] = ""
 
     logger.info("[Step 5] : Kill BGPd daemon on R3..")
 
@@ -742,14 +746,19 @@ def test_BGP_GR_chaos_37_p1(request):
 
     # Start BGPd daemon on R3
     start_router_daemons(tgen, "r3", ["bgpd"])
-
+    
     for addr_type in ADDR_TYPES:
         # Verify r_bit
-        result = verify_r_bit(tgen, topo, addr_type, input_dict, dut="r1", peer="r3")
+        result = verify_r_bit(tgen, topo, addr_type, input_dict, dut="r3", peer="r1")
         assert result is True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result
         )
 
+        result = verify_r_bit(tgen, topo, addr_type, input_dict, dut="r1", peer="r3")
+        assert result is not True, "Testcase {} : Failed \n Error {}".format(
+            tc_name, result
+        )
+
         # Verifying RIB routes
         input_dict_1 = {key: topo["routers"][key] for key in ["r3"]}
         result = verify_rib(tgen, addr_type, dut, input_dict_1)

zebra/main.c

@@ -65,8 +65,6 @@ struct mgmt_be_client *mgmt_be_client;
 /* Route retain mode flag. */
 int retain_mode = 0;
 
-int graceful_restart;
-
 /* Receive buffer size for kernel control sockets */
 #define RCVBUFSIZE_MIN 4194304
 #ifdef HAVE_NETLINK
@@ -88,15 +86,14 @@ const struct option longopts[] = {
 	{ "socket", required_argument, NULL, 'z' },
 	{ "ecmp", required_argument, NULL, 'e' },
 	{ "retain", no_argument, NULL, 'r' },
-	{ "graceful_restart", required_argument, NULL, 'K' },
 	{ "asic-offload", optional_argument, NULL, OPTION_ASIC_OFFLOAD },
 	{ "v6-with-v4-nexthops", no_argument, NULL, OPTION_V6_WITH_V4_NEXTHOP },
 #ifdef HAVE_NETLINK
 	{ "vrfwnetns", no_argument, NULL, 'n' },
 	{ "nl-bufsize", required_argument, NULL, 's' },
 	{ "v6-rr-semantics", no_argument, NULL, OPTION_V6_RR_SEMANTICS },
 #endif /* HAVE_NETLINK */
-	{"routing-table", optional_argument, NULL, 'R'},
+	{ "routing-table", optional_argument, NULL, 'R' },
 	{ 0 }
 };
 
@@ -326,7 +323,6 @@ int main(int argc, char **argv)
 	bool v6_with_v4_nexthop = false;
 	bool notify_on_ack = true;
 
-	graceful_restart = 0;
 	vrf_configure_backend(VRF_BACKEND_VRF_LITE);
 
 	frr_preinit(&zebra_di, argc, argv);
@@ -342,7 +338,6 @@ int main(int argc, char **argv)
 		    "  -z, --socket              Set path of zebra socket\n"
 		    "  -e, --ecmp                Specify ECMP to use.\n"
 		    "  -r, --retain              When program terminates, retain added route by zebra.\n"
-		    "  -K, --graceful_restart    Graceful restart at the kernel level, timer in seconds for expiration\n"
 		    "  -A, --asic-offload        FRR is interacting with an asic underneath the linux kernel\n"
 		    "      --v6-with-v4-nexthops Underlying dataplane supports v6 routes with v4 nexthops"
 #ifdef HAVE_NETLINK
@@ -352,8 +347,7 @@ int main(int argc, char **argv)
 #else
 		    "  -s,                       Set kernel socket receive buffer size\n"
 #endif /* HAVE_NETLINK */
-		    "  -R, --routing-table       Set kernel routing table\n"
-	);
+		    "  -R, --routing-table       Set kernel routing table\n");
 
 	while (1) {
 		int opt = frr_getopt(argc, argv, NULL);
@@ -397,9 +391,6 @@ int main(int argc, char **argv)
 		case 'r':
 			retain_mode = 1;
 			break;
-		case 'K':
-			graceful_restart = atoi(optarg);
-			break;
 		case 's':
 			rcvbufsize = atoi(optarg);
 			if (rcvbufsize < RCVBUFSIZE_MIN)
@@ -488,11 +479,25 @@ int main(int argc, char **argv)
 	*  Clean up zebra-originated routes. The requests will be sent to OS
 	*  immediately, so originating PID in notifications from kernel
 	*  will be equal to the current getpid(). To know about such routes,
-	* we have to have route_read() called before.
+	*  we have to have route_read() called before.
+	*  If FRR is gracefully restarting, we either wait for clients
+	*  (e.g., BGP) to signal GR is complete else we wait for specified
+	*  duration.
 	*/
 	zrouter.startup_time = monotime(NULL);
-	event_add_timer(zrouter.master, rib_sweep_route, NULL, graceful_restart,
-			&zrouter.sweeper);
+	zrouter.rib_sweep_time = 0;
+	zrouter.graceful_restart = zebra_di.graceful_restart;
+	if (!zrouter.graceful_restart)
+		event_add_timer(zrouter.master, rib_sweep_route, NULL, 0, NULL);
+	else {
+		int gr_cleanup_time;
+
+		gr_cleanup_time = zebra_di.gr_cleanup_time
+					  ? zebra_di.gr_cleanup_time
+					  : ZEBRA_GR_DEFAULT_RIB_SWEEP_TIME;
+		event_add_timer(zrouter.master, rib_sweep_route, NULL,
+				gr_cleanup_time, &zrouter.t_rib_sweep);
+	}
 
 	/* Needed for BSD routing socket. */
 	pid = getpid();

zebra/rib.h

@@ -622,10 +622,10 @@ static inline struct nexthop_group *rib_get_fib_backup_nhg(
 }
 
 extern void zebra_gr_process_client(afi_t afi, vrf_id_t vrf_id, uint8_t proto,
-				    uint8_t instance);
+				    uint8_t instance, time_t restart_time);
 
 extern int rib_add_gr_run(afi_t afi, vrf_id_t vrf_id, uint8_t proto,
-			  uint8_t instance);
+			  uint8_t instance, time_t restart_time);
 
 extern void zebra_vty_init(void);