Fix subnet and default routes on different FIBs on the same subnet.

These two bugs are closely related.  The root cause is that ifa_ifwithnet
does not consider FIBs when searching for an interface address.

sys/net/if_var.h
sys/net/if.c
	Add a fib argument to ifa_ifwithnet and ifa_ifwithdstadddr.  Those
	functions will only return an address whose interface fib equals the
	argument.

sys/net/route.c
	Update calls to ifa_ifwithnet and ifa_ifwithdstaddr with fib
	arguments.

sys/netinet/in.c
	Update in_addprefix to consider the interface fib when adding
	prefixes.  This will prevent it from not adding a subnet route when
	one already exists on a different fib.

sys/net/rtsock.c
sys/netinet/in_pcb.c
sys/netinet/ip_output.c
sys/netinet/ip_options.c
sys/netinet6/nd6.c
	Add RT_DEFAULT_FIB arguments to ifa_ifwithdstaddr and ifa_ifwithnet.
	In some cases it there wasn't a clear specific fib number to use.
	In others, I was unable to test those functions so I chose
	RT_DEFAULT_FIB to minimize divergence from current behavior.  I will
	fix some of the latter changes along with PR kern/187553.

tests/sys/netinet/fibs_test.sh
tests/sys/netinet/udp_dontroute.c
tests/sys/netinet/Makefile
	Revert r263738.  The udp_dontroute test was right all along.
	However, bugs kern/187550 and kern/187553 cancelled each other out
	when it came to this test.  Because of kern/187553, ifa_ifwithnet
	searched the default fib instead of the requested one, but because
	of kern/187550, there was an applicable subnet route on the default
	fib.  The new test added in r263738 doesn't work right, however.  I
	can verify with dtrace that ifa_ifwithnet returned the wrong address
	before I applied this commit, but route(8) miraculously found the
	correct interface to use anyway.  I don't know how.

	Clear expected failure messages for kern/187550 and kern/187552.

PR:		kern/187550
PR:		kern/187552
Reviewed by:	melifaro
MFC after:	3 weeks
Sponsored by:	Spectra Logic
This commit is contained in:
Alan Somers 2014-04-24 23:56:56 +00:00
parent e13aabb24f
commit 0cfee0c223
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=264905
12 changed files with 158 additions and 63 deletions

View File

