Skip to content

Commit e962370

Browse files
vivek-cumulusPdoijode
vivek-cumulus
authored and
Pdoijode
committed
*: 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 <vivek@nvidia.com>
1 parent 25a5eb5 commit e962370

20 files changed

+227
-67
lines changed

‎bgpd/bgp_main.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,9 @@ int main(int argc, char **argv)
519519
bgp_option_set(BGP_OPT_NO_FIB);
520520
if (no_zebra_flag)
521521
bgp_option_set(BGP_OPT_NO_ZEBRA);
522-
SET_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART);
522+
if (bgpd_di.graceful_restart) {
523+
SET_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART);
524+
}
523525
bgp_error_init();
524526
/* Initializations. */
525527
libagentx_init();

‎lib/libfrr.c‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -102,37 +102,41 @@ static void opt_extend(const struct optspec *os)
102102
#define OPTION_SCRIPTDIR 1009
103103

104104
static const struct option lo_always[] = {
105-
{"help", no_argument, NULL, 'h'},
106-
{"version", no_argument, NULL, 'v'},
107-
{"daemon", no_argument, NULL, 'd'},
108-
{"module", no_argument, NULL, 'M'},
109-
{"profile", required_argument, NULL, 'F'},
110-
{"pathspace", required_argument, NULL, 'N'},
111-
{"vrfdefaultname", required_argument, NULL, 'o'},
112-
{"vty_socket", required_argument, NULL, OPTION_VTYSOCK},
113-
{"moduledir", required_argument, NULL, OPTION_MODULEDIR},
114-
{"scriptdir", required_argument, NULL, OPTION_SCRIPTDIR},
115-
{"log", required_argument, NULL, OPTION_LOG},
116-
{"log-level", required_argument, NULL, OPTION_LOGLEVEL},
117-
{"command-log-always", no_argument, NULL, OPTION_LOGGING},
118-
{"limit-fds", required_argument, NULL, OPTION_LIMIT_FDS},
119-
{NULL}};
105+
{ "help", no_argument, NULL, 'h' },
106+
{ "version", no_argument, NULL, 'v' },
107+
{ "daemon", no_argument, NULL, 'd' },
108+
{ "module", no_argument, NULL, 'M' },
109+
{ "profile", required_argument, NULL, 'F' },
110+
{ "pathspace", required_argument, NULL, 'N' },
111+
{ "vrfdefaultname", required_argument, NULL, 'o' },
112+
{ "graceful_restart", optional_argument, NULL, 'K' },
113+
{ "vty_socket", required_argument, NULL, OPTION_VTYSOCK },
114+
{ "moduledir", required_argument, NULL, OPTION_MODULEDIR },
115+
{ "scriptdir", required_argument, NULL, OPTION_SCRIPTDIR },
116+
{ "log", required_argument, NULL, OPTION_LOG },
117+
{ "log-level", required_argument, NULL, OPTION_LOGLEVEL },
118+
{ "command-log-always", no_argument, NULL, OPTION_LOGGING },
119+
{ "limit-fds", required_argument, NULL, OPTION_LIMIT_FDS },
120+
{ NULL }
121+
};
120122
static const struct optspec os_always = {
121-
"hvdM:F:N:o:",
123+
"hvdM:F:N:o:K::",
122124
" -h, --help Display this help and exit\n"
123125
" -v, --version Print program version\n"
124126
" -d, --daemon Runs in daemon mode\n"
125127
" -M, --module Load specified module\n"
126128
" -F, --profile Use specified configuration profile\n"
127129
" -N, --pathspace Insert prefix into config & socket paths\n"
128130
" -o, --vrfdefaultname Set default VRF name.\n"
131+
" -K, --graceful_restart FRR starting in Graceful Restart mode, with optional route-cleanup timer\n"
129132
" --vty_socket Override vty socket path\n"
130133
" --moduledir Override modules directory\n"
131134
" --scriptdir Override scripts directory\n"
132135
" --log Set Logging to stdout, syslog, or file:<name>\n"
133136
" --log-level Set Logging Level to use, debug, info, warn, etc\n"
134137
" --limit-fds Limit number of fds supported\n",
135-
lo_always};
138+
lo_always
139+
};
136140

