Pass path_id_tx to the Adj-RIB-Out
authorclaudio <claudio@openbsd.org>
Fri, 8 Jul 2022 10:01:52 +0000 (10:01 +0000)
committerclaudio <claudio@openbsd.org>
Fri, 8 Jul 2022 10:01:52 +0000 (10:01 +0000)
Adjust prefix_adjout_update() to properly handle path_id_tx.
Move the lookup of the prefix out of prefix_adjout_update() and to
up_generate_updates(). While that code uses prefix_adjout_lookup() to
find the current prefix in the Adj-RIB-Out and add-path aware function
will use prefix_adjout_get().

In up_generate_default() just use 0 for path_id_tx since for this peer
that is the only prefix installed into the Adj-RIB-Out.

OK tb@

usr.sbin/bgpd/rde.h
usr.sbin/bgpd/rde_rib.c
usr.sbin/bgpd/rde_update.c

index 56c567f..0a48587 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: rde.h,v 1.257 2022/07/08 08:11:25 claudio Exp $ */
+/*     $OpenBSD: rde.h,v 1.258 2022/07/08 10:01:52 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -616,8 +616,9 @@ int          prefix_update(struct rib *, struct rde_peer *, uint32_t,
 int             prefix_withdraw(struct rib *, struct rde_peer *, uint32_t,
                    struct bgpd_addr *, int);
 void            prefix_add_eor(struct rde_peer *, uint8_t);
-void            prefix_adjout_update(struct rde_peer *, struct filterstate *,
-                   struct bgpd_addr *, int, uint8_t);
+void            prefix_adjout_update(struct prefix *, struct rde_peer *,
+                   struct filterstate *, struct bgpd_addr *, int,
+                   uint32_t, uint8_t);
 void            prefix_adjout_withdraw(struct prefix *);
 void            prefix_adjout_destroy(struct prefix *);
 void            prefix_adjout_dump(struct rde_peer *, void *,
index 69db4e8..74da950 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: rde_rib.c,v 1.239 2022/07/08 08:11:25 claudio Exp $ */
+/*     $OpenBSD: rde_rib.c,v 1.240 2022/07/08 10:01:52 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org>
@@ -917,14 +917,14 @@ prefix_get(struct rib *rib, struct rde_peer *peer, uint32_t path_id,
  * Returns NULL if not found.
  */
 struct prefix *
