Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/netinet6 Tidy up in6_select*



details:   https://anonhg.NetBSD.org/src/rev/884ca7842830
branches:  trunk
changeset: 348820:884ca7842830
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Thu Nov 10 04:13:53 2016 +0000

description:
Tidy up in6_select*

This change tidies up in6_select* functions, especially
selectroute.

selectroute is annoying because:
- It returns both/either of a rtentry and/or an ifp
  - Yes, it may return only an ifp!
    - It is valid but selectroute shouldn't handle the case
  - Such conditional behavior makes it difficult
    to apply locking/psref thingy
- It may return a rtentry even if error
- It may use opt->ip6po_nextroute rtcache implicitly
  - The caller can know if it is used
    by rtcache_validate(&opt->ip6po_nextroute)
    but it's racy in MP-safe world
  - Even if it uses opt->ip6po_nextroute, it may
    return a rtentry that isn't derived from the rtcache

The change includes:
- Rename selectroute to in6_selectroute
  - Let a remaining caller of selectroute, in6_selectif,
    use in6_selectroute instead
- Let in6_selectroute return only an rtentry
  - If error, it doesn't return an rtentry
  - A caller gets an ifp from a returned rtentry
- Allow in6_selectroute to modify a passed rtcache
  and a caller can know if opt->ip6po_nextroute is
  used via the rtcache
- Let callers (ip6_output and in6_selectif) handle
  the case that only an ifp is required

Inspired by OpenBSD
Proposed on tech-kern and tech-net
LGTM by roy@

diffstat:

 sys/netinet6/in6_src.c    |  167 ++++++++++++++-------------------------------
 sys/netinet6/ip6_output.c |   52 ++++++++-----
 sys/netinet6/ip6_var.h    |    5 +-
 3 files changed, 86 insertions(+), 138 deletions(-)

diffs (truncated from 405 to 300 lines):

