When bgpd generates an UPDATE to update or withdraw prefixes it does this
from rde_generate_updates() and then decends into up_generate_update().
Now there is up_test_update() that checks if a new prefix is actually OK
to be distributed. It checks things for route reflectors and the common
communities (NO_EXPORT, ...). There are a few more checks that are pure
peer config checks and those should be moved up to rde_generate_updates().
Last but not least there is this bit about ORIGINATOR_ID which seems
sensible but on second thought I think it is actually wrong and an
extension on top of the RFC. Since I think this code currently has not the
right withdraw behaviour I decided it is the best to just remove it.
This code simplifies the return of up_test_update() to a pure true / false
case and make up_generate_update() simpler. Also I think doing the peer
checks early on will improve performance.
Please review :)
--
:wq Claudio
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.510
diff -u -p -r1.510 rde.c
--- rde.c 30 Dec 2020 07:29:56 -0000 1.510
+++ rde.c 7 Jan 2021 17:04:53 -0000
@@ -2814,7 +2814,8 @@ rde_send_kroute(struct rib *rib, struct
void
rde_generate_updates(struct rib *rib, struct prefix *new, struct prefix *old)
{
- struct rde_peer *peer;
+ struct rde_peer *peer;
+ u_int8_t aid;
/*
* If old is != NULL we know it was active and should be removed.
@@ -2824,6 +2825,11 @@ rde_generate_updates(struct rib *rib, st
if (old == NULL && new == NULL)
return;
+ if (new)
+ aid = new->pt->aid;
+ else
+ aid = old->pt->aid;
+
LIST_FOREACH(peer, &peerlist, peer_l) {
if (peer->conf.id == 0)
continue;
@@ -2831,6 +2837,14 @@ rde_generate_updates(struct rib *rib, st
continue;
if (peer->state != PEER_UP)
continue;
+ /* check if peer actually supports the address family */
+ if (peer->capa.mp[aid] == 0)
+ continue;
+ /* skip peers with special export types */
+ if (peer->conf.export_type == EXPORT_NONE ||
+ peer->conf.export_type == EXPORT_DEFAULT_ROUTE)
+ continue;
+
up_generate_updates(out_rules, peer, new, old);
}
}
Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
retrieving revision 1.123
diff -u -p -r1.123 rde_update.c
--- rde_update.c 24 Jan 2020 05:44:05 -0000 1.123
+++ rde_update.c 7 Jan 2021 18:13:45 -0000
@@ -47,11 +47,9 @@ static struct community comm_no_expsubco
static int
up_test_update(struct rde_peer *peer, struct prefix *p)
{
- struct bgpd_addr addr;
struct rde_aspath *asp;
struct rde_community *comm;
struct rde_peer *prefp;
- struct attr *attr;
if (p == NULL)
/* no prefix available */
@@ -70,10 +68,6 @@ up_test_update(struct rde_peer *peer, st
if (asp->flags & F_ATTR_LOOP)
fatalx("try to send out a looped path");
- pt_getaddr(p->pt, &addr);
- if (peer->capa.mp[addr.aid] == 0)
- return (-1);
-
if (!prefp->conf.ebgp && !peer->conf.ebgp) {
/*
* route reflector redistribution rules:
@@ -90,16 +84,6 @@ up_test_update(struct rde_peer *peer, st
return (0);
}
- /* export type handling */
- if (peer->conf.export_type == EXPORT_NONE ||
- peer->conf.export_type == EXPORT_DEFAULT_ROUTE) {
- /*
- * no need to withdraw old prefix as this will be
- * filtered out as well.
- */
- return (-1);
- }
-
/* well known communities */
if (community_match(comm, &comm_no_advertise, NULL))
return (0);
@@ -110,18 +94,6 @@ up_test_update(struct rde_peer *peer, st
return (0);
}
- /*
- * Don't send messages back to originator
- * this is not specified in the RFC but seems logical.
- */
- if ((attr = attr_optget(asp, ATTR_ORIGINATOR_ID)) != NULL) {
- if (memcmp(attr->data, &peer->remote_bgpid,
- sizeof(peer->remote_bgpid)) == 0) {
- /* would cause loop don't send */
- return (-1);
- }
- }
-
return (1);
}
@@ -149,13 +121,8 @@ withdraw:
peer->up_wcnt++;
}
} else {
- switch (up_test_update(peer, new)) {
- case 1:
- break;
- case 0:
+ if (!up_test_update(peer, new)) {
goto withdraw;
- case -1:
- return;
}
rde_filterstate_prep(&state, prefix_aspath(new),
