Bug 187552

Summary:	default route uses the wrong interface when multiple interfaces have the same subnet but different fibs
Product:	Base System	Reporter:	Alan Somers <asomers>
Component:	kern	Assignee:	Alan Somers <asomers>
Status:	Closed FIXED
Severity:	Affects Some People
Priority:	Normal
Version:	CURRENT
Hardware:	Any
OS:	Any

Description Alan Somers freebsd_committer

2014-03-13 19:50:00 UTC

If multiple interfaces share the same subnet but different fibs, and they both have default routes, then the second and subsequent default routes will erroneously use the same interface as the first default route.  Instead, each default route should use the interface associated with its fib.

Fix: 

Working on a patch ...
How-To-Repeat: # ifconfig tap0 create
# ifconfig tap1 create
# setfib 2 ifconfig tap0 192.0.2.2/24 fib 2
# setfib 3 ifconfig tap1 192.0.2.3/24 fib 3
# setfib 2 netstat -rn -f inet
Routing tables (fib: 2)

Internet:
Destination        Gateway            Flags    Netif Expire
default            192.0.2.1          UGS      tap0
192.0.2.0/24       link#3             U        tap0
# setfib 3 netstat -rn -f inet
Routing tables (fib: 3)

Internet:
Destination        Gateway            Flags    Netif Expire
default            192.0.2.1          UGS      tap0


Notice that the default route uses tap0 in both fibs.  It should use tap1 in fib 3.

Comment 1 Alan Somers freebsd_committer

2014-03-13 19:53:32 UTC

Responsible Changed
From-To: freebsd-bugs->asomers

I'll take it.  I already have a patch

Comment 2 dfilter service freebsd_committer

2014-03-20 20:39:48 UTC

Author: asomers
Date: Thu Mar 20 20:39:41 2014
New Revision: 263445
URL: http://svnweb.freebsd.org/changeset/base/263445

Log:
  Add several ATF tests that deal with multiple fibs.  They're described in
  several different PRs, but the tests share some common code, so I'm
  committing them together.
  
  sbin/ifconfig/tests
  sbin/ifconfig/tests/fibs_test.sh
  sbin/ifconfig/tests/Makefile
  sbin/ifconfig/Makefile
  	Add fibs_test.sh, which regresses bin/187551
  
  tests/sys/netinet
  tests/sys/netinet/fibs_test.sh
  tests/sys/netinet/udp_dontroute.c
  tests/sys/netinet/Makefile
  tests/sys/Makefile
  	Add fibs_test.sh, which regresses kern/167947, kern/187552
  	kern/187549, kern/187550, and kern/187553
  
  etc/mtree/BSD.tests.dist
  	Add newly created directories
  
  PR:		bin/187551
  PR:		kern/167947
  PR:		kern/187552
  PR:		kern/187549
  PR:		kern/187550
  PR:		kern/187553
  Discussed with:	melifaro
  MFC after:	3 weeks
  Sponsored by:	Spectra Logic Corporation

Added:
  head/sbin/ifconfig/tests/
  head/sbin/ifconfig/tests/Makefile   (contents, props changed)
  head/sbin/ifconfig/tests/fibs_test.sh   (contents, props changed)
  head/tests/sys/netinet/
  head/tests/sys/netinet/Makefile   (contents, props changed)
  head/tests/sys/netinet/fibs_test.sh   (contents, props changed)
  head/tests/sys/netinet/udp_dontroute.c   (contents, props changed)
Modified:
  head/etc/mtree/BSD.tests.dist
  head/sbin/ifconfig/Makefile
  head/tests/sys/Makefile

Modified: head/etc/mtree/BSD.tests.dist
==============================================================================
--- head/etc/mtree/BSD.tests.dist	Thu Mar 20 20:33:18 2014	(r263444)
+++ head/etc/mtree/BSD.tests.dist	Thu Mar 20 20:39:41 2014	(r263445)
@@ -75,6 +75,8 @@
             ..
             growfs
             ..
