From 56d80c2c2a9bd3b28305b98c6f01bf8971e5098b Mon Sep 17 00:00:00 2001 From: claudio Date: Thu, 27 May 2021 08:45:24 +0000 Subject: [PATCH] When generating updates for a peer that has 'rde evaluate all' set the old prefix pointer is most probably NULL. If a secondary route is removed the withdraw would not happen because old == NULL which skips the withdraw. Access to old is only needed to extract the prefix. So instead extract the prefix early and use it for both cases. So if 'rde evaluate all' is used the code tries all prefixes and if none is allowed a withdraw is issued. Problem noticed and fix tested by Pier Carlo Chiodi --- usr.sbin/bgpd/rde_update.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/usr.sbin/bgpd/rde_update.c b/usr.sbin/bgpd/rde_update.c index 8910ca0db63..b20da366937 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.127 2021/05/06 09:18:54 claudio Exp $ */ +/* $OpenBSD: rde_update.c,v 1.128 2021/05/27 08:45:24 claudio Exp $ */ /* * Copyright (c) 2004 Claudio Jeker @@ -101,17 +101,23 @@ up_generate_updates(struct filter_head *rules, struct rde_peer *peer, struct filterstate state; struct bgpd_addr addr; int need_withdraw; + uint8_t prefixlen; -again: if (new == NULL) { if (old == NULL) - /* no prefix to withdraw */ + /* no prefix to update or withdraw */ return; + pt_getaddr(old->pt, &addr); + prefixlen = old->pt->prefixlen; + } else { + pt_getaddr(new->pt, &addr); + prefixlen = new->pt->prefixlen; + } +again: + if (new == NULL) { /* withdraw prefix */ - pt_getaddr(old->pt, &addr); - if (prefix_adjout_withdraw(peer, &addr, - old->pt->prefixlen) == 1) { + if (prefix_adjout_withdraw(peer, &addr, prefixlen) == 1) { peer->prefix_out_cnt--; peer->up_wcnt++; } @@ -143,10 +149,8 @@ again: rde_filterstate_prep(&state, prefix_aspath(new), prefix_communities(new), prefix_nexthop(new), prefix_nhflags(new)); - pt_getaddr(new->pt, &addr); if (rde_filter(rules, peer, prefix_peer(new), &addr, - new->pt->prefixlen, prefix_vstate(new), &state) == - ACTION_DENY) { + prefixlen, prefix_vstate(new), &state) == ACTION_DENY) { rde_filterstate_clean(&state); if (peer->flags & PEERFLAG_EVALUATE_ALL) new = LIST_NEXT(new, entry.list.rib); -- 2.20.1