From d40281a0c91dd5e8f6375678685389acaf2e949e Mon Sep 17 00:00:00 2001 From: claudio Date: Tue, 26 Jul 2022 16:32:29 +0000 Subject: [PATCH] Refactor nexthop tracking and remove all the kif_kr code. There is no need to track directly connected routes per kif. The only use case was for nexthop validation but that can be done by storing the ifindex in struct knexthop. OK tb@ --- usr.sbin/bgpd/kroute.c | 258 +++++------------------------------------ 1 file changed, 30 insertions(+), 228 deletions(-) diff --git a/usr.sbin/bgpd/kroute.c b/usr.sbin/bgpd/kroute.c index 49a73201822..3293950d8ae 100644 --- a/usr.sbin/bgpd/kroute.c +++ b/usr.sbin/bgpd/kroute.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kroute.c,v 1.279 2022/07/23 10:24:01 claudio Exp $ */ +/* $OpenBSD: kroute.c,v 1.280 2022/07/26 16:32:29 claudio Exp $ */ /* * Copyright (c) 2022 Claudio Jeker @@ -86,6 +86,7 @@ struct knexthop { RB_ENTRY(knexthop) entry; struct bgpd_addr nexthop; void *kroute; + u_short ifindex; }; struct kredist_node { @@ -96,19 +97,6 @@ struct kredist_node { uint8_t dynamic; }; -struct kif_kr { - LIST_ENTRY(kif_kr) entry; - struct kroute *kr; -}; - -struct kif_kr6 { - LIST_ENTRY(kif_kr6) entry; - struct kroute6 *kr; -}; - -LIST_HEAD(kif_kr_head, kif_kr); -LIST_HEAD(kif_kr6_head, kif_kr6); - struct kif { char ifname[IFNAMSIZ]; uint64_t baudrate; @@ -124,8 +112,6 @@ struct kif { struct kif_node { RB_ENTRY(kif_node) entry; struct kif k; - struct kif_kr_head kroute_l; - struct kif_kr6_head kroute6_l; }; int ktable_new(u_int, u_int, char *, int); @@ -176,16 +162,10 @@ int kif_insert(struct kif_node *); int kif_remove(struct kif_node *); void kif_clear(void); -int kif_kr_insert(struct kroute *); -int kif_kr_remove(struct kroute *); - -int kif_kr6_insert(struct kroute6 *); -int kif_kr6_remove(struct kroute6 *); - int kroute_validate(struct kroute *); int kroute6_validate(struct kroute6 *); void knexthop_validate(struct ktable *, struct knexthop *); -void knexthop_track(struct ktable *, void *); +void knexthop_track(struct ktable *, u_short); void knexthop_send_update(struct knexthop *); struct kroute *kroute_match(struct ktable *, struct bgpd_addr *, int); struct kroute6 *kroute6_match(struct ktable *, struct bgpd_addr *, int); @@ -1760,11 +1740,6 @@ kroute_insert(struct ktable *kt, struct kroute_full *kf) krm->next = kr; } - if ((kr->flags & (F_KERNEL | F_CONNECTED)) == - (F_KERNEL | F_CONNECTED)) - if (kif_kr_insert(kr) == -1) - return (-1); - if (kf->flags & F_BGPD) if (send_rtmsg(kr_state.fd, RTM_ADD, kt, kf)) kr->flags |= F_BGPD_INSERTED; @@ -1801,11 +1776,6 @@ kroute_insert(struct ktable *kt, struct kroute_full *kf) kr6m->next = kr6; } - if ((kr6->flags & (F_KERNEL | F_CONNECTED)) == - (F_KERNEL | F_CONNECTED)) - if (kif_kr6_insert(kr6) == -1) - return (-1); - if (kf->flags & F_BGPD) if (send_rtmsg(kr_state.fd, RTM_ADD, kt, kf)) kr6->flags |= F_BGPD_INSERTED; @@ -1877,15 +1847,7 @@ kroute_remove(struct ktable *kt, struct kroute *kr) /* again remove only once */ kr_redistribute(IMSG_NETWORK_REMOVE, kt, kr_tofull(kr)); - if (kr->flags & F_CONNECTED) - if (kif_kr_remove(kr) == -1) { - rtlabel_unref(kr->labelid); - free(kr); - return (-1); - } - rtlabel_unref(kr->labelid); - free(kr); return (0); } @@ -1995,15 +1957,7 @@ kroute6_remove(struct ktable *kt, struct kroute6 *kr) /* again remove only once */ kr_redistribute(IMSG_NETWORK_REMOVE, kt, kr6_tofull(kr)); - if (kr->flags & F_CONNECTED) - if (kif_kr6_remove(kr) == -1) { - rtlabel_unref(kr->labelid); - free(kr); - return (-1); - } - rtlabel_unref(kr->labelid); - free(kr); return (0); } @@ -2081,9 +2035,6 @@ kif_find(int ifindex) int kif_insert(struct kif_node *kif) { - LIST_INIT(&kif->kroute_l); - LIST_INIT(&kif->kroute6_l); - if (RB_INSERT(kif_tree, &kit, kif) != NULL) { log_warnx("RB_INSERT(kif_tree, &kit, kif)"); free(kif); @@ -2097,31 +2048,23 @@ int kif_remove(struct kif_node *kif) { struct ktable *kt; - struct kif_kr *kkr; - struct kif_kr6 *kkr6; + + kif->flags &= ~IFF_UP; + + /* + * TODO, remove all kroutes using this interface, + * the kernel does this for us but better to do it + * here as well. + */ + + if ((kt = ktable_get(kif->k.rdomain)) != NULL) + knexthop_track(kt, kif->k.ifindex); if (RB_REMOVE(kif_tree, &kit, kif) == NULL) { log_warnx("RB_REMOVE(kif_tree, &kit, kif)"); return (-1); } - if ((kt = ktable_get(kif->k.rdomain)) == NULL) - goto done; - - while ((kkr = LIST_FIRST(&kif->kroute_l)) != NULL) { - LIST_REMOVE(kkr, entry); - kkr->kr->flags &= ~F_NEXTHOP; - kroute_remove(kt, kkr->kr); - free(kkr); - } - - while ((kkr6 = LIST_FIRST(&kif->kroute6_l)) != NULL) { - LIST_REMOVE(kkr6, entry); - kkr6->kr->flags &= ~F_NEXTHOP; - kroute6_remove(kt, kkr6->kr); - free(kkr6); - } -done: free(kif); return (0); } @@ -2135,124 +2078,6 @@ kif_clear(void) kif_remove(kif); } -int -kif_kr_insert(struct kroute *kr) -{ - struct kif_node *kif; - struct kif_kr *kkr; - - if ((kif = kif_find(kr->ifindex)) == NULL) { - if (kr->ifindex) - log_warnx("%s: interface with index %u not found", - __func__, kr->ifindex); - return (0); - } - - if (kif->k.nh_reachable) - kr->flags &= ~F_DOWN; - else - kr->flags |= F_DOWN; - - if ((kkr = calloc(1, sizeof(*kkr))) == NULL) { - log_warn("%s", __func__); - return (-1); - } - - kkr->kr = kr; - - LIST_INSERT_HEAD(&kif->kroute_l, kkr, entry); - - return (0); -} - -int -kif_kr_remove(struct kroute *kr) -{ - struct kif_node *kif; - struct kif_kr *kkr; - - if ((kif = kif_find(kr->ifindex)) == NULL) { - if (kr->ifindex) - log_warnx("%s: interface with index %u not found", - __func__, kr->ifindex); - return (0); - } - - for (kkr = LIST_FIRST(&kif->kroute_l); kkr != NULL && kkr->kr != kr; - kkr = LIST_NEXT(kkr, entry)) - ; /* nothing */ - - if (kkr == NULL) { - log_warnx("%s: can't remove connected route from interface " - "with index %u: not found", __func__, kr->ifindex); - return (-1); - } - - LIST_REMOVE(kkr, entry); - free(kkr); - - return (0); -} - -int -kif_kr6_insert(struct kroute6 *kr) -{ - struct kif_node *kif; - struct kif_kr6 *kkr6; - - if ((kif = kif_find(kr->ifindex)) == NULL) { - if (kr->ifindex) - log_warnx("%s: interface with index %u not found", - __func__, kr->ifindex); - return (0); - } - - if (kif->k.nh_reachable) - kr->flags &= ~F_DOWN; - else - kr->flags |= F_DOWN; - - if ((kkr6 = calloc(1, sizeof(*kkr6))) == NULL) { - log_warn("%s", __func__); - return (-1); - } - - kkr6->kr = kr; - - LIST_INSERT_HEAD(&kif->kroute6_l, kkr6, entry); - - return (0); -} - -int -kif_kr6_remove(struct kroute6 *kr) -{ - struct kif_node *kif; - struct kif_kr6 *kkr6; - - if ((kif = kif_find(kr->ifindex)) == NULL) { - if (kr->ifindex) - log_warnx("%s: interface with index %u not found", - __func__, kr->ifindex); - return (0); - } - - for (kkr6 = LIST_FIRST(&kif->kroute6_l); kkr6 != NULL && kkr6->kr != kr; - kkr6 = LIST_NEXT(kkr6, entry)) - ; /* nothing */ - - if (kkr6 == NULL) { - log_warnx("%s: can't remove connected route from interface " - "with index %u: not found", __func__, kr->ifindex); - return (-1); - } - - LIST_REMOVE(kkr6, entry); - free(kkr6); - - return (0); -} - /* * nexthop validation */ @@ -2286,7 +2111,6 @@ kif_depend_state(struct kif *kif) if (!(kif->flags & IFF_UP)) return (0); - if (kif->if_type == IFT_CARP && kif->link_state == LINK_STATE_UNKNOWN) return (0); @@ -2362,8 +2186,10 @@ knexthop_validate(struct ktable *kt, struct knexthop *kn) * the route remains the same then the NH state has not * changed. State changes are tracked by knexthop_track(). */ - if (kr != oldk) + if (kr != oldk) { + kn->ifindex = kr->ifindex; knexthop_send_update(kn); + } break; case AID_INET6: kr6 = kroute6_match(kt, &kn->nexthop, 0); @@ -2373,20 +2199,22 @@ knexthop_validate(struct ktable *kt, struct knexthop *kn) kr6->flags |= F_NEXTHOP; } - if (kr6 != oldk) + if (kr6 != oldk) { + kn->ifindex = kr6->ifindex; knexthop_send_update(kn); + } break; } } void -knexthop_track(struct ktable *kt, void *krp) +knexthop_track(struct ktable *kt, u_short ifindex) { struct knexthop *kn; RB_FOREACH(kn, knexthop_tree, KT2KNT(kt)) - if (kn->kroute == krp) - knexthop_send_update(kn); + if (kn->ifindex == ifindex) + knexthop_validate(kt, kn); } void @@ -2664,8 +2492,6 @@ if_change(u_short ifindex, int flags, struct if_data *ifd) { struct ktable *kt; struct kif_node *kif; - struct kif_kr *kkr; - struct kif_kr6 *kkr6; uint8_t reachable; if ((kif = kif_find(ifindex)) == NULL) { @@ -2696,26 +2522,10 @@ if_change(u_short ifindex, int flags, struct if_data *ifd) kif->k.nh_reachable = reachable; kt = ktable_get(kif->k.rdomain); - if (kt == NULL) return; - LIST_FOREACH(kkr, &kif->kroute_l, entry) { - if (reachable) - kkr->kr->flags &= ~F_DOWN; - else - kkr->kr->flags |= F_DOWN; - - knexthop_track(kt, kkr->kr); - } - LIST_FOREACH(kkr6, &kif->kroute6_l, entry) { - if (reachable) - kkr6->kr->flags &= ~F_DOWN; - else - kkr6->kr->flags |= F_DOWN; - - knexthop_track(kt, kkr6->kr); - } + knexthop_track(kt, ifindex); } void @@ -3359,19 +3169,15 @@ kr_fib_change(struct ktable *kt, struct kroute_full *kf, int type, int mpath) kt, kr_tofull(kr)); if ((oflags & F_CONNECTED) && - !(flags & F_CONNECTED)) { - kif_kr_remove(kr); + !(flags & F_CONNECTED)) kr_redistribute(IMSG_NETWORK_ADD, kt, kr_tofull(kr)); - } if ((flags & F_CONNECTED) && - !(oflags & F_CONNECTED)) { - kif_kr_insert(kr); + !(oflags & F_CONNECTED)) kr_redistribute(IMSG_NETWORK_ADD, kt, kr_tofull(kr)); - } if (kr->flags & F_NEXTHOP && changed) - knexthop_track(kt, kr); + knexthop_track(kt, kf->ifindex); } } else { add4: @@ -3433,20 +3239,16 @@ add4: kt, kr6_tofull(kr6)); if ((oflags & F_CONNECTED) && - !(flags & F_CONNECTED)) { - kif_kr6_remove(kr6); + !(flags & F_CONNECTED)) kr_redistribute(IMSG_NETWORK_ADD, kt, kr6_tofull(kr6)); - } if ((flags & F_CONNECTED) && - !(oflags & F_CONNECTED)) { - kif_kr6_insert(kr6); + !(oflags & F_CONNECTED)) kr_redistribute(IMSG_NETWORK_ADD, kt, kr6_tofull(kr6)); - } if (kr6->flags & F_NEXTHOP && changed) - knexthop_track(kt, kr6); + knexthop_track(kt, kf->ifindex); } } else { add6: -- 2.20.1