+            ifconfig
+            ..
             mdconfig
             ..
         ..
@@ -91,6 +93,8 @@
         sys
             kern
             ..
+            netinet
+            ..
         ..
         usr.bin
             apply

Modified: head/sbin/ifconfig/Makefile
==============================================================================
--- head/sbin/ifconfig/Makefile	Thu Mar 20 20:33:18 2014	(r263444)
+++ head/sbin/ifconfig/Makefile	Thu Mar 20 20:39:41 2014	(r263445)
@@ -63,4 +63,8 @@ MAN=	ifconfig.8
 CFLAGS+= -Wall -Wmissing-prototypes -Wcast-qual -Wwrite-strings -Wnested-externs
 WARNS?=	2
 
+.if ${MK_TESTS} != "no"
+SUBDIR+=    tests
+.endif
+
 .include <bsd.prog.mk>

Added: head/sbin/ifconfig/tests/Makefile
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/sbin/ifconfig/tests/Makefile	Thu Mar 20 20:39:41 2014	(r263445)
@@ -0,0 +1,9 @@
+# $FreeBSD$
+
+.include <bsd.own.mk>
+
+TESTSDIR=	${TESTSBASE}/sbin/ifconfig
+
+ATF_TESTS_SH=	fibs_test
+
+.include <bsd.test.mk>

Added: head/sbin/ifconfig/tests/fibs_test.sh
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/sbin/ifconfig/tests/fibs_test.sh	Thu Mar 20 20:39:41 2014	(r263445)
@@ -0,0 +1,126 @@
+#
+#  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$
+
+
+# Regression test for bin/187551
+atf_test_case process_fib cleanup
+process_fib_head()
+{
+	atf_set "descr" "ifconfig will set its process fib whenever configuring an interface with nondefault fib"
+	atf_set "require.user" "root"
+	atf_set "require.config" "fibs"
+}
+process_fib_body()
+{
+	atf_expect_fail "bin/187551 ifconfig should change its process fib when configuring an interface with nondefault fib"
+	# Configure the TAP interface to use an RFC5737 nonrouteable address
+	# and a non-default fib
+	ADDR="192.0.2.2"
+	SUBNET="192.0.2.0"
+	MASK="24"
+
+	# 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 1
+
+	# Configure a TAP interface
+	get_tap
+	ktrace ifconfig $TAP ${ADDR}/${MASK} fib $FIB0
+	if kdump -s | egrep -q 'CALL[[:space:]]+setfib'; then
+		atf_pass
+	else
+		atf_fail "ifconfig never called setfib(2)"
+	fi
+}
+
+process_fib_cleanup()
+{
+	cleanup_tap
+}
+
+atf_init_test_cases()
+{
+	atf_add_test_case process_fib
+}
+
+
+# parameter numfibs	The number of fibs to lookup
+get_fibs()
+{
+	NUMFIBS=$1
+	net_fibs=`sysctl -n net.fibs`
+	i=0
+	while [ $i -lt "$NUMFIBS" ]; do
+		fib=`atf_config_get "fibs" | \
+			awk -v i=$(( i + 1 )) '{print $i}'`
+		echo "fib is ${fib}"
+		eval FIB${i}=${fib}
+		if [ "$fib" -ge "$net_fibs" ]; then
+			atf_skip "The ${i}th configured fib is ${fib}, which is not less than net.fibs, which is ${net_fibs}"
+		fi
+		i=$(( $i + 1 ))
+	done
+}
+
+
+
+# Creates a new tap(4) interface, registers it for cleanup, and returns the
+# name via the environment variable TAP
+get_tap()
+{
+	local TAPN=0
+	while ! ifconfig tap${TAPN} create > /dev/null 2>&1; do
+		if [ "$TAPN" -ge 8 ]; then
+			atf_skip "Could not create a tap(4) interface"
+		else
+			TAPN=$(($TAPN + 1))
+		fi
+	done
+	local TAPD=tap${TAPN}
+	# Record the TAP device so we can clean it up later
+	echo ${TAPD} >> "tap_devices_to_cleanup"
+	TAP=${TAPD}
+}
+
+
+
+
+cleanup_tap()
+{
+	for TAPD in `cat "tap_devices_to_cleanup"`; do
+		ifconfig ${TAPD} destroy
+	done
+}
+

