netdev - Re: [net PATCH 2/2] tools: selftests: add test for changing routes with PTMU exceptions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <43a47c65-d608-e05d-385e-f9b4b7f49ac1@gmail.com>
Date:   Wed, 6 Jan 2021 10:03:19 -0700
From:   David Ahern <dsahern@...il.com>
To:     Sean Tranchetti <stranche@...cinc.com>, davem@...emloft.net,
        netdev@...r.kernel.org, kuba@...nel.org
Cc:     subashab@...earurora.org,
        Sean Tranchetti <stranche@...eaurora.org>,
        Stefano Brivio <sbrivio@...hat.com>
Subject: Re: [net PATCH 2/2] tools: selftests: add test for changing routes
 with PTMU exceptions

On 1/5/21 5:22 PM, Sean Tranchetti wrote:
> From: Sean Tranchetti <stranche@...eaurora.org>
> 
> Adds new 2 new tests to the PTMU script: pmtu_ipv4/6_route_change.
> 
> These tests explicitly test for a recently discovered problem in the
> IPv6 routing framework where PMTU exceptions were not properly released
> when replacing a route via "ip route change ...".
> 
> After creating PMTU exceptions, the route from the device A to R1 will be
> replaced with a new route, then device A will be deleted. If the PMTU
> exceptions were properly cleaned up by the kernel, this device deletion
> will succeed. Otherwise, the unregistration of the device will stall, and
> messages such as the following will be logged in dmesg:
> 
> unregister_netdevice: waiting for veth_A-R1 to become free. Usage count = 4
> 
> Signed-off-by: Sean Tranchetti <stranche@...eaurora.org>
> ---
>  tools/testing/selftests/net/pmtu.sh | 71 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 69 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index 464e31e..64cd2e2 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -162,7 +162,15 @@
>  # - list_flush_ipv6_exception
>  #	Using the same topology as in pmtu_ipv6, create exceptions, and check
>  #	they are shown when listing exception caches, gone after flushing them
> -
> +#
> +# - pmtu_ipv4_route_change
> +#	Use the same topology as in pmtu_ipv4, but issue a route replacement
> +#	command and delete the corresponding device afterward. This tests for
> +#	proper cleanup of the PMTU exceptions by the route replacement path.
> +#	Device unregistration should complete successfully
> +#
> +# - pmtu_ipv6_route_change
> +#	Same as above but with IPv6
>  
>  # Kselftest framework requirement - SKIP code is 4.
>  ksft_skip=4
> @@ -224,7 +232,9 @@ tests="
>  	cleanup_ipv4_exception		ipv4: cleanup of cached exceptions	1
>  	cleanup_ipv6_exception		ipv6: cleanup of cached exceptions	1
>  	list_flush_ipv4_exception	ipv4: list and flush cached exceptions	1
> -	list_flush_ipv6_exception	ipv6: list and flush cached exceptions	1"
> +	list_flush_ipv6_exception	ipv6: list and flush cached exceptions	1
> +	pmtu_ipv4_route_change		ipv4: PMTU exception w/route replace	1
> +	pmtu_ipv6_route_change		ipv6: PMTU exception w/route replace	1"
>  
>  NS_A="ns-A"
>  NS_B="ns-B"
> @@ -1782,6 +1792,63 @@ test_list_flush_ipv6_exception() {
>  	return ${fail}
>  }
>  
> +test_pmtu_ipvX_route_change() {
> +	family=${1}
> +
> +	setup namespaces routing || return 2
> +	trace "${ns_a}"  veth_A-R1    "${ns_r1}" veth_R1-A \
> +	      "${ns_r1}" veth_R1-B    "${ns_b}"  veth_B-R1 \
> +	      "${ns_a}"  veth_A-R2    "${ns_r2}" veth_R2-A \
> +	      "${ns_r2}" veth_R2-B    "${ns_b}"  veth_B-R2
> +
> +	if [ ${family} -eq 4 ]; then
> +		ping=ping
> +		dst1="${prefix4}.${b_r1}.1"
> +		dst2="${prefix4}.${b_r2}.1"
> +		gw="${prefix4}.${a_r1}.2"
> +	else
> +		ping=${ping6}
> +		dst1="${prefix6}:${b_r1}::1"
> +		dst2="${prefix6}:${b_r2}::1"
> +		gw="${prefix6}:${a_r1}::2"
> +	fi
> +
> +	# Set up initial MTU values
> +	mtu "${ns_a}"  veth_A-R1 2000
> +	mtu "${ns_r1}" veth_R1-A 2000
> +	mtu "${ns_r1}" veth_R1-B 1400
> +	mtu "${ns_b}"  veth_B-R1 1400
> +
> +	mtu "${ns_a}"  veth_A-R2 2000
> +	mtu "${ns_r2}" veth_R2-A 2000
> +	mtu "${ns_r2}" veth_R2-B 1500
> +	mtu "${ns_b}"  veth_B-R2 1500
> +
> +	# Create route exceptions
> +	run_cmd ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst1}
> +	run_cmd ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst2}
> +
> +	# Check that exceptions have been created with the correct PMTU
> +	pmtu_1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})"
> +	check_pmtu_value "1400" "${pmtu_1}" "exceeding MTU" || return 1
> +	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
> +	check_pmtu_value "1500" "${pmtu_2}" "exceeding MTU" || return 1
> +
> +	# Replace the route from A to R1
> +	run_cmd ${ns_a} ip route change default via ${gw}
> +
> +	# Delete the device in A
> +	run_cmd ${ns_a} ip link del "veth_A-R1"
> +}
> +
> +test_pmtu_ipv4_route_change() {
> +	test_pmtu_ipvX_route_change 4
> +}
> +
> +test_pmtu_ipv6_route_change() {
> +	test_pmtu_ipvX_route_change 6
> +}
> +
>  usage() {
>  	echo
>  	echo "$0 [OPTIONS] [TEST]..."
> 

Thanks for adding the tests.
Reviewed-by: David Ahern <dsahern@...nel.org>