On 03/04/14(Thu) 10:40, Martin Pieuchot wrote:
> I just committed a diff to reuse the logic of rtinit() to manipulate
> routes to loopback used in our IPv6 stack.
>
> Since this diff turned rtinit() into a wrapper around rt_ifa_add() and
> rt_ifa_del(), here's a diff to get completely rid of it. These new
> functions come with a man page which is far from being perfect but at
> least explains which routes are automatically managed by the kernel.
>
> This diff has another advantage, since the new API requires the caller
> to specify a `dst' argument, it allows us to remove some "save and
> restore" logic in in.c
>
> I totally agree that this API might be improved, but I believe it's a
> good step and this diff should not introduce any change in behavior.
>
> As you can see it's mostly IPv4 centric, since the IPv6 stack reroll
> its own functions to manipulate "connected routes". Hopefully somebody
> can try to replace the remaining rtrequest1(9) call with a priority
> of RTP_CONNECTED by this new API.
>
> Ok?
Com'on, this is a simple diff with documentation...
>
> Index: share/man/man9/Makefile
> ===================================================================
> RCS file: /cvs/src/share/man/man9/Makefile,v
> retrieving revision 1.206
> diff -u -p -r1.206 Makefile
> --- share/man/man9/Makefile 2 Apr 2014 13:10:48 -0000 1.206
> +++ share/man/man9/Makefile 3 Apr 2014 08:24:48 -0000
> @@ -25,8 +25,8 @@ MAN= altq.9 aml_evalnode.9 atomic_add_in
> panic.9 pci_conf_read.9 pci_intr_map.9 pfind.9 physio.9 pmap.9 \
> pool.9 powerhook_establish.9 ppsratecheck.9 printf.9 psignal.9 \
> radio.9 arc4random.9 rasops.9 ratecheck.9 resettodr.9 rssadapt.9 \
> - route.9 rt_timer_add.9 rtable_add.9 rtlabel_id2name.9 rtrequest1.9 \
> - rwlock.9 sensor_attach.9 \
> + rt_ifa_add.9 rt_timer_add.9 route.9 rtable_add.9 rtlabel_id2name.9 \
> + rtrequest1.9 rwlock.9 sensor_attach.9 \
> shutdownhook_establish.9 tsleep.9 spl.9 startuphook_establish.9 \
> socreate.9 sosplice.9 style.9 syscall.9 systrace.9 sysctl_int.9 \
> task_add.9 tc_init.9 time.9 timeout.9 tvtohz.9 uiomove.9 uvm.9 \
> @@ -313,13 +313,15 @@ MLINKS+=rssadapt.9 ieee80211_rssadapt_ch
> MLINKS+=route.9 rt_lookup.9 route.9 rtalloc.9 route.9 rtalloc_noclone.9 \
> route.9 rtalloc1.9 route.9 rtfree.9 route.9 rt_setgate.9 \
> route.9 rtredirect.9 route.9 rtdeletemsg.9
> -MLINKS+=rtable_add.9 rtable_exists.9 rtable_add.9 rtable_get.9 \
> - rtable_add.9 rtable_l2.9 rtable_add.9 rtable_l2set.9
> +MLINKS+=rt_ifa_add.9 rt_ifa_del.9 rt_ifa_add.9 rt_ifa_addloop.9 \
> + rt_ifa_add.9 rt_ifa_delloop.9
> MLINKS+=rt_timer_add.9 rt_timer_queue_create.9 \
> rt_timer_add.9 rt_timer_queue_count.9 \
> rt_timer_add.9 rt_timer_queue_change.9 \
> rt_timer_add.9 rt_timer_queue_destroy.9 \
> rt_timer_add.9 rt_timer_remove_all.9
> +MLINKS+=rtable_add.9 rtable_exists.9 rtable_add.9 rtable_get.9 \
> + rtable_add.9 rtable_l2.9 rtable_add.9 rtable_l2set.9
> MLINKS+=rtlabel_id2name.9 rtlabel_name2id.9 \
> rtlabel_id2name.9 rtlabel_id2sa.9 rtlabel_id2name.9 rtlabel_unref.9
> MLINKS+=rwlock.9 rw_init.9 rwlock.9 rw_enter.9 rwlock.9 rw_exit.9 \
> Index: share/man/man9/rt_ifa_add.9
> ===================================================================
> RCS file: share/man/man9/rt_ifa_add.9
> diff -N share/man/man9/rt_ifa_add.9
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ share/man/man9/rt_ifa_add.9 3 Apr 2014 08:24:48 -0000
> @@ -0,0 +1,104 @@
> +.\" $OpenBSD$
> +.\"
> +.\" Copyright (c) 2014 Martin Pieuchot
> +.\"
> +.\" Permission to use, copy, modify, and distribute this software for any
> +.\" purpose with or without fee is hereby granted, provided that the above
> +.\" copyright notice and this permission notice appear in all copies.
> +.\"
> +.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> +.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> +.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> +.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> +.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> +.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> +.\"
> +.Dd $Mdocdate$
> +.Dt RT_IFA_ADD 9
> +.Os
> +.Sh NAME
> +.Nm rt_ifa_add ,
> +.Nm rt_ifa_del ,
> +.Nm rt_ifa_addloop ,
> +.Nm rt_ifa_delloop
> +.Nd add or delete routes associated with an address
> +.Sh SYNOPSIS
> +.In sys/socket.h
> +.In net/if.h
> +.In net/route.h
> +.Ft int
> +.Fn rt_ifa_add "struct ifaddr *ifa" "int flags" "struct sockaddr *dst"
> +.Ft int
> +.Fn rt_ifa_del "struct ifaddr *ifa" "int flags" "struct sockaddr *dst"
> +.Ft void
> +.Fn rt_ifa_addloop "struct ifaddr *ifa"
> +.Ft void
> +.Fn rt_ifa_delloop "struct ifaddr *ifa"
> +.Sh DESCRIPTION
> +These functions create and delete routes required by the network stack
> +and managed by the kernel.
> +.Bl -tag -width rt_ifa_addloopxx
> +.It Fn rt_ifa_add
> +Creates and associates a connected route with
> +.Fa ifa .
> +.Pp
> +Connected routes are most of the time routes to prefix and should be created
> +with
> +.Dv RTF_CLONING
> +in
> +.Fa flags
> +and the address of
> +.Fa ifa
> +in
> +.Fa dst .
> +But for Point-to-Point interfaces, connected routes are routes to host and
> +should be created
> +with
> +.Dv RTF_HOST
> +in
> +.Fa flags
> +and the destination address in
> +.Fa dst .
> +Connected routes have a priority of
> +.Dv RTP_CONNECTED .
> +.It Fn rt_ifa_del
> +Removes the connected route associated with
> +.Fa ifa .
> +.It Fn rt_ifa_addloop
> +Creates and associates a local route
> +with
> +.Fa ifa .
> +.Pp
> +Local routes are used to not send packets destined to a local address on the
> +wire and instead redirect them to
> +.Xr lo 4 .
> +They have the lowest priority available,
> +.Dv RTP_LOCAL ,
> +and contain a special flag,
> +.Dv RTF_LOCAL ,
> +that can be checked to determine if the address is configured on the system.
> +.It Fn rt_ifa_delloop
> +Removes the local route associated with
> +.Fa ifa .
> +.El
> +.Sh CONTEXT
> +.Fn rt_ifa_add ,
> +.Fn rt_ifa_del ,
> +.Fn rt_ifa_addloop ,
> +and
> +.Fn rt_ifa_delloop
> +can be called during autoconf, from process context, or from interrupt
> context.
> +.Sh RETURN VALUES
> +.Fn rt_ifa_add
> +and
> +.Fn rt_ifa_del
> +will return
> +.Dv 0
> +on success and the return value of
> +.Xr rtrequest1 9
> +otherwise.
> +.Sh SEE ALSO
> +.Xr lo 4 ,
> +.Xr route 4 ,
> +.Xr rtrequest1 9
> Index: sys/net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.282
> diff -u -p -r1.282 if.c
> --- sys/net/if.c 20 Mar 2014 13:19:06 -0000 1.282
> +++ sys/net/if.c 3 Apr 2014 08:24:58 -0000
> @@ -368,7 +368,7 @@ if_free_sadl(struct ifnet *ifp)
> return;
>
> s = splnet();
> - rtinit(ifa, RTM_DELETE, 0);
> + rt_ifa_del(ifa, 0, ifa->ifa_addr);
> ifa_del(ifp, ifa);
> ifafree(ifp->if_lladdr);
> ifp->if_lladdr = NULL;
> Index: sys/net/route.c
> ===================================================================
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.158
> diff -u -p -r1.158 route.c
> --- sys/net/route.c 3 Apr 2014 08:22:10 -0000 1.158
> +++ sys/net/route.c 3 Apr 2014 08:25:02 -0000
> @@ -150,9 +150,6 @@ int rtflushclone1(struct radix_node *, v
> void rtflushclone(struct radix_node_head *, struct rtentry *);
> int rt_if_remove_rtdelete(struct radix_node *, void *, u_int);
>
> -int rt_ifa_add(struct ifaddr *, int, struct sockaddr *);
> -int rt_ifa_del(struct ifaddr *, int, struct sockaddr *);
> -
> #define LABELID_MAX 50000
>
> struct rt_label {
> @@ -1077,28 +1074,6 @@ rt_maskedcopy(struct sockaddr *src, stru
> *cp2++ = *cp1++ & *cp3++;
> if (cp2 < cplim2)
> bzero((caddr_t)cp2, (unsigned)(cplim2 - cp2));
> -}
> -
> -/*
> - * Set up a routing table entry, normally
> - * for an interface.
> - */
> -int
> -rtinit(struct ifaddr *ifa, int cmd, int flags)
> -{
> - struct sockaddr *dst;
> - int error;
> -
> - KASSERT(cmd == RTM_ADD || cmd == RTM_DELETE);
> -
> - dst = flags & RTF_HOST ? ifa->ifa_dstaddr : ifa->ifa_addr;
> -
> - if (cmd == RTM_ADD)
> - error = rt_ifa_add(ifa, flags, dst);
> - else
> - error = rt_ifa_del(ifa, flags, dst);
> -
> - return (error);
> }
>
> int
> Index: sys/net/route.h
> ===================================================================
> RCS file: /cvs/src/sys/net/route.h,v
> retrieving revision 1.90
> diff -u -p -r1.90 route.h
> --- sys/net/route.h 3 Apr 2014 08:22:10 -0000 1.90
> +++ sys/net/route.h 3 Apr 2014 08:25:04 -0000
> @@ -401,7 +401,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_ifa_add(struct ifaddr *, int, struct sockaddr *);
> +int rt_ifa_del(struct ifaddr *, int, struct sockaddr *);
> void rt_ifa_addloop(struct ifaddr *);
> void rt_ifa_delloop(struct ifaddr *);
> int rtioctl(u_long, caddr_t, struct proc *);
> Index: sys/netinet/in.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 in.c
> --- sys/netinet/in.c 27 Mar 2014 10:39:23 -0000 1.94
> +++ sys/netinet/in.c 3 Apr 2014 08:25:08 -0000
> @@ -328,10 +328,9 @@ in_control(struct socket *so, u_long cmd
> return (error);
> }
> if (ia->ia_flags & IFA_ROUTE) {
> - ia->ia_ifa.ifa_dstaddr = sintosa(&oldaddr);
> - rtinit(&ia->ia_ifa, RTM_DELETE, RTF_UP | RTF_HOST);
> - ia->ia_ifa.ifa_dstaddr = sintosa(&ia->ia_dstaddr);
> - rtinit(&ia->ia_ifa, RTM_ADD, RTF_UP | RTF_HOST);
> + rt_ifa_del(&ia->ia_ifa, RTF_HOST, sintosa(&oldaddr));
> + rt_ifa_add(&ia->ia_ifa, RTF_HOST,
> + ia->ia_ifa.ifa_dstaddr);
> }
> splx(s);
> break;
> @@ -773,7 +772,7 @@ in_addhost(struct in_ifaddr *ia0)
> return (0);
> }
>
> - error = rtinit(&ia0->ia_ifa, RTM_ADD, RTF_UP | RTF_HOST);
> + error = rt_ifa_add(&ia0->ia_ifa, RTF_HOST, ia0->ia_ifa.ifa_dstaddr);
> if (!error)
> ia0->ia_flags |= IFA_ROUTE;
>
> @@ -807,16 +806,17 @@ in_scrubhost(struct in_ifaddr *ia0)
> if ((ia->ia_flags & IFA_ROUTE) != 0)
> continue;
>
> - rtinit(&ia0->ia_ifa, RTM_DELETE, RTF_UP | RTF_HOST);
> + rt_ifa_del(&ia0->ia_ifa, RTF_HOST, ia0->ia_ifa.ifa_dstaddr);
> ia0->ia_flags &= ~IFA_ROUTE;
> - error = rtinit(&ia->ia_ifa, RTM_ADD, RTF_UP | RTF_HOST);
> + error = rt_ifa_add(&ia->ia_ifa, RTF_HOST,
> + ia->ia_ifa.ifa_dstaddr);
> if (!error)
> ia->ia_flags |= IFA_ROUTE;
>
> return (error);
> }
>
> - rtinit(&ia0->ia_ifa, RTM_DELETE, RTF_UP | RTF_HOST);
> + rt_ifa_del(&ia0->ia_ifa, RTF_HOST, ia0->ia_ifa.ifa_dstaddr);
> ia0->ia_flags &= ~IFA_ROUTE;
>
> return (0);
> @@ -858,7 +858,7 @@ in_addprefix(struct in_ifaddr *ia0)
> /* move to a real interface instead of carp interface */
> if (ia->ia_ifp->if_type == IFT_CARP &&
> ia0->ia_ifp->if_type != IFT_CARP) {
> - rtinit(&ia->ia_ifa, RTM_DELETE, RTF_UP | RTF_CLONING);
> + rt_ifa_del(&ia->ia_ifa, 0, ia->ia_ifa.ifa_addr);
> ia->ia_flags &= ~IFA_ROUTE;
> break;
> }
> @@ -873,7 +873,7 @@ in_addprefix(struct in_ifaddr *ia0)
> /*
> * noone seem to have prefix route. insert it.
> */
> - error = rtinit(&ia0->ia_ifa, RTM_ADD, RTF_UP | RTF_CLONING);
> + error = rt_ifa_add(&ia0->ia_ifa, RTF_CLONING, ia0->ia_ifa.ifa_addr);
> if (!error)
> ia0->ia_flags |= IFA_ROUTE;
> return error;
> @@ -918,9 +918,10 @@ in_scrubprefix(struct in_ifaddr *ia0)
> /*
> * if we got a matching prefix route, move IFA_ROUTE to him
> */
> - rtinit(&ia0->ia_ifa, RTM_DELETE, RTF_UP | RTF_CLONING);
> + rt_ifa_del(&ia0->ia_ifa, 0, ia0->ia_ifa.ifa_addr);
> ia0->ia_flags &= ~IFA_ROUTE;
> - error = rtinit(&ia->ia_ifa, RTM_ADD, RTF_UP | RTF_CLONING);
> + error = rt_ifa_add(&ia->ia_ifa, RTF_CLONING,
> + ia->ia_ifa.ifa_addr);
> if (error == 0)
> ia->ia_flags |= IFA_ROUTE;
> return error;
> @@ -929,7 +930,7 @@ in_scrubprefix(struct in_ifaddr *ia0)
> /*
> * noone seem to have prefix route. remove it.
> */
> - rtinit(&ia0->ia_ifa, RTM_DELETE, RTF_UP | RTF_CLONING);
> + rt_ifa_del(&ia0->ia_ifa, 0, ia0->ia_ifa.ifa_addr);
> ia0->ia_flags &= ~IFA_ROUTE;
> return 0;
> }
> Index: sys/netinet6/in6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6.c,v
> retrieving revision 1.134
> diff -u -p -r1.134 in6.c
> --- sys/netinet6/in6.c 3 Apr 2014 08:22:10 -0000 1.134
> +++ sys/netinet6/in6.c 3 Apr 2014 08:25:09 -0000
> @@ -750,9 +750,11 @@ in6_update_ifa(struct ifnet *ifp, struct
> */
> if (dst6.sin6_family == AF_INET6 &&
> !IN6_ARE_ADDR_EQUAL(&dst6.sin6_addr, &ia6->ia_dstaddr.sin6_addr)) {
> + struct ifaddr *ifa = &ia6->ia_ifa;
> + int e;
>
> - if ((ia6->ia_flags & IFA_ROUTE) != 0 &&
> - rtinit(&ia6->ia_ifa, RTM_DELETE, RTF_UP | RTF_HOST)) {
> + if (((ia6->ia_flags & IFA_ROUTE) != 0) &&
> + (e = rt_ifa_del(ifa, RTF_HOST, ifa->ifa_dstaddr) != 0)) {
> nd6log((LOG_ERR, "in6_update_ifa: failed to remove "
> "a route to the old destination: %s\n",
> inet_ntop(AF_INET6, &ia6->ia_addr.sin6_addr,
> @@ -1020,8 +1022,7 @@ in6_purgeaddr(struct ifaddr *ifa)
> if ((ia6->ia_flags & IFA_ROUTE) != 0 && ia6->ia_dstaddr.sin6_len != 0) {
> int e;
>
> - if ((e = rtinit(&ia6->ia_ifa, RTM_DELETE,
> - RTF_UP | RTF_HOST)) != 0) {
> + if ((e = rt_ifa_del(ifa, RTF_HOST, ifa->ifa_dstaddr)) != 0) {
> char addr[INET6_ADDRSTRLEN];
> log(LOG_ERR, "in6_purgeaddr: failed to remove "
> "a route to the p2p destination: %s on %s, "
> @@ -1388,7 +1389,9 @@ in6_ifinit(struct ifnet *ifp, struct in6
> */
> plen = in6_mask2len(&ia6->ia_prefixmask.sin6_addr, NULL); /* XXX */
> if (plen == 128 && ia6->ia_dstaddr.sin6_family == AF_INET6) {
> - if ((error = rtinit(&ia6->ia_ifa, RTM_ADD, RTF_UP | RTF_HOST)))
> + ifa = &ia6->ia_ifa;
> + error = rt_ifa_add(ifa, RTF_HOST, ifa->ifa_dstaddr);
> + if (error != 0)
> return (error);
> ia6->ia_flags |= IFA_ROUTE;
> }
>