Modified: head/tests/sys/Makefile
==============================================================================
--- head/tests/sys/Makefile	Thu Mar 20 20:33:18 2014	(r263444)
+++ head/tests/sys/Makefile	Thu Mar 20 20:39:41 2014	(r263445)
@@ -5,6 +5,7 @@
 .PATH: ${.CURDIR}/..
 
 TESTS_SUBDIRS+=		kern
+TESTS_SUBDIRS+=		netinet
 TESTSDIR= ${TESTSBASE}/sys
 
 KYUAFILE= yes

Added: head/tests/sys/netinet/Makefile
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/tests/sys/netinet/Makefile	Thu Mar 20 20:39:41 2014	(r263445)
@@ -0,0 +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>

Added: head/tests/sys/netinet/fibs_test.sh
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/tests/sys/netinet/fibs_test.sh	Thu Mar 20 20:39:41 2014	(r263445)
@@ -0,0 +1,371 @@
+#
+#  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$
+
+# All of the tests in this file requires the test-suite config variable "fibs"
+# to be defined to a space-delimited list of FIBs that may be used for testing.
+
+# arpresolve should check the interface fib for routes to a target when
+# creating an ARP table entry.  This is a regression for kern/167947, where
+# arpresolve only checked the default route.
+#
+# Outline:
+# Create two tap(4) interfaces
+# Simulate a crossover cable between them by using net/socat
+# Use nping (from security/nmap) to send an ICMP echo request from one
+# interface to the other, spoofing the source IP.  The source IP must be
+# spoofed, or else it will already have an entry in the arp table. 
+# Check whether an arp entry exists for the spoofed IP
+atf_test_case arpresolve_checks_interface_fib cleanup
+arpresolve_checks_interface_fib_head()
+{
+	atf_set "descr" "arpresolve should check the interface fib, not the default fib, for routes"
+	atf_set "require.user" "root"
+	atf_set "require.config" "fibs"
+	atf_set "require.progs" "socat nping"
+}
+arpresolve_checks_interface_fib_body()
+{
+	atf_expect_fail "kern/167947 arpresolve checks only the default FIB for the interface route"
+	# Configure the TAP interfaces to use a RFC5737 nonrouteable addresses
+	# and a non-default fib
+	ADDR0="192.0.2.2"
+	ADDR1="192.0.2.3"
+	SUBNET="192.0.2.0"
+	# Due to bug TBD (regressed by multiple_fibs_on_same_subnet) we need
+	# diffferent subnet masks, or FIB1 won't have a subnet route.
+	MASK0="24"
+	MASK1="25"
+	# Spoof a MAC that is reserved per RFC7042
+	SPOOF_ADDR="192.0.2.4"
+	SPOOF_MAC="00:00:5E:00:53:00"
+
+	# 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
+
+	# Configure TAP interfaces
+	setup_tap "$FIB0" ${ADDR0} ${MASK0}
+	TAP0=$TAP
+	setup_tap "$FIB1" ${ADDR1} ${MASK1}
+	TAP1=$TAP
+
+	# Simulate a crossover cable
+	socat /dev/${TAP0} /dev/${TAP1} &
+	SOCAT_PID=$!
+	echo ${SOCAT_PID} >> "processes_to_kill"
+	
+	# Send an ICMP echo request with a spoofed source IP
+	setfib 2 nping -c 1 -e ${TAP0} -S ${SPOOF_ADDR} \
+		--source-mac ${SPOOF_MAC} --icmp --icmp-type "echo-request" \
+		--icmp-code 0 --icmp-id 0xdead --icmp-seq 1 --data 0xbeef \
+		${ADDR1}
+	# For informational and debugging purposes only, look for the
+	# characteristic error message
+	dmesg | grep "llinfo.*${SPOOF_ADDR}"
+	# Check that the ARP entry exists
+	atf_check -o match:"${SPOOF_ADDR}.*expires" setfib 3 arp ${SPOOF_ADDR}
+}
+arpresolve_checks_interface_fib_cleanup()
+{
+	for PID in `cat "processes_to_kill"`; do
+		kill $PID
+	done
+	cleanup_tap
+}
+
+
+# Regression test for kern/187549
+atf_test_case loopback_and_network_routes_on_nondefault_fib cleanup
+loopback_and_network_routes_on_nondefault_fib_head()
+{
+	atf_set "descr" "When creating and deleting loopback routes, use the interface's fib"
+	atf_set "require.user" "root"
+	atf_set "require.config" "fibs"
+}
+
+loopback_and_network_routes_on_nondefault_fib_body()
+{
+	atf_expect_fail "kern/187549 Host and network routes for a new interface appear in the wrong FIB"
+	# Configure the TAP interface to use an RFC5737 nonrouteable address
+	# and a non-default fib
+	ADDR="192.0.2.2"
+	SUBNET="192.0.2.0"
+	MASK="24"
+
+	# 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 1
+
+	# Configure a TAP interface
+	setup_tap ${FIB0} ${ADDR} ${MASK}
+
+	# Check whether the host route exists in only the correct FIB
+	setfib ${FIB0} netstat -rn -f inet | grep -q "^${ADDR}.*UHS.*lo0"
+	if [ 0 -ne $? ]; then
+		setfib ${FIB0} netstat -rn -f inet
+		atf_fail "Host route did not appear in the correct FIB"
+	fi
+	setfib 0 netstat -rn -f inet | grep -q "^${ADDR}.*UHS.*lo0"
+	if [ 0 -eq $? ]; then
+		setfib 0 netstat -rn -f inet
+		atf_fail "Host route appeared in the wrong FIB"
+	fi
+
+	# Check whether the network route exists in only the correct FIB
+	setfib ${FIB0} netstat -rn -f inet | \
+		grep -q "^${SUBNET}/${MASK}.*${TAPD}"
+	if [ 0 -ne $? ]; then
+		setfib ${FIB0} netstat -rn -f inet
+		atf_fail "Network route did not appear in the correct FIB"
+	fi
+	setfib 0 netstat -rn -f inet | \
+		grep -q "^${SUBNET}/${MASK}.*${TAPD}"
+	if [ 0 -eq $? ]; then
+		setfib ${FIB0} netstat -rn -f inet
+		atf_fail "Network route appeared in the wrong FIB"
+	fi
+}
+
+loopback_and_network_routes_on_nondefault_fib_cleanup()
+{
+	cleanup_tap
+}
+
+
+# Regression test for kern/187552
+atf_test_case default_route_with_multiple_fibs_on_same_subnet cleanup
+default_route_with_multiple_fibs_on_same_subnet_head()
+{
+	atf_set "descr" "Multiple interfaces on the same subnet but with different fibs can both have default routes"
+	atf_set "require.user" "root"
+	atf_set "require.config" "fibs"
+}
+
+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"
+	ADDR1="192.0.2.3"
+	GATEWAY="192.0.2.1"
+	SUBNET="192.0.2.0"
+	MASK="24"
+
+	# 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
+
+	# Configure TAP interfaces
+	setup_tap "$FIB0" ${ADDR0} ${MASK}
+	TAP0=$TAP
+	setup_tap "$FIB1" ${ADDR1} ${MASK}
+	TAP1=$TAP
+
+	# Attempt to add default routes
+	setfib ${FIB0} route add default ${GATEWAY}
+	setfib ${FIB1} route add default ${GATEWAY}
+
+	# Verify that the default route exists for both fibs, with their
+	# respective interfaces.
+	atf_check -o match:"^default.*${TAP0}$" \
+		setfib ${FIB0} netstat -rn -f inet
+	atf_check -o match:"^default.*${TAP1}$" \
+		setfib ${FIB1} netstat -rn -f inet
+}
+
+default_route_with_multiple_fibs_on_same_subnet_cleanup()
+{
+	cleanup_tap
+}
+
+
+# Regression test for kern/187550
+atf_test_case subnet_route_with_multiple_fibs_on_same_subnet cleanup
+subnet_route_with_multiple_fibs_on_same_subnet_head()
+{
+	atf_set "descr" "Multiple FIBs can have subnet routes for the same subnet"
+	atf_set "require.user" "root"
+	atf_set "require.config" "fibs"
+}
+
+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"
+	ADDR1="192.0.2.3"
+	SUBNET="192.0.2.0"
+	MASK="24"
+
+	# 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
+
+	# Configure TAP interfaces
+	setup_tap "$FIB0" ${ADDR0} ${MASK}
+	setup_tap "$FIB1" ${ADDR1} ${MASK}
+
+	# Check that a subnet route exists on both fibs
+	atf_check -o ignore setfib "$FIB0" route get $ADDR1
+	atf_check -o ignore setfib "$FIB1" route get $ADDR0
+}
+
+subnet_route_with_multiple_fibs_on_same_subnet_cleanup()
+{
+	cleanup_tap
+}
+
+# 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
+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"
+}
+
+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
+	ADDR="192.0.2.2"
+	SUBNET="192.0.2.0"
+	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 1
+
+	# Configure a TAP interface
+	setup_tap ${FIB0} ${ADDR} ${MASK}
+
+	# 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}
+}
+
+udp_dontroute_cleanup()
+{
+	cleanup_tap
+}
+
+
+atf_init_test_cases()
+{
+	atf_add_test_case arpresolve_checks_interface_fib
+	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 udp_dontroute
+}
+
+# Looks up one or more fibs from the configuration data and validates them.
+# Returns the results in the env varilables FIB0, FIB1, etc.
+
+# parameter numfibs	The number of fibs to lookup
+get_fibs()
+{
+	NUMFIBS=$1
+	net_fibs=`sysctl -n net.fibs`
+	i=0
+	while [ $i -lt "$NUMFIBS" ]; do
+		fib=`atf_config_get "fibs" | \
+			awk -v i=$(( i + 1 )) '{print $i}'`
+		echo "fib is ${fib}"
+		eval FIB${i}=${fib}
+		if [ "$fib" -ge "$net_fibs" ]; then
+			atf_skip "The ${i}th configured fib is ${fib}, which is not less than net.fibs, which is ${net_fibs}"
+		fi
+		i=$(( $i + 1 ))
+	done
+}
+
+# Creates a new tap(4) interface, registers it for cleanup, and returns the
+# name via the environment variable TAP
+get_tap()
+{
+	local TAPN=0
+	while ! ifconfig tap${TAPN} create > /dev/null 2>&1; do
+		if [ "$TAPN" -ge 8 ]; then
+			atf_skip "Could not create a tap(4) interface"
+		else
+			TAPN=$(($TAPN + 1))
+		fi
+	done
+	local TAPD=tap${TAPN}
+	# Record the TAP device so we can clean it up later
+	echo ${TAPD} >> "tap_devices_to_cleanup"
+	TAP=${TAPD}
+}
+
+# Create a tap(4) interface, configure it, and register it for cleanup.
+# parameters:
+# fib
+# IP address
+# Netmask in number of bits (eg 24 or 8)
+# Return: the tap interface name as the env variable TAP
+setup_tap()
+{
+	local FIB=$1
+	local ADDR=$2
+	local MASK=$3
+	get_tap
+	echo setfib ${FIB} ifconfig $TAP ${ADDR}/${MASK} fib $FIB
+	setfib ${FIB} ifconfig $TAP ${ADDR}/${MASK} fib $FIB
+}
+
+cleanup_tap()
+{
+	for TAPD in `cat "tap_devices_to_cleanup"`; do
+		ifconfig ${TAPD} destroy
+	done
+}

