On Wed, Jun 08, 2022 at 10:47:48PM +0200, Claudio Jeker wrote:
> and here is the updated diff I forgot to include 

I think I've now done what I reasonably can do to review this. I buy
that this mostly preserves behavior and I'm convinced that it is a step
in the right direction, especially given the outline of your planned
next steps.

I'm ok with this going in once you think you've seen enough tests.

A few more things I spotted below.

> 
> -- 
> :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  8 Jun 2022 16:23:51 -0000
> @@ -125,12 +125,11 @@ int     kroute6_compare(struct kroute6_node 
>  int  knexthop_compare(struct knexthop_node *, struct knexthop_node *);
>  int  kredist_compare(struct kredist_node *, struct kredist_node *);
>  int  kif_compare(struct kif_node *, struct kif_node *);
> -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 +137,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 +187,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 +1780,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 +1933,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 +3180,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,145 +3216,49 @@ 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)
> +             if (dispatch_rtmsg_addr(rtm, &kl) == -1)
>                       continue;
>  
> -             /* Skip ARP/ND cache and broadcast routes. */
> -             if (rtm->rtm_flags & (RTF_LLINFO|RTF_BROADCAST))
> -                     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) {

I'd probably avoid the line break with sizeof(*kr).

>                               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);

If kl.label is the empty string, this will now set errno to EINVAL. This
was previously avoided with the NULL check for label.

It might be appropriate to add proper error checking down the road.

>  
> -                     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;
> -                             }

I can't spot where this setting of F_CONNECTED is handled after the
refactor. The path doing sa2addr(..., &kl->nexthop, NULL) in the new
dispatch_rtmsg_addr() explicitly avoids setting that flag.

Perhaps this is part of the switch to using RTF_GATEWAY you mentioned...

> -
> -                             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;
> -                             }

ditto.

> -
> -                             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) {
> +                     if (kl.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) {
> -                     if (rtm->rtm_priority == kr_state.fib_prio) {
> +             } else if (kl.prefix.aid == AID_INET6) {
> +                     if ((kr6 = calloc(1,
> +                         sizeof(struct kroute6_node))) == NULL) {

Same re linewrap and sizeof(*kr6)

> +                             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);

Same here re EINVAL if kl.label is "" and future error checking.

> +
> +                     if (kl.priority == kr_state.fib_prio) {
>                               send_rt6msg(kr_state.fd, RTM_DELETE, kt,
>                                   &kr6->r);
>                               rtlabel_unref(kr6->r.labelid);
> @@ -3454,8 +3355,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 +3384,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;
> +
> +                     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 +3430,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 *kl)
>  {
> -     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(kl, 0, sizeof(*kl));
> +     kl->flags = F_KERNEL;
> +
>       if (rtm->rtm_flags & RTF_STATIC)
> -             flags |= F_STATIC;
> +             kl->flags |= F_STATIC;
>       if (rtm->rtm_flags & RTF_BLACKHOLE)
> -             flags |= F_BLACKHOLE;
> +             kl->flags |= F_BLACKHOLE;
>       if (rtm->rtm_flags & RTF_REJECT)
> -             flags |= F_REJECT;
> +             kl->flags |= F_REJECT;
>       if (rtm->rtm_flags & RTF_DYNAMIC)
> -             flags |= F_DYNAMIC;
> -#ifdef RTF_MPATH
> -     if (rtm->rtm_flags & RTF_MPATH)
> -             mpath = 1;
> -#endif
> +             kl->flags |= F_DYNAMIC;
>  
> -     prio = rtm->rtm_priority;
> +     kl->priority = rtm->rtm_priority;
>       label = (struct sockaddr_rtlabel *)rti_info[RTAX_LABEL];
> +     if (label != NULL)
> +             if (strlcpy(kl->label, label->sr_label, sizeof(kl->label)) >=
> +                 sizeof(kl->label))
> +                     fatalx("rtm label overflow");
>  
> +     sa2addr(sa, &kl->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(
> -                                 sa_in->sin_addr.s_addr);
> +                             kl->prefixlen =
> +                                 mask2prefixlen(sa_in->sin_addr.s_addr);
>               } else if (rtm->rtm_flags & RTF_HOST)
> -                     prefixlen = 32;
> +                     kl->prefixlen = 32;
>               else
> -                     prefixlen =
> -                         prefixlen_classful(prefix.v4.s_addr);
> +                     kl->prefixlen =
> +                         prefixlen_classful(kl->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);
> +                             kl->prefixlen = mask2prefixlen6(sa_in6);
>               } else if (rtm->rtm_flags & RTF_HOST)
> -                     prefixlen = 128;
> +                     kl->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(&kl->prefix), kl->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 */
> +                     kl->flags |= F_CONNECTED;
> +                     kl->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], &kl->nexthop, NULL);
>                       break;
>               }
> +     } else {
> +             kl->flags |= F_CONNECTED;
> +             kl->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;
>                               } 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 +3641,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 +3648,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 +3724,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 +3731,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;

Reply via email to