Download raw body.
bgpd: cleanup AID / AFI handling
Looping over all valid AID should use:
for (i = AID_MIN; i < AID_MAX; i++)
Now this diff tries to enforce this everywhere and in the few cases where
this is not the case I use:
for (i = AID_UNSPEC; i < AID_MAX; i++)
For aid2afi() make sure that AID_UNSPEC returns an error and not some bad
value.
Additionally check in some cases not only for AID_MAX but also for
AID_MIN. E.g. when sending router-refresh messages.
Now the add-path capability uses AID_UNSPEC to indicate if it is in use
or not. It also used this for our announce settings but that is not needed
so only do this trick in the negotiated values.
--
:wq Claudio
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
diff -u -p -r1.456 parse.y
--- parse.y 18 Mar 2024 14:54:52 -0000 1.456
+++ parse.y 19 Mar 2024 15:03:03 -0000
@@ -1894,7 +1894,7 @@ peeropts : REMOTEAS as4number {
uint16_t afi;
if ($3 == SAFI_NONE) {
- for (aid = 0; aid < AID_MAX; aid++) {
+ for (aid = AID_MIN; aid < AID_MAX; aid++) {
if (aid2afi(aid, &afi, &safi) == -1 ||
afi != $2)
continue;
@@ -1927,11 +1927,11 @@ peeropts : REMOTEAS as4number {
int8_t *ap = curpeer->conf.capabilities.add_path;
uint8_t i;
- for (i = 0; i < AID_MAX; i++)
+ for (i = AID_MIN; i < AID_MAX; i++)
if ($4)
- *ap++ |= CAPA_AP_RECV;
+ ap[i] |= CAPA_AP_RECV;
else
- *ap++ &= ~CAPA_AP_RECV;
+ ap[i] &= ~CAPA_AP_RECV;
}
| ANNOUNCE ADDPATH SEND STRING addpathextra addpathmax {
int8_t *ap = curpeer->conf.capabilities.add_path;
@@ -1945,9 +1945,7 @@ peeropts : REMOTEAS as4number {
"for 'add-path send no'");
YYERROR;
}
- for (i = 0; i < AID_MAX; i++)
- *ap++ &= ~CAPA_AP_SEND;
- break;
+ mode = ADDPATH_EVAL_NONE;
} else if (!strcmp($4, "all")) {
free($4);
if ($5 != 0 || $6 != 0) {
@@ -1971,8 +1969,12 @@ peeropts : REMOTEAS as4number {
free($4);
YYERROR;
}
- for (i = 0; i < AID_MAX; i++)
- *ap++ |= CAPA_AP_SEND;
+ for (i = AID_MIN; i < AID_MAX; i++) {
+ if (mode != ADDPATH_EVAL_NONE)
+ ap[i] |= CAPA_AP_SEND;
+ else
+ ap[i] &= ~CAPA_AP_SEND;
+ }
curpeer->conf.eval.mode = mode;
curpeer->conf.eval.extrapaths = $5;
curpeer->conf.eval.maxpaths = $6;
@@ -4611,7 +4613,6 @@ struct peer *
alloc_peer(void)
{
struct peer *p;
- uint8_t i;
if ((p = calloc(1, sizeof(struct peer))) == NULL)
fatal("new_peer");
@@ -4622,8 +4623,6 @@ alloc_peer(void)
p->conf.distance = 1;
p->conf.export_type = EXPORT_UNSET;
p->conf.announce_capa = 1;
- for (i = 0; i < AID_MAX; i++)
- p->conf.capabilities.mp[i] = 0;
p->conf.capabilities.refresh = 1;
p->conf.capabilities.grestart.restart = 1;
p->conf.capabilities.as4byte = 1;
Index: printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
diff -u -p -r1.169 printconf.c
--- printconf.c 10 Jan 2024 13:31:09 -0000 1.169
+++ printconf.c 19 Mar 2024 14:51:59 -0000
@@ -920,7 +920,7 @@ print_announce(struct peer_config *p, co
if (p->announce_capa == 0)
printf("%s\tannounce capabilities no\n", c);
- for (aid = 0; aid < AID_MAX; aid++)
+ for (aid = AID_MIN; aid < AID_MAX; aid++)
if (p->capabilities.mp[aid])
printf("%s\tannounce %s\n", c, aid2str(aid));
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.623 rde.c
--- rde.c 22 Feb 2024 06:45:22 -0000 1.623
+++ rde.c 19 Mar 2024 15:09:40 -0000
@@ -442,7 +442,7 @@ rde_dispatch_imsg_session(struct imsgbuf
log_warnx("%s: wrong imsg len", __func__);
break;
}
- if (aid >= AID_MAX) {
+ if (aid < AID_MIN || aid >= AID_MAX) {
log_warnx("%s: bad AID", __func__);
break;
}
@@ -1328,7 +1328,7 @@ rde_dispatch_imsg_peer(struct rde_peer *
log_warnx("route refresh: wrong imsg len");
break;
}
- if (rr.aid >= AID_MAX) {
+ if (rr.aid < AID_MIN || rr.aid >= AID_MAX) {
log_peer_warnx(&peer->conf,
"route refresh: bad AID %d", rr.aid);
break;
@@ -3326,7 +3326,7 @@ rde_update_queue_pending(void)
continue;
if (peer->throttled)
continue;
- for (aid = 0; aid < AID_MAX; aid++) {
+ for (aid = AID_MIN; aid < AID_MAX; aid++) {
if (!RB_EMPTY(&peer->updates[aid]) ||
!RB_EMPTY(&peer->withdraws[aid]))
return 1;
@@ -3821,7 +3821,7 @@ rde_softreconfig_in_done(void *arg, uint
peer->reconf_out = 0;
} else if (peer->export_type == EXPORT_DEFAULT_ROUTE) {
/* just resend the default route */
- for (aid = 0; aid < AID_MAX; aid++) {
+ for (aid = AID_MIN; aid < AID_MAX; aid++) {
if (peer->capa.mp[aid])
up_generate_default(peer, aid);
}
@@ -3831,7 +3831,7 @@ rde_softreconfig_in_done(void *arg, uint
RECONF_RELOAD;
} else if (peer->reconf_rib) {
/* dump the full table to neighbors that changed rib */
- for (aid = 0; aid < AID_MAX; aid++) {
+ for (aid = AID_MIN; aid < AID_MAX; aid++) {
if (peer->capa.mp[aid])
peer_dump(peer, aid);
}
Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
diff -u -p -r1.35 rde_peer.c
--- rde_peer.c 3 Feb 2024 09:26:52 -0000 1.35
+++ rde_peer.c 19 Mar 2024 15:18:35 -0000
@@ -44,18 +44,15 @@ peer_has_as4byte(struct rde_peer *peer)
return (peer->capa.as4byte);
}
+/*
+ * Check if ADD_PATH is enabled for aid and mode (rx / tx). If aid is
+ * AID_UNSPEC then the function returns true if any aid has mode enabled.
+ */
int
peer_has_add_path(struct rde_peer *peer, uint8_t aid, int mode)
{
if (aid >= AID_MAX)
return 0;
- if (aid == AID_UNSPEC) {
- /* check if at capability is set for at least one AID */
- for (aid = AID_MIN; aid < AID_MAX; aid++)
- if (peer->capa.add_path[aid] & mode)
- return 1;
- return 0;
- }
return (peer->capa.add_path[aid] & mode);
}
@@ -444,7 +441,7 @@ peer_up(struct rde_peer *peer, struct se
}
peer->state = PEER_UP;
- for (i = 0; i < AID_MAX; i++) {
+ for (i = AID_MIN; i < AID_MAX; i++) {
if (peer->capa.mp[i])
peer_dump(peer, i);
}
@@ -500,7 +497,7 @@ peer_flush(struct rde_peer *peer, uint8_
/* every route is gone so reset staletime */
if (aid == AID_UNSPEC) {
uint8_t i;
- for (i = 0; i < AID_MAX; i++)
+ for (i = AID_MIN; i < AID_MAX; i++)
peer->staletime[i] = 0;
} else {
peer->staletime[aid] = 0;
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
diff -u -p -r1.463 session.c
--- session.c 19 Feb 2024 10:15:35 -0000 1.463
+++ session.c 19 Mar 2024 15:18:52 -0000
@@ -68,7 +68,7 @@ void session_tcp_established(struct peer
void session_capa_ann_none(struct peer *);
int session_capa_add(struct ibuf *, uint8_t, uint8_t);
int session_capa_add_mp(struct ibuf *, uint8_t);
-int session_capa_add_afi(struct peer *, struct ibuf *, uint8_t, uint8_t);
+int session_capa_add_afi(struct ibuf *, uint8_t, uint8_t);
struct bgp_msg *session_newmsg(enum msg_type, uint16_t);
int session_sendmsg(struct bgp_msg *, struct peer *);
void session_open(struct peer *);
@@ -1357,8 +1357,7 @@ session_capa_add_mp(struct ibuf *buf, ui
}
int
-session_capa_add_afi(struct peer *p, struct ibuf *b, uint8_t aid,
- uint8_t flags)
+session_capa_add_afi(struct ibuf *b, uint8_t aid, uint8_t flags)
{
u_int errs = 0;
uint16_t afi;
@@ -1492,7 +1491,7 @@ session_open(struct peer *p)
}
/* multiprotocol extensions, RFC 4760 */
- for (i = 0; i < AID_MAX; i++)
+ for (i = AID_MIN; i < AID_MAX; i++)
if (p->capa.ann.mp[i]) { /* 4 bytes data */
errs += session_capa_add(opb, CAPA_MP, 4);
errs += session_capa_add_mp(opb, i);
@@ -1517,7 +1516,7 @@ session_open(struct peer *p)
int rst = 0;
uint16_t hdr = 0;
- for (i = 0; i < AID_MAX; i++) {
+ for (i = AID_MIN; i < AID_MAX; i++) {
if (p->capa.neg.grestart.flags[i] & CAPA_GR_RESTARTING)
rst++;
}
@@ -1536,7 +1535,7 @@ session_open(struct peer *p)
}
/* advertisement of multiple paths, RFC7911 */
- if (p->capa.ann.add_path[0]) { /* variable */
+ if (p->capa.ann.add_path[AID_MIN]) { /* variable */
uint8_t aplen;
if (mpcapa)
@@ -1547,12 +1546,12 @@ session_open(struct peer *p)
if (mpcapa) {
for (i = AID_MIN; i < AID_MAX; i++) {
if (p->capa.ann.mp[i]) {
- errs += session_capa_add_afi(p, opb,
+ errs += session_capa_add_afi(opb,
i, p->capa.ann.add_path[i]);
}
}
} else { /* AID_INET */
- errs += session_capa_add_afi(p, opb, AID_INET,
+ errs += session_capa_add_afi(opb, AID_INET,
p->capa.ann.add_path[AID_INET]);
}
}
@@ -1759,7 +1758,7 @@ session_neighbor_rrefresh(struct peer *p
if (!(p->capa.neg.refresh || p->capa.neg.enhanced_rr))
return (-1);
- for (i = 0; i < AID_MAX; i++) {
+ for (i = AID_MIN; i < AID_MAX; i++) {
if (p->capa.neg.mp[i] != 0)
session_rrefresh(p, i, ROUTE_REFRESH_REQUEST);
}
@@ -1828,7 +1827,7 @@ session_graceful_restart(struct peer *p)
timer_set(&p->timers, Timer_RestartTimeout,
p->capa.neg.grestart.timeout);
- for (i = 0; i < AID_MAX; i++) {
+ for (i = AID_MIN; i < AID_MAX; i++) {
if (p->capa.neg.grestart.flags[i] & CAPA_GR_PRESENT) {
if (imsg_rde(IMSG_SESSION_STALE, p->conf.id,
&i, sizeof(i)) == -1)
@@ -1854,7 +1853,7 @@ session_graceful_stop(struct peer *p)
{
uint8_t i;
- for (i = 0; i < AID_MAX; i++) {
+ for (i = AID_MIN; i < AID_MAX; i++) {
/*
* Only flush if the peer is restarting and the timeout fired.
* In all other cases the session was already flushed when the
@@ -2485,7 +2484,6 @@ parse_notification(struct peer *peer)
uint8_t capa_code;
uint8_t capa_len;
size_t reason_len;
- uint8_t i;
/* just log */
p = peer->rbuf->rptr;
@@ -2545,8 +2543,8 @@ parse_notification(struct peer *peer)
datalen -= capa_len;
switch (capa_code) {
case CAPA_MP:
- for (i = 0; i < AID_MAX; i++)
- peer->capa.ann.mp[i] = 0;
+ memset(peer->capa.ann.mp, 0,
+ sizeof(peer->capa.ann.mp));
log_peer_warnx(&peer->conf,
"disabling multiprotocol capability");
break;
@@ -2840,12 +2838,11 @@ capa_neg_calc(struct peer *p, uint8_t *s
(p->capa.ann.refresh && p->capa.peer.refresh) != 0;
p->capa.neg.enhanced_rr =
(p->capa.ann.enhanced_rr && p->capa.peer.enhanced_rr) != 0;
-
p->capa.neg.as4byte =
(p->capa.ann.as4byte && p->capa.peer.as4byte) != 0;
/* MP: both side must agree on the AFI,SAFI pair */
- for (i = 0; i < AID_MAX; i++) {
+ for (i = AID_MIN; i < AID_MAX; i++) {
if (p->capa.ann.mp[i] && p->capa.peer.mp[i])
p->capa.neg.mp[i] = 1;
else
@@ -2866,7 +2863,7 @@ capa_neg_calc(struct peer *p, uint8_t *s
* supporting graceful restart.
*/
- for (i = 0; i < AID_MAX; i++) {
+ for (i = AID_MIN; i < AID_MAX; i++) {
int8_t negflags;
/* disable GR if the AFI/SAFI is not present */
@@ -2898,26 +2895,24 @@ capa_neg_calc(struct peer *p, uint8_t *s
if (p->capa.ann.grestart.restart == 0)
p->capa.neg.grestart.restart = 0;
-
/*
* ADD-PATH: set only those bits where both sides agree.
* For this compare our send bit with the recv bit from the peer
* and vice versa.
* The flags are stored from this systems view point.
+ * At index 0 the flags are set if any per-AID flag is set.
*/
memset(p->capa.neg.add_path, 0, sizeof(p->capa.neg.add_path));
- if (p->capa.ann.add_path[0]) {
- for (i = AID_MIN; i < AID_MAX; i++) {
- if ((p->capa.ann.add_path[i] & CAPA_AP_RECV) &&
- (p->capa.peer.add_path[i] & CAPA_AP_SEND)) {
- p->capa.neg.add_path[i] |= CAPA_AP_RECV;
- p->capa.neg.add_path[0] |= CAPA_AP_RECV;
- }
- if ((p->capa.ann.add_path[i] & CAPA_AP_SEND) &&
- (p->capa.peer.add_path[i] & CAPA_AP_RECV)) {
- p->capa.neg.add_path[i] |= CAPA_AP_SEND;
- p->capa.neg.add_path[0] |= CAPA_AP_SEND;
- }
+ for (i = AID_MIN; i < AID_MAX; i++) {
+ if ((p->capa.ann.add_path[i] & CAPA_AP_RECV) &&
+ (p->capa.peer.add_path[i] & CAPA_AP_SEND)) {
+ p->capa.neg.add_path[i] |= CAPA_AP_RECV;
+ p->capa.neg.add_path[0] |= CAPA_AP_RECV;
+ }
+ if ((p->capa.ann.add_path[i] & CAPA_AP_SEND) &&
+ (p->capa.peer.add_path[i] & CAPA_AP_RECV)) {
+ p->capa.neg.add_path[i] |= CAPA_AP_SEND;
+ p->capa.neg.add_path[0] |= CAPA_AP_SEND;
}
}
@@ -3323,7 +3318,7 @@ session_dispatch_imsg(struct imsgbuf *im
log_warnx("no such peer: id=%u", peerid);
break;
}
- if (rr.aid >= AID_MAX)
+ if (rr.aid < AID_MIN || rr.aid >= AID_MAX)
fatalx("IMSG_REFRESH: bad AID");
session_rrefresh(p, rr.aid, rr.subtype);
break;
@@ -3338,7 +3333,7 @@ session_dispatch_imsg(struct imsgbuf *im
log_warnx("no such peer: id=%u", peerid);
break;
}
- if (aid >= AID_MAX)
+ if (aid < AID_MIN || aid >= AID_MAX)
fatalx("IMSG_SESSION_RESTARTED: bad AID");
if (p->capa.neg.grestart.flags[aid] &
CAPA_GR_RESTARTING) {
Index: util.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
diff -u -p -r1.82 util.c
--- util.c 22 Feb 2024 06:45:22 -0000 1.82
+++ util.c 19 Mar 2024 15:03:58 -0000
@@ -922,7 +922,7 @@ aid2str(uint8_t aid)
int
aid2afi(uint8_t aid, uint16_t *afi, uint8_t *safi)
{
- if (aid < AID_MAX) {
+ if (aid != AID_UNSPEC && aid < AID_MAX) {
*afi = aid_vals[aid].afi;
*safi = aid_vals[aid].safi;
return (0);
@@ -935,7 +935,7 @@ afi2aid(uint16_t afi, uint8_t safi, uint
{
uint8_t i;
- for (i = 0; i < AID_MAX; i++)
+ for (i = AID_MIN; i < AID_MAX; i++)
if (aid_vals[i].afi == afi && aid_vals[i].safi == safi) {
*aid = i;
return (0);
@@ -960,7 +960,7 @@ af2aid(sa_family_t af, uint8_t safi, uin
if (safi == 0) /* default to unicast subclass */
safi = SAFI_UNICAST;
- for (i = 0; i < AID_MAX; i++)
+ for (i = AID_UNSPEC; i < AID_MAX; i++)
if (aid_vals[i].af == af && aid_vals[i].safi == safi) {
*aid = i;
return (0);
bgpd: cleanup AID / AFI handling