Added: head/tests/sys/netinet/udp_dontroute.c
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/tests/sys/netinet/udp_dontroute.c	Thu Mar 20 20:39:41 2014	(r263445)
@@ -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);
+}
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

Comment 3 Mark Linimon freebsd_committer

2014-04-20 23:05:24 UTC

State Changed
From-To: open->closed

see kern/187549.

Comment 4 Alan Somers freebsd_committer

2014-04-21 17:18:55 UTC

State Changed
From-To: closed->open

Actually, linimon, this is still open.  Commit 263445 merely added 
regression tests; it did not fix the bug.

Comment 5 dfilter service freebsd_committer

2014-04-25 00:57:04 UTC

Author: asomers
Date: Thu Apr 24 23:56:56 2014
New Revision: 264905
URL: http://svnweb.freebsd.org/changeset/base/264905

Log:
  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

Added:
  head/tests/sys/netinet/udp_dontroute.c
     - copied unchanged from r263737, head/tests/sys/netinet/udp_dontroute.c
Modified:
  head/sys/net/if.c
  head/sys/net/if_var.h
  head/sys/net/route.c
  head/sys/net/rtsock.c
  head/sys/netinet/in.c
  head/sys/netinet/in_pcb.c
  head/sys/netinet/ip_options.c
  head/sys/netinet/ip_output.c
  head/sys/netinet6/nd6.c
  head/tests/sys/netinet/Makefile
  head/tests/sys/netinet/fibs_test.sh