@ -1653,7 +1653,7 @@ ifa_ifwithbroadaddr(struct sockaddr *addr)
*/
/*ARGSUSED*/
struct ifaddr *
ifa_ifwithdstaddr(struct sockaddr *addr)
ifa_ifwithdstaddr(struct sockaddr *addr, int fibnum)
{
struct ifnet *ifp;
struct ifaddr *ifa;
@ -1662,6 +1662,8 @@ ifa_ifwithdstaddr(struct sockaddr *addr)
TAILQ_FOREACH(ifp, &V_ifnet, if_link) {
if ((ifp->if_flags & IFF_POINTOPOINT) == 0)
continue;
if ((ifp->if_fib != fibnum))
continue;
IF_ADDR_RLOCK(ifp);
TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
if (ifa->ifa_addr->sa_family != addr->sa_family)
@ -1686,7 +1688,7 @@ ifa_ifwithdstaddr(struct sockaddr *addr)
* is most specific found.
*/
struct ifaddr *
ifa_ifwithnet(struct sockaddr *addr, int ignore_ptp)
ifa_ifwithnet(struct sockaddr *addr, int ignore_ptp, int fibnum)
{
struct ifnet *ifp;
struct ifaddr *ifa;
@ -1706,12 +1708,14 @@ ifa_ifwithnet(struct sockaddr *addr, int ignore_ptp)
/*
* Scan though each interface, looking for ones that have addresses
* in this address family. Maintain a reference on ifa_maybe once
* we find one, as we release the IF_ADDR_RLOCK() that kept it stable
* when we move onto the next interface.
* in this address family and the requested fib. Maintain a reference
* on ifa_maybe once we find one, as we release the IF_ADDR_RLOCK() that
* kept it stable when we move onto the next interface.
*/
IFNET_RLOCK_NOSLEEP();
TAILQ_FOREACH(ifp, &V_ifnet, if_link) {
if (ifp->if_fib != fibnum)
continue;
IF_ADDR_RLOCK(ifp);
TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
char *cp, *cp2, *cp3;

View File

@ -495,8 +495,8 @@ int ifa_switch_loopback_route(struct ifaddr *, struct sockaddr *, int fib);
struct ifaddr *ifa_ifwithaddr(struct sockaddr *);
int ifa_ifwithaddr_check(struct sockaddr *);
struct ifaddr *ifa_ifwithbroadaddr(struct sockaddr *);
struct ifaddr *ifa_ifwithdstaddr(struct sockaddr *);
struct ifaddr *ifa_ifwithnet(struct sockaddr *, int);
struct ifaddr *ifa_ifwithdstaddr(struct sockaddr *, int);
struct ifaddr *ifa_ifwithnet(struct sockaddr *, int, int);
struct ifaddr *ifa_ifwithroute(int, struct sockaddr *, struct sockaddr *);
struct ifaddr *ifa_ifwithroute_fib(int, struct sockaddr *, struct sockaddr *, u_int);
struct ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *);

View File

@ -582,7 +582,7 @@ rtredirect_fib(struct sockaddr *dst,
}
/* verify the gateway is directly reachable */
if ((ifa = ifa_ifwithnet(gateway, 0)) == NULL) {
if ((ifa = ifa_ifwithnet(gateway, 0, fibnum)) == NULL) {
error = ENETUNREACH;
goto out;
}
@ -739,7 +739,7 @@ ifa_ifwithroute_fib(int flags, struct sockaddr *dst, struct sockaddr *gateway,
*/
ifa = NULL;
if (flags & RTF_HOST)
ifa = ifa_ifwithdstaddr(dst);
ifa = ifa_ifwithdstaddr(dst, fibnum);
if (ifa == NULL)
ifa = ifa_ifwithaddr(gateway);
} else {
@ -748,10 +748,10 @@ ifa_ifwithroute_fib(int flags, struct sockaddr *dst, struct sockaddr *gateway,
* or host, the gateway may still be on the
* other end of a pt to pt link.
*/
ifa = ifa_ifwithdstaddr(gateway);
ifa = ifa_ifwithdstaddr(gateway, fibnum);
}
if (ifa == NULL)
ifa = ifa_ifwithnet(gateway, 0);
ifa = ifa_ifwithnet(gateway, 0, fibnum);
if (ifa == NULL) {
struct rtentry *rt = rtalloc1_fib(gateway, 0, RTF_RNH_LOCKED, fibnum);
if (rt == NULL)
@ -865,7 +865,7 @@ rt_getifa_fib(struct rt_addrinfo *info, u_int fibnum)
*/
if (info->rti_ifp == NULL && ifpaddr != NULL &&
ifpaddr->sa_family == AF_LINK &&
(ifa = ifa_ifwithnet(ifpaddr, 0)) != NULL) {
(ifa = ifa_ifwithnet(ifpaddr, 0, fibnum)) != NULL) {
info->rti_ifp = ifa->ifa_ifp;
ifa_free(ifa);
}

View File

@ -735,7 +735,8 @@ route_output(struct mbuf *m, struct socket *so)
rt->rt_ifp->if_type == IFT_PROPVIRTUAL) {
struct ifaddr *ifa;
ifa = ifa_ifwithnet(info.rti_info[RTAX_DST], 1);
ifa = ifa_ifwithnet(info.rti_info[RTAX_DST], 1,
RT_DEFAULT_FIB);
if (ifa != NULL)
rt_maskedcopy(ifa->ifa_addr,
&laddr,

View File

@ -617,7 +617,7 @@ in_addprefix(struct in_ifaddr *target, int flags)
{
struct in_ifaddr *ia;
struct in_addr prefix, mask, p, m;
int error, fibnum;
int error;
if ((flags & RTF_HOST) != 0) {
prefix = target->ia_dstaddr.sin_addr;
@ -628,9 +628,8 @@ in_addprefix(struct in_ifaddr *target, int flags)
prefix.s_addr &= mask.s_addr;
}
fibnum = rt_add_addr_allfibs ? RT_ALL_FIBS : target->ia_ifp->if_fib;
IN_IFADDR_RLOCK();
/* Look for an existing address with the same prefix, mask, and fib */
TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) {
if (rtinitflags(ia)) {
p = ia->ia_dstaddr.sin_addr;
@ -646,6 +645,8 @@ in_addprefix(struct in_ifaddr *target, int flags)
mask.s_addr != m.s_addr)
continue;
}
if (target->ia_ifp->if_fib != ia->ia_ifp->if_fib)
continue;
/*
* If we got a matching prefix route inserted by other
@ -664,6 +665,10 @@ in_addprefix(struct in_ifaddr *target, int flags)
IN_IFADDR_RUNLOCK();
return (EEXIST);
} else {
int fibnum;
fibnum = rt_add_addr_allfibs ? RT_ALL_FIBS :
target->ia_ifp->if_fib;
rt_addrmsg(RTM_ADD, &target->ia_ifa, fibnum);
IN_IFADDR_RUNLOCK();
return (0);

View File

@ -745,9 +745,11 @@ in_pcbladdr(struct inpcb *inp, struct in_addr *faddr, struct in_addr *laddr,
struct in_ifaddr *ia;
struct ifnet *ifp;
ia = ifatoia(ifa_ifwithdstaddr((struct sockaddr *)sin));
ia = ifatoia(ifa_ifwithdstaddr((struct sockaddr *)sin,
RT_DEFAULT_FIB));
if (ia == NULL)
ia = ifatoia(ifa_ifwithnet((struct sockaddr *)sin, 0));
ia = ifatoia(ifa_ifwithnet((struct sockaddr *)sin, 0,
RT_DEFAULT_FIB));
if (ia == NULL) {
error = ENETUNREACH;
goto done;
@ -862,9 +864,10 @@ in_pcbladdr(struct inpcb *inp, struct in_addr *faddr, struct in_addr *laddr,
sain.sin_len = sizeof(struct sockaddr_in);
sain.sin_addr.s_addr = faddr->s_addr;
ia = ifatoia(ifa_ifwithdstaddr(sintosa(&sain)));
ia = ifatoia(ifa_ifwithdstaddr(sintosa(&sain), RT_DEFAULT_FIB));
if (ia == NULL)
ia = ifatoia(ifa_ifwithnet(sintosa(&sain), 0));
ia = ifatoia(ifa_ifwithnet(sintosa(&sain), 0,
RT_DEFAULT_FIB));
if (ia == NULL)
ia = ifatoia(ifa_ifwithaddr(sintosa(&sain)));

View File

@ -227,8 +227,11 @@ ip_dooptions(struct mbuf *m, int pass)
if (opt == IPOPT_SSRR) {
#define INA struct in_ifaddr *
#define SA struct sockaddr *
if ((ia = (INA)ifa_ifwithdstaddr((SA)&ipaddr)) == NULL)
ia = (INA)ifa_ifwithnet((SA)&ipaddr, 0);
if ((ia = (INA)ifa_ifwithdstaddr((SA)&ipaddr,
RT_DEFAULT_FIB)) == NULL) {
ia = (INA)ifa_ifwithnet((SA)&ipaddr, 0,
RT_DEFAULT_FIB);
}
} else
/* XXX MRT 0 for routing */
ia = ip_rtaddr(ipaddr.sin_addr, M_GETFIB(m));

View File

@ -230,7 +230,8 @@ ip_output(struct mbuf *m, struct mbuf *opt, struct route *ro, int flags,
*/
if (flags & IP_SENDONES) {
if ((ia = ifatoia(ifa_ifwithbroadaddr(sintosa(dst)))) == NULL &&
(ia = ifatoia(ifa_ifwithdstaddr(sintosa(dst)))) == NULL) {
(ia = ifatoia(ifa_ifwithdstaddr(sintosa(dst),
RT_DEFAULT_FIB))) == NULL) {
IPSTAT_INC(ips_noroute);
error = ENETUNREACH;
goto bad;
@ -241,8 +242,10 @@ ip_output(struct mbuf *m, struct mbuf *opt, struct route *ro, int flags,
ip->ip_ttl = 1;
isbroadcast = 1;
} else if (flags & IP_ROUTETOIF) {
if ((ia = ifatoia(ifa_ifwithdstaddr(sintosa(dst)))) == NULL &&
(ia = ifatoia(ifa_ifwithnet(sintosa(dst), 0))) == NULL) {
if ((ia = ifatoia(ifa_ifwithdstaddr(sintosa(dst),
RT_DEFAULT_FIB))) == NULL &&
(ia = ifatoia(ifa_ifwithnet(sintosa(dst), 0,
RT_DEFAULT_FIB))) == NULL) {
IPSTAT_INC(ips_noroute);
error = ENETUNREACH;
goto bad;

View File

@ -945,7 +945,7 @@ nd6_is_new_addr_neighbor(struct sockaddr_in6 *addr, struct ifnet *ifp)
* If the address is assigned on the node of the other side of
* a p2p interface, the address should be a neighbor.
*/
dstaddr = ifa_ifwithdstaddr((struct sockaddr *)addr);
dstaddr = ifa_ifwithdstaddr((struct sockaddr *)addr, RT_DEFAULT_FIB);
if (dstaddr != NULL) {
if (dstaddr->ifa_ifp == ifp) {
ifa_free(dstaddr);

View File

@ -1,7 +1,12 @@
# $FreeBSD$
TESTSDIR= ${TESTSBASE}/sys/netinet
BINDIR= ${TESTSDIR}
ATF_TESTS_SH+= fibs_test
PROG= udp_dontroute
SRCS= udp_dontroute.c
NO_MAN=
WARNS?= 6
.include <bsd.test.mk>

View File

@ -175,7 +175,6 @@ default_route_with_multiple_fibs_on_same_subnet_head()
default_route_with_multiple_fibs_on_same_subnet_body()
{
atf_expect_fail "kern/187552 default route uses the wrong interface when multiple interfaces have the same subnet but different fibs"
# Configure the TAP interfaces to use a RFC5737 nonrouteable addresses
# and a non-default fib
ADDR0="192.0.2.2"
@ -225,7 +224,6 @@ subnet_route_with_multiple_fibs_on_same_subnet_head()
subnet_route_with_multiple_fibs_on_same_subnet_body()
{
atf_expect_fail "kern/187550 Multiple interfaces on different FIBs but the same subnet don't all have a subnet route"
# Configure the TAP interfaces to use a RFC5737 nonrouteable addresses
# and a non-default fib
ADDR0="192.0.2.2"
@ -253,66 +251,54 @@ subnet_route_with_multiple_fibs_on_same_subnet_cleanup()
cleanup_tap
}
# Regression test for kern/187553 "Source address selection for UDP packets
# with SO_DONTROUTE uses the default FIB". The original complaint was that a
# UDP packet with SO_DONTROUTE set would select a source address from an
# interface on the default FIB instead of the process FIB.
# Test that source address selection works correctly for UDP packets with
# SO_DONTROUTE set that are sent on non-default FIBs.
# This bug was discovered with "setfib 1 netperf -t UDP_STREAM -H some_host"
# Regression test for kern/187553
#
# The root cause was that ifa_ifwithnet() did not have a fib argument. It
# would return an address from an interface on any FIB that had a subnet route
# for the destination. If more than one were available, it would choose the
# most specific. The root cause is most easily tested by creating two
# interfaces with overlapping subnet routes, adding a default route to the
# interface with the less specific subnet route, and looking up a host that
# requires the default route using the FIB of the interface with the less
# specific subnet route. "route get" should provide a route that uses the
# interface on the chosen FIB. However, absent the patch for this bug it will
# instead use the other interface.
atf_test_case src_addr_selection_by_subnet cleanup
src_addr_selection_by_subnet_head()
# most specific. This is most easily tested by creating a FIB without a
# default route, then trying to send a UDP packet with SO_DONTROUTE set to an
# address which is not routable on that FIB. Absent the fix for this bug,
# in_pcbladdr would choose an interface on any FIB with a default route. With
# the fix, you will get EUNREACH or ENETUNREACH.
atf_test_case udp_dontroute cleanup
udp_dontroute_head()
{
atf_set "descr" "Source address selection for UDP packets with SO_DONTROUTE on non-default FIBs works"
atf_set "require.user" "root"
atf_set "require.config" "fibs"
}
src_addr_selection_by_subnet_body()
udp_dontroute_body()
{
atf_expect_fail "kern/187553 Source address selection for UDP packets with SO_DONTROUTE uses the default FIB"
# Configure the TAP interface to use an RFC5737 nonrouteable address
# and a non-default fib
ADDR0="192.0.2.2"
ADDR1="192.0.2.3"
GATEWAY0="192.0.2.1"
TARGET="192.0.2.128"
ADDR="192.0.2.2"
SUBNET="192.0.2.0"
MASK0="25"
MASK1="26"
MASK="24"
# Use a different IP on the same subnet as the target
TARGET="192.0.2.100"
# Check system configuration
if [ 0 != `sysctl -n net.add_addr_allfibs` ]; then
atf_skip "This test requires net.add_addr_allfibs=0"
fi
get_fibs 2
get_fibs 1
# Configure a TAP interface
setup_tap ${FIB0} ${ADDR0} ${MASK0}
TAP0=${TAP}
setup_tap ${FIB1} ${ADDR1} ${MASK1}
TAP1=${TAP}
setup_tap ${FIB0} ${ADDR} ${MASK}
# Add a gateway to the interface with the less specific subnet route
setfib ${FIB0} route add default ${GATEWAY0}
# Lookup a route
echo "Looking up route to ${TARGET} with fib ${FIB0}"
echo "Expected behavior is to use interface ${TAP0}"
atf_check -o match:"interface:.${TAP0}" setfib ${FIB0} route -n get ${TARGET}
# Send a UDP packet with SO_DONTROUTE. In the failure case, it will
# return ENETUNREACH
SRCDIR=`atf_get_srcdir`
atf_check -o ignore setfib ${FIB0} ${SRCDIR}/udp_dontroute ${TARGET}
}
src_addr_selection_by_subnet_cleanup()
udp_dontroute_cleanup()
{
cleanup_tap
}
@ -324,7 +310,7 @@ atf_init_test_cases()
atf_add_test_case loopback_and_network_routes_on_nondefault_fib
atf_add_test_case default_route_with_multiple_fibs_on_same_subnet
atf_add_test_case subnet_route_with_multiple_fibs_on_same_subnet
atf_add_test_case src_addr_selection_by_subnet
atf_add_test_case udp_dontroute
}
# Looks up one or more fibs from the configuration data and validates them.

View File

@ -0,0 +1,85 @@
/*
* Copyright (c) 2014 Spectra Logic Corporation
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions, and the following disclaimer,
* without modification.
* 2. Redistributions in binary form must reproduce at minimum a disclaimer
* substantially similar to the "NO WARRANTY" disclaimer below
* ("Disclaimer") and any redistribution must be conditioned upon
* including a substantially similar Disclaimer requirement for further
* binary redistribution.
*
* NO WARRANTY
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* HOLDERS OR CONTRIBUTORS BE LIABLE FOR SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
* STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
* IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGES.
*
* Authors: Alan Somers (Spectra Logic Corporation)
*
* $FreeBSD$
*/
#include <arpa/inet.h>
#include <netinet/in.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <err.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
/*
* Sends a single UDP packet to the provided address, with SO_DONTROUTE set
* I couldn't find a way to do this with builtin utilities like nc(1)
*/
int main(int argc, char **argv)
{
struct sockaddr_in dst;
int s;
int opt;
int ret;
const char* buf = "Hello, World!";
if (argc != 2) {
fprintf(stderr, "Usage: %s ip_address\n", argv[0]);
exit(2);
}
s = socket(PF_INET, SOCK_DGRAM, 0);
if (s < 0)
err(errno, "socket");
opt = 1;
ret = setsockopt(s, SOL_SOCKET, SO_DONTROUTE, &opt, sizeof(opt));
if (ret == -1)
err(errno, "setsockopt(SO_DONTROUTE)");
dst.sin_len = sizeof(dst);
dst.sin_family = AF_INET;
dst.sin_port = htons(46120);
dst.sin_addr.s_addr = inet_addr(argv[1]);
if (dst.sin_addr.s_addr == htonl(INADDR_NONE)) {
fprintf(stderr, "Invalid address: %s\n", argv[1]);
exit(2);
}
ret = sendto(s, buf, strlen(buf), 0, (struct sockaddr*)&dst,
dst.sin_len);
if (ret == -1)
err(errno, "sendto");
return (0);
}