Skip to content

Commit 555d8fc

Browse files
vivek-cumulusPdoijode
vivek-cumulus
authored and
Pdoijode
committed
bgpd: Refine restarter operation - R-bit & F-bit
Introduce BGP-wide flags to denote if BGP has started gracefully and GR is in progress or not. Use this for setting of the R-bit in the GR capability, and not a timer which is set for any new instance creation. Mark graceful restart is complete when the deferred path selection has been done and route sync with zebra as well as deferred EOR advertisement has been initiated. Introduce a function to check on F-bit setting rather than just base it on configuration. Subsequent commits will extend these functionalities. Signed-off-by: Vivek Venkatraman <[email protected]>
1 parent 15403f5 commit 555d8fc

File tree

8 files changed

+121
-53
lines changed

8 files changed

+121
-53
lines changed

‎bgpd/bgp_fsm.c‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2046,9 +2046,10 @@ static int bgp_start_deferral_timer(struct bgp *bgp, afi_t afi, safi_t safi,
20462046
}
20472047
gr_info->eor_required++;
20482048
/* Send message to RIB indicating route update pending */
2049-
if (gr_info->af_enabled[afi][safi] == false) {
2050-
gr_info->af_enabled[afi][safi] = true;
2051-
/* Send message to RIB */
2049+
if (gr_info->af_enabled == false) {
2050+
gr_info->af_enabled = true;
2051+
gr_info->route_sync = false;
2052+
bgp->gr_route_sync_pending = true;
20522053
bgp_zebra_update(bgp, afi, safi,
20532054
ZEBRA_CLIENT_ROUTE_UPDATE_PENDING);
20542055
}
@@ -2082,7 +2083,7 @@ static int bgp_update_gr_info(struct peer *peer, afi_t afi, safi_t safi)
20822083
if (BGP_PEER_GRACEFUL_RESTART_CAPABLE(peer)
20832084
&& BGP_PEER_RESTARTING_MODE(peer)) {
20842085
/* Check if the forwarding state is preserved */
2085-
if (CHECK_FLAG(bgp->flags, BGP_FLAG_GR_PRESERVE_FWD)) {
2086+
if (bgp_gr_is_forwarding_preserved(bgp)) {
20862087
gr_info = &(bgp->gr_info[afi][safi]);
20872088
ret = bgp_start_deferral_timer(bgp, afi, safi, gr_info);
20882089
}
@@ -2199,8 +2200,7 @@ bgp_establish(struct peer_connection *connection)
21992200
} else {
22002201
if (BGP_PEER_GRACEFUL_RESTART_CAPABLE(peer) &&
22012202
BGP_PEER_RESTARTING_MODE(peer) &&
2202-
CHECK_FLAG(peer->bgp->flags,
2203-
BGP_FLAG_GR_PRESERVE_FWD))
2203+
bgp_gr_is_forwarding_preserved(peer->bgp))
22042204
peer->bgp->gr_info[afi][safi]
22052205
.eor_required++;
22062206
}
@@ -2702,10 +2702,14 @@ bgp_peer_inherit_global_gr_mode(struct peer *peer,
27022702
switch (global_gr_mode) {
27032703
case GLOBAL_HELPER:
27042704
BGP_PEER_GR_HELPER_ENABLE(peer);
2705+
break;
27052706
case GLOBAL_GR:
27062707
BGP_PEER_GR_ENABLE(peer);
2708+
break;
27072709
case GLOBAL_DISABLE:
27082710
BGP_PEER_GR_DISABLE(peer);
2711+
break;
2712+
case GLOBAL_INVALID:
27092713
default:
27102714
zlog_err("Unexpected Global GR mode %d", global_gr_mode);
27112715
}

‎bgpd/bgp_main.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,13 +511,15 @@ int main(int argc, char **argv)
511511

512512
/* BGP master init. */
513513
bgp_master_init(frr_init(), buffer_size, addresses);
514+
bm->startup_time = monotime(NULL);
514515
bm->port = bgp_port;
515516
if (bgp_port == 0)
516517
bgp_option_set(BGP_OPT_NO_LISTEN);
517518
if (no_fib_flag || no_zebra_flag)
518519
bgp_option_set(BGP_OPT_NO_FIB);
519520
if (no_zebra_flag)
520521
bgp_option_set(BGP_OPT_NO_ZEBRA);
522+
SET_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART);
521523
bgp_error_init();
522524
/* Initializations. */
523525
libagentx_init();

‎bgpd/bgp_open.c‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,15 +1587,12 @@ static void bgp_peer_send_gr_capability(struct stream *s, struct peer *peer,
15871587
uint32_t restart_time;
15881588
unsigned long capp = 0;
15891589
unsigned long rcapp = 0;
1590+
struct bgp *bgp = peer->bgp;
15901591

15911592
if (!CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART)
15921593
&& !CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART_HELPER))
15931594
return;
15941595