Modified: head/sys/net/if.c
==============================================================================
--- head/sys/net/if.c	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/sys/net/if.c	Thu Apr 24 23:56:56 2014	(r264905)
@@ -1653,7 +1653,7 @@ done:
  */
 /*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 @@ done:
  * 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
 
 	/*
 	 * 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;

Modified: head/sys/net/if_var.h
==============================================================================
--- head/sys/net/if_var.h	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/sys/net/if_var.h	Thu Apr 24 23:56:56 2014	(r264905)
@@ -495,8 +495,8 @@ int	ifa_switch_loopback_route(struct ifa
 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 *);

Modified: head/sys/net/route.c
==============================================================================
--- head/sys/net/route.c	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/sys/net/route.c	Thu Apr 24 23:56:56 2014	(r264905)
@@ -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 so
 		 */
 		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 so
 		 * 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, 
 	 */
 	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);
 	}

Modified: head/sys/net/rtsock.c
==============================================================================
--- head/sys/net/rtsock.c	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/sys/net/rtsock.c	Thu Apr 24 23:56:56 2014	(r264905)
@@ -735,7 +735,8 @@ route_output(struct mbuf *m, struct sock
 			    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,

Modified: head/sys/netinet/in.c
==============================================================================
--- head/sys/netinet/in.c	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/sys/netinet/in.c	Thu Apr 24 23:56:56 2014	(r264905)
@@ -617,7 +617,7 @@ in_addprefix(struct in_ifaddr *target, i
 {
 	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, i
 		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, i
 			    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, i
 				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);

Modified: head/sys/netinet/in_pcb.c
==============================================================================
--- head/sys/netinet/in_pcb.c	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/sys/netinet/in_pcb.c	Thu Apr 24 23:56:56 2014	(r264905)
@@ -745,9 +745,11 @@ in_pcbladdr(struct inpcb *inp, struct in
 		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
 		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)));
 

