From 6543303ad4d509340643ae1f2b585f6278cf0463 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Tue, 7 Feb 2023 15:14:40 +0100 Subject: BFD notifications respect protocol loop settings --- nest/bfd.h | 5 ++-- proto/bfd/bfd.c | 63 +++++++++++++++++++++++++++++++++++++++++---------- proto/bgp/bgp.c | 2 +- proto/ospf/neighbor.c | 2 +- proto/rip/rip.c | 2 +- proto/static/static.c | 2 +- 6 files changed, 58 insertions(+), 18 deletions(-) diff --git a/nest/bfd.h b/nest/bfd.h index 37561266..e8977dbd 100644 --- a/nest/bfd.h +++ b/nest/bfd.h @@ -35,6 +35,7 @@ struct bfd_request { void (*hook)(struct bfd_request *); void *data; + struct birdloop *target; struct bfd_session *session; @@ -57,14 +58,14 @@ static inline struct bfd_options * bfd_new_options(void) #ifdef CONFIG_BFD -struct bfd_request * bfd_request_session(pool *p, ip_addr addr, ip_addr local, struct iface *iface, struct iface *vrf, void (*hook)(struct bfd_request *), void *data, const struct bfd_options *opts); +struct bfd_request * bfd_request_session(pool *p, ip_addr addr, ip_addr local, struct iface *iface, struct iface *vrf, void (*hook)(struct bfd_request *), void *data, struct birdloop *target, const struct bfd_options *opts); void bfd_update_request(struct bfd_request *req, const struct bfd_options *opts); static inline void cf_check_bfd(int use UNUSED) { } #else -static inline struct bfd_request * bfd_request_session(pool *p UNUSED, ip_addr addr UNUSED, ip_addr local UNUSED, struct iface *iface UNUSED, struct iface *vrf UNUSED, void (*hook)(struct bfd_request *) UNUSED, void *data UNUSED, const struct bfd_options *opts UNUSED) { return NULL; } +static inline struct bfd_request * bfd_request_session(pool *p UNUSED, ip_addr addr UNUSED, ip_addr local UNUSED, struct iface *iface UNUSED, struct iface *vrf UNUSED, void (*hook)(struct bfd_request *) UNUSED, void *data UNUSED, struct birdloop *target UNUSED, const struct bfd_options *opts UNUSED) { return NULL; } static inline void bfd_update_request(struct bfd_request *req UNUSED, const struct bfd_options *opts UNUSED) { }; static inline void cf_check_bfd(int use) { if (use) cf_error("BFD not available"); } diff --git a/proto/bfd/bfd.c b/proto/bfd/bfd.c index 575ebc3c..59134a22 100644 --- a/proto/bfd/bfd.c +++ b/proto/bfd/bfd.c @@ -657,7 +657,17 @@ bfd_request_notify(struct bfd_request *req, u8 state, u8 diag) req->down = (old_state == BFD_STATE_UP) && (state == BFD_STATE_DOWN); if (req->hook) + { + struct birdloop *target = !birdloop_inside(req->target) ? req->target : NULL; + + if (target) + birdloop_enter(target); + req->hook(req); + + if (target) + birdloop_leave(target); + } } static int @@ -676,7 +686,6 @@ bfd_add_request(struct bfd_proto *p, struct bfd_request *req) uint ifindex = req->iface ? req->iface->index : 0; struct bfd_session *s = bfd_find_session_by_addr(p, req->addr, ifindex); - u8 state, diag; if (!s) s = bfd_add_session(p, req->addr, req->local, req->iface, &req->opts); @@ -686,11 +695,15 @@ bfd_add_request(struct bfd_proto *p, struct bfd_request *req) req->session = s; bfd_lock_sessions(p); - state = s->loc_state; - diag = s->loc_diag; + + int notify = !NODE_VALID(&s->n); + if (notify) + add_tail(&p->notify_list, &s->n); + bfd_unlock_sessions(p); - bfd_request_notify(req, state, diag); + if (notify) + ev_send(&global_event_list, &p->notify_event); return 1; } @@ -698,6 +711,26 @@ bfd_add_request(struct bfd_proto *p, struct bfd_request *req) static void bfd_pickup_requests(void *_data UNUSED) { + /* NOTE TO MY FUTURE SELF + * + * Functions bfd_take_requests() and bfd_drop_requests() need to have + * consistent &bfd_global.wait_list and this is ensured only by having these + * functions called from bfd_start() and bfd_shutdown() which are both called + * in PROTO_LOCKED_FROM_MAIN context, i.e. always from &main_birdloop. + * + * This pickup event is also called in &main_birdloop, therefore we can + * freely do BFD_LOCK/BFD_UNLOCK while processing all the requests. All BFD + * protocols capable of bfd_add_request() are either started before this code + * happens or after that. + * + * If BFD protocols could start in parallel with this routine, they might + * miss some of the waiting requests, thus if anybody tries to start + * protocols or run this pickup event outside &main_birdloop in future, they + * shall ensure that this race condition is mitigated somehow. + * + * Thank you, my future self, for understanding. Have a nice day! + */ + node *n; WALK_LIST(n, bfd_global.proto_list) { @@ -714,12 +747,16 @@ bfd_pickup_requests(void *_data UNUSED) } BFD_LOCK; - node *rn, *rnxt; - WALK_LIST_DELSAFE(rn, rnxt, bfd_global.pickup_list) + while (!EMPTY_LIST(bfd_global.pickup_list)) { - rem_node(rn); - add_tail(&bfd_global.wait_list, rn); - bfd_request_notify(SKIP_BACK(struct bfd_request, n, rn), BFD_STATE_ADMIN_DOWN, 0); + struct bfd_request *req = SKIP_BACK(struct bfd_request, n, HEAD(bfd_global.pickup_list)); + rem_node(&req->n); + BFD_UNLOCK; + + bfd_request_notify(req, BFD_STATE_ADMIN_DOWN, 0); + + BFD_LOCK; + add_tail(&bfd_global.wait_list, &req->n); } BFD_UNLOCK; } @@ -749,7 +786,6 @@ bfd_drop_requests(struct bfd_proto *p) rem_node(&req->n); add_tail(&bfd_global.pickup_list, &req->n); req->session = NULL; - bfd_request_notify(req, BFD_STATE_ADMIN_DOWN, 0); } ev_send(&global_event_list, &bfd_pickup_event); @@ -766,6 +802,7 @@ struct bfd_request * bfd_request_session(pool *p, ip_addr addr, ip_addr local, struct iface *iface, struct iface *vrf, void (*hook)(struct bfd_request *), void *data, + struct birdloop *target, const struct bfd_options *opts) { struct bfd_request *req = ralloc(p, &bfd_request_class); @@ -778,8 +815,10 @@ bfd_request_session(pool *p, ip_addr addr, ip_addr local, if (opts) req->opts = *opts; + ASSERT_DIE(target || !hook); req->hook = hook; req->data = data; + req->target = target; req->session = NULL; @@ -854,7 +893,7 @@ bfd_neigh_notify(struct neighbor *nb) if ((nb->scope > 0) && !n->req) { ip_addr local = ipa_nonzero(n->local) ? n->local : nb->ifa->ip; - n->req = bfd_request_session(p->p.pool, n->addr, local, nb->iface, p->p.vrf, NULL, NULL, NULL); + n->req = bfd_request_session(p->p.pool, n->addr, local, nb->iface, p->p.vrf, NULL, NULL, NULL, NULL); } if ((nb->scope <= 0) && n->req) @@ -871,7 +910,7 @@ bfd_start_neighbor(struct bfd_proto *p, struct bfd_neighbor *n) if (n->multihop) { - n->req = bfd_request_session(p->p.pool, n->addr, n->local, NULL, p->p.vrf, NULL, NULL, NULL); + n->req = bfd_request_session(p->p.pool, n->addr, n->local, NULL, p->p.vrf, NULL, NULL, NULL, NULL); return; } diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index f350c8ca..104e5862 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -1469,7 +1469,7 @@ bgp_update_bfd(struct bgp_proto *p, const struct bfd_options *bfd) if (bfd && !p->bfd_req && !bgp_is_dynamic(p)) p->bfd_req = bfd_request_session(p->p.pool, p->remote_ip, p->local_ip, p->cf->multihop ? NULL : p->neigh->iface, - p->p.vrf, bgp_bfd_notify, p, bfd); + p->p.vrf, bgp_bfd_notify, p, p->p.loop, bfd); if (!bfd && p->bfd_req) { diff --git a/proto/ospf/neighbor.c b/proto/ospf/neighbor.c index ca369819..b0fdc42f 100644 --- a/proto/ospf/neighbor.c +++ b/proto/ospf/neighbor.c @@ -777,7 +777,7 @@ ospf_neigh_update_bfd(struct ospf_neighbor *n, int use_bfd) if (use_bfd && !n->bfd_req) n->bfd_req = bfd_request_session(n->pool, n->ip, n->ifa->addr->ip, n->ifa->iface, p->p.vrf, - ospf_neigh_bfd_hook, n, NULL); + ospf_neigh_bfd_hook, n, p->p.loop, NULL); if (!use_bfd && n->bfd_req) { diff --git a/proto/rip/rip.c b/proto/rip/rip.c index 97d1dd80..2c504112 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -543,7 +543,7 @@ rip_update_bfd(struct rip_proto *p, struct rip_neighbor *n) ip_addr saddr = rip_is_v2(p) ? n->ifa->sk->saddr : n->nbr->ifa->ip; n->bfd_req = bfd_request_session(p->p.pool, n->nbr->addr, saddr, n->nbr->iface, p->p.vrf, - rip_bfd_notify, n, NULL); + rip_bfd_notify, n, p->p.loop, NULL); } if (!use_bfd && n->bfd_req) diff --git a/proto/static/static.c b/proto/static/static.c index 82fbfe7a..86142790 100644 --- a/proto/static/static.c +++ b/proto/static/static.c @@ -213,7 +213,7 @@ static_update_bfd(struct static_proto *p, struct static_route *r) // ip_addr local = ipa_nonzero(r->local) ? r->local : nb->ifa->ip; r->bfd_req = bfd_request_session(p->p.pool, r->via, nb->ifa->ip, nb->iface, p->p.vrf, - static_bfd_notify, r, NULL); + static_bfd_notify, r, p->p.loop, NULL); } if (!bfd_up && r->bfd_req) -- cgit v1.2.3