From: Claudio Jeker Subject: bgpd: start rework the rde_filter code To: tech@openbsd.org Date: Tue, 10 Feb 2026 16:12:24 +0100 After implementing rde_filterset which improved rde_attr_set() a fair bit it is time to tackle filter_rules. Right now I'm just looking at the out filters (which are already per peer). So as a first step I decoupled these filters from the in filters by adding a new simplified rde_filter_out() function. Also rde_filter_match() is now just using the filter_match object and not the full rule. For that the peer.ebgp and peer.ibgp checks had to be moved up into rde_filter() which is a bit strange because of skip steps. Long term I will also redo the in filters but those will follow later and then skip steps will go away. So this is just a big shuffle that should have no visible effect. -- :wq Claudio Index: rde.h =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v diff -u -p -r1.340 rde.h --- rde.h 4 Feb 2026 11:41:11 -0000 1.340 +++ rde.h 9 Feb 2026 19:33:08 -0000 @@ -563,6 +563,9 @@ void rde_filter_calc_skip_steps(struct f enum filter_actions rde_filter(struct filter_head *, struct rde_peer *, struct rde_peer *, struct bgpd_addr *, uint8_t, struct filterstate *); +enum filter_actions rde_filter_out(struct filter_head *, struct rde_peer *, + struct rde_peer *, struct bgpd_addr *, uint8_t, + struct filterstate *); /* rde_prefix.c */ void pt_init(void); Index: rde_filter.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v diff -u -p -r1.142 rde_filter.c --- rde_filter.c 4 Feb 2026 13:49:23 -0000 1.142 +++ rde_filter.c 9 Feb 2026 21:46:28 -0000 @@ -243,71 +243,66 @@ rde_prefix_match(struct filter_prefix *f } static int -rde_filter_match(struct filter_rule *f, struct rde_peer *peer, +rde_filter_match(struct filter_match *match, struct rde_peer *peer, struct rde_peer *from, struct filterstate *state, struct bgpd_addr *prefix, uint8_t plen) { struct rde_aspath *asp = &state->aspath; int i; - if (f->peer.ebgp && !peer->conf.ebgp) - return (0); - if (f->peer.ibgp && peer->conf.ebgp) - return (0); - - if (f->match.ovs.is_set) { - if ((state->vstate & ROA_MASK) != f->match.ovs.validity) + if (match->ovs.is_set) { + if ((state->vstate & ROA_MASK) != match->ovs.validity) return (0); } - if (f->match.avs.is_set) { - if (((state->vstate >> 4) & ASPA_MASK) != f->match.avs.validity) + if (match->avs.is_set) { + if (((state->vstate >> 4) & ASPA_MASK) != match->avs.validity) return (0); } - if (asp != NULL && f->match.as.type != AS_UNDEF) { - if (aspath_match(asp->aspath, &f->match.as, + if (asp != NULL && match->as.type != AS_UNDEF) { + if (aspath_match(asp->aspath, &match->as, peer->conf.remote_as) == 0) return (0); } - if (asp != NULL && f->match.aslen.type != ASLEN_NONE) - if (aspath_lenmatch(asp->aspath, f->match.aslen.type, - f->match.aslen.aslen) == 0) + if (asp != NULL && match->aslen.type != ASLEN_NONE) + if (aspath_lenmatch(asp->aspath, match->aslen.type, + match->aslen.aslen) == 0) return (0); for (i = 0; i < MAX_COMM_MATCH; i++) { - if (f->match.community[i].flags == 0) + if (match->community[i].flags == 0) break; if (community_match(&state->communities, - &f->match.community[i], peer) == 0) + &match->community[i], peer) == 0) return (0); } - if (f->match.maxcomm != 0) { - if (f->match.maxcomm > + if (match->maxcomm != 0) { + if (match->maxcomm > community_count(&state->communities, COMMUNITY_TYPE_BASIC)) return (0); } - if (f->match.maxextcomm != 0) { - if (f->match.maxextcomm > + if (match->maxextcomm != 0) { + if (match->maxextcomm > community_count(&state->communities, COMMUNITY_TYPE_EXT)) return (0); } - if (f->match.maxlargecomm != 0) { - if (f->match.maxlargecomm > + if (match->maxlargecomm != 0) { + if (match->maxlargecomm > community_count(&state->communities, COMMUNITY_TYPE_LARGE)) return (0); } - if (f->match.nexthop.flags != 0) { + if (match->nexthop.flags != 0) { struct bgpd_addr *nexthop, *cmpaddr; if (state->nexthop == NULL) /* no nexthop, skip */ return (0); nexthop = &state->nexthop->exit_nexthop; - if (f->match.nexthop.flags == FILTER_NEXTHOP_ADDR) - cmpaddr = &f->match.nexthop.addr; + if (match->nexthop.flags == FILTER_NEXTHOP_ADDR) + cmpaddr = &match->nexthop.addr; else cmpaddr = &from->remote_addr; if (cmpaddr->aid != nexthop->aid) @@ -330,8 +325,8 @@ rde_filter_match(struct filter_rule *f, } /* origin-set lookups match only on ROA_VALID */ - if (asp != NULL && f->match.originset.ps != NULL) { - if (trie_roa_check(&f->match.originset.ps->th, prefix, plen, + if (asp != NULL && match->originset.ps != NULL) { + if (trie_roa_check(&match->originset.ps->th, prefix, plen, aspath_origin(asp->aspath)) != ROA_VALID) return (0); } @@ -339,13 +334,13 @@ rde_filter_match(struct filter_rule *f, /* * prefixset and prefix filter rules are mutual exclusive */ - if (f->match.prefixset.flags != 0) { - if (f->match.prefixset.ps == NULL || - !trie_match(&f->match.prefixset.ps->th, prefix, plen, - (f->match.prefixset.flags & PREFIXSET_FLAG_LONGER))) + if (match->prefixset.flags != 0) { + if (match->prefixset.ps == NULL || + !trie_match(&match->prefixset.ps->th, prefix, plen, + (match->prefixset.flags & PREFIXSET_FLAG_LONGER))) return (0); - } else if (f->match.prefix.addr.aid != 0) - return (rde_prefix_match(&f->match.prefix, prefix, plen)); + } else if (match->prefix.addr.aid != 0) + return (rde_prefix_match(&match->prefix, prefix, plen)); /* matched somewhen or is anymatch rule */ return (1); @@ -359,14 +354,14 @@ rde_filter_skip_rule(struct rde_peer *pe if (peer == NULL || f == NULL) return (0); - if (f->peer.groupid != 0 && - f->peer.groupid != peer->conf.groupid) - return (1); - if (f->peer.peerid != 0 && f->peer.peerid != peer->conf.id) return (1); + if (f->peer.groupid != 0 && + f->peer.groupid != peer->conf.groupid) + return (1); + if (f->peer.remote_as != 0 && f->peer.remote_as != peer->conf.remote_as) return (1); @@ -1005,17 +1000,26 @@ rde_filter(struct filter_head *rules, st RDE_FILTER_TEST_ATTRIB( (f->peer.peerid && f->peer.peerid != peer->conf.id), - f->skip[RDE_FILTER_SKIP_PEERID]); + f->skip[RDE_FILTER_SKIP_PEERID]); RDE_FILTER_TEST_ATTRIB( (f->peer.groupid && f->peer.groupid != peer->conf.groupid), - f->skip[RDE_FILTER_SKIP_GROUPID]); + f->skip[RDE_FILTER_SKIP_GROUPID]); RDE_FILTER_TEST_ATTRIB( (f->peer.remote_as && f->peer.remote_as != peer->conf.remote_as), - f->skip[RDE_FILTER_SKIP_REMOTE_AS]); + f->skip[RDE_FILTER_SKIP_REMOTE_AS]); + + RDE_FILTER_TEST_ATTRIB( + (f->peer.ebgp && !peer->conf.ebgp), + TAILQ_NEXT(f, entry)); + + RDE_FILTER_TEST_ATTRIB( + (f->peer.ibgp && peer->conf.ebgp), + TAILQ_NEXT(f, entry)); - if (rde_filter_match(f, peer, from, state, prefix, plen)) { + if (rde_filter_match(&f->match, peer, from, state, + prefix, plen)) { rde_apply_set(f->rde_set, peer, from, state, prefix->aid); if (f->action != ACTION_NONE) @@ -1025,6 +1029,43 @@ rde_filter(struct filter_head *rules, st } f = TAILQ_NEXT(f, entry); nextrule: ; + } + return (action); +} + +enum filter_actions +rde_filter_out(struct filter_head *rules, struct rde_peer *peer, + struct rde_peer *from, struct bgpd_addr *prefix, uint8_t plen, + struct filterstate *state) +{ + struct filter_rule *f; + enum filter_actions action = ACTION_DENY; /* default deny */ + + if (state->aspath.flags & F_ATTR_PARSE_ERR) + /* + * don't try to filter bad updates just deny them + * so they act as implicit withdraws + */ + return (ACTION_DENY); + + if (rules == NULL) + return (action); + + if (prefix->aid == AID_FLOWSPECv4 || prefix->aid == AID_FLOWSPECv6) + return (ACTION_ALLOW); + + f = TAILQ_FIRST(rules); + while (f != NULL) { + if (rde_filter_match(&f->match, peer, from, state, + prefix, plen)) { + rde_apply_set(f->rde_set, peer, from, state, + prefix->aid); + if (f->action != ACTION_NONE) + action = f->action; + if (f->quick) + return (action); + } + f = TAILQ_NEXT(f, entry); } return (action); } Index: rde_update.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v diff -u -p -r1.192 rde_update.c --- rde_update.c 24 Dec 2025 07:59:55 -0000 1.192 +++ rde_update.c 31 Dec 2025 23:22:47 -0000 @@ -177,7 +177,7 @@ up_process_prefix(struct rde_peer *peer, rde_filterstate_prep(&state, new); pt_getaddr(new->pt, &addr); - if (rde_filter(peer->out_rules, peer, prefix_peer(new), &addr, + if (rde_filter_out(peer->out_rules, peer, prefix_peer(new), &addr, new->pt->prefixlen, &state) == ACTION_DENY) { rde_filterstate_clean(&state); return UP_FILTERED; @@ -435,8 +435,8 @@ up_generate_default(struct rde_peer *pee addr.aid = aid; /* outbound filter as usual */ - if (rde_filter(peer->out_rules, peer, peerself, &addr, 0, &state) == - ACTION_DENY) { + if (rde_filter_out(peer->out_rules, peer, peerself, &addr, 0, + &state) == ACTION_DENY) { rde_filterstate_clean(&state); return; }