From 442a03204b9c6c8c7e885744f7634b690a07ffb3 Mon Sep 17 00:00:00 2001 From: claudio Date: Thu, 12 Dec 2024 20:19:03 +0000 Subject: [PATCH] Cache the Adj-RIB-Out for sessions that have not been down for more than INTERVAL_SESSION_DOWN (3600) seconds. Rebuilding the Adj-RIB-Out is a lot of work while keeping the RIB in sync is reasonably trivial. So avoid the work for the case that a session was just quickly reset. This only works if the same peer settings are used in the old and new session. For this introduce a IMSG_SESSION_DELETE that tells the RDE to remove the peer and split peer_down into a part that takes the session down (and clears the Adj-RIB-In) and a part the frees the peer (peer_delete). The SE now sends an IMSG_SESSION_ADD command on first connect and skips that imsg on later connects unless IMSG_SESSION_DELETE was called before. During config reload the IMSG_SESSION_ADD calls only need to happen when the RDE actually has that information. OK tb@ --- usr.sbin/bgpd/bgpd.h | 3 +- usr.sbin/bgpd/rde.c | 8 +++- usr.sbin/bgpd/rde.h | 6 ++- usr.sbin/bgpd/rde_peer.c | 91 ++++++++++++++++++++-------------------- usr.sbin/bgpd/rde_rib.c | 22 ++++++++-- usr.sbin/bgpd/session.c | 44 ++++++++++++++----- usr.sbin/bgpd/session.h | 3 +- 7 files changed, 112 insertions(+), 65 deletions(-) diff --git a/usr.sbin/bgpd/bgpd.h b/usr.sbin/bgpd/bgpd.h index 6035a322142..3885954318f 100644 --- a/usr.sbin/bgpd/bgpd.h +++ b/usr.sbin/bgpd/bgpd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: bgpd.h,v 1.504 2024/12/11 09:19:44 claudio Exp $ */ +/* $OpenBSD: bgpd.h,v 1.505 2024/12/12 20:19:03 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -683,6 +683,7 @@ enum imsg_type { IMSG_SESSION_ADD, IMSG_SESSION_UP, IMSG_SESSION_DOWN, + IMSG_SESSION_DELETE, IMSG_SESSION_STALE, IMSG_SESSION_NOGRACE, IMSG_SESSION_FLUSH, diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c index 409fb3ede0a..407f07d6878 100644 --- a/usr.sbin/bgpd/rde.c +++ b/usr.sbin/bgpd/rde.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.c,v 1.645 2024/12/11 09:19:44 claudio Exp $ */ +/* $OpenBSD: rde.c,v 1.646 2024/12/12 20:19:03 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -433,6 +433,12 @@ rde_dispatch_imsg_session(struct imsgbuf *imsgbuf) } peer_down(peer); break; + case IMSG_SESSION_DELETE: + /* silently ignore deletes for unknown peers */ + if ((peer = peer_get(peerid)) == NULL) + break; + peer_delete(peer); + break; case IMSG_SESSION_STALE: case IMSG_SESSION_NOGRACE: case IMSG_SESSION_FLUSH: diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h index c8a31e6e6d3..f28ff29683f 100644 --- a/usr.sbin/bgpd/rde.h +++ b/usr.sbin/bgpd/rde.h @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.h,v 1.308 2024/12/11 09:19:44 claudio Exp $ */ +/* $OpenBSD: rde.h,v 1.309 2024/12/12 20:19:03 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Claudio Jeker and @@ -289,7 +289,7 @@ struct prefix { #define PREFIX_FLAG_WITHDRAW 0x0001 /* enqueued on withdraw queue */ #define PREFIX_FLAG_UPDATE 0x0002 /* enqueued on update queue */ #define PREFIX_FLAG_DEAD 0x0004 /* locked but removed */ -#define PREFIX_FLAG_STALE 0x0008 /* stale entry (graceful reload) */ +#define PREFIX_FLAG_STALE 0x0008 /* stale entry (for addpath) */ #define PREFIX_FLAG_MASK 0x000f /* mask for the prefix types */ #define PREFIX_FLAG_ADJOUT 0x0010 /* prefix is in the adj-out rib */ #define PREFIX_FLAG_EOR 0x0020 /* prefix is EoR */ @@ -369,6 +369,7 @@ void rde_generate_updates(struct rib_entry *, struct prefix *, void peer_up(struct rde_peer *, struct session_up *); void peer_down(struct rde_peer *); +void peer_delete(struct rde_peer *); void peer_flush(struct rde_peer *, uint8_t, time_t); void peer_stale(struct rde_peer *, uint8_t, int); void peer_blast(struct rde_peer *, uint8_t); @@ -603,6 +604,7 @@ void prefix_adjout_update(struct prefix *, struct rde_peer *, struct filterstate *, struct pt_entry *, uint32_t); void prefix_adjout_withdraw(struct prefix *); void prefix_adjout_destroy(struct prefix *); +void prefix_adjout_flush_pending(struct rde_peer *); int prefix_adjout_reaper(struct rde_peer *); int prefix_dump_new(struct rde_peer *, uint8_t, unsigned int, void *, void (*)(struct prefix *, void *), diff --git a/usr.sbin/bgpd/rde_peer.c b/usr.sbin/bgpd/rde_peer.c index 0a6f61f42ea..60d8491349d 100644 --- a/usr.sbin/bgpd/rde_peer.c +++ b/usr.sbin/bgpd/rde_peer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde_peer.c,v 1.41 2024/12/11 09:19:44 claudio Exp $ */ +/* $OpenBSD: rde_peer.c,v 1.42 2024/12/12 20:19:03 claudio Exp $ */ /* * Copyright (c) 2019 Claudio Jeker @@ -82,7 +82,7 @@ peer_shutdown(void) struct rde_peer *peer, *np; RB_FOREACH_SAFE(peer, peer_tree, &peertable, np) - peer_down(peer); + peer_delete(peer); while (!RB_EMPTY(&zombietable)) peer_reaper(NULL); @@ -241,7 +241,8 @@ peer_generate_update(struct rde_peer *peer, struct rib_entry *re, /* skip ourself */ if (peer == peerself) return; - if (!peer_is_up(peer)) + /* skip peers that never had a session open */ + if (peer->state == PEER_NONE) return; /* skip peers using a different rib */ if (peer->loc_rib_id != re->rib_id) @@ -286,28 +287,6 @@ rde_generate_updates(struct rib_entry *re, struct prefix *newpath, /* * Various RIB walker callbacks. */ -static void -peer_adjout_clear_upcall(struct prefix *p, void *arg) -{ - prefix_adjout_destroy(p); -} - -static void -peer_adjout_stale_upcall(struct prefix *p, void *arg) -{ - if (p->flags & PREFIX_FLAG_DEAD) { - return; - } else if (p->flags & PREFIX_FLAG_WITHDRAW) { - /* no need to keep stale withdraws, they miss all attributes */ - prefix_adjout_destroy(p); - return; - } else if (p->flags & PREFIX_FLAG_UPDATE) { - RB_REMOVE(prefix_tree, &prefix_peer(p)->updates[p->pt->aid], p); - p->flags &= ~PREFIX_FLAG_UPDATE; - } - p->flags |= PREFIX_FLAG_STALE; -} - struct peer_flush { struct rde_peer *peer; time_t staletime; @@ -361,6 +340,7 @@ void peer_up(struct rde_peer *peer, struct session_up *sup) { uint8_t i; + int force_sync = 1; if (peer->state == PEER_ERR) { /* @@ -369,21 +349,33 @@ peer_up(struct rde_peer *peer, struct session_up *sup) */ rib_dump_terminate(peer); peer_imsg_flush(peer); - if (prefix_dump_new(peer, AID_UNSPEC, 0, NULL, - peer_adjout_clear_upcall, NULL, NULL) == -1) - fatal("%s: prefix_dump_new", __func__); peer_flush(peer, AID_UNSPEC, 0); peer->stats.prefix_cnt = 0; - peer->stats.prefix_out_cnt = 0; peer->state = PEER_DOWN; } - peer->remote_bgpid = sup->remote_bgpid; - peer->short_as = sup->short_as; + + /* + * Check if no value changed during flap to decide if the RIB + * is in sync. The capa check is maybe too strict but it should + * not matter for normal operation. + */ + if (memcmp(&peer->remote_addr, &sup->remote_addr, + sizeof(sup->remote_addr)) == 0 && + memcmp(&peer->local_v4_addr, &sup->local_v4_addr, + sizeof(sup->local_v4_addr)) == 0 && + memcmp(&peer->local_v6_addr, &sup->local_v6_addr, + sizeof(sup->local_v6_addr)) == 0 && + memcmp(&peer->capa, &sup->capa, sizeof(sup->capa)) == 0) + force_sync = 0; + peer->remote_addr = sup->remote_addr; peer->local_v4_addr = sup->local_v4_addr; peer->local_v6_addr = sup->local_v6_addr; + memcpy(&peer->capa, &sup->capa, sizeof(sup->capa)); + /* the Adj-RIB-Out does not depend on those */ + peer->remote_bgpid = sup->remote_bgpid; peer->local_if_scope = sup->if_scope; - memcpy(&peer->capa, &sup->capa, sizeof(peer->capa)); + peer->short_as = sup->short_as; /* clear eor markers depending on GR flags */ if (peer->capa.grestart.restart) { @@ -396,9 +388,16 @@ peer_up(struct rde_peer *peer, struct session_up *sup) } peer->state = PEER_UP; - for (i = AID_MIN; i < AID_MAX; i++) { - if (peer->capa.mp[i]) - peer_dump(peer, i); + if (!force_sync) { + for (i = AID_MIN; i < AID_MAX; i++) { + if (peer->capa.mp[i]) + peer_blast(peer, i); + } + } else { + for (i = AID_MIN; i < AID_MAX; i++) { + if (peer->capa.mp[i]) + peer_dump(peer, i); + } } } @@ -416,16 +415,24 @@ peer_down(struct rde_peer *peer) * and flush all pending imsg from the SE. */ rib_dump_terminate(peer); + prefix_adjout_flush_pending(peer); peer_imsg_flush(peer); /* flush Adj-RIB-In */ peer_flush(peer, AID_UNSPEC, 0); peer->stats.prefix_cnt = 0; +} + +void +peer_delete(struct rde_peer *peer) +{ + if (peer->state != PEER_DOWN) + peer_down(peer); /* free filters */ filterlist_free(peer->out_rules); - RB_REMOVE(peer_tree, &peertable, peer); + RB_REMOVE(peer_tree, &peertable, peer); while (RB_INSERT(peer_tree, &zombietable, peer) != NULL) { log_warnx("zombie peer conflict"); peer->conf.id = arc4random(); @@ -481,17 +488,12 @@ peer_stale(struct rde_peer *peer, uint8_t aid, int flushall) * and flush all pending imsg from the SE. */ rib_dump_terminate(peer); + prefix_adjout_flush_pending(peer); peer_imsg_flush(peer); if (flushall) peer_flush(peer, aid, 0); - /* XXX this is not quite correct */ - /* mark Adj-RIB-Out stale for this peer */ - if (prefix_dump_new(peer, aid, 0, NULL, - peer_adjout_stale_upcall, NULL, NULL) == -1) - fatal("%s: prefix_dump_new", __func__); - /* make sure new prefixes start on a higher timestamp */ while (now >= getmonotime()) sleep(1); @@ -504,10 +506,7 @@ peer_stale(struct rde_peer *peer, uint8_t aid, int flushall) static void peer_blast_upcall(struct prefix *p, void *ptr) { - if (p->flags & PREFIX_FLAG_STALE) { - /* remove stale entries */ - prefix_adjout_destroy(p); - } else if (p->flags & PREFIX_FLAG_DEAD) { + if (p->flags & PREFIX_FLAG_DEAD) { /* ignore dead prefixes, they will go away soon */ } else if ((p->flags & PREFIX_FLAG_MASK) == 0) { /* put entries on the update queue if not already on a queue */ diff --git a/usr.sbin/bgpd/rde_rib.c b/usr.sbin/bgpd/rde_rib.c index 1980e87a858..3477d686404 100644 --- a/usr.sbin/bgpd/rde_rib.c +++ b/usr.sbin/bgpd/rde_rib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde_rib.c,v 1.265 2024/12/11 09:19:44 claudio Exp $ */ +/* $OpenBSD: rde_rib.c,v 1.266 2024/12/12 20:19:03 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Claudio Jeker @@ -1273,7 +1273,6 @@ prefix_adjout_update(struct prefix *p, struct rde_peer *peer, /* nothing changed */ p->validation_state = state->vstate; p->lastchange = getmonotime(); - p->flags &= ~PREFIX_FLAG_STALE; return; } @@ -1344,7 +1343,6 @@ prefix_adjout_withdraw(struct prefix *p) /* already a withdraw, shortcut */ if (p->flags & PREFIX_FLAG_WITHDRAW) { p->lastchange = getmonotime(); - p->flags &= ~PREFIX_FLAG_STALE; return; } /* pending update just got withdrawn */ @@ -1417,6 +1415,24 @@ prefix_adjout_destroy(struct prefix *p) } } +void +prefix_adjout_flush_pending(struct rde_peer *peer) +{ + struct prefix *p, *np; + uint8_t aid; + + for (aid = AID_MIN; aid < AID_MAX; aid++) { + RB_FOREACH_SAFE(p, prefix_tree, &peer->withdraws[aid], np) { + prefix_adjout_destroy(p); + } + RB_FOREACH_SAFE(p, prefix_tree, &peer->updates[aid], np) { + p->flags &= ~PREFIX_FLAG_UPDATE; + RB_REMOVE(prefix_tree, &peer->updates[aid], p); + peer->stats.pending_update--; + } + } +} + int prefix_adjout_reaper(struct rde_peer *peer) { diff --git a/usr.sbin/bgpd/session.c b/usr.sbin/bgpd/session.c index 597ac25fd0b..5467d6eb717 100644 --- a/usr.sbin/bgpd/session.c +++ b/usr.sbin/bgpd/session.c @@ -1,4 +1,4 @@ -/* $OpenBSD: session.c,v 1.502 2024/12/10 14:34:51 claudio Exp $ */ +/* $OpenBSD: session.c,v 1.503 2024/12/12 20:19:03 claudio Exp $ */ /* * Copyright (c) 2003, 2004, 2005 Henning Brauer @@ -270,6 +270,9 @@ session_main(int debug, int verbose) NULL); timer_remove_all(&p->timers); tcp_md5_del_listener(conf, p); + if (imsg_rde(IMSG_SESSION_DELETE, + p->conf.id, NULL, 0) == -1) + fatalx("imsg_compose error"); msgbuf_free(p->wbuf); RB_REMOVE(peer_head, &conf->peers, p); log_peer_warnx(&p->conf, "removed"); @@ -405,6 +408,12 @@ session_main(int debug, int verbose) case Timer_SessionDown: timer_stop(&p->timers, Timer_SessionDown); + + if (imsg_rde(IMSG_SESSION_DELETE, + p->conf.id, NULL, 0) == -1) + fatalx("imsg_compose error"); + p->rdesession = 0; + /* finally delete this cloned peer */ if (p->template) p->reconf_action = @@ -3462,9 +3471,13 @@ session_up(struct peer *p) timer_stop(&p->timers, Timer_SessionDown); - if (imsg_rde(IMSG_SESSION_ADD, p->conf.id, - &p->conf, sizeof(p->conf)) == -1) - fatalx("imsg_compose error"); + if (!p->rdesession) { + /* inform rde about new peer */ + if (imsg_rde(IMSG_SESSION_ADD, p->conf.id, + &p->conf, sizeof(p->conf)) == -1) + fatalx("imsg_compose error"); + p->rdesession = 1; + } if (p->local.aid == AID_INET) { sup.local_v4_addr = p->local; @@ -3632,10 +3645,15 @@ merge_peers(struct bgpd_config *c, struct bgpd_config *nc) imsg_compose(ibuf_main, IMSG_PFKEY_RELOAD, p->conf.id, 0, -1, NULL, 0); - /* sync the RDE in case we keep the peer */ - if (imsg_rde(IMSG_SESSION_ADD, p->conf.id, - &p->conf, sizeof(struct peer_config)) == -1) - fatalx("imsg_compose error"); + /* + * If the session is established or the SessionDown timer is + * running sync with the RDE + */ + if (p->rdesession) { + if (imsg_rde(IMSG_SESSION_ADD, p->conf.id, + &p->conf, sizeof(struct peer_config)) == -1) + fatalx("imsg_compose error"); + } /* apply the config to all clones of a template */ if (p->conf.template) { @@ -3645,9 +3663,13 @@ merge_peers(struct bgpd_config *c, struct bgpd_config *nc) continue; session_template_clone(xp, NULL, xp->conf.id, xp->conf.remote_as); - if (imsg_rde(IMSG_SESSION_ADD, xp->conf.id, - &xp->conf, sizeof(xp->conf)) == -1) - fatalx("imsg_compose error"); + + if (p->rdesession) { + if (imsg_rde(IMSG_SESSION_ADD, + xp->conf.id, &xp->conf, + sizeof(xp->conf)) == -1) + fatalx("imsg_compose error"); + } } } } diff --git a/usr.sbin/bgpd/session.h b/usr.sbin/bgpd/session.h index 34482524ae6..e5be693dead 100644 --- a/usr.sbin/bgpd/session.h +++ b/usr.sbin/bgpd/session.h @@ -1,4 +1,4 @@ -/* $OpenBSD: session.h,v 1.181 2024/12/10 14:34:51 claudio Exp $ */ +/* $OpenBSD: session.h,v 1.182 2024/12/12 20:19:03 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -228,6 +228,7 @@ struct peer { uint8_t passive; uint8_t throttled; uint8_t rpending; + uint8_t rdesession; }; extern time_t pauseaccept;