This diff changes the way session or peer related imsgs are handled.
Instead of passing the imsg.hdr.peerid down and doing the lookup for the
peer in each function move that code up into the imsg handler.
The plan is to add an imsg queue per peer later on to make the processing
of messages more fair than what happens currently.
While there change some fatalx to warnx since it is fine to just ignore
bad GR restart messages.
OK?
--
:wq Claudio
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.492
diff -u -p -r1.492 rde.c
--- rde.c 13 Dec 2019 14:10:56 -0000 1.492
+++ rde.c 13 Dec 2019 15:09:09 -0000
@@ -49,7 +49,7 @@
void rde_sighdlr(int);
void rde_dispatch_imsg_session(struct imsgbuf *);
void rde_dispatch_imsg_parent(struct imsgbuf *);
-int rde_update_dispatch(struct imsg *);
+void rde_update_dispatch(struct rde_peer *, struct imsg *);
int rde_update_update(struct rde_peer *, struct filterstate *,
struct bgpd_addr *, u_int8_t);
void rde_update_withdraw(struct rde_peer *, struct bgpd_addr *,
@@ -98,11 +98,11 @@ void peer_shutdown(void);
int peer_localaddrs(struct rde_peer *, struct bgpd_addr *);
struct rde_peer *peer_match(struct ctl_neighbor *, u_int32_t);
struct rde_peer *peer_add(u_int32_t, struct peer_config *);
-void peer_up(u_int32_t, struct session_up *);
-void peer_down(u_int32_t);
+void peer_up(struct rde_peer *, struct session_up *);
+void peer_down(struct rde_peer *);
void peer_flush(struct rde_peer *, u_int8_t, time_t);
-void peer_stale(u_int32_t, u_int8_t);
-void peer_dump(u_int32_t, u_int8_t);
+void peer_stale(struct rde_peer *, u_int8_t);
+void peer_dump(struct rde_peer *, u_int8_t);
static void peer_recv_eor(struct rde_peer *, u_int8_t);
static void peer_send_eor(struct rde_peer *, u_int8_t);
@@ -380,7 +380,12 @@ rde_dispatch_imsg_session(struct imsgbuf
switch (imsg.hdr.type) {
case IMSG_UPDATE:
- rde_update_dispatch(&imsg);
+ if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
+ log_warnx("rde_dispatch: unknown peer id %d",
+ imsg.hdr.peerid);
+ break;
+ }
+ rde_update_dispatch(peer, &imsg);
break;
case IMSG_SESSION_ADD:
if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(pconf))
@@ -392,10 +397,20 @@ rde_dispatch_imsg_session(struct imsgbuf
if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(sup))
fatalx("incorrect size of session request");
memcpy(&sup, imsg.data, sizeof(sup));
- peer_up(imsg.hdr.peerid, &sup);
+ if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
+ log_warnx("rde_dispatch: unknown peer id %d",
+ imsg.hdr.peerid);
+ break;
+ }
+ peer_up(peer, &sup);
break;
case IMSG_SESSION_DOWN:
- peer_down(imsg.hdr.peerid);
+ if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
+ log_warnx("rde_dispatch: unknown peer id %d",
+ imsg.hdr.peerid);
+ break;
+ }
+ peer_down(peer);
break;
case IMSG_SESSION_STALE:
if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(aid)) {
@@ -403,9 +418,16 @@ rde_dispatch_imsg_session(struct imsgbuf
break;
}
memcpy(&aid, imsg.data, sizeof(aid));
- if (aid >= AID_MAX)
- fatalx("IMSG_SESSION_STALE: bad AID");
- peer_stale(imsg.hdr.peerid, aid);
+ if (aid >= AID_MAX) {
+ log_warnx("IMSG_SESSION_STALE: bad AID");
+ break;
+ }
+ if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
+ log_warnx("rde_dispatch: unknown peer id %d",
+ imsg.hdr.peerid);
+ break;
+ }
+ peer_stale(peer, aid);
break;
case IMSG_SESSION_FLUSH:
if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(aid)) {
@@ -413,8 +435,10 @@ rde_dispatch_imsg_session(struct imsgbuf
break;
}
memcpy(&aid, imsg.data, sizeof(aid));
- if (aid >= AID_MAX)
- fatalx("IMSG_SESSION_FLUSH: bad AID");
+ if (aid >= AID_MAX) {
+ log_warnx("IMSG_SESSION_FLUSH: bad AID");
+ break;
+ }
if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
log_warnx("rde_dispatch: unknown peer id %d",
imsg.hdr.peerid);
@@ -428,8 +452,10 @@ rde_dispatch_imsg_session(struct imsgbuf
break;
}
memcpy(&aid, imsg.data, sizeof(aid));
- if (aid >= AID_MAX)
- fatalx("IMSG_SESSION_RESTARTED: bad AID");
+ if (aid >= AID_MAX) {
+ log_warnx("IMSG_SESSION_RESTARTED: bad AID");
+ break;
+ }
if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
log_warnx("rde_dispatch: unknown peer id %d",
imsg.hdr.peerid);
@@ -444,9 +470,16 @@ rde_dispatch_imsg_session(struct imsgbuf
break;
}
memcpy(&aid, imsg.data, sizeof(aid));
- if (aid >= AID_MAX)
- fatalx("IMSG_REFRESH: bad AID");
- peer_dump(imsg.hdr.peerid, aid);
+ if (aid >= AID_MAX) {
+ log_warnx("IMSG_REFRESH: bad AID");
+ break;
+ }
+ if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
+ log_warnx("rde_dispatch: unknown peer id %d",
+ imsg.hdr.peerid);
+ break;
+ }
+ peer_dump(peer, aid);
break;
case IMSG_NETWORK_ADD:
if (imsg.hdr.len - IMSG_HEADER_SIZE !=
@@ -1022,13 +1055,12 @@ rde_dispatch_imsg_parent(struct imsgbuf
}
/* handle routing updates from the session engine. */
-int
-rde_update_dispatch(struct imsg *imsg)
+void
+rde_update_dispatch(struct rde_peer *peer, struct imsg *imsg)
{
struct filterstate state;
struct bgpd_addr prefix;
struct mpattr mpa;
- struct rde_peer *peer;
u_char *p, *mpp = NULL;
int error = -1, pos = 0;
u_int16_t afi, len, mplen;
@@ -1038,17 +1070,11 @@ rde_update_dispatch(struct imsg *imsg)
u_int8_t aid, prefixlen, safi, subtype;
u_int32_t fas;
- peer = peer_get(imsg->hdr.peerid);
- if (peer == NULL) /* unknown peer, cannot happen */
- return (-1);
- if (peer->state != PEER_UP)
- return (-1); /* peer is not yet up, cannot happen */
-
p = imsg->data;
if (imsg->hdr.len < IMSG_HEADER_SIZE + 2) {
rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRLIST, NULL, 0);
- return (-1);
+ return;
}
memcpy(&len, p, 2);
@@ -1056,7 +1082,7 @@ rde_update_dispatch(struct imsg *imsg)
p += 2;
if (imsg->hdr.len < IMSG_HEADER_SIZE + 2 + withdrawn_len + 2) {
rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRLIST, NULL, 0);
- return (-1);
+ return;
}
p += withdrawn_len;
@@ -1066,7 +1092,7 @@ rde_update_dispatch(struct imsg *imsg)
if (imsg->hdr.len <
IMSG_HEADER_SIZE + 2 + withdrawn_len + 2 + attrpath_len) {
rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRLIST, NULL, 0);
- return (-1);
+ return;
}
nlri_len =
@@ -1078,12 +1104,12 @@ rde_update_dispatch(struct imsg *imsg)
/* crap at end of update which should not be there */
rde_update_err(peer, ERR_UPDATE,
ERR_UPD_ATTRLIST, NULL, 0);
- return (-1);
+ return;
}
if (withdrawn_len == 0) {
/* EoR marker */
peer_recv_eor(peer, AID_INET);
- return (0);
+ return;
}
}
@@ -1394,8 +1420,6 @@ rde_update_dispatch(struct imsg *imsg)
done:
rde_filterstate_clean(&state);
rde_send_pftable_commit();
-
- return (error);
}
int
@@ -3290,7 +3314,7 @@ rde_softreconfig_in_done(void *arg, u_in
/* dump the full table to neighbors that changed rib */
for (aid = 0; aid < AID_MAX; aid++) {
if (peer->capa.mp[aid])
- peer_dump(peer->conf.id, aid);
+ peer_dump(peer, aid);
}
}
}
@@ -3711,17 +3735,10 @@ peer_adjout_stale_upcall(struct prefix *
}
void
-peer_up(u_int32_t id, struct session_up *sup)
+peer_up(struct rde_peer *peer, struct session_up *sup)
{
- struct rde_peer *peer;
u_int8_t i;
- peer = peer_get(id);
- if (peer == NULL) {
- log_warnx("peer_up: unknown peer id %d", id);
- return;
- }
-
if (peer->state == PEER_ERR) {
/*
* There is a race condition when doing PEER_ERR -> PEER_DOWN.
@@ -3742,7 +3759,8 @@ peer_up(u_int32_t id, struct session_up
if (peer_localaddrs(peer, &sup->local_addr)) {
peer->state = PEER_DOWN;
- imsg_compose(ibuf_se, IMSG_SESSION_DOWN, id, 0, -1, NULL, 0);
+ imsg_compose(ibuf_se, IMSG_SESSION_DOWN, peer->conf.id, 0, -1,
+ NULL, 0);
return;
}
@@ -3750,20 +3768,13 @@ peer_up(u_int32_t id, struct session_up
for (i = 0; i < AID_MAX; i++) {
if (peer->capa.mp[i])
- peer_dump(id, i);
+ peer_dump(peer, i);
}
}
void
-peer_down(u_int32_t id)
+peer_down(struct rde_peer *peer)
{
- struct rde_peer *peer;
-
- peer = peer_get(id);
- if (peer == NULL) {
- log_warnx("peer_down: unknown peer id %d", id);
- return;
- }
peer->remote_bgpid = 0;
peer->state = PEER_DOWN;
/* stop all pending dumps which may depend on this peer */
@@ -3858,17 +3869,10 @@ peer_flush(struct rde_peer *peer, u_int8
}
void
-peer_stale(u_int32_t id, u_int8_t aid)
+peer_stale(struct rde_peer *peer, u_int8_t aid)
{
- struct rde_peer *peer;
time_t now;
- peer = peer_get(id);
- if (peer == NULL) {
- log_warnx("peer_stale: unknown peer id %d", id);
- return;
- }
-
/* flush the now even staler routes out */
if (peer->staletime[aid])
peer_flush(peer, aid, peer->staletime[aid]);
@@ -3887,16 +3891,8 @@ peer_stale(u_int32_t id, u_int8_t aid)
}
void
-peer_dump(u_int32_t id, u_int8_t aid)
+peer_dump(struct rde_peer *peer, u_int8_t aid)
{
- struct rde_peer *peer;
-
- peer = peer_get(id);
- if (peer == NULL) {
- log_warnx("peer_dump: unknown peer id %d", id);
- return;
- }
-
if (peer->conf.export_type == EXPORT_NONE) {
/* nothing to send apart from the marker */
if (peer->capa.grestart.restart)
@@ -4220,7 +4216,7 @@ rde_shutdown(void)
/* First all peers go down */
for (i = 0; i <= peertable.peer_hashmask; i++)
while ((p = LIST_FIRST(&peertable.peer_hashtbl[i])) != NULL)
- peer_down(p->conf.id);
+ peer_down(p);
/* free filters */
filterlist_free(out_rules);