diff -r 56f5de4bc603 -r 884ca7842830 sys/netinet6/in6_src.c
--- a/sys/netinet6/in6_src.c    Thu Nov 10 03:32:04 2016 +0000
+++ b/sys/netinet6/in6_src.c    Thu Nov 10 04:13:53 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: in6_src.c,v 1.73 2016/10/31 04:57:10 ozaki-r Exp $     */
+/*     $NetBSD: in6_src.c,v 1.74 2016/11/10 04:13:53 ozaki-r Exp $     */
 /*     $KAME: in6_src.c,v 1.159 2005/10/19 01:40:32 t-momose Exp $     */
 
 /*
@@ -66,7 +66,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in6_src.c,v 1.73 2016/10/31 04:57:10 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in6_src.c,v 1.74 2016/11/10 04:13:53 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -122,9 +122,6 @@
 
 int ip6_prefer_tempaddr = 0;
 
-static int selectroute(struct sockaddr_in6 *, struct ip6_pktopts *,
-       struct ip6_moptions *, struct route *, struct ifnet **, struct psref *,
-       struct rtentry **, int, int);
 static int in6_selectif(struct sockaddr_in6 *, struct ip6_pktopts *,
        struct ip6_moptions *, struct route *, struct ifnet **, struct psref *);
 
@@ -590,28 +587,20 @@
 #undef PSREF
 }
 
-static int
-selectroute(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, 
-       struct ip6_moptions *mopts, struct route *ro, struct ifnet **retifp, 
-       struct psref *psref, struct rtentry **retrt, int clone, int norouteok)
+int
+in6_selectroute(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts,
+    struct route **ro, struct rtentry **retrt, bool count_discard)
 {
        int error = 0;
-       struct ifnet *ifp = NULL;
        struct rtentry *rt = NULL;
-       struct sockaddr_in6 *sin6_next;
-       struct in6_pktinfo *pi = NULL;
-       struct in6_addr *dst;
        union {
                struct sockaddr         dst;
                struct sockaddr_in6     dst6;
        } u;
 
        KASSERT(ro != NULL);
-       KASSERT(retifp != NULL && psref != NULL);
        KASSERT(retrt != NULL);
 
-       dst = &dstsock->sin6_addr;
-
 #if 0
        if (dstsock->sin6_addr.s6_addr32[0] == 0 &&
            dstsock->sin6_addr.s6_addr32[1] == 0 &&
@@ -625,41 +614,13 @@
        }
 #endif
 
-       /* If the caller specify the outgoing interface explicitly, use it. */
-       if (opts && (pi = opts->ip6po_pktinfo) != NULL && pi->ipi6_ifindex) {
-               /* XXX boundary check is assumed to be already done. */
-               ifp = if_get_byindex(pi->ipi6_ifindex, psref);
-               if (ifp != NULL &&
-                   (norouteok || IN6_IS_ADDR_MULTICAST(dst))) {
-                       /*
-                        * we do not have to check or get the route for
-                        * multicast.
-                        */
-                       goto done;
-               } else {
-                       if_put(ifp, psref);
-                       ifp = NULL;
-                       goto getroute;
-               }
-       }
-
-       /*
-        * If the destination address is a multicast address and the outgoing
-        * interface for the address is specified by the caller, use it.
-        */
-       if (IN6_IS_ADDR_MULTICAST(dst) && mopts != NULL) {
-               ifp = if_get_byindex(mopts->im6o_multicast_if_index, psref);
-               if (ifp != NULL)
-                       goto done; /* we do not need a route for multicast. */
-       }
-
-  getroute:
        /*
         * If the next hop address for the packet is specified by the caller,
         * use it as the gateway.
         */
        if (opts && opts->ip6po_nexthop) {
                struct route *ron;
+               struct sockaddr_in6 *sin6_next;
 
                sin6_next = satosin6(opts->ip6po_nexthop);
 
@@ -674,28 +635,19 @@
                 * by that address must be a neighbor of the sending host.
                 */
                ron = &opts->ip6po_nextroute;
-               if ((rt = rtcache_lookup(ron, sin6tosa(sin6_next))) == NULL ||
-                   (rt->rt_flags & RTF_GATEWAY) != 0 ||
+               rt = rtcache_lookup(ron, sin6tosa(sin6_next));
+               if (rt == NULL || (rt->rt_flags & RTF_GATEWAY) != 0 ||
                    !nd6_is_addr_neighbor(sin6_next, rt->rt_ifp)) {
                        rtcache_free(ron);
+                       if (rt != NULL && count_discard)
+                               in6_ifstat_inc(rt->rt_ifp, ifs6_out_discard);
+                       rt = NULL;
                        error = EHOSTUNREACH;
                        goto done;
                }
-               ifp = rt->rt_ifp;
-               if (ifp != NULL) {
-                       if (!if_is_deactivated(ifp))
-                               if_acquire_NOMPSAFE(ifp, psref);
-                       else
-                               ifp = NULL;
-               }
+               *ro = ron;
 
-               /*
-                * When cloning is required, try to allocate a route to the
-                * destination so that the caller can store path MTU
-                * information.
-                */
-               if (!clone)
-                       goto done;
+               goto done;
        }
 
        /*
@@ -705,27 +657,10 @@
         */
        u.dst6 = *dstsock;
        u.dst6.sin6_scope_id = 0;
-       rt = rtcache_lookup1(ro, &u.dst, clone);
-
-       /*
-        * do not care about the result if we have the nexthop
-        * explicitly specified.
-        */
-       if (opts && opts->ip6po_nexthop)
-               goto done;
+       rt = rtcache_lookup1(*ro, &u.dst, 1);
 
        if (rt == NULL)
                error = EHOSTUNREACH;
-       else {
-               if_put(ifp, psref);
-               ifp = rt->rt_ifp;
-               if (ifp != NULL) {
-                       if (!if_is_deactivated(ifp))
-                               if_acquire_NOMPSAFE(ifp, psref);
-                       else
-                               ifp = NULL;
-               }
-       }
 
        /*
         * Check if the outgoing interface conflicts with
@@ -735,28 +670,20 @@
         *  our own addresses.)
         */
        if (opts && opts->ip6po_pktinfo && opts->ip6po_pktinfo->ipi6_ifindex) {
-               if (!(ifp->if_flags & IFF_LOOPBACK) &&
-                   ifp->if_index != opts->ip6po_pktinfo->ipi6_ifindex) {
+               if (!(rt->rt_ifp->if_flags & IFF_LOOPBACK) &&
+                   rt->rt_ifp->if_index != opts->ip6po_pktinfo->ipi6_ifindex) {
+                       if (rt != NULL && count_discard)
+                               in6_ifstat_inc(rt->rt_ifp, ifs6_out_discard);
                        error = EHOSTUNREACH;
-                       goto done;
+                       rt = NULL;
                }
        }
 
-  done:
-       if (ifp == NULL && rt == NULL) {
-               /*
-                * This can happen if the caller did not pass a cached route
-                * nor any other hints.  We treat this case an error.
-                */
-               error = EHOSTUNREACH;
-       }
+done:
        if (error == EHOSTUNREACH)
                IP6_STATINC(IP6_STAT_NOROUTE);
-
-       *retifp = ifp;
-       *retrt = rt;    /* rt may be NULL */
-
-       return (error);
+       *retrt = rt;
+       return error;
 }
 
 static int
@@ -764,19 +691,42 @@
        struct ip6_moptions *mopts, struct route *ro, struct ifnet **retifp,
        struct psref *psref)
 {
-       int error, clone;
+       int error;
        struct rtentry *rt = NULL;
+       struct in6_addr *dst;
+       struct in6_pktinfo *pi = NULL;
 
        KASSERT(retifp != NULL);
        *retifp = NULL;
+       dst = &dstsock->sin6_addr;
 
-       clone = IN6_IS_ADDR_MULTICAST(&dstsock->sin6_addr) ? 0 : 1;
-       if ((error = selectroute(dstsock, opts, mopts, ro, retifp, psref,
-           &rt, clone, 1)) != 0) {
-               return (error);
+       /* If the caller specify the outgoing interface explicitly, use it. */
+       if (opts && (pi = opts->ip6po_pktinfo) != NULL && pi->ipi6_ifindex) {
+               /* XXX boundary check is assumed to be already done. */
+               *retifp = if_get_byindex(pi->ipi6_ifindex, psref);
+               if (*retifp != NULL)
+                       return 0;
+               goto getroute;
        }
 
        /*
+        * If the destination address is a multicast address and the outgoing
+        * interface for the address is specified by the caller, use it.
+        */
+       if (IN6_IS_ADDR_MULTICAST(dst) && mopts != NULL) {
+               *retifp = if_get_byindex(mopts->im6o_multicast_if_index, psref);
+               if (*retifp != NULL)
+                       return 0; /* we do not need a route for multicast. */
+       }
+
+getroute:
+       error = in6_selectroute(dstsock, opts, &ro, &rt, false);
+       if (error != 0)
+               return error;
+
+       *retifp = if_get_byindex(rt->rt_ifp->if_index, psref);
+
+       /*
         * do not use a rejected or black hole route.
         * XXX: this check should be done in the L2 output routine.
         * However, if we skipped this check here, we'd see the following
@@ -793,7 +743,7 @@
         * Although this may not be very harmful, it should still be confusing.
         * We thus reject the case here.
         */
-       if (rt && (rt->rt_flags & (RTF_REJECT | RTF_BLACKHOLE)))
+       if ((rt->rt_flags & (RTF_REJECT | RTF_BLACKHOLE)))
                return (rt->rt_flags & RTF_HOST ? EHOSTUNREACH : ENETUNREACH);
 
        /*
@@ -803,7 +753,7 @@
         * destination address (which should probably be one of our own
         * addresses.)
         */
-       if (rt && rt->rt_ifa && rt->rt_ifa->ifa_ifp &&
+       if (rt->rt_ifa && rt->rt_ifa->ifa_ifp &&
            rt->rt_ifa->ifa_ifp != *retifp &&
            !if_is_deactivated(rt->rt_ifa->ifa_ifp)) {
                if_put(*retifp, psref);
@@ -815,19 +765,6 @@
 }
 
 /*
- * close - meaningful only for bsdi and freebsd.
- */
-
-int
-in6_selectroute(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, 
-       struct ip6_moptions *mopts, struct route *ro, struct ifnet **retifp, 
-       struct psref *psref, struct rtentry **retrt, int clone)
-{
-       return selectroute(dstsock, opts, mopts, ro, retifp, psref,
-           retrt, clone, 0);
-}
-
-/*
  * Default hop limit selection. The precedence is as follows:
  * 1. Hoplimit value specified via ioctl.
  * 2. (If the outgoing interface is detected) the current
diff -r 56f5de4bc603 -r 884ca7842830 sys/netinet6/ip6_output.c
--- a/sys/netinet6/ip6_output.c Thu Nov 10 03:32:04 2016 +0000
+++ b/sys/netinet6/ip6_output.c Thu Nov 10 04:13:53 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ip6_output.c,v 1.177 2016/11/07 01:55:17 ozaki-r Exp $ */
+/*     $NetBSD: ip6_output.c,v 1.178 2016/11/10 04:13:53 ozaki-r Exp $ */
 /*     $KAME: ip6_output.c,v 1.172 2001/03/25 09:55:56 itojun Exp $    */
 
 /*



Home | Main Index | Thread Index | Old Index