On Tue, Aug 27, 2013 at 03:38:49PM +0200, Martin Pieuchot wrote:
> So I started to play with the routine table and I'm slowly trying to
> unify the various code paths to add and delete route entries. The
> diff below is a first step, it splits rtinit() into rt_add() and
> rt_delete() there should be no functional change.
>
> ok?
That makes soooo much more sense. :-).
ok krw@ (note: not a routing table guru!)
.... Ken
>
> Index: net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.262
> diff -u -p -r1.262 if.c
> --- net/if.c 20 Aug 2013 09:14:22 -0000 1.262
> +++ net/if.c 27 Aug 2013 13:33:03 -0000
> @@ -353,7 +353,7 @@ if_free_sadl(struct ifnet *ifp)
> return;
>
> s = splnet();
> - rtinit(ifa, RTM_DELETE, 0);
> + rt_del(ifa, 0);
> #if 0
> ifa_del(ifp, ifa);
> ifp->if_lladdr = NULL;
> Index: net/route.c
> ===================================================================
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.144
> diff -u -p -r1.144 route.c
> --- net/route.c 28 Mar 2013 23:10:05 -0000 1.144
> +++ net/route.c 27 Aug 2013 13:33:03 -0000
> @@ -1069,11 +1069,10 @@ rt_maskedcopy(struct sockaddr *src, stru
> * for an interface.
> */
> int
> -rtinit(struct ifaddr *ifa, int cmd, int flags)
> +rt_add(struct ifaddr *ifa, int flags)
> {
> struct rtentry *rt;
> - struct sockaddr *dst, *deldst;
> - struct mbuf *m = NULL;
> + struct sockaddr *dst;
> struct rtentry *nrt = NULL;
> int error;
> struct rt_addrinfo info;
> @@ -1081,35 +1080,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
> u_short rtableid = ifa->ifa_ifp->if_rdomain;
>
> dst = flags & RTF_HOST ? ifa->ifa_dstaddr : ifa->ifa_addr;
> - if (cmd == RTM_DELETE) {
> - if ((flags & RTF_HOST) == 0 && ifa->ifa_netmask) {
> - m = m_get(M_DONTWAIT, MT_SONAME);
> - if (m == NULL)
> - return (ENOBUFS);
> - deldst = mtod(m, struct sockaddr *);
> - rt_maskedcopy(dst, deldst, ifa->ifa_netmask);
> - dst = deldst;
> - }
> - if ((rt = rtalloc1(dst, 0, rtableid)) != NULL) {
> - rt->rt_refcnt--;
> - /* try to find the right route */
> - while (rt && rt->rt_ifa != ifa)
> - rt = (struct rtentry *)
> - ((struct radix_node *)rt)->rn_dupedkey;
> - if (!rt) {
> - if (m != NULL)
> - (void) m_free(m);
> - return (flags & RTF_HOST ? EHOSTUNREACH
> - : ENETUNREACH);
> - }
> - }
> - }
> bzero(&info, sizeof(info));
> info.rti_ifa = ifa;
> info.rti_flags = flags | ifa->ifa_flags;
> info.rti_info[RTAX_DST] = dst;
> - if (cmd == RTM_ADD)
> - info.rti_info[RTAX_GATEWAY] = ifa->ifa_addr;
> + info.rti_info[RTAX_GATEWAY] = ifa->ifa_addr;
> info.rti_info[RTAX_LABEL] =
> rtlabel_id2sa(ifa->ifa_ifp->if_rtlabelid, &sa_rl);
>
> @@ -1120,22 +1095,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
> * change it to meet bsdi4 behavior.
> */
> info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;
> - error = rtrequest1(cmd, &info, RTP_CONNECTED, &nrt, rtableid);
> - if (cmd == RTM_DELETE) {
> - if (error == 0 && (rt = nrt) != NULL) {
> - rt_newaddrmsg(cmd, ifa, error, nrt);
> - if (rt->rt_refcnt <= 0) {
> - rt->rt_refcnt++;
> - rtfree(rt);
> - }
> - }
> - if (m != NULL)
> - (void) m_free(m);
> - }
> - if (cmd == RTM_ADD && error == 0 && (rt = nrt) != NULL) {
> + error = rtrequest1(RTM_ADD, &info, RTP_CONNECTED, &nrt, rtableid);
> + if (error == 0 && (rt = nrt) != NULL) {
> rt->rt_refcnt--;
> if (rt->rt_ifa != ifa) {
> - printf("rtinit: wrong ifa (%p) was (%p)\n",
> + printf("%s: wrong ifa (%p) was (%p)\n", __func__,
> ifa, rt->rt_ifa);
> if (rt->rt_ifa->ifa_rtrequest)
> rt->rt_ifa->ifa_rtrequest(RTM_DELETE, rt, NULL);
> @@ -1146,8 +1110,68 @@ rtinit(struct ifaddr *ifa, int cmd, int
> if (ifa->ifa_rtrequest)
> ifa->ifa_rtrequest(RTM_ADD, rt, NULL);
> }
> - rt_newaddrmsg(cmd, ifa, error, nrt);
> + rt_newaddrmsg(RTM_ADD, ifa, error, nrt);
> + }
> +
> + return (error);
> +}
> +
> +int
> +rt_del(struct ifaddr *ifa, int flags)
> +{
> + struct rtentry *rt;
> + struct sockaddr *dst, *deldst;
> + struct mbuf *m = NULL;
> + struct rtentry *nrt = NULL;
> + int error;
> + struct rt_addrinfo info;
> + u_short rtableid = ifa->ifa_ifp->if_rdomain;
> +
> + dst = flags & RTF_HOST ? ifa->ifa_dstaddr : ifa->ifa_addr;
> + if ((flags & RTF_HOST) == 0 && ifa->ifa_netmask) {
> + m = m_get(M_DONTWAIT, MT_SONAME);
> + if (m == NULL)
> + return (ENOBUFS);
> + deldst = mtod(m, struct sockaddr *);
> + rt_maskedcopy(dst, deldst, ifa->ifa_netmask);
> + dst = deldst;
> }
> + if ((rt = rtalloc1(dst, 0, rtableid)) != NULL) {
> + rt->rt_refcnt--;
> + /* try to find the right route */
> + while (rt && rt->rt_ifa != ifa)
> + rt = (struct rtentry *)
> + ((struct radix_node *)rt)->rn_dupedkey;
> + if (!rt) {
> + if (m != NULL)
> + m_free(m);
> + return (flags & RTF_HOST ? EHOSTUNREACH : ENETUNREACH);
> + }
> + }
> +
> + bzero(&info, sizeof(info));
> + info.rti_ifa = ifa;
> + info.rti_flags = flags | ifa->ifa_flags;
> + info.rti_info[RTAX_DST] = dst;
> +
> + /*
> + * XXX here, it seems that we are assuming that ifa_netmask is NULL
> + * for RTF_HOST. bsdi4 passes NULL explicitly (via intermediate
> + * variable) when RTF_HOST is 1. still not sure if i can safely
> + * change it to meet bsdi4 behavior.
> + */
> + info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;
> + error = rtrequest1(RTM_DELETE, &info, RTP_CONNECTED, &nrt, rtableid);
> + if (error == 0 && (rt = nrt) != NULL) {
> + rt_newaddrmsg(RTM_DELETE, ifa, error, nrt);
> + if (rt->rt_refcnt <= 0) {
> + rt->rt_refcnt++;
> + rtfree(rt);
> + }
> + }
> + if (m != NULL)
> + (void) m_free(m);
> +
> return (error);
> }
>
> Index: net/route.h
> ===================================================================
> RCS file: /cvs/src/sys/net/route.h,v
> retrieving revision 1.78
> diff -u -p -r1.78 route.h
> --- net/route.h 19 Sep 2012 16:14:01 -0000 1.78
> +++ net/route.h 27 Aug 2013 13:33:03 -0000
> @@ -396,7 +396,8 @@ struct rtentry *
> rtalloc1(struct sockaddr *, int, u_int);
> void rtfree(struct rtentry *);
> int rt_getifa(struct rt_addrinfo *, u_int);
> -int rtinit(struct ifaddr *, int, int);
> +int rt_add(struct ifaddr *, int);
> +int rt_del(struct ifaddr *, int);
> int rtioctl(u_long, caddr_t, struct proc *);
> void rtredirect(struct sockaddr *, struct sockaddr *,
> struct sockaddr *, int, struct sockaddr *,
> Index: netinet/in.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.83
> diff -u -p -r1.83 in.c
> --- netinet/in.c 19 Aug 2013 08:45:34 -0000 1.83
> +++ netinet/in.c 27 Aug 2013 13:33:04 -0000
> @@ -323,9 +323,9 @@ in_control(struct socket *so, u_long cmd
> }
> if (ia->ia_flags & IFA_ROUTE) {
> ia->ia_ifa.ifa_dstaddr = sintosa(&oldaddr);
> - rtinit(&(ia->ia_ifa), (int)RTM_DELETE, RTF_HOST);
> + rt_del(&ia->ia_ifa, RTF_HOST);
> ia->ia_ifa.ifa_dstaddr = sintosa(&ia->ia_dstaddr);
> - rtinit(&(ia->ia_ifa), (int)RTM_ADD, RTF_HOST|RTF_UP);
> + rt_add(&ia->ia_ifa, RTF_HOST | RTF_UP);
> }
> splx(s);
> break;
> @@ -777,8 +777,7 @@ in_addprefix(struct in_ifaddr *target, i
> /* move to a real interface instead of carp interface */
> if (ia->ia_ifp->if_type == IFT_CARP &&
> target->ia_ifp->if_type != IFT_CARP) {
> - rtinit(&(ia->ia_ifa), (int)RTM_DELETE,
> - rtinitflags(ia));
> + rt_del(&ia->ia_ifa, rtinitflags(ia));
> ia->ia_flags &= ~IFA_ROUTE;
> break;
> }
> @@ -793,7 +792,7 @@ in_addprefix(struct in_ifaddr *target, i
> /*
> * noone seem to have prefix route. insert it.
> */
> - error = rtinit(&target->ia_ifa, (int)RTM_ADD, flags);
> + error = rt_add(&target->ia_ifa, flags);
> if (!error)
> target->ia_flags |= IFA_ROUTE;
> return error;
> @@ -842,12 +841,10 @@ in_scrubprefix(struct in_ifaddr *target)
> * if we got a matching prefix route, move IFA_ROUTE to him
> */
> if ((ia->ia_flags & IFA_ROUTE) == 0) {
> - rtinit(&(target->ia_ifa), (int)RTM_DELETE,
> - rtinitflags(target));
> + rt_del(&target->ia_ifa, rtinitflags(target));
> target->ia_flags &= ~IFA_ROUTE;
>
> - error = rtinit(&ia->ia_ifa, (int)RTM_ADD,
> - rtinitflags(ia) | RTF_UP);
> + error = rt_add(&ia->ia_ifa, rtinitflags(ia) | RTF_UP);
> if (error == 0)
> ia->ia_flags |= IFA_ROUTE;
> return error;
> @@ -857,7 +854,7 @@ in_scrubprefix(struct in_ifaddr *target)
> /*
> * noone seem to have prefix route. remove it.
> */
> - rtinit(&(target->ia_ifa), (int)RTM_DELETE, rtinitflags(target));
> + rt_del(&target->ia_ifa, rtinitflags(target));
> target->ia_flags &= ~IFA_ROUTE;
> return 0;
> }
> Index: netinet6/in6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6.c,v
> retrieving revision 1.118
> diff -u -p -r1.118 in6.c
> --- netinet6/in6.c 26 Aug 2013 07:15:58 -0000 1.118
> +++ netinet6/in6.c 27 Aug 2013 13:33:04 -0000
> @@ -200,7 +200,7 @@ in6_ifloop_request(int cmd, struct ifadd
>
> /*
> * Report the addition/removal of the address to the routing socket.
> - * XXX: since we called rtinit for a p2p interface with a destination,
> + * XXX: since we called rt_add for a p2p interface with a destination,
> * we end up reporting twice in such a case. Should we rather
> * omit the second report?
> */
> @@ -932,7 +932,7 @@ in6_update_ifa(struct ifnet *ifp, struct
> int e;
>
> if ((ia->ia_flags & IFA_ROUTE) != 0 &&
> - (e = rtinit(&(ia->ia_ifa), (int)RTM_DELETE, RTF_HOST)) !=
> 0) {
> + (e = rt_del(&ia->ia_ifa, RTF_HOST)) != 0) {
> nd6log((LOG_ERR, "in6_update_ifa: failed to remove "
> "a route to the old destination: %s\n",
> ip6_sprintf(&ia->ia_addr.sin6_addr)));
> @@ -1193,8 +1193,7 @@ in6_purgeaddr(struct ifaddr *ifa)
> if ((ia->ia_flags & IFA_ROUTE) != 0 && ia->ia_dstaddr.sin6_len != 0) {
> int e;
>
> - if ((e = rtinit(&(ia->ia_ifa), (int)RTM_DELETE, RTF_HOST))
> - != 0) {
> + if ((e = rt_del(&ia->ia_ifa, RTF_HOST)) != 0) {
> log(LOG_ERR, "in6_purgeaddr: failed to remove "
> "a route to the p2p destination: %s on %s, "
> "errno=%d\n",
> @@ -1535,8 +1534,7 @@ in6_ifinit(struct ifnet *ifp, struct in6
> */
> plen = in6_mask2len(&ia->ia_prefixmask.sin6_addr, NULL); /* XXX */
> if (plen == 128 && ia->ia_dstaddr.sin6_family == AF_INET6) {
> - if ((error = rtinit(&(ia->ia_ifa), (int)RTM_ADD,
> - RTF_UP | RTF_HOST)) != 0)
> + if ((error = rt_add(&ia->ia_ifa, RTF_UP | RTF_HOST)) != 0)
> return (error);
> ia->ia_flags |= IFA_ROUTE;
> }
>