Modified: head/sys/netinet/ip_options.c
==============================================================================
--- head/sys/netinet/ip_options.c	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/sys/netinet/ip_options.c	Thu Apr 24 23:56:56 2014	(r264905)
@@ -227,8 +227,11 @@ dropit:
 			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));

Modified: head/sys/netinet/ip_output.c
==============================================================================
--- head/sys/netinet/ip_output.c	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/sys/netinet/ip_output.c	Thu Apr 24 23:56:56 2014	(r264905)
@@ -230,7 +230,8 @@ again:
 	 */
 	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 @@ again:
 		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;

Modified: head/sys/netinet6/nd6.c
==============================================================================
--- head/sys/netinet6/nd6.c	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/sys/netinet6/nd6.c	Thu Apr 24 23:56:56 2014	(r264905)
@@ -945,7 +945,7 @@ nd6_is_new_addr_neighbor(struct sockaddr
 	 * 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);

Modified: head/tests/sys/netinet/Makefile
==============================================================================
--- head/tests/sys/netinet/Makefile	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/tests/sys/netinet/Makefile	Thu Apr 24 23:56:56 2014	(r264905)
@@ -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>

Modified: head/tests/sys/netinet/fibs_test.sh
==============================================================================
--- head/tests/sys/netinet/fibs_test.sh	Thu Apr 24 23:28:09 2014	(r264904)
+++ head/tests/sys/netinet/fibs_test.sh	Thu Apr 24 23:56:56 2014	(r264905)
@@ -175,7 +175,6 @@ default_route_with_multiple_fibs_on_same
 
 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_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_
 	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}
