On Wed, Jun 08, 2022 at 01:28:14PM +0200, Claudio Jeker wrote:
> The idea behind this diff is to use struct kroute_full in more places.
> Mainly when parsing route messages. This allows to factor out the pure
> route message parsing into dispatch_rtmsg_addr() and use it in both the
> gerneral routing socket code but also in fetchtables(). Which removes some
> duplicated code.
>
> As a second step then this kroute_full is applied to the kroute tree. Here
> I limited my changes to be minimal for now. A follow up will look more
> into kr_fib_change() and tries to streamline that code and the bits in
> fetchtable().
>
> Also struct kroute_full could be used in more places with the goal to have
> less mostly similar function for IPv4, IPv6 and the L3VPNs. Again this is
> for later.
>
> This should behave the same as before but dispatch_rtmsg_addr() decides
> now in a different way if a route is connected or not (using the
> RTF_GATEWAY flag for this). With this loopback routes show now up as
> connected. It should have no noticable impact on routing.
>
> Please give this a good test.
I spent some time with this diff. This is outside my comfort zone. I'll
need to read this once more with a fresh head.
I like the direction and less spaghetti is good.
Some small remarks and questions inline.
> --
> :wq Claudio
>
> Index: kroute.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.251
> diff -u -p -r1.251 kroute.c
> --- kroute.c 7 Jun 2022 16:42:07 -0000 1.251
> +++ kroute.c 7 Jun 2022 17:05:37 -0000
> @@ -130,7 +130,7 @@ void kr_fib_update_prio(u_int, uint8_t);
> struct kroute_node *kroute_find(struct ktable *, in_addr_t, uint8_t,
> uint8_t);
> struct kroute_node *kroute_matchgw(struct kroute_node *,
> - struct sockaddr_in *);
> + struct bgpd_addr *);
> int kroute_insert(struct ktable *, struct kroute_node *);
> int kroute_remove(struct ktable *, struct kroute_node *);
> void kroute_clear(struct ktable *);
> @@ -138,7 +138,7 @@ void kroute_clear(struct ktable *);
> struct kroute6_node *kroute6_find(struct ktable *, const struct in6_addr *,
> uint8_t, uint8_t);
> struct kroute6_node *kroute6_matchgw(struct kroute6_node *,
> - struct sockaddr_in6 *);
> + struct bgpd_addr *);
> int kroute6_insert(struct ktable *, struct kroute6_node *);
> int kroute6_remove(struct ktable *, struct kroute6_node *);
> void kroute6_clear(struct ktable *);
> @@ -188,8 +188,9 @@ int send_rt6msg(int, int, struct ktable
> int dispatch_rtmsg(u_int);
> int fetchtable(struct ktable *);
> int fetchifs(int);
> -int dispatch_rtmsg_addr(struct rt_msghdr *,
> - struct sockaddr *[RTAX_MAX], struct ktable *);
> +int dispatch_rtmsg_addr(struct rt_msghdr *, struct kroute_full *);
> +int kr_fib_delete(struct ktable *, struct kroute_full *, int);
> +int kr_fib_change(struct ktable *, struct kroute_full *, int, int);
>
> RB_PROTOTYPE(kroute_tree, kroute_node, entry, kroute_compare)
> RB_GENERATE(kroute_tree, kroute_node, entry, kroute_compare)
> @@ -1780,21 +1781,21 @@ kroute_find(struct ktable *kt, in_addr_t
> }
>
> struct kroute_node *
> -kroute_matchgw(struct kroute_node *kr, struct sockaddr_in *sa_in)
> +kroute_matchgw(struct kroute_node *kr, struct bgpd_addr *gw)
> {
> in_addr_t nexthop;
>
> - if (sa_in == NULL) {
> + if (gw->aid != AID_INET) {
> log_warnx("%s: no nexthop defined", __func__);
> return (NULL);
> }
> - nexthop = sa_in->sin_addr.s_addr;
> + nexthop = gw->v4.s_addr;
>
> - while (kr) {
> + do {
> if (kr->r.nexthop.s_addr == nexthop)
> return (kr);
> kr = kr->next;
> - }
> + } while (kr);
>
> return (NULL);
> }
> @@ -1933,21 +1934,21 @@ kroute6_find(struct ktable *kt, const st
> }
>
> struct kroute6_node *
> -kroute6_matchgw(struct kroute6_node *kr, struct sockaddr_in6 *sa_in6)
> +kroute6_matchgw(struct kroute6_node *kr, struct bgpd_addr *gw)
> {
> struct in6_addr nexthop;
>
> - if (sa_in6 == NULL) {
> + if (gw->aid != AID_INET6) {
> log_warnx("%s: no nexthop defined", __func__);
> return (NULL);
> }
> - memcpy(&nexthop, &sa_in6->sin6_addr, sizeof(nexthop));
> + nexthop = gw->v6;
>
> - while (kr) {
> + do {
> if (memcmp(&kr->r.nexthop, &nexthop, sizeof(nexthop)) == 0)
> return (kr);
> kr = kr->next;
> - }
> + } while (kr);
>
> return (NULL);
> }
> @@ -3180,12 +3181,9 @@ fetchtable(struct ktable *kt)
> int mib[7];
> char *buf = NULL, *next, *lim;
> struct rt_msghdr *rtm;
> - struct sockaddr *sa, *gw, *rti_info[RTAX_MAX];
> - struct sockaddr_in *sa_in;
> - struct sockaddr_in6 *sa_in6;
> - struct sockaddr_rtlabel *label;
> - struct kroute_node *kr = NULL;
> - struct kroute6_node *kr6 = NULL;
> + struct kroute_full kl;
> + struct kroute_node *kr;
> + struct kroute6_node *kr6;
>
> mib[0] = CTL_NET;
> mib[1] = PF_ROUTE;
> @@ -3219,144 +3217,48 @@ fetchtable(struct ktable *kt)
> rtm = (struct rt_msghdr *)next;
> if (rtm->rtm_version != RTM_VERSION)
> continue;
> - sa = (struct sockaddr *)(next + rtm->rtm_hdrlen);
> - get_rtaddrs(rtm->rtm_addrs, sa, rti_info);
> -
> - if ((sa = rti_info[RTAX_DST]) == NULL)
> - continue;
>
> - /* Skip ARP/ND cache and broadcast routes. */
> - if (rtm->rtm_flags & (RTF_LLINFO|RTF_BROADCAST))
> + if (dispatch_rtmsg_addr(rtm, &kl) == -1)
> continue;
>
> - switch (sa->sa_family) {
> - case AF_INET:
> - if ((kr = calloc(1, sizeof(struct kroute_node))) ==
> - NULL) {
> - log_warn("%s", __func__);
> - free(buf);
> - return (-1);
> - }
> -
> - kr->r.flags = F_KERNEL;
> - kr->r.priority = rtm->rtm_priority;
> - kr->r.ifindex = rtm->rtm_index;
> - kr->r.prefix.s_addr =
> - ((struct sockaddr_in *)sa)->sin_addr.s_addr;
> - sa_in = (struct sockaddr_in *)rti_info[RTAX_NETMASK];
> - if (rtm->rtm_flags & RTF_STATIC)
> - kr->r.flags |= F_STATIC;
> - if (rtm->rtm_flags & RTF_BLACKHOLE)
> - kr->r.flags |= F_BLACKHOLE;
> - if (rtm->rtm_flags & RTF_REJECT)
> - kr->r.flags |= F_REJECT;
> - if (rtm->rtm_flags & RTF_DYNAMIC)
> - kr->r.flags |= F_DYNAMIC;
> - if (sa_in != NULL) {
> - if (sa_in->sin_len == 0)
> - break;
> - kr->r.prefixlen =
> - mask2prefixlen(sa_in->sin_addr.s_addr);
> - } else if (rtm->rtm_flags & RTF_HOST)
> - kr->r.prefixlen = 32;
> - else
> - kr->r.prefixlen =
> - prefixlen_classful(kr->r.prefix.s_addr);
> - if ((label = (struct sockaddr_rtlabel *)
> - rti_info[RTAX_LABEL]) != NULL) {
> - kr->r.labelid =
> - rtlabel_name2id(label->sr_label);
> - }
> - break;
> - case AF_INET6:
> - if ((kr6 = calloc(1, sizeof(struct kroute6_node))) ==
> - NULL) {
> + if (kl.prefix.aid == AID_INET) {
> + if ((kr = calloc(1,
> + sizeof(struct kroute_node))) == NULL) {
> log_warn("%s", __func__);
> - free(buf);
> return (-1);
> }
> + kr->r.prefix.s_addr = kl.prefix.v4.s_addr;
> + kr->r.prefixlen = kl.prefixlen;
> + if (kl.nexthop.aid == AID_INET)
> + kr->r.nexthop.s_addr = kl.nexthop.v4.s_addr;
> + kr->r.flags = kl.flags;
> + kr->r.ifindex = kl.ifindex;
> + kr->r.priority = kl.priority;
> + kr->r.labelid = rtlabel_name2id(kl.label);
>
> - kr6->r.flags = F_KERNEL;
> - kr6->r.priority = rtm->rtm_priority;
> - kr6->r.ifindex = rtm->rtm_index;
> - memcpy(&kr6->r.prefix,
> - &((struct sockaddr_in6 *)sa)->sin6_addr,
> - sizeof(kr6->r.prefix));
> -
> - sa_in6 = (struct sockaddr_in6 *)rti_info[RTAX_NETMASK];
> - if (rtm->rtm_flags & RTF_STATIC)
> - kr6->r.flags |= F_STATIC;
> - if (rtm->rtm_flags & RTF_BLACKHOLE)
> - kr6->r.flags |= F_BLACKHOLE;
> - if (rtm->rtm_flags & RTF_REJECT)
> - kr6->r.flags |= F_REJECT;
> - if (rtm->rtm_flags & RTF_DYNAMIC)
> - kr6->r.flags |= F_DYNAMIC;
> - if (sa_in6 != NULL) {
> - if (sa_in6->sin6_len == 0)
> - break;
> - kr6->r.prefixlen = mask2prefixlen6(sa_in6);
> - } else if (rtm->rtm_flags & RTF_HOST)
> - kr6->r.prefixlen = 128;
> - else
> - fatalx("INET6 route without netmask");
> - if ((label = (struct sockaddr_rtlabel *)
> - rti_info[RTAX_LABEL]) != NULL) {
> - kr6->r.labelid =
> - rtlabel_name2id(label->sr_label);
> - }
> - break;
> - default:
> - continue;
> - }
> -
> - if ((gw = rti_info[RTAX_GATEWAY]) != NULL)
> - switch (gw->sa_family) {
> - case AF_INET:
> - if (kr == NULL)
> - fatalx("v4 gateway for !v4 dst?!");
> -
> - if (rtm->rtm_flags & RTF_CONNECTED) {
> - kr->r.flags |= F_CONNECTED;
> - break;
> - }
> -
> - kr->r.nexthop.s_addr =
> - ((struct sockaddr_in *)gw)->sin_addr.s_addr;
> - break;
> - case AF_INET6:
> - if (kr6 == NULL)
> - fatalx("v6 gateway for !v6 dst?!");
> -
> - if (rtm->rtm_flags & RTF_CONNECTED) {
> - kr6->r.flags |= F_CONNECTED;
> - break;
> - }
> -
> - memcpy(&kr6->r.nexthop,
> - &((struct sockaddr_in6 *)gw)->sin6_addr,
> - sizeof(kr6->r.nexthop));
> - break;
> - case AF_LINK:
> - /*
> - * Traditional BSD connected routes have
> - * a gateway of type AF_LINK.
> - */
> - if (sa->sa_family == AF_INET)
> - kr->r.flags |= F_CONNECTED;
> - else if (sa->sa_family == AF_INET6)
> - kr6->r.flags |= F_CONNECTED;
> - break;
> - }
> -
> - if (sa->sa_family == AF_INET) {
> if (rtm->rtm_priority == kr_state.fib_prio) {
> send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r);
> rtlabel_unref(kr->r.labelid);
> free(kr);
> } else
> kroute_insert(kt, kr);
> - } else if (sa->sa_family == AF_INET6) {
> + } else if (kl.prefix.aid == AID_INET6) {
> + if ((kr6 = calloc(1,
> + sizeof(struct kroute6_node))) == NULL) {
> + log_warn("%s", __func__);
> + return (-1);
> + }
> + kr6->r.prefix = kl.prefix.v6;
> + kr6->r.prefixlen = kl.prefixlen;
> + if (kl.nexthop.aid == AID_INET6)
> + kr6->r.nexthop = kl.nexthop.v6;
> + else
> + kr6->r.nexthop = in6addr_any;
> + kr6->r.flags = kl.flags;
> + kr6->r.ifindex = kl.ifindex;
> + kr6->r.priority = kl.priority;
> + kr6->r.labelid = rtlabel_name2id(kl.label);
> +
> if (rtm->rtm_priority == kr_state.fib_prio) {
> send_rt6msg(kr_state.fd, RTM_DELETE, kt,
> &kr6->r);
> @@ -3454,8 +3356,9 @@ dispatch_rtmsg(u_int rdomain)
> char *next, *lim;
> struct rt_msghdr *rtm;
> struct if_msghdr ifm;
> - struct sockaddr *sa, *rti_info[RTAX_MAX];
> + struct kroute_full kl;
> struct ktable *kt;
> + int mpath = 0;
>
> if ((n = read(kr_state.fd, &buf, sizeof(buf))) == -1) {
> if (errno == EAGAIN || errno == EINTR)
> @@ -3482,23 +3385,34 @@ dispatch_rtmsg(u_int rdomain)
> case RTM_ADD:
> case RTM_CHANGE:
> case RTM_DELETE:
> - sa = (struct sockaddr *)(next + rtm->rtm_hdrlen);
> - get_rtaddrs(rtm->rtm_addrs, sa, rti_info);
> -
> if (rtm->rtm_pid == kr_state.pid) /* cause by us */
> continue;
>
> - if (rtm->rtm_errno) /* failed attempts */
> - continue;
> + /* failed attempts */
> + if (rtm->rtm_errno || !(rtm->rtm_flags & RTF_DONE))
> + return (-1);
>
> - if (rtm->rtm_flags & RTF_LLINFO) /* arp cache */
> + if ((kt = ktable_get(rtm->rtm_tableid)) == NULL)
> continue;
>
> - if ((kt = ktable_get(rtm->rtm_tableid)) == NULL)
> + if (dispatch_rtmsg_addr(rtm, &kl) == -1)
> continue;
>
> - if (dispatch_rtmsg_addr(rtm, rti_info, kt) == -1)
> - return (-1);
> + if (rtm->rtm_flags & RTF_MPATH)
> + mpath = 1;
I guess the #ifdef RTF_MPATH was dropped since kroute.c is no longer
built outside of OpenBSD with recent -portable changes?
> +
> + switch (rtm->rtm_type) {
> + case RTM_ADD:
> + case RTM_CHANGE:
> + if (kr_fib_change(kt, &kl, rtm->rtm_type,
> + mpath) == -1)
> + return -1;
> + break;
> + case RTM_DELETE:
> + if (kr_fib_delete(kt, &kl, mpath) == -1)
> + return -1;
> + break;
> + }
> break;
> case RTM_IFINFO:
> memcpy(&ifm, next, sizeof(ifm));
> @@ -3517,191 +3431,190 @@ dispatch_rtmsg(u_int rdomain)
> }
>
> int
> -dispatch_rtmsg_addr(struct rt_msghdr *rtm, struct sockaddr
> *rti_info[RTAX_MAX],
> - struct ktable *kt)
> +dispatch_rtmsg_addr(struct rt_msghdr *rtm, struct kroute_full *kr)
This is the only kroute_full called kr in the entire file. I would
prefer calling it kl (or kf as in the older code you don't touch).
> {
> - struct sockaddr *sa;
> + struct sockaddr *sa, *rti_info[RTAX_MAX];
> struct sockaddr_in *sa_in;
> struct sockaddr_in6 *sa_in6;
> struct sockaddr_rtlabel *label;
> - struct kroute_node *kr;
> - struct kroute6_node *kr6;
> - struct bgpd_addr prefix;
> - int flags, oflags, mpath = 0, changed = 0;
> - int rtlabel_changed = 0;
> - uint16_t ifindex, new_labelid;
> - uint8_t prefixlen;
> - uint8_t prio;
> -
> - flags = F_KERNEL;
> - ifindex = 0;
> - prefixlen = 0;
> - bzero(&prefix, sizeof(prefix));
> +
> + sa = (struct sockaddr *)((char *)rtm + rtm->rtm_hdrlen);
> + get_rtaddrs(rtm->rtm_addrs, sa, rti_info);
> +
> + /* Skip ARP/ND cache and broadcast routes. */
> + if (rtm->rtm_flags & (RTF_LLINFO|RTF_BROADCAST))
> + return (-1);
>
> if ((sa = rti_info[RTAX_DST]) == NULL) {
> - log_warnx("empty route message");
> - return (0);
> + log_warnx("route message without destination");
> + return (-1);
> }
>
> + memset(kr, 0, sizeof(*kr));
> + kr->flags = F_KERNEL;
> +
> if (rtm->rtm_flags & RTF_STATIC)
> - flags |= F_STATIC;
> + kr->flags |= F_STATIC;
> if (rtm->rtm_flags & RTF_BLACKHOLE)
> - flags |= F_BLACKHOLE;
> + kr->flags |= F_BLACKHOLE;
> if (rtm->rtm_flags & RTF_REJECT)
> - flags |= F_REJECT;
> + kr->flags |= F_REJECT;
> if (rtm->rtm_flags & RTF_DYNAMIC)
> - flags |= F_DYNAMIC;
> -#ifdef RTF_MPATH
> - if (rtm->rtm_flags & RTF_MPATH)
> - mpath = 1;
> -#endif
> + kr->flags |= F_DYNAMIC;
>
> - prio = rtm->rtm_priority;
> + kr->priority = rtm->rtm_priority;
> label = (struct sockaddr_rtlabel *)rti_info[RTAX_LABEL];
> + if (label != NULL)
> + if (strlcpy(kr->label, label->sr_label, sizeof(kr->label)) >=
> + sizeof(kr->label))
> + fatalx("rtm label overflow");
>
> + sa2addr(sa, &kr->prefix, NULL);
> switch (sa->sa_family) {
> case AF_INET:
> - prefix.aid = AID_INET;
> - prefix.v4.s_addr = ((struct sockaddr_in *)sa)->sin_addr.s_addr;
> sa_in = (struct sockaddr_in *)rti_info[RTAX_NETMASK];
> if (sa_in != NULL) {
> if (sa_in->sin_len != 0)
> - prefixlen = mask2prefixlen(
> + kr->prefixlen = mask2prefixlen(
> sa_in->sin_addr.s_addr);
I would wrap this the same way as the assignment a few lines down:
kr->prefixlen =
mask2prefixlen(sa_in->sin_addr.s_addr);
> } else if (rtm->rtm_flags & RTF_HOST)
> - prefixlen = 32;
> + kr->prefixlen = 32;
> else
> - prefixlen =
> - prefixlen_classful(prefix.v4.s_addr);
> + kr->prefixlen =
> + prefixlen_classful(kr->prefix.v4.s_addr);
> break;
> case AF_INET6:
> - prefix.aid = AID_INET6;
> - memcpy(&prefix.v6, &((struct sockaddr_in6 *)sa)->sin6_addr,
> - sizeof(struct in6_addr));
> sa_in6 = (struct sockaddr_in6 *)rti_info[RTAX_NETMASK];
> if (sa_in6 != NULL) {
> if (sa_in6->sin6_len != 0)
> - prefixlen = mask2prefixlen6(sa_in6);
> + kr->prefixlen = mask2prefixlen6(sa_in6);
> } else if (rtm->rtm_flags & RTF_HOST)
> - prefixlen = 128;
> + kr->prefixlen = 128;
> else
> fatalx("in6 net addr without netmask");
> break;
> default:
> - return (0);
> + return (-1);
> }
>
> - if ((sa = rti_info[RTAX_GATEWAY]) != NULL)
> + if ((sa = rti_info[RTAX_GATEWAY]) == NULL) {
> + log_warnx("route %s/%u without gateway",
> + log_addr(&kr->prefix), kr->prefixlen);
> + return (-1);
> + }
> +
> + if (rtm->rtm_flags & RTF_GATEWAY) {
> switch (sa->sa_family) {
> case AF_LINK:
> - flags |= F_CONNECTED;
> - ifindex = rtm->rtm_index;
> - sa = NULL;
> - mpath = 0; /* link local stuff can't be mpath */
> + kr->flags |= F_CONNECTED;
> + kr->ifindex = rtm->rtm_index;
> break;
> case AF_INET:
> case AF_INET6:
> - if (rtm->rtm_flags & RTF_CONNECTED) {
> - flags |= F_CONNECTED;
> - ifindex = rtm->rtm_index;
> - sa = NULL;
> - mpath = 0; /* link local stuff can't be mpath */
> - }
> + sa2addr(rti_info[RTAX_GATEWAY], &kr->nexthop, NULL);
> break;
> }
> + } else {
> + kr->flags |= F_CONNECTED;
> + kr->ifindex = rtm->rtm_index;
> + }
>
> - if (rtm->rtm_type == RTM_DELETE) {
> - switch (prefix.aid) {
> - case AID_INET:
> - sa_in = (struct sockaddr_in *)sa;
> - if ((kr = kroute_find(kt, prefix.v4.s_addr,
> - prefixlen, prio)) == NULL)
> - return (0);
> - if (!(kr->r.flags & F_KERNEL))
> - return (0);
> + return (0);
> +}
>
> - if (mpath)
> - /* get the correct route */
> - if ((kr = kroute_matchgw(kr, sa_in)) == NULL) {
> - log_warnx("%s[delete]: "
> - "mpath route not found", __func__);
> - return (0);
> - }
> +int
> +kr_fib_delete(struct ktable *kt, struct kroute_full *kl, int mpath)
> +{
> + struct kroute_node *kr;
> + struct kroute6_node *kr6;
>
> - if (kroute_remove(kt, kr) == -1)
> - return (-1);
> - break;
> - case AID_INET6:
> - sa_in6 = (struct sockaddr_in6 *)sa;
> - if ((kr6 = kroute6_find(kt, &prefix.v6, prefixlen,
> - prio)) == NULL)
> - return (0);
> - if (!(kr6->r.flags & F_KERNEL))
> - return (0);
> + switch (kl->prefix.aid) {
> + case AID_INET:
> + if ((kr = kroute_find(kt, kl->prefix.v4.s_addr,
> + kl->prefixlen, kl->priority)) == NULL)
> + return (0);
> + if (!(kr->r.flags & F_KERNEL))
> + return (0);
>
> - if (mpath)
> - /* get the correct route */
> - if ((kr6 = kroute6_matchgw(kr6, sa_in6)) ==
> - NULL) {
> - log_warnx("%s[delete]: IPv6 mpath "
> - "route not found", __func__);
> - return (0);
> - }
> + if (mpath) {
> + /* get the correct route */
> + if ((kr = kroute_matchgw(kr, &kl->nexthop)) == NULL) {
> + log_warnx("delete %s/%u: route not found",
> + log_addr(&kl->prefix), kl->prefixlen);
> + return (0);
> + }
> + }
> + if (kroute_remove(kt, kr) == -1)
> + return (-1);
> + break;
> + case AID_INET6:
> + if ((kr6 = kroute6_find(kt, &kl->prefix.v6, kl->prefixlen,
> + kl->priority)) == NULL)
> + return (0);
> + if (!(kr6->r.flags & F_KERNEL))
> + return (0);
>
> - if (kroute6_remove(kt, kr6) == -1)
> - return (-1);
> - break;
> + if (mpath) {
> + /* get the correct route */
> + if ((kr6 = kroute6_matchgw(kr6, &kl->nexthop)) ==
> + NULL) {
> + log_warnx("delete %s/%u: route not found",
> + log_addr(&kl->prefix), kl->prefixlen);
> + return (0);
> + }
> }
> - return (0);
> + if (kroute6_remove(kt, kr6) == -1)
> + return (-1);
> + break;
> }
> + return (0);
> +}
>
> - if (sa == NULL && !(flags & F_CONNECTED)) {
> - log_warnx("%s: no nexthop for %s/%u",
> - __func__, log_addr(&prefix), prefixlen);
> - return (0);
> - }
> +int
> +kr_fib_change(struct ktable *kt, struct kroute_full *kl, int type, int mpath)
> +{
> + struct kroute_node *kr;
> + struct kroute6_node *kr6;
> + int flags, oflags;
> + int changed = 0, rtlabel_changed = 0;
> + uint16_t new_labelid;
>
> - switch (prefix.aid) {
> + flags = kl->flags;
> + switch (kl->prefix.aid) {
> case AID_INET:
> - sa_in = (struct sockaddr_in *)sa;
> - if ((kr = kroute_find(kt, prefix.v4.s_addr, prefixlen,
> - prio)) != NULL) {
> + if ((kr = kroute_find(kt, kl->prefix.v4.s_addr, kl->prefixlen,
> + kl->priority)) != NULL) {
> if (kr->r.flags & F_KERNEL) {
> /* get the correct route */
> - if (mpath && rtm->rtm_type == RTM_CHANGE &&
> - (kr = kroute_matchgw(kr, sa_in)) == NULL) {
> + if (mpath && type == RTM_CHANGE &&
> + (kr = kroute_matchgw(kr, &kl->nexthop)) ==
> + NULL) {
> log_warnx("%s[change]: "
> "mpath route not found", __func__);
> goto add4;
> - } else if (mpath && rtm->rtm_type == RTM_ADD)
> + } else if (mpath && type == RTM_ADD)
> goto add4;
>
> - if (sa_in != NULL) {
> + if (kl->nexthop.aid == AID_INET) {
> if (kr->r.nexthop.s_addr !=
> - sa_in->sin_addr.s_addr)
> + kl->nexthop.v4.s_addr)
> changed = 1;
> kr->r.nexthop.s_addr =
> - sa_in->sin_addr.s_addr;
> + kl->nexthop.v4.s_addr;
why is there no kr->r.ifindex assignment here?
> } else {
> if (kr->r.nexthop.s_addr != 0)
> changed = 1;
> kr->r.nexthop.s_addr = 0;
> + kr->r.ifindex = kl->ifindex;
> }
>
> if (kr->r.flags & F_NEXTHOP)
> flags |= F_NEXTHOP;
>
> - if (label != NULL) {
> - new_labelid =
> - rtlabel_name2id(label->sr_label);
> - if (kr->r.labelid != new_labelid) {
> - rtlabel_unref(kr->r.labelid);
> - kr->r.labelid = new_labelid;
> - rtlabel_changed = 1;
> - }
> - } else if (kr->r.labelid) {
> + new_labelid = rtlabel_name2id(kl->label);
> + if (kr->r.labelid != new_labelid) {
> rtlabel_unref(kr->r.labelid);
> - kr->r.labelid = 0;
> + kr->r.labelid = new_labelid;
> rtlabel_changed = 1;
> }
>
> @@ -3729,10 +3642,6 @@ dispatch_rtmsg_addr(struct rt_msghdr *rt
> if (kr->r.flags & F_NEXTHOP && changed)
> knexthop_track(kt, kr);
> }
> - } else if (rtm->rtm_type == RTM_CHANGE) {
> - log_warnx("%s: change req for %s/%u: not in table",
> - __func__, log_addr(&prefix), prefixlen);
> - return (0);
> } else {
> add4:
> if ((kr = calloc(1,
> @@ -3740,70 +3649,54 @@ add4:
> log_warn("%s", __func__);
> return (-1);
> }
> - kr->r.prefix.s_addr = prefix.v4.s_addr;
> - kr->r.prefixlen = prefixlen;
> - if (sa_in != NULL)
> - kr->r.nexthop.s_addr = sa_in->sin_addr.s_addr;
> - else
> - kr->r.nexthop.s_addr = 0;
> + kr->r.prefix.s_addr = kl->prefix.v4.s_addr;
> + kr->r.prefixlen = kl->prefixlen;
> + if (kl->nexthop.aid == AID_INET)
> + kr->r.nexthop.s_addr = kl->nexthop.v4.s_addr;
> kr->r.flags = flags;
> - kr->r.ifindex = ifindex;
> - kr->r.priority = prio;
> + kr->r.ifindex = kl->ifindex;
> + kr->r.priority = kl->priority;
> + kr->r.labelid = rtlabel_name2id(kl->label);
>
> - if (label) {
> - kr->r.labelid =
> - rtlabel_name2id(label->sr_label);
> - }
> kroute_insert(kt, kr);
> }
> break;
> case AID_INET6:
> - sa_in6 = (struct sockaddr_in6 *)sa;
> - if ((kr6 = kroute6_find(kt, &prefix.v6, prefixlen, prio)) !=
> - NULL) {
> + if ((kr6 = kroute6_find(kt, &kl->prefix.v6, kl->prefixlen,
> + kl->priority)) != NULL) {
> if (kr6->r.flags & F_KERNEL) {
> /* get the correct route */
> - if (mpath && rtm->rtm_type == RTM_CHANGE &&
> - (kr6 = kroute6_matchgw(kr6, sa_in6)) ==
> - NULL) {
> + if (mpath && type == RTM_CHANGE &&
> + (kr6 = kroute6_matchgw(kr6, &kl->nexthop))
> + == NULL) {
> log_warnx("%s[change]: IPv6 mpath "
> "route not found", __func__);
> goto add6;
> - } else if (mpath && rtm->rtm_type == RTM_ADD)
> + } else if (mpath && type == RTM_ADD)
> goto add6;
>
> - if (sa_in6 != NULL) {
> + if (kl->nexthop.aid == AID_INET6) {
> if (memcmp(&kr6->r.nexthop,
> - &sa_in6->sin6_addr,
> + &kl->nexthop.v6,
> sizeof(struct in6_addr)))
> changed = 1;
> - memcpy(&kr6->r.nexthop,
> - &sa_in6->sin6_addr,
> - sizeof(struct in6_addr));
> + kr6->r.nexthop = kl->nexthop.v6;
> } else {
> if (memcmp(&kr6->r.nexthop,
> &in6addr_any,
> sizeof(struct in6_addr)))
> changed = 1;
> - memcpy(&kr6->r.nexthop,
> - &in6addr_any,
> - sizeof(struct in6_addr));
> + kr6->r.nexthop = in6addr_any;
> + kr6->r.ifindex = kl->ifindex;
> }
>
> if (kr6->r.flags & F_NEXTHOP)
> flags |= F_NEXTHOP;
>
> - if (label != NULL) {
> - new_labelid =
> - rtlabel_name2id(label->sr_label);
> - if (kr6->r.labelid != new_labelid) {
> - rtlabel_unref(kr6->r.labelid);
> - kr6->r.labelid = new_labelid;
> - rtlabel_changed = 1;
> - }
> - } else if (kr6->r.labelid) {
> + new_labelid = rtlabel_name2id(kl->label);
> + if (kr6->r.labelid != new_labelid) {
> rtlabel_unref(kr6->r.labelid);
> - kr6->r.labelid = 0;
> + kr6->r.labelid = new_labelid;
> rtlabel_changed = 1;
> }
>
> @@ -3832,10 +3725,6 @@ add4:
> if (kr6->r.flags & F_NEXTHOP && changed)
> knexthop_track(kt, kr6);
> }
> - } else if (rtm->rtm_type == RTM_CHANGE) {
> - log_warnx("%s: change req for %s/%u: not in table",
> - __func__, log_addr(&prefix), prefixlen);
> - return (0);
> } else {
> add6:
> if ((kr6 = calloc(1,
> @@ -3843,23 +3732,17 @@ add6:
> log_warn("%s", __func__);
> return (-1);
> }
> - memcpy(&kr6->r.prefix, &prefix.v6,
> - sizeof(struct in6_addr));
> - kr6->r.prefixlen = prefixlen;
> - if (sa_in6 != NULL)
> - memcpy(&kr6->r.nexthop, &sa_in6->sin6_addr,
> - sizeof(struct in6_addr));
> + kr6->r.prefix = kl->prefix.v6;
> + kr6->r.prefixlen = kl->prefixlen;
> + if (kl->nexthop.aid == AID_INET6)
> + kr6->r.nexthop = kl->nexthop.v6;
> else
> - memcpy(&kr6->r.nexthop, &in6addr_any,
> - sizeof(struct in6_addr));
> + kr6->r.nexthop = in6addr_any;
> kr6->r.flags = flags;
> - kr6->r.ifindex = ifindex;
> - kr6->r.priority = prio;
> + kr6->r.ifindex = kl->ifindex;
> + kr6->r.priority = kl->priority;
> + kr6->r.labelid = rtlabel_name2id(kl->label);
>
> - if (label) {
> - kr6->r.labelid =
> - rtlabel_name2id(label->sr_label);
> - }
> kroute6_insert(kt, kr6);
> }
> break;
>
