From 957262a22d3885a56acef60552cfebc760c0d09c Mon Sep 17 00:00:00 2001 From: claudio Date: Wed, 2 Mar 2022 14:44:46 +0000 Subject: [PATCH] Refactor prefix_adjout_withdraw() Just pass the prefix to be withdrawn to the function and move the lookup up. Adjust how the various accounting vars are updated so that the values are decremented in the right cases. Do the same accounting dance for prefix_adjout_destroy(). Adjust rde_up_flush_upcall() to directly call prefix_adjout_withdraw() without calling it via up_generate_updates(). OK tb@ --- usr.sbin/bgpd/rde.c | 8 +++---- usr.sbin/bgpd/rde.h | 5 ++-- usr.sbin/bgpd/rde_rib.c | 47 +++++++++++++++++++++++--------------- usr.sbin/bgpd/rde_update.c | 10 ++++---- 4 files changed, 37 insertions(+), 33 deletions(-) diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c index e08fec4394e..41d8372b732 100644 --- a/usr.sbin/bgpd/rde.c +++ b/usr.sbin/bgpd/rde.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.c,v 1.538 2022/02/28 12:52:38 claudio Exp $ */ +/* $OpenBSD: rde.c,v 1.539 2022/03/02 14:44:46 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -3050,9 +3050,7 @@ rde_generate_updates(struct rib *rib, struct prefix *new, struct prefix *old, static void rde_up_flush_upcall(struct prefix *p, void *ptr) { - struct rde_peer *peer = ptr; - - up_generate_updates(out_rules, peer, NULL, p); + prefix_adjout_withdraw(p); } u_char queue_buf[4096]; @@ -3444,7 +3442,7 @@ rde_reload_done(void) if (peer->reconf_rib) { if (prefix_dump_new(peer, AID_UNSPEC, - RDE_RUNNER_ROUNDS, peer, rde_up_flush_upcall, + RDE_RUNNER_ROUNDS, NULL, rde_up_flush_upcall, rde_softreconfig_in_done, NULL) == -1) fatal("%s: prefix_dump_new", __func__); log_peer_info(&peer->conf, "flushing Adj-RIB-Out"); diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h index ef7b733f9de..a7a8907bf58 100644 --- a/usr.sbin/bgpd/rde.h +++ b/usr.sbin/bgpd/rde.h @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.h,v 1.244 2022/02/25 11:36:54 claudio Exp $ */ +/* $OpenBSD: rde.h,v 1.245 2022/03/02 14:44:46 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Claudio Jeker and @@ -596,8 +596,7 @@ int prefix_withdraw(struct rib *, struct rde_peer *, uint32_t, void prefix_add_eor(struct rde_peer *, uint8_t); int prefix_adjout_update(struct rde_peer *, struct filterstate *, struct bgpd_addr *, int, uint8_t); -int prefix_adjout_withdraw(struct rde_peer *, struct bgpd_addr *, - int); +void prefix_adjout_withdraw(struct prefix *); void prefix_adjout_destroy(struct prefix *p); void prefix_adjout_dump(struct rde_peer *, void *, void (*)(struct prefix *, void *)); diff --git a/usr.sbin/bgpd/rde_rib.c b/usr.sbin/bgpd/rde_rib.c index b987129dfab..79579890259 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.230 2022/03/01 09:39:36 claudio Exp $ */ +/* $OpenBSD: rde_rib.c,v 1.231 2022/03/02 14:44:46 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Claudio Jeker @@ -1251,37 +1251,40 @@ prefix_adjout_update(struct rde_peer *peer, struct filterstate *state, * Withdraw a prefix from the Adj-RIB-Out, this unlinks the aspath but leaves * the prefix in the RIB linked to the peer withdraw list. */ -int -prefix_adjout_withdraw(struct rde_peer *peer, struct bgpd_addr *prefix, - int prefixlen) +void +prefix_adjout_withdraw(struct prefix *p) { - struct prefix *p; - - p = prefix_adjout_get(peer, 0, prefix, prefixlen); - if (p == NULL) /* Got a dummy withdrawn request. */ - return (0); + struct rde_peer *peer = prefix_peer(p); if ((p->flags & PREFIX_FLAG_ADJOUT) == 0) fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__); - /* already a withdraw, error */ - if (p->flags & PREFIX_FLAG_WITHDRAW) - log_warnx("%s: prefix already withdrawed", __func__); + /* already a withdraw, shortcut */ + if (p->flags & PREFIX_FLAG_WITHDRAW) { + p->lastchange = getmonotime(); + p->flags &= ~PREFIX_FLAG_STALE; + return; + } /* pending update just got withdrawn */ - if (p->flags & PREFIX_FLAG_UPDATE) + if (p->flags & PREFIX_FLAG_UPDATE) { RB_REMOVE(prefix_tree, &peer->updates[p->pt->aid], p); + peer->up_nlricnt--; + } /* unlink prefix if it was linked (not a withdraw or dead) */ - if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) + if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) { prefix_unlink(p); + peer->prefix_out_cnt--; + } /* nothing needs to be done for PREFIX_FLAG_DEAD and STALE */ p->flags &= ~PREFIX_FLAG_MASK; p->lastchange = getmonotime(); p->flags |= PREFIX_FLAG_WITHDRAW; - if (RB_INSERT(prefix_tree, &peer->withdraws[prefix->aid], p) != NULL) + if (RB_INSERT(prefix_tree, &peer->withdraws[p->pt->aid], p) != NULL) fatalx("%s: RB tree invariant violated", __func__); - return (1); + + peer->up_wcnt++; } void @@ -1298,13 +1301,19 @@ prefix_adjout_destroy(struct prefix *p) return; } - if (p->flags & PREFIX_FLAG_WITHDRAW) + if (p->flags & PREFIX_FLAG_WITHDRAW) { RB_REMOVE(prefix_tree, &peer->withdraws[p->pt->aid], p); - if (p->flags & PREFIX_FLAG_UPDATE) + peer->up_wcnt--; + } + if (p->flags & PREFIX_FLAG_UPDATE) { RB_REMOVE(prefix_tree, &peer->updates[p->pt->aid], p); + peer->up_nlricnt--; + } /* unlink prefix if it was linked (not a withdraw or dead) */ - if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) + if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) { prefix_unlink(p); + peer->prefix_out_cnt--; + } /* nothing needs to be done for PREFIX_FLAG_DEAD and STALE */ p->flags &= ~PREFIX_FLAG_MASK; diff --git a/usr.sbin/bgpd/rde_update.c b/usr.sbin/bgpd/rde_update.c index d8460f8f6ef..6273a819cc6 100644 --- a/usr.sbin/bgpd/rde_update.c +++ b/usr.sbin/bgpd/rde_update.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde_update.c,v 1.134 2022/03/01 09:53:42 claudio Exp $ */ +/* $OpenBSD: rde_update.c,v 1.135 2022/03/02 14:44:46 claudio Exp $ */ /* * Copyright (c) 2004 Claudio Jeker @@ -102,6 +102,7 @@ up_generate_updates(struct filter_head *rules, struct rde_peer *peer, { struct filterstate state; struct bgpd_addr addr; + struct prefix *p; int need_withdraw; uint8_t prefixlen; @@ -119,10 +120,8 @@ up_generate_updates(struct filter_head *rules, struct rde_peer *peer, again: if (new == NULL) { /* withdraw prefix */ - if (prefix_adjout_withdraw(peer, &addr, prefixlen) == 1) { - peer->prefix_out_cnt--; - peer->up_wcnt++; - } + if ((p = prefix_adjout_get(peer, 0, &addr, prefixlen)) != NULL) + prefix_adjout_withdraw(p); } else { need_withdraw = 0; /* @@ -655,7 +654,6 @@ up_dump_prefix(u_char *buf, int len, struct prefix_tree *prefix_head, if (withdraw) { /* prefix no longer needed, remove it */ prefix_adjout_destroy(p); - peer->up_wcnt--; peer->prefix_sent_withdraw++; } else { /* prefix still in Adj-RIB-Out, keep it */ -- 2.20.1