-
-	# Add a gateway to the interface with the less specific subnet route
-	setfib ${FIB0} route add default ${GATEWAY0}
+	setup_tap ${FIB0} ${ADDR} ${MASK}
 
-	# 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.

Copied: head/tests/sys/netinet/udp_dontroute.c (from r263737, head/tests/sys/netinet/udp_dontroute.c)
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/tests/sys/netinet/udp_dontroute.c	Thu Apr 24 23:56:56 2014	(r264905, copy of r263737, head/tests/sys/netinet/udp_dontroute.c)
@@ -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);
+}
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

Comment 6 Alan Somers freebsd_committer

2014-04-25 15:44:23 UTC

State Changed
From-To: open->patched

Patched by r264905

Comment 7 commit-hook freebsd_committer

2014-09-18 21:48:10 UTC

A commit references this bug:

Author: asomers
Date: Thu Sep 18 21:48:06 UTC 2014
New revision: 271840
URL: http://svnweb.freebsd.org/changeset/base/271840

Log:
  MFC r264905 and r266860, except for the ATF parts.

  r266860
  Fix unintended KBI change from r264905.  Add _fib versions of
  ifa_ifwithnet() and ifa_ifwithdstaddr()  The legacy functions will call the
  _fib() versions with RT_ALL_FIBS, preserving legacy behavior.

  sys/net/if_var.h
  sys/net/if.c
          Add legacy-compatible functions as described above.  Ensure legacy
          behavior when RT_ALL_FIBS is passed as fibnum.

  sys/netinet/in_pcb.c
  sys/netinet/ip_output.c
  sys/netinet/ip_options.c
  sys/net/route.c
  sys/net/rtsock.c
  sys/netinet6/nd6.c
          Call with _fib() functions if we must use a specific fib, or the
          legacy functions otherwise.

  r264905
  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.

  PR:             kern/187550
  PR:             kern/187552

Changes:
_U  stable/9/
_U  stable/9/sys/
_U  stable/9/sys/net/
  stable/9/sys/net/if.c
  stable/9/sys/net/if_var.h
  stable/9/sys/net/route.c
  stable/9/sys/netinet/in.c

Comment 8 Alan Somers freebsd_committer

2014-09-18 22:00:49 UTC

MFCed to stable/9 by 271840 and stable/10 by 267186.