The peer flags (mainly rde evaluate all but also transparent-as) and the
export options (none, default) are not properly handled on a config
reload. In both cases a full session restart is needed after the config
reload (with a bit of extra wait time to ensure that the peer config is
actually up to date).
The following diff should fix this.
Add an extra reload barrier (IMSG_RECONF_DRAIN) to the sync of the peer
config from the session engine to the rde. This way it can be ensured that
the peer config is up to date in the RDE before hitting reconfiguration
function.
Store the export_type and the peer flags outside of peer->conf. Adjust all
users of these two fields so they only look at the copies in peer.
This way the RDE is able to notice that a value has changed during reload.
Flush the Adj-RIB-Out for a peer where either the export_type changed or
where rde evaluate or transparent-as is changed. After the flush the RIB
is rebuilt in a 2nd step.
Fix multiple issues in the rde_softreconfig_in_done handler that resulted
in multiple runs of the out stage of the softreconfig pipeline.
OK?
--
:wq Claudio
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.519
diff -u -p -r1.519 rde.c
--- rde.c 27 Apr 2021 09:07:10 -0000 1.519
+++ rde.c 5 May 2021 12:04:55 -0000
@@ -666,6 +666,10 @@ badnetdel:
rde_dump_ctx_throttle(imsg.hdr.pid, 1);
}
break;
+ case IMSG_RECONF_DRAIN:
+ imsg_compose(ibuf_se, IMSG_RECONF_DRAIN, 0, 0,
+ -1, NULL, 0);
+ break;
default:
break;
}
@@ -2136,7 +2140,7 @@ rde_update_log(const char *message, u_in
char *p = NULL;
if (!((conf->log & BGPD_LOG_UPDATES) ||
- (peer->conf.flags & PEERFLAG_LOG_UPDATES)))
+ (peer->flags & PEERFLAG_LOG_UPDATES)))
return;
if (next != NULL)
@@ -2919,7 +2923,7 @@ rde_generate_updates(struct rib *rib, st
if (peer->state != PEER_UP)
continue;
/* handle evaluate all, keep track if it is needed */
- if (peer->conf.flags & PEERFLAG_EVALUATE_ALL)
+ if (peer->flags & PEERFLAG_EVALUATE_ALL)
rde_eval_all = 1;
else if (eval_all)
/* skip default peers if the best path didn't change */
@@ -2931,8 +2935,8 @@ rde_generate_updates(struct rib *rib, st
if (peer->capa.mp[aid] == 0)
continue;
/* skip peers with special export types */
- if (peer->conf.export_type == EXPORT_NONE ||
- peer->conf.export_type == EXPORT_DEFAULT_ROUTE)
+ if (peer->export_type == EXPORT_NONE ||
+ peer->export_type == EXPORT_DEFAULT_ROUTE)
continue;
up_generate_updates(out_rules, peer, new, old);
@@ -3289,13 +3293,34 @@ rde_reload_done(void)
continue;
peer->reconf_out = 0;
peer->reconf_rib = 0;
+ if (peer->export_type != peer->conf.export_type) {
+ log_peer_info(&peer->conf, "export type change, "
+ "reloading");
+ peer->reconf_rib = 1;
+ }
+ if ((peer->flags & PEERFLAG_EVALUATE_ALL) !=
+ (peer->conf.flags & PEERFLAG_EVALUATE_ALL)) {
+ log_peer_info(&peer->conf, "rde evaluate change, "
+ "reloading");
+ peer->reconf_rib = 1;
+ }
+ if ((peer->flags & PEERFLAG_TRANS_AS) !=
+ (peer->conf.flags & PEERFLAG_TRANS_AS)) {
+ log_peer_info(&peer->conf, "transparent-as change, "
+ "reloading");
+ peer->reconf_rib = 1;
+ }
if (peer->loc_rib_id != rib_find(peer->conf.rib)) {
log_peer_info(&peer->conf, "rib change, reloading");
peer->loc_rib_id = rib_find(peer->conf.rib);
if (peer->loc_rib_id == RIB_NOTFOUND)
fatalx("King Bula's peer met an unknown RIB");
peer->reconf_rib = 1;
- softreconfig++;
+ }
+ peer->export_type = peer->conf.export_type;
+ peer->flags = peer->conf.flags;
+
+ if (peer->reconf_rib) {
if (prefix_dump_new(peer, AID_UNSPEC,
RDE_RUNNER_ROUNDS, NULL, rde_up_flush_upcall,
rde_softreconfig_in_done, NULL) == -1)
@@ -3362,15 +3387,15 @@ rde_reload_done(void)
log_info("RDE reconfigured");
+ softreconfig++;
if (reload > 0) {
- softreconfig++;
if (rib_dump_new(RIB_ADJ_IN, AID_UNSPEC, RDE_RUNNER_ROUNDS,
- rib_byid(RIB_ADJ_IN), rde_softreconfig_in,
- rde_softreconfig_in_done, NULL) == -1)
+ NULL, rde_softreconfig_in, rde_softreconfig_in_done,
+ NULL) == -1)
fatal("%s: rib_dump_new", __func__);
log_info("running softreconfig in");
} else {
- rde_softreconfig_in_done(NULL, AID_UNSPEC);
+ rde_softreconfig_in_done((void *)1, AID_UNSPEC);
}
}
@@ -3380,14 +3405,13 @@ rde_softreconfig_in_done(void *arg, u_in
struct rde_peer *peer;
u_int16_t i;
- if (arg != NULL) {
- softreconfig--;
- /* one guy done but other dumps are still running */
- if (softreconfig > 0)
- return;
+ softreconfig--;
+ /* one guy done but other dumps are still running */
+ if (softreconfig > 0)
+ return;
+ if (arg == NULL)
log_info("softreconfig in done");
- }
/* now do the Adj-RIB-Out sync and a possible FIB sync */
softreconfig = 0;
@@ -3419,11 +3443,10 @@ rde_softreconfig_in_done(void *arg, u_in
u_int8_t aid;
if (peer->reconf_out) {
- if (peer->conf.export_type == EXPORT_NONE) {
+ if (peer->export_type == EXPORT_NONE) {
/* nothing to do here */
peer->reconf_out = 0;
- } else if (peer->conf.export_type ==
- EXPORT_DEFAULT_ROUTE) {
+ } else if (peer->export_type == EXPORT_DEFAULT_ROUTE) {
/* just resend the default route */
for (aid = 0; aid < AID_MAX; aid++) {
if (peer->capa.mp[aid])
@@ -3739,7 +3762,7 @@ rde_as4byte(struct rde_peer *peer)
static int
rde_no_as_set(struct rde_peer *peer)
{
- return (peer->conf.flags & PEERFLAG_NO_AS_SET);
+ return (peer->flags & PEERFLAG_NO_AS_SET);
}
/* End-of-RIB marker, RFC 4724 */
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.237
diff -u -p -r1.237 rde.h
--- rde.h 2 Mar 2021 09:45:07 -0000 1.237
+++ rde.h 8 Mar 2021 11:41:28 -0000
@@ -103,12 +103,14 @@ struct rde_peer {
u_int32_t up_nlricnt;
u_int32_t up_wcnt;
enum peer_state state;
+ enum export_type export_type;
u_int16_t loc_rib_id;
u_int16_t short_as;
u_int16_t mrt_idx;
u_int8_t reconf_out; /* out filter changed */
u_int8_t reconf_rib; /* rib changed */
u_int8_t throttled;
+ u_int8_t flags;
};
#define AS_SET 1
Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
retrieving revision 1.6
diff -u -p -r1.6 rde_peer.c
--- rde_peer.c 4 Dec 2020 11:57:13 -0000 1.6
+++ rde_peer.c 5 May 2021 09:26:26 -0000
@@ -170,6 +170,8 @@ peer_add(u_int32_t id, struct peer_confi
if (peer->loc_rib_id == RIB_NOTFOUND)
fatalx("King Bula's new peer met an unknown RIB");
peer->state = PEER_NONE;
+ peer->export_type = peer->conf.export_type;
+ peer->flags = peer->conf.flags;
SIMPLEQ_INIT(&peer->imsg_queue);
head = PEER_HASH(id);
@@ -429,11 +431,11 @@ peer_stale(struct rde_peer *peer, u_int8
void
peer_dump(struct rde_peer *peer, u_int8_t aid)
{
- if (peer->conf.export_type == EXPORT_NONE) {
+ if (peer->export_type == EXPORT_NONE) {
/* nothing to send apart from the marker */
if (peer->capa.grestart.restart)
prefix_add_eor(peer, aid);
- } else if (peer->conf.export_type == EXPORT_DEFAULT_ROUTE) {
+ } else if (peer->export_type == EXPORT_DEFAULT_ROUTE) {
up_generate_default(out_rules, peer, aid);
rde_up_dump_done(peer, aid);
} else {
Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
retrieving revision 1.126
diff -u -p -r1.126 rde_update.c
--- rde_update.c 20 Apr 2021 11:19:56 -0000 1.126
+++ rde_update.c 5 May 2021 09:29:34 -0000
@@ -135,7 +135,7 @@ again:
* skip the filters.
*/
if (need_withdraw &&
- !(peer->conf.flags & PEERFLAG_EVALUATE_ALL)) {
+ !(peer->flags & PEERFLAG_EVALUATE_ALL)) {
new = NULL;
goto again;
}
@@ -148,7 +148,7 @@ again:
new->pt->prefixlen, prefix_vstate(new), &state) ==
ACTION_DENY) {
rde_filterstate_clean(&state);
- if (peer->conf.flags & PEERFLAG_EVALUATE_ALL)
+ if (peer->flags & PEERFLAG_EVALUATE_ALL)
new = LIST_NEXT(new, entry.list.rib);
else
new = NULL;
@@ -360,7 +360,7 @@ up_generate_attr(u_char *buf, int len, s
break;
case ATTR_ASPATH:
if (!peer->conf.ebgp ||
- peer->conf.flags & PEERFLAG_TRANS_AS)
+ peer->flags & PEERFLAG_TRANS_AS)
pdata = aspath_prepend(asp->aspath,
peer->conf.local_as, 0, &plen);
else
@@ -399,7 +399,7 @@ up_generate_attr(u_char *buf, int len, s
*/
if (asp->flags & F_ATTR_MED && (!peer->conf.ebgp ||
asp->flags & F_ATTR_MED_ANNOUNCE ||
- peer->conf.flags & PEERFLAG_TRANS_AS)) {
+ peer->flags & PEERFLAG_TRANS_AS)) {
tmp32 = htonl(asp->med);
if ((r = attr_write(buf + wlen, len,
ATTR_OPTIONAL, ATTR_MED, &tmp32, 4)) == -1)
@@ -439,7 +439,7 @@ up_generate_attr(u_char *buf, int len, s
case ATTR_AS4_PATH:
if (neednewpath) {
if (!peer->conf.ebgp ||
- peer->conf.flags & PEERFLAG_TRANS_AS)
+ peer->flags & PEERFLAG_TRANS_AS)
pdata = aspath_prepend(asp->aspath,
peer->conf.local_as, 0, &plen);
else
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.413
diff -u -p -r1.413 session.c
--- session.c 3 May 2021 14:08:09 -0000 1.413
+++ session.c 5 May 2021 09:13:41 -0000
@@ -2734,10 +2734,24 @@ session_dispatch_imsg(struct imsgbuf *ib
}
break;
case IMSG_RECONF_DRAIN:
- if (idx != PFD_PIPE_MAIN)
- fatalx("reconf request not from parent");
- imsg_compose(ibuf_main, IMSG_RECONF_DRAIN, 0, 0,
- -1, NULL, 0);
+ switch (idx) {
+ case PFD_PIPE_ROUTE:
+ if (nconf != NULL)
+ fatalx("got unexpected %s from RDE",
+ "IMSG_RECONF_DONE");
+ imsg_compose(ibuf_main, IMSG_RECONF_DONE, 0, 0,
+ -1, NULL, 0);
+ break;
+ case PFD_PIPE_MAIN:
+ if (nconf == NULL)
+ fatalx("got unexpected %s from parent",
+ "IMSG_RECONF_DONE");
+ imsg_compose(ibuf_main, IMSG_RECONF_DRAIN, 0, 0,
+ -1, NULL, 0);
+ break;
+ default:
+ fatalx("reconf request not from parent or RDE");
+ }
break;
case IMSG_RECONF_DONE:
if (idx != PFD_PIPE_MAIN)
@@ -2771,8 +2785,10 @@ session_dispatch_imsg(struct imsgbuf *ib
nconf = NULL;
pending_reconf = 0;
log_info("SE reconfigured");
- imsg_compose(ibuf_main, IMSG_RECONF_DONE, 0, 0,
- -1, NULL, 0);
+ /*
+ * IMSG_RECONF_DONE is sent when the RDE drained
+ * the peer config sent in merge_peers().
+ */
break;
case IMSG_IFINFO:
if (idx != PFD_PIPE_MAIN)
@@ -3342,6 +3358,9 @@ merge_peers(struct bgpd_config *c, struc
}
}
}
+
+ if (imsg_rde(IMSG_RECONF_DRAIN, 0, NULL, 0) == -1)
+ fatalx("imsg_compose error");
/* pfkeys of new peers already loaded by the parent process */
RB_FOREACH_SAFE(np, peer_head, &nc->peers, next) {
