Module Name: src
Committed By: ozaki-r
Date: Thu Nov 10 04:13:53 UTC 2016
Modified Files:
src/sys/netinet6: in6_src.c ip6_output.c ip6_var.h
Log Message:
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@
To generate a diff of this commit:
cvs rdiff -u -r1.73 -r1.74 src/sys/netinet6/in6_src.c
cvs rdiff -u -r1.177 -r1.178 src/sys/netinet6/ip6_output.c
cvs rdiff -u -r1.69 -r1.70 src/sys/netinet6/ip6_var.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/netinet6/in6_src.c
diff -u src/sys/netinet6/in6_src.c:1.73 src/sys/netinet6/in6_src.c:1.74
--- src/sys/netinet6/in6_src.c:1.73 Mon Oct 31 04:57:10 2016
+++ src/sys/netinet6/in6_src.c Thu Nov 10 04:13:53 2016
@@ -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 @@ struct in6_addrpolicy defaultaddrpolicy;
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 @@ exit:
#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 @@ selectroute(struct sockaddr_in6 *dstsock
}
#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 @@ selectroute(struct sockaddr_in6 *dstsock
* 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 @@ selectroute(struct sockaddr_in6 *dstsock
*/
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 @@ selectroute(struct sockaddr_in6 *dstsock
* 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 @@ in6_selectif(struct sockaddr_in6 *dstsoc
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 @@ in6_selectif(struct sockaddr_in6 *dstsoc
* 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 @@ in6_selectif(struct sockaddr_in6 *dstsoc
* 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 @@ in6_selectif(struct sockaddr_in6 *dstsoc
}
/*
- * 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
Index: src/sys/netinet6/ip6_output.c
diff -u src/sys/netinet6/ip6_output.c:1.177 src/sys/netinet6/ip6_output.c:1.178
--- src/sys/netinet6/ip6_output.c:1.177 Mon Nov 7 01:55:17 2016
+++ src/sys/netinet6/ip6_output.c Thu Nov 10 04:13:53 2016
@@ -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 $ */
/*
@@ -62,7 +62,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip6_output.c,v 1.177 2016/11/07 01:55:17 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip6_output.c,v 1.178 2016/11/10 04:13:53 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -408,6 +408,9 @@ ip6_output(
sizeof(struct ip6_hdr) + optlen);
}
+ /* Need to save for pmtu */
+ finaldst = ip6->ip6_dst;
+
/*
* If there is a routing header, replace destination address field
* with the first hop of the routing header.
@@ -417,7 +420,6 @@ ip6_output(
rh = (struct ip6_rthdr *)(mtod(exthdrs.ip6e_rthdr,
struct ip6_rthdr *));
- finaldst = ip6->ip6_dst; /* need to save for pmtu */
error = ip6_handle_rthdr(rh, ip6);
if (error != 0)
@@ -499,12 +501,31 @@ ip6_output(
ip6 = mtod(m, struct ip6_hdr *);
sockaddr_in6_init(&dst_sa, &ip6->ip6_dst, 0, 0, 0);
- if ((error = in6_selectroute(&dst_sa, opt, im6o, ro,
- &ifp, &psref, &rt, 0)) != 0) {
- if (ifp != NULL)
- in6_ifstat_inc(ifp, ifs6_out_discard);
- goto bad;
+
+ /* We do not need a route for multicast */
+ if (IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst)) {
+ struct in6_pktinfo *pi = NULL;
+
+ /*
+ * If the outgoing interface for the address is specified by
+ * the caller, use it.
+ */
+ if (opt && (pi = opt->ip6po_pktinfo) != NULL) {
+ /* XXX boundary check is assumed to be already done. */
+ ifp = if_get_byindex(pi->ipi6_ifindex, &psref);
+ } else if (im6o != NULL) {
+ ifp = if_get_byindex(im6o->im6o_multicast_if_index,
+ &psref);
+ }
}
+
+ if (ifp == NULL) {
+ error = in6_selectroute(&dst_sa, opt, &ro, &rt, true);
+ if (error != 0)
+ goto bad;
+ ifp = if_get_byindex(rt->rt_ifp->if_index, &psref);
+ }
+
if (rt == NULL) {
/*
* If in6_selectroute() does not return a route entry,
@@ -578,18 +599,9 @@ ip6_output(
goto bad;
}
- if (rt == NULL || IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst)) {
- dst = satocsin6(rtcache_getdst(ro));
- KASSERT(dst != NULL);
- } else if (opt && rtcache_validate(&opt->ip6po_nextroute) != NULL) {
- /*
- * The nexthop is explicitly specified by the
- * application. We assume the next hop is an IPv6
- * address.
- */
- dst = (struct sockaddr_in6 *)opt->ip6po_nexthop;
- } else if ((rt->rt_flags & RTF_GATEWAY))
- dst = (struct sockaddr_in6 *)rt->rt_gateway;
+ if (rt != NULL && (rt->rt_flags & RTF_GATEWAY) &&
+ !IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst))
+ dst = satocsin6(rt->rt_gateway);
else
dst = satocsin6(rtcache_getdst(ro));
Index: src/sys/netinet6/ip6_var.h
diff -u src/sys/netinet6/ip6_var.h:1.69 src/sys/netinet6/ip6_var.h:1.70
--- src/sys/netinet6/ip6_var.h:1.69 Mon Oct 31 04:16:25 2016
+++ src/sys/netinet6/ip6_var.h Thu Nov 10 04:13:53 2016
@@ -1,4 +1,4 @@
-/* $NetBSD: ip6_var.h,v 1.69 2016/10/31 04:16:25 ozaki-r Exp $ */
+/* $NetBSD: ip6_var.h,v 1.70 2016/11/10 04:13:53 ozaki-r Exp $ */
/* $KAME: ip6_var.h,v 1.33 2000/06/11 14:59:20 jinmei Exp $ */
/*
@@ -400,8 +400,7 @@ int in6_selectsrc(struct sockaddr_in6 *,
struct ip6_moptions *, struct route *, struct in6_addr *,
struct ifnet **, struct psref *, struct in6_addr *);
int in6_selectroute(struct sockaddr_in6 *, struct ip6_pktopts *,
- struct ip6_moptions *, struct route *, struct ifnet **,
- struct psref *, struct rtentry **, int);
+ struct route **, struct rtentry **, bool);
int ip6_get_membership(const struct sockopt *, struct ifnet **, void *,
size_t);