1595-
if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
1596-
zlog_debug("[BGP_GR] Sending helper Capability for Peer :%s :",
1597-
peer->host);
1598-
15991596
SET_FLAG(peer->cap, PEER_CAP_RESTART_ADV);
16001597
stream_putc(s, BGP_OPEN_OPT_CAP);
16011598
capp = stream_get_endp(s); /* Set Capability Len Pointer */
@@ -1605,54 +1602,57 @@ static void bgp_peer_send_gr_capability(struct stream *s, struct peer *peer,
16051602
/* Set Restart Capability Len Pointer */
16061603
rcapp = stream_get_endp(s);
16071604
stream_putc(s, 0);
1608-
restart_time = peer->bgp->restart_time;
1609-
if (peer->bgp->t_startup) {
1605+
restart_time = bgp->restart_time;
1606+
if (bgp_in_graceful_restart()) {
16101607
SET_FLAG(restart_time, GRACEFUL_RESTART_R_BIT);
16111608
SET_FLAG(peer->cap, PEER_CAP_GRACEFUL_RESTART_R_BIT_ADV);
1612-
if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
1613-
zlog_debug("[BGP_GR] Sending R-Bit for peer: %s",
1614-
peer->host);
16151609
}
16161610

1617-
if (CHECK_FLAG(peer->bgp->flags, BGP_FLAG_GRACEFUL_NOTIFICATION)) {
1611+
if (CHECK_FLAG(bgp->flags, BGP_FLAG_GRACEFUL_NOTIFICATION)) {
16181612
SET_FLAG(restart_time, GRACEFUL_RESTART_N_BIT);
16191613
SET_FLAG(peer->cap, PEER_CAP_GRACEFUL_RESTART_N_BIT_ADV);
1620-
if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
1621-
zlog_debug("[BGP_GR] Sending N-Bit for peer: %s",
1622-
peer->host);
16231614
}
16241615

16251616
stream_putw(s, restart_time);
16261617

1618+
if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
1619+
zlog_debug("%s: Sending GR Capability, Restart time %d R-bit %s, N-bit %s",
1620+
peer->host, bgp->restart_time,
1621+
CHECK_FLAG(peer->cap,
1622+
PEER_CAP_GRACEFUL_RESTART_R_BIT_ADV)
1623+
? "SET"
1624+
: "NOT-SET",
1625+
CHECK_FLAG(peer->cap,
1626+
PEER_CAP_GRACEFUL_RESTART_N_BIT_ADV)
1627+
? "SET"
1628+
: "NOT-SET");
1629+
16271630
/* Send address-family specific graceful-restart capability
16281631
* only when GR config is present
16291632
*/
16301633
if (CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART)) {
1631-
if (CHECK_FLAG(peer->bgp->flags, BGP_FLAG_GR_PRESERVE_FWD)
1632-
&& BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
1633-
zlog_debug("[BGP_GR] F bit Set");
1634-
16351634
FOREACH_AFI_SAFI (afi, safi) {
1635+
bool f_bit = false;
1636+
16361637
if (!peer->afc[afi][safi])
16371638
continue;
16381639

1639-
if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
1640-
zlog_debug(
1641-
"[BGP_GR] Sending GR Capability for AFI :%d :, SAFI :%d:",
1642-
afi, safi);
1643-
16441640
/* Convert AFI, SAFI to values for
16451641
* packet.
16461642
*/
16471643
bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi,
16481644
&pkt_safi);
16491645
stream_putw(s, pkt_afi);
16501646
stream_putc(s, pkt_safi);
1651-
if (CHECK_FLAG(peer->bgp->flags,
1652-
BGP_FLAG_GR_PRESERVE_FWD))
1653-
stream_putc(s, GRACEFUL_RESTART_F_BIT);
1654-
else
1655-
stream_putc(s, 0);
1647+
1648+
f_bit = bgp_gr_is_forwarding_preserved(bgp);
1649+
1650+
if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
1651+
zlog_debug("... F-bit %s for %s",
1652+
f_bit ? "SET" : "NOT-SET",
1653+
get_afi_safi_str(afi, safi, false));
1654+
1655+
stream_putc(s, f_bit ? GRACEFUL_RESTART_F_BIT : 0);
16561656
}
16571657
}
16581658

‎bgpd/bgp_packet.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1296,7 +1296,7 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
12961296
stream_putc(s, 0);
12971297
gr_restart_time = peer->bgp->restart_time;
12981298

1299-
if (peer->bgp->t_startup) {
1299+
if (bgp_in_graceful_restart()) {
13001300
SET_FLAG(gr_restart_time, GRACEFUL_RESTART_R_BIT);
13011301
SET_FLAG(peer->cap, PEER_CAP_GRACEFUL_RESTART_R_BIT_ADV);
13021302
}

‎bgpd/bgp_route.c‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3877,6 +3877,7 @@ void bgp_best_path_select_defer(struct bgp *bgp, afi_t afi, safi_t safi)
38773877
struct bgp_dest *dest;
38783878
int cnt = 0;
38793879
struct afi_safi_info *thread_info;
3880+
bool route_sync_pending = false;
38803881

38813882
if (bgp->gr_info[afi][safi].t_route_select) {
38823883
struct event *t = bgp->gr_info[afi][safi].t_route_select;
@@ -3886,7 +3887,7 @@ void bgp_best_path_select_defer(struct bgp *bgp, afi_t afi, safi_t safi)
38863887
EVENT_OFF(bgp->gr_info[afi][safi].t_route_select);
38873888
}
38883889

3889-
if (BGP_DEBUG(update, UPDATE_OUT)) {
3890+
if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) {
38903891
zlog_debug("%s: processing route for %s : cnt %d", __func__,
38913892
get_afi_safi_str(afi, safi, false),
38923893
bgp->gr_info[afi][safi].gr_deferred);
@@ -3919,6 +3920,21 @@ void bgp_best_path_select_defer(struct bgp *bgp, afi_t afi, safi_t safi)
39193920
/* Send route processing complete message to RIB */
39203921
bgp_zebra_update(bgp, afi, safi,
39213922
ZEBRA_CLIENT_ROUTE_UPDATE_COMPLETE);
3923+
bgp->gr_info[afi][safi].route_sync = true;
3924+
3925+
/* If this instance is all done, check for GR completion overall */
3926+
FOREACH_AFI_SAFI_NSF (afi, safi) {
3927+
if (bgp->gr_info[afi][safi].af_enabled &&
3928+
!bgp->gr_info[afi][safi].route_sync) {
3929+
route_sync_pending = true;
3930+
break;
3931+
}
3932+
}
3933+
3934+
if (!route_sync_pending) {
3935+
bgp->gr_route_sync_pending = false;
3936+
bgp_update_gr_completion();
3937+
}
39223938
return;
39233939
}
39243940

‎bgpd/bgp_vty.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3652,7 +3652,7 @@ DEFUN (bgp_neighbor_graceful_restart_disable_set,
36523652

36533653
ret = bgp_neighbor_graceful_restart(peer, PEER_DISABLE_CMD);
36543654
if (ret == BGP_GR_SUCCESS) {
3655-
if (peer->bgp->t_startup)
3655+
if (bgp_in_graceful_restart())
36563656
bgp_peer_gr_flags_update(peer);
36573657

36583658
VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer);

‎bgpd/bgpd.c‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3389,14 +3389,6 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer,
33893389
return 0;
33903390
}
33913391

3392-
static void bgp_startup_timer_expire(struct event *thread)
3393-
{
3394-
struct bgp *bgp;
3395-
3396-
bgp = EVENT_ARG(thread);
3397-
bgp->t_startup = NULL;
3398-
}
3399-
34003392
/*
34013393
* On shutdown we call the cleanup function which
34023394
* does a free of the link list nodes, free up
@@ -3555,9 +3547,6 @@ static struct bgp *bgp_create(as_t *as, const char *name,
35553547
if (name)
35563548
bgp->name = XSTRDUP(MTYPE_BGP_NAME, name);
35573549

3558-
event_add_timer(bm->master, bgp_startup_timer_expire, bgp,
3559-
bgp->restart_time, &bgp->t_startup);
3560-
35613550
/* printable name we can use in debug messages */
35623551
if (inst_type == BGP_INSTANCE_TYPE_DEFAULT) {
35633552
bgp->name_pretty = XSTRDUP(MTYPE_BGP_NAME, "VRF default");
@@ -3964,7 +3953,6 @@ int bgp_delete(struct bgp *bgp)
39643953
EVENT_OFF(bgp->t_revalidate[afi][safi]);
39653954

39663955
EVENT_OFF(bgp->t_condition_check);
3967-
EVENT_OFF(bgp->t_startup);
39683956
EVENT_OFF(bgp->t_maxmed_onstartup);
39693957
EVENT_OFF(bgp->t_update_delay);
39703958
EVENT_OFF(bgp->t_establish_wait);

‎bgpd/bgpd.h‎

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ struct bgp_master {
167167
#define BM_FLAG_GR_RESTARTER (1 << 3)
168168
#define BM_FLAG_GR_DISABLED (1 << 4)
169169
#define BM_FLAG_GR_PRESERVE_FWD (1 << 5)
170+
#define BM_FLAG_GRACEFUL_RESTART (1 << 6)
171+
#define BM_FLAG_GR_COMPLETE (1 << 7)
170172

171173
#define BM_FLAG_GR_CONFIGURED (BM_FLAG_GR_RESTARTER | BM_FLAG_GR_DISABLED)
172174

@@ -176,6 +178,9 @@ struct bgp_master {
176178
uint32_t select_defer_time;
177179
uint32_t rib_stale_time;
178180

181+
time_t startup_time;
182+
time_t gr_completion_time;
183+
179184
bool terminating; /* global flag that sigint terminate seen */
180185

181186
/* TOS value for outgoing packets in BGP connections */
@@ -305,9 +310,9 @@ struct graceful_restart_info {
305310
/* Best route select */
306311
struct event *t_route_select;
307312
/* AFI, SAFI enabled */
308-
bool af_enabled[AFI_MAX][SAFI_MAX];
313+
bool af_enabled;
309314
/* Route update completed */
310-
bool route_sync[AFI_MAX][SAFI_MAX];
315+
bool route_sync;
311316
};
312317

313318
enum global_mode {
@@ -442,9 +447,6 @@ struct bgp {
442447
struct as_confed *confed_peers;
443448
int confed_peers_cnt;
444449

445-
/* start-up timer on only once at the beginning */
446-
struct event *t_startup;
447-
448450
uint32_t v_maxmed_onstartup; /* Duration of max-med on start-up */
449451
#define BGP_MAXMED_ONSTARTUP_UNCONFIGURED 0 /* 0 means off, its the default */
450452
uint32_t maxmed_onstartup_value; /* Max-med value when active on
@@ -559,6 +561,9 @@ struct bgp {
559561
*/
560562
enum zebra_gr_mode present_zebra_gr_state;
561563

564+
/* Is deferred path selection still not complete? */
565+
bool gr_route_sync_pending;
566+
562567
/* BGP Per AF flags */
563568
uint16_t af_flags[AFI_MAX][SAFI_MAX];
564569
#define BGP_CONFIG_DAMPENING (1 << 0)
@@ -2754,6 +2759,59 @@ static inline bool bgp_in_graceful_shutdown(struct bgp *bgp)
27542759
!!CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_SHUTDOWN));
27552760
}
27562761

2762+
static inline bool bgp_in_graceful_restart(void)
2763+
{
2764+
/* True if BGP has (re)started gracefully (based on flags
2765+
* noted at startup) and GR is not complete.
2766+
*/
2767+
return (CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART) &&
2768+
!CHECK_FLAG(bm->flags, BM_FLAG_GR_COMPLETE));
2769+
}
2770+
2771+
static inline bool bgp_is_graceful_restart_complete(void)
2772+
{
2773+
/* True if BGP has (re)started gracefully (based on flags
2774+
* noted at startup) and GR is marked as complete.
2775+
*/
2776+
return (CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART) &&
2777+
CHECK_FLAG(bm->flags, BM_FLAG_GR_COMPLETE));
2778+
}
2779+
2780+
static inline void bgp_update_gr_completion(void)
2781+
{
2782+
struct listnode *node, *nnode;
2783+
struct bgp *bgp;
2784+
2785+
/*
2786+
* Check and mark GR complete. This is done when deferred
2787+
* path selection has been completed for all instances and
2788+
* route-advertisement/EOR and route-sync with zebra has
2789+
* been invoked.
2790+
*/
2791+
if (!CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART) ||
2792+
CHECK_FLAG(bm->flags, BM_FLAG_GR_COMPLETE))
2793+
return;
2794+
2795+
for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) {
2796+
if (bgp->gr_route_sync_pending)
2797+
return;
2798+
}
2799+
2800+
SET_FLAG(bm->flags, BM_FLAG_GR_COMPLETE);
2801+
bm->gr_completion_time = monotime(NULL);
2802+
}
2803+
2804+
static inline bool bgp_gr_is_forwarding_preserved(struct bgp *bgp)
2805+
{
2806+
/*
2807+
* Is forwarding state preserved? Based either on config
2808+
* or if BGP restarted gracefully.
2809+
* TBD: Additional AFI/SAFI based checks etc.
2810+
*/
2811+
return (CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART) ||
2812+
CHECK_FLAG(bgp->flags, BGP_FLAG_GR_PRESERVE_FWD));
2813+
}
2814+
27572815
/* For benefit of rfapi */
27582816
extern struct peer *peer_new(struct bgp *bgp);
27592817

0 commit comments

Comments
 (0)