137141
static bool logging_to_stdout = false; /* set when --log stdout specified */
138142

@@ -358,6 +362,8 @@ void frr_preinit(struct frr_daemon_info *daemon, int argc, char **argv)
358362
strlcpy(frr_protonameinst, di->logname, sizeof(frr_protonameinst));
359363

360364
di->cli_mode = FRR_CLI_CLASSIC;
365+
di->graceful_restart = false;
366+
di->gr_cleanup_time = 0;
361367

362368
/* we may be starting with extra FDs open for whatever purpose,
363369
* e.g. logging, some module, etc. Recording them here allows later
@@ -520,6 +526,11 @@ static int frr_opt(int opt)
520526
di->db_file = optarg;
521527
break;
522528
#endif
529+
case 'K':
530+
di->graceful_restart = true;
531+
if (optarg)
532+
di->gr_cleanup_time = atoi(optarg);
533+
break;
523534
case 'C':
524535
if (di->flags & FRR_NO_SPLIT_CONFIG)
525536
return 1;

‎lib/libfrr.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ struct frr_daemon_info {
118118
bool dryrun;
119119
bool daemon_mode;
120120
bool terminal;
121+
bool graceful_restart;
122+
int gr_cleanup_time;
121123
enum frr_cli_mode cli_mode;
122124

123125
struct event *read_in;

‎tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-1.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1047,8 +1047,9 @@ def test_BGP_GR_TC_4_p0(request):
10471047
tc_name, result
10481048
)
10491049

1050+
#R-bit must not be set if the helper node R2 restarts
10501051
result = verify_r_bit(tgen, topo, addr_type, input_dict, dut="r1", peer="r2")
1051-
assert result is True, "Testcase {} : Failed \n Error {}".format(
1052+
assert result is not True, "Testcase {} : Failed \n Error {}".format(
10521053
tc_name, result
10531054
)
10541055

@@ -1209,7 +1210,11 @@ def test_BGP_GR_TC_5_1_2_p1(request):
12091210
logger.info("[Phase 2] : Restart BGPd on router R2. ")
12101211
kill_router_daemons(tgen, "r2", ["bgpd"])
12111212

1213+
#Start BGP with -K option to start BGP gracefully
1214+
tgen.net["r2"].daemons_options["bgpd"] = "-K "
12121215
start_router_daemons(tgen, "r2", ["bgpd"])
1216+
#Unset -K after starting BGP
1217+
tgen.net["r2"].daemons_options["bgpd"] = ""
12131218

12141219
logger.info("[Phase 4] : R2 is UP now, so time to collect GR stats ")
12151220

‎tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-2.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,12 @@ def test_BGP_GR_TC_8_p1(request):
361361
kill_router_daemons(tgen, "r1", ["bgpd"])
362362

363363
logger.info("[Phase 3] : R1 is about to come up now ")
364+
#Set -K option to start BGP gracefully
365+
tgen.net["r1"].daemons_options["bgpd"] = "-K "
366+
364367
start_router_daemons(tgen, "r1", ["bgpd"])
368+
#Unset -K after starting BGP
369+
tgen.net["r1"].daemons_options["bgpd"] = ""
365370

366371
logger.info("[Phase 4] : R2 is UP now, so time to collect GR stats ")
367372

‎tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-3.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,7 +1577,12 @@ def test_BGP_GR_TC_9_p1(request):
15771577
)
15781578

15791579
logger.info("[Phase 5] : R2 is about to come up now ")
1580+
#Set -K option to start BGP gracefully
1581+
tgen.net["r2"].daemons_options["bgpd"] = "-K "
1582+
15801583
start_router_daemons(tgen, "r2", ["bgpd"])
1584+
#Unset -K after starting BGP
1585+
tgen.net["r2"].daemons_options["bgpd"] = ""
15811586

15821587
logger.info("[Phase 4] : R2 is UP now, so time to collect GR stats ")
15831588

‎tests/topotests/bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2-2.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,12 @@ def test_BGP_GR_chaos_28_p1(request):
548548
logger.info("[Step 3] : Start BGPd daemon on R1..")
549549

550550
# Start BGPd daemon on R1
551+
#Set -K option to start BGP gracefully
552+
tgen.net["r1"].daemons_options["bgpd"] = "-K "
551553
start_router_daemons(tgen, "r1", ["bgpd"])
554+
#Unset -K after starting BGP
555+
tgen.net["r1"].daemons_options["bgpd"] = ""
556+
552557

553558
logger.info("[Step 4] : Start BGPd daemon on R3..")
554559

‎tests/topotests/bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2-3.py‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,11 @@ def test_BGP_GR_chaos_37_p1(request):
725725
logger.info("[Step 4] : Start BGPd daemon on R1..")
726726

727727
# Start BGPd daemon on R1
728+
#Set -K option to start BGP gracefully
729+
tgen.net["r1"].daemons_options["bgpd"] = "-K "
728730
start_router_daemons(tgen, "r1", ["bgpd"])
731+
#Unset -K after starting BGP
732+
tgen.net["r1"].daemons_options["bgpd"] = ""
729733

730734
logger.info("[Step 5] : Kill BGPd daemon on R3..")
731735

@@ -742,14 +746,19 @@ def test_BGP_GR_chaos_37_p1(request):
742746

743747
# Start BGPd daemon on R3
744748
start_router_daemons(tgen, "r3", ["bgpd"])
745-
749+
746750
for addr_type in ADDR_TYPES:
747751
# Verify r_bit
748-
result = verify_r_bit(tgen, topo, addr_type, input_dict, dut="r1", peer="r3")
752+
result = verify_r_bit(tgen, topo, addr_type, input_dict, dut="r3", peer="r1")
749753
assert result is True, "Testcase {} : Failed \n Error {}".format(
750754
tc_name, result
751755
)
752756

757+
result = verify_r_bit(tgen, topo, addr_type, input_dict, dut="r1", peer="r3")
758+
assert result is not True, "Testcase {} : Failed \n Error {}".format(
759+
tc_name, result
760+
)
761+
753762
# Verifying RIB routes
754763
input_dict_1 = {key: topo["routers"][key] for key in ["r3"]}
755764
result = verify_rib(tgen, addr_type, dut, input_dict_1)

‎tests/topotests/bgp_local_asn/test_bgp_local_asn_topo1.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,12 @@ def test_verify_bgp_local_as_GR_EBGP_p0(request):
11501150
assert result is True, "Testcase {} :Failed \n Error {}".format(tc_name, result)
11511151

11521152
logger.info("[Phase 5] : R3 is about to come up now ")
1153+
1154+
#Set -K option to start BGP gracefully
1155+
tgen.net["r3"].daemons_options["bgpd"] = "-K "
11531156
start_router_daemons(tgen, "r3", ["bgpd"])
1157+
#Unset -K after starting BGP
1158+
tgen.net["r3"].daemons_options["bgpd"] = ""
11541159

11551160
logger.info("[Phase 5] : R3 is UP Now ! ")
11561161

‎tests/topotests/bgp_local_asn_dot/test_bgp_local_asn_dot_topo1.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,7 +1171,12 @@ def test_verify_bgp_local_as_GR_EBGP_p0(request):
11711171
assert result is True, "Testcase {} :Failed \n Error {}".format(tc_name, result)
11721172

11731173
logger.info("[Phase 5] : R3 is about to come up now ")
1174+
1175+
#Set -K option to start BGP gracefully
1176+
tgen.net["r3"].daemons_options["bgpd"] = "-K "
11741177
start_router_daemons(tgen, "r3", ["bgpd"])
1178+
#Unset -K after starting BGP
1179+
tgen.net["r3"].daemons_options["bgpd"] = ""
11751180

11761181
logger.info("[Phase 5] : R3 is UP Now ! ")
11771182

0 commit comments

Comments
 (0)