Next step on the epic saga of cleaning up kroute.c
Refactor kroute_remove() so that a struct kroute_full can be passed to the
function. It updates the struct kroute_full with the route that got removed.
I split the code into kroute[46]_remove() to make kroute_remove() less
cluttered. The return value of those two functions is a bit overloaded but
I hope to make this better further down the road.
--
:wq Claudio
Index: kroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.283
diff -u -p -r1.283 kroute.c
--- kroute.c 27 Jul 2022 17:23:17 -0000 1.283
+++ kroute.c 28 Jul 2022 09:20:50 -0000
@@ -123,10 +123,6 @@ int kr4_change(struct ktable *, struct k
int kr6_change(struct ktable *, struct kroute_full *);
int krVPN4_change(struct ktable *, struct kroute_full *);
int krVPN6_change(struct ktable *, struct kroute_full *);
-int kr4_delete(struct ktable *, struct kroute_full *);
-int kr6_delete(struct ktable *, struct kroute_full *);
-int krVPN4_delete(struct ktable *, struct kroute_full *);
-int krVPN6_delete(struct ktable *, struct kroute_full *);
int kr_net_match(struct ktable *, struct network_config *, uint16_t, int);
struct network *kr_net_find(struct ktable *, struct network *);
void kr_net_clear(struct ktable *);
@@ -144,18 +140,17 @@ struct kroute *kroute_find(struct ktable
uint8_t, uint8_t);
struct kroute *kroute_matchgw(struct kroute *, struct bgpd_addr *);
int kroute_insert(struct ktable *, struct kroute_full *);
-int kroute_remove(struct ktable *, struct kroute *);
+int kroute_remove(struct ktable *, struct kroute_full *, int);
void kroute_clear(struct ktable *);
struct kroute6 *kroute6_find(struct ktable *, const struct bgpd_addr *,
uint8_t, uint8_t);
struct kroute6 *kroute6_matchgw(struct kroute6 *, struct bgpd_addr *);
-int kroute6_remove(struct ktable *, struct kroute6 *);
void kroute6_clear(struct ktable *);
struct knexthop *knexthop_find(struct ktable *, struct bgpd_addr *);
int knexthop_insert(struct ktable *, struct knexthop *);
-int knexthop_remove(struct ktable *, struct knexthop *);
+void knexthop_remove(struct ktable *, struct knexthop *);
void knexthop_clear(struct ktable *);
struct kif_node *kif_find(int);
@@ -662,18 +657,7 @@ kr_delete(u_int rtableid, struct kroute_
return (0);
kf->flags |= F_BGPD;
kf->priority = RTP_MINE;
- switch (kf->prefix.aid) {
- case AID_INET:
- return (kr4_delete(kt, kf));
- case AID_INET6:
- return (kr6_delete(kt, kf));
- case AID_VPN_IPv4:
- return (krVPN4_delete(kt, kf));
- case AID_VPN_IPv6:
- return (krVPN6_delete(kt, kf));
- }
- log_warnx("%s: not handled AID", __func__);
- return (-1);
+ return kroute_remove(kt, kf, 1);
}
int
@@ -689,18 +673,12 @@ kr_flush(u_int rtableid)
RB_FOREACH_SAFE(kr, kroute_tree, &kt->krt, next)
if ((kr->flags & F_BGPD_INSERTED)) {
- if (kt->fib_sync) /* coupled */
- send_rtmsg(kr_state.fd, RTM_DELETE, kt,
- kr_tofull(kr));
- if (kroute_remove(kt, kr) == -1)
+ if (kroute_remove(kt, kr_tofull(kr), 1) == -1)
return (-1);
}
RB_FOREACH_SAFE(kr6, kroute6_tree, &kt->krt6, next6)
if ((kr6->flags & F_BGPD_INSERTED)) {
- if (kt->fib_sync) /* coupled */
- send_rtmsg(kr_state.fd, RTM_DELETE, kt,
- kr6_tofull(kr6));
- if (kroute6_remove(kt, kr6) == -1)
+ if (kroute_remove(kt, kr6_tofull(kr6), 1) == -1)
return (-1);
}
@@ -708,86 +686,6 @@ kr_flush(u_int rtableid)
return (0);
}
-int
-kr4_delete(struct ktable *kt, struct kroute_full *kf)
-{
- struct kroute *kr;
-
- if ((kr = kroute_find(kt, &kf->prefix, kf->prefixlen,
- kf->priority)) == NULL)
- return (0);
-
- if (!(kr->flags & F_BGPD_INSERTED))
- return (0);
-
- send_rtmsg(kr_state.fd, RTM_DELETE, kt, kr_tofull(kr));
-
- if (kroute_remove(kt, kr) == -1)
- return (-1);
-
- return (0);
-}
-
-int
-kr6_delete(struct ktable *kt, struct kroute_full *kf)
-{
- struct kroute6 *kr6;
-
- if ((kr6 = kroute6_find(kt, &kf->prefix, kf->prefixlen,
- kf->priority)) == NULL)
- return (0);
-
- if (!(kr6->flags & F_BGPD_INSERTED))
- return (0);
-
- send_rtmsg(kr_state.fd, RTM_DELETE, kt, kr6_tofull(kr6));
-
- if (kroute6_remove(kt, kr6) == -1)
- return (-1);
-
- return (0);
-}
-
-int
-krVPN4_delete(struct ktable *kt, struct kroute_full *kf)
-{
- struct kroute *kr;
-
- if ((kr = kroute_find(kt, &kf->prefix, kf->prefixlen,
- kf->priority)) == NULL)
- return (0);
-
- if (!(kr->flags & F_BGPD_INSERTED))
- return (0);
-
- send_rtmsg(kr_state.fd, RTM_DELETE, kt, kr_tofull(kr));
-
- if (kroute_remove(kt, kr) == -1)
- return (-1);
-
- return (0);
-}
-
-int
-krVPN6_delete(struct ktable *kt, struct kroute_full *kf)
-{
- struct kroute6 *kr6;
-
- if ((kr6 = kroute6_find(kt, &kf->prefix, kf->prefixlen,
- kf->priority)) == NULL)
- return (0);
-
- if (!(kr6->flags & F_BGPD_INSERTED))
- return (0);
-
- send_rtmsg(kr_state.fd, RTM_DELETE, kt, kr6_tofull(kr6));
-
- if (kroute6_remove(kt, kr6) == -1)
- return (-1);
-
- return (0);
-}
-
void
kr_shutdown(void)
{
@@ -1711,7 +1609,7 @@ kroute_insert(struct ktable *kt, struct
{
struct kroute *kr, *krm;
struct kroute6 *kr6, *kr6m;
- struct knexthop *h;
+ struct knexthop *n;
uint32_t mplslabel = 0;
if (kf->prefix.aid == AID_VPN_IPv4 ||
@@ -1800,10 +1698,10 @@ kroute_insert(struct ktable *kt, struct
/* XXX this is wrong for nexthop validated via BGP */
if (!(kf->flags & F_BGPD)) {
- RB_FOREACH(h, knexthop_tree, KT2KNT(kt))
- if (prefix_compare(&kf->prefix, &h->nexthop,
+ RB_FOREACH(n, knexthop_tree, KT2KNT(kt))
+ if (prefix_compare(&kf->prefix, &n->nexthop,
kf->prefixlen) == 0)
- knexthop_validate(kt, h);
+ knexthop_validate(kt, n);
if (krm == NULL)
/* redistribute multipath routes only once */
@@ -1814,57 +1712,163 @@ kroute_insert(struct ktable *kt, struct
}
-int
-kroute_remove(struct ktable *kt, struct kroute *kr)
+static int
+kroute4_remove(struct ktable *kt, struct kroute_full *kf, int any)
{
- struct kroute *krm;
- struct knexthop *s;
+ struct kroute *kr, *krm;
+ int multipath = 0;
- if ((krm = RB_FIND(kroute_tree, &kt->krt, kr)) == NULL) {
- log_warnx("%s: failed to find %s/%u", __func__,
- inet_ntoa(kr->prefix), kr->prefixlen);
+ if ((kr = kroute_find(kt, &kf->prefix, kf->prefixlen,
+ kf->priority)) == NULL)
+ return (-1);
+
+ if ((kr->flags & F_BGPD) != (kf->flags & F_BGPD)) {
+ log_warnx("%s: wrong type for %s/%u", __func__,
+ log_addr(&kf->prefix), kf->prefixlen);
+ if (!(kf->flags & F_BGPD))
+ kr->flags &= ~F_BGPD_INSERTED;
return (-1);
}
+ /* get the correct route to remove */
+ if (any) {
+ krm = kr;
+ } else {
+ if ((krm = kroute_matchgw(kr, &kf->nexthop)) == NULL) {
+ log_warnx("delete %s/%u: route not found",
+ log_addr(&kf->prefix), kf->prefixlen);
+ return (-2);
+ }
+ }
+
if (krm == kr) {
/* head element */
- if (RB_REMOVE(kroute_tree, &kt->krt, kr) == NULL) {
- log_warnx("%s: failed for %s/%u", __func__,
- inet_ntoa(kr->prefix), kr->prefixlen);
- return (-1);
+ RB_REMOVE(kroute_tree, &kt->krt, kr);
+ if (kr->next != NULL) {
+ kr = kr->next;
+ if (RB_INSERT(kroute_tree, &kt->krt, kr) != NULL) {
+ log_warnx("%s: failed to add %s/%u",
+ __func__, inet_ntoa(kr->prefix),
+ kr->prefixlen);
+ return (-2);
+ }
+ multipath = 1;
}
+ } else {
+ /* somewhere in the list */
+ while (kr->next != krm && kr->next != NULL)
+ kr = kr->next;
+ if (kr->next == NULL) {
+ log_warnx("%s: multipath list corrupted for %s/%u",
+ __func__, inet_ntoa(kr->prefix), kr->prefixlen);
+ return (-2);
+ }
+ kr->next = krm->next;
+ multipath = 1;
+ }
+ *kf = *kr_tofull(krm);
+
+ rtlabel_unref(krm->labelid);
+ free(krm);
+ return (multipath);
+}
+
+static int
+kroute6_remove(struct ktable *kt, struct kroute_full *kf, int any)
+{
+ struct kroute6 *kr, *krm;
+ int multipath = 0;
+
+ if ((kr = kroute6_find(kt, &kf->prefix, kf->prefixlen,
+ kf->priority)) == NULL)
+ return (-1);
+
+ if ((kr->flags & F_BGPD) != (kf->flags & F_BGPD)) {
+ log_warnx("%s: wrong type for %s/%u", __func__,
+ log_addr(&kf->prefix), kf->prefixlen);
+ if (!(kf->flags & F_BGPD))
+ kr->flags &= ~F_BGPD_INSERTED;
+ return (-1);
+ }
+
+ /* get the correct route to remove */
+ if (any) {
+ krm = kr;
+ } else {
+ if ((krm = kroute6_matchgw(kr, &kf->nexthop)) == NULL) {
+ log_warnx("delete %s/%u: route not found",
+ log_addr(&kf->prefix), kf->prefixlen);
+ return (-2);
+ }
+ }
+
+ if (krm == kr) {
+ /* head element */
+ RB_REMOVE(kroute6_tree, &kt->krt6, kr);
if (kr->next != NULL) {
- if (RB_INSERT(kroute_tree, &kt->krt, kr->next) !=
- NULL) {
+ kr = kr->next;
+ if (RB_INSERT(kroute6_tree, &kt->krt6, kr) != NULL) {
log_warnx("%s: failed to add %s/%u", __func__,
- inet_ntoa(kr->prefix), kr->prefixlen);
- return (-1);
+ log_in6addr(&kr->prefix), kr->prefixlen);
+ return (-2);
}
+ multipath = 1;
}
} else {
/* somewhere in the list */
- while (krm->next != kr && krm->next != NULL)
- krm = krm->next;
- if (krm->next == NULL) {
+ while (kr->next != krm && kr->next != NULL)
+ kr = kr->next;
+ if (kr->next == NULL) {
log_warnx("%s: multipath list corrupted for %s/%u",
- __func__, inet_ntoa(kr->prefix), kr->prefixlen);
- return (-1);
+ __func__, log_in6addr(&kr->prefix), kr->prefixlen);
+ return (-2);
}
- krm->next = kr->next;
+ kr->next = krm->next;
+ multipath = 1;
+ }
+ *kf = *kr6_tofull(krm);
+
+ rtlabel_unref(krm->labelid);
+ free(krm);
+ return (multipath);
+}
+
+
+int
+kroute_remove(struct ktable *kt, struct kroute_full *kf, int any)
+{
+ struct knexthop *n;
+ int multipath;
+
+ switch (kf->prefix.aid) {
+ case AID_INET:
+ multipath = kroute4_remove(kt, kf, any);
+ break;
+ case AID_INET6:
+ multipath = kroute6_remove(kt, kf, any);
+ break;
+ default:
+ log_warnx("%s: not handled AID", __func__);
+ return (-1);
}
+ if (multipath < 0)
+ return (multipath + 1);
+
/* check whether a nexthop depends on this kroute */
- if (kr->flags & F_NEXTHOP)
- RB_FOREACH(s, knexthop_tree, KT2KNT(kt))
- if (s->kroute == kr)
- knexthop_validate(kt, s);
+ if (kf->flags & F_NEXTHOP)
+ RB_FOREACH(n, knexthop_tree, KT2KNT(kt))
+ if (prefix_compare(&kf->prefix, &n->nexthop,
+ kf->prefixlen) == 0)
+ knexthop_validate(kt, n);
- if (!(kr->flags & F_BGPD) && kr == krm && kr->next == NULL)
+ if (!((kf->flags & F_BGPD) || multipath))
/* again remove only once */
- kr_redistribute(IMSG_NETWORK_REMOVE, kt, kr_tofull(kr));
+ kr_redistribute(IMSG_NETWORK_REMOVE, kt, kf);
+
+ if (kf->flags & F_BGPD_INSERTED)
+ send_rtmsg(kr_state.fd, RTM_DELETE, kt, kf);
- rtlabel_unref(kr->labelid);
- free(kr);
return (0);
}
@@ -1874,7 +1878,7 @@ kroute_clear(struct ktable *kt)
struct kroute *kr;
while ((kr = RB_MIN(kroute_tree, &kt->krt)) != NULL)
- kroute_remove(kt, kr);
+ kroute_remove(kt, kr_tofull(kr), 1);
}
struct kroute6 *
@@ -1924,67 +1928,13 @@ kroute6_matchgw(struct kroute6 *kr, stru
return (NULL);
}
-int
-kroute6_remove(struct ktable *kt, struct kroute6 *kr)
-{
- struct kroute6 *krm;
- struct knexthop *s;
-
- if ((krm = RB_FIND(kroute6_tree, &kt->krt6, kr)) == NULL) {
- log_warnx("%s: failed for %s/%u", __func__,
- log_in6addr(&kr->prefix), kr->prefixlen);
- return (-1);
- }
-
- if (krm == kr) {
- /* head element */
- if (RB_REMOVE(kroute6_tree, &kt->krt6, kr) == NULL) {
- log_warnx("%s: failed for %s/%u", __func__,
- log_in6addr(&kr->prefix), kr->prefixlen);
- return (-1);
- }
- if (kr->next != NULL) {
- if (RB_INSERT(kroute6_tree, &kt->krt6, kr->next) !=
- NULL) {
- log_warnx("%s: failed to add %s/%u", __func__,
- log_in6addr(&kr->prefix), kr->prefixlen);
- return (-1);
- }
- }
- } else {
- /* somewhere in the list */
- while (krm->next != kr && krm->next != NULL)
- krm = krm->next;
- if (krm->next == NULL) {
- log_warnx("%s: multipath list corrupted for %s/%u",
- __func__, log_in6addr(&kr->prefix), kr->prefixlen);
- return (-1);
- }
- krm->next = kr->next;
- }
-
- /* check whether a nexthop depends on this kroute */
- if (kr->flags & F_NEXTHOP)
- RB_FOREACH(s, knexthop_tree, KT2KNT(kt))
- if (s->kroute == kr)
- knexthop_validate(kt, s);
-
- if (!(kr->flags & F_BGPD) && kr == krm && kr->next == NULL)
- /* again remove only once */
- kr_redistribute(IMSG_NETWORK_REMOVE, kt, kr6_tofull(kr));
-
- rtlabel_unref(kr->labelid);
- free(kr);
- return (0);
-}
-
void
kroute6_clear(struct ktable *kt)
{
struct kroute6 *kr;
while ((kr = RB_MIN(kroute6_tree, &kt->krt6)) != NULL)
- kroute6_remove(kt, kr);
+ kroute_remove(kt, kr6_tofull(kr), 1);
}
struct knexthop *
@@ -2013,19 +1963,12 @@ knexthop_insert(struct ktable *kt, struc
return (0);
}
-int
+void
knexthop_remove(struct ktable *kt, struct knexthop *kn)
{
kroute_detach_nexthop(kt, kn);
-
- if (RB_REMOVE(knexthop_tree, KT2KNT(kt), kn) == NULL) {
- log_warnx("%s: failed for %s", __func__,
- log_addr(&kn->nexthop));
- return (-1);
- }
-
+ RB_REMOVE(knexthop_tree, KT2KNT(kt), kn);
free(kn);
- return (0);
}
void
@@ -3082,51 +3025,7 @@ dispatch_rtmsg_addr(struct rt_msghdr *rt
int
kr_fib_delete(struct ktable *kt, struct kroute_full *kf, int mpath)
{
- struct kroute *kr;
- struct kroute6 *kr6;
-
- switch (kf->prefix.aid) {
- case AID_INET:
- if ((kr = kroute_find(kt, &kf->prefix, kf->prefixlen,
- kf->priority)) == NULL)
- return (0);
- if (mpath) {
- /* get the correct route */
- if ((kr = kroute_matchgw(kr, &kf->nexthop)) == NULL) {
- log_warnx("delete %s/%u: route not found",
- log_addr(&kf->prefix), kf->prefixlen);
- return (0);
- }
- }
- if (kf->flags & F_BGPD) {
- kr->flags &= ~F_BGPD_INSERTED;
- return (0);
- }
- if (kroute_remove(kt, kr) == -1)
- return (-1);
- break;
- case AID_INET6:
- if ((kr6 = kroute6_find(kt, &kf->prefix, kf->prefixlen,
- kf->priority)) == NULL)
- return (0);
- if (mpath) {
- /* get the correct route */
- if ((kr6 = kroute6_matchgw(kr6, &kf->nexthop)) ==
- NULL) {
- log_warnx("delete %s/%u: route not found",
- log_addr(&kf->prefix), kf->prefixlen);
- return (0);
- }
- }
- if (kf->flags & F_BGPD) {
- kr6->flags &= ~F_BGPD_INSERTED;
- return (0);
- }
- if (kroute6_remove(kt, kr6) == -1)
- return (-1);
- break;
- }
- return (0);
+ return kroute_remove(kt, kf, !mpath);
}
int