-prefix_adjout_get(struct rde_peer *peer, uint32_t path_id,
+prefix_adjout_get(struct rde_peer *peer, uint32_t path_id_tx,
     struct bgpd_addr *prefix, int prefixlen)
 {
        struct prefix xp;
 
        memset(&xp, 0, sizeof(xp));
        xp.pt = pt_fill(prefix, prefixlen);
-       xp.path_id_tx = path_id;
+       xp.path_id_tx = path_id_tx;
 
        return RB_FIND(prefix_index, &peer->adj_rib_out, &xp);
 }
@@ -1168,14 +1168,14 @@ prefix_add_eor(struct rde_peer *peer, uint8_t aid)
  * Put a prefix from the Adj-RIB-Out onto the update queue.
  */
 void
-prefix_adjout_update(struct rde_peer *peer, struct filterstate *state,
-    struct bgpd_addr *prefix, int prefixlen, uint8_t vstate)
+prefix_adjout_update(struct prefix *p, struct rde_peer *peer,
+    struct filterstate *state, struct bgpd_addr *prefix, int prefixlen,
+    uint32_t path_id_tx, uint8_t vstate)
 {
        struct rde_aspath *asp;
        struct rde_community *comm;
-       struct prefix *p;
 
-       if ((p = prefix_adjout_get(peer, 0, prefix, prefixlen)) == NULL) {
+       if (p == NULL) {
                p = prefix_alloc();
                /* initally mark DEAD so code below is skipped */
                p->flags |= PREFIX_FLAG_ADJOUT | PREFIX_FLAG_DEAD;
@@ -1185,7 +1185,7 @@ prefix_adjout_update(struct rde_peer *peer, struct filterstate *state,
                        p->pt = pt_add(prefix, prefixlen);
                pt_ref(p->pt);
                p->peer = peer;
-               p->path_id_tx = 0 /* XXX force this for now */;
+               p->path_id_tx = path_id_tx;
 
                if (RB_INSERT(prefix_index, &peer->adj_rib_out, p) != NULL)
                        fatalx("%s: RB index invariant violated", __func__);
@@ -1194,7 +1194,14 @@ prefix_adjout_update(struct rde_peer *peer, struct filterstate *state,
        if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
                fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__);
        if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) {
-               if (prefix_nhflags(p) == state->nhflags &&
+               /*
+                * XXX for now treat a different path_id_tx like different
+                * attributes and force out an update. It is unclear how
+                * common it is to have equivalent updates from alternative
+                * paths.
+                */
+               if (p->path_id_tx == path_id_tx &&
+                   prefix_nhflags(p) == state->nhflags &&
                    prefix_nexthop(p) == state->nexthop &&
                    communities_equal(&state->communities,
                    prefix_communities(p)) &&
@@ -1224,6 +1231,15 @@ prefix_adjout_update(struct rde_peer *peer, struct filterstate *state,
        /* nothing needs to be done for PREFIX_FLAG_DEAD and STALE */
        p->flags &= ~PREFIX_FLAG_MASK;
 
+       /* update path_id_tx now that the prefix is unlinked */
+       if (p->path_id_tx != path_id_tx) {
+               /* path_id_tx is part of the index so remove and re-insert p */
+               RB_REMOVE(prefix_index, &peer->adj_rib_out, p);
+               p->path_id_tx = path_id_tx;
+               if (RB_INSERT(prefix_index, &peer->adj_rib_out, p) != NULL)
+                       fatalx("%s: RB index invariant violated", __func__);
+       }
+
        if ((asp = path_lookup(&state->aspath)) == NULL) {
                /* Path not available, create and link a new one. */
                asp = path_copy(path_get(), &state->aspath);
@@ -1235,7 +1251,7 @@ prefix_adjout_update(struct rde_peer *peer, struct filterstate *state,
                comm = communities_link(&state->communities);
        }
 
-       prefix_link(p, NULL, p->pt, peer, 0, /* XXX */ 0, asp, comm,
+       prefix_link(p, NULL, p->pt, peer, 0, p->path_id_tx, asp, comm,
            state->nexthop, state->nhflags, vstate);
        peer->prefix_out_cnt++;
 
index a14e778..335884a 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: rde_update.c,v 1.141 2022/06/27 13:26:51 claudio Exp $ */
+/*     $OpenBSD: rde_update.c,v 1.142 2022/07/08 10:01:52 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -117,6 +117,8 @@ up_generate_updates(struct filter_head *rules, struct rde_peer *peer,
                prefixlen = new->pt->prefixlen;
        }
 
+       p = prefix_adjout_lookup(peer, &addr, prefixlen);
+
        while (new != NULL) {
                need_withdraw = 0;
                /*
@@ -200,8 +202,8 @@ up_generate_updates(struct filter_head *rules, struct rde_peer *peer,
                /* from here on we know this is an update */
 
                up_prep_adjout(peer, &state, addr.aid);
-               prefix_adjout_update(peer, &state, &addr,
-                   new->pt->prefixlen, prefix_vstate(new));
+               prefix_adjout_update(p, peer, &state, &addr,
+                   new->pt->prefixlen, new->path_id_tx, prefix_vstate(new));
                rde_filterstate_clean(&state);
 
                /* max prefix checker outbound */
@@ -218,7 +220,7 @@ up_generate_updates(struct filter_head *rules, struct rde_peer *peer,
        }
 
        /* withdraw prefix */
-       if ((p = prefix_adjout_get(peer, 0, &addr, prefixlen)) != NULL)
+       if (p != NULL)
                prefix_adjout_withdraw(p);
 }
 
@@ -234,6 +236,7 @@ up_generate_default(struct filter_head *rules, struct rde_peer *peer,
        extern struct rde_peer  *peerself;
        struct filterstate       state;
        struct rde_aspath       *asp;
+       struct prefix           *p;
        struct bgpd_addr         addr;
 
        if (peer->capa.mp[aid] == 0)
@@ -253,6 +256,8 @@ up_generate_default(struct filter_head *rules, struct rde_peer *peer,
 
        bzero(&addr, sizeof(addr));
        addr.aid = aid;
+       p = prefix_adjout_lookup(peer, &addr, 0);
+
        /* outbound filter as usual */
        if (rde_filter(rules, peer, peerself, &addr, 0, ROA_NOTFOUND,
            &state) == ACTION_DENY) {
@@ -261,7 +266,7 @@ up_generate_default(struct filter_head *rules, struct rde_peer *peer,
        }
 
        up_prep_adjout(peer, &state, addr.aid);
-       prefix_adjout_update(peer, &state, &addr, 0, ROA_NOTFOUND);
+       prefix_adjout_update(p, peer, &state, &addr, 0, 0, ROA_NOTFOUND);
        rde_filterstate_clean(&state);
 
        /* max prefix checker outbound */