[net-next] selftests: net: Introduce first PMTU test

One single test implemented so far: test_pmtu_vti6_exception
checks that the PMTU of a route exception, caused by a tunnel
exceeding the link layer MTU, is affected by administrative
changes of the tunnel MTU. Creation of the route exception is
checked too.

Requested-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
This test will currently fail without "[PATCH net v2] ipv6: Reflect
MTU changes on PMTU of exceptions for MTU-less routes"

 tools/testing/selftests/net/Makefile |   2 +-
 tools/testing/selftests/net/pmtu.sh  | 159 +++++++++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/net/pmtu.sh

On 3/5/18 3:45 PM, Stefano Brivio wrote:
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> new file mode 100755
> index 000000000000..eb186ca3e5e4
> --- /dev/null
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -0,0 +1,159 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Check that route PMTU values match expectations
> +#
> +# Tests currently implemented:
> +#
> +# - test_pmtu_vti6_exception
> +#	Set up vti6 tunnel on top of veth, with xfrm states and policies, in two
> +#	namespaces with matching endpoints. Check that route exception is
> +#	created by exceeding link layer MTU with ping to other endpoint. Then
> +#	decrease and increase MTU of tunnel, checking that route exception PMTU
> +#	changes accordingly
> +
> +NS_A="ns-$(mktemp -u XXXXXX)"
> +NS_B="ns-$(mktemp -u XXXXXX)"
> +ns_a="ip netns exec ${NS_A}"
> +ns_b="ip netns exec ${NS_B}"
> +
> +veth6_a_addr="fd00:1::a"
> +veth6_b_addr="fd00:1::b"
> +veth6_mask="64"
> +
> +vti6_a_addr="fd00:2::a"
> +vti6_b_addr="fd00:2::b"
> +vti6_mask="64"
> +
> +setup_namespaces() {
> +	ip netns add ${NS_A} || return 1
> +	ip netns add ${NS_B}

for basic config commands that should work every encasing in
set -e
...
set +e

simplifies error handling. IMO, it is relevant for netns and veth config
commands. Not so much for the xfrm commands which need to load modules
or depend on features that need config options.

> +
> +	return 0
> +}
> +
> +setup_veth() {
> +	${ns_a} ip link add veth_a type veth peer name veth_b || return 1
> +	${ns_a} ip link set veth_b netns ${NS_B}
> +	
> +	${ns_a} ip link set veth_a up
> +	${ns_b} ip link set veth_b up
> +
> +	${ns_a} ip addr add ${veth6_a_addr}/${veth6_mask} dev veth_a
> +	${ns_b} ip addr add ${veth6_b_addr}/${veth6_mask} dev veth_b
> +
> +	return 0
> +}
> +
> +setup_vti6() {
> +	${ns_a} ip link add vti_a type vti6 local ${veth6_a_addr} remote ${veth6_b_addr} key 10 || return 1
> +	${ns_b} ip link add vti_b type vti6 local ${veth6_b_addr} remote ${veth6_a_addr} key 10
> +
> +	${ns_a} ip link set vti_a up
> +	${ns_b} ip link set vti_b up
> +
> +	${ns_a} ip addr add ${vti6_a_addr}/${vti6_mask} dev vti_a
> +	${ns_b} ip addr add ${vti6_b_addr}/${vti6_mask} dev vti_b
> +
> +	return 0
> +}
> +
> +setup_xfrm() {
> +	${ns_a} ip -6 xfrm state add src ${veth6_a_addr} dst ${veth6_b_addr} spi 0x1000 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel || return 1
> +	${ns_a} ip -6 xfrm state add src ${veth6_b_addr} dst ${veth6_a_addr} spi 0x1001 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel
> +	${ns_a} ip -6 xfrm policy add dir out mark 10 tmpl src ${veth6_a_addr} dst ${veth6_b_addr} proto esp mode tunnel
> +	${ns_a} ip -6 xfrm policy add dir in mark 10 tmpl src ${veth6_b_addr} dst ${veth6_a_addr} proto esp mode tunnel
> +
> +	${ns_b} ip -6 xfrm state add src ${veth6_a_addr} dst ${veth6_b_addr} spi 0x1000 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel
> +	${ns_b} ip -6 xfrm state add src ${veth6_b_addr} dst ${veth6_a_addr} spi 0x1001 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel
> +	${ns_b} ip -6 xfrm policy add dir out mark 10 tmpl src ${veth6_b_addr} dst ${veth6_a_addr} proto esp mode tunnel
> +	${ns_b} ip -6 xfrm policy add dir in mark 10 tmpl src ${veth6_a_addr} dst ${veth6_b_addr} proto esp mode tunnel
> +
> +	return 0
> +}
> +
> +setup() {
> +	tunnel_type="$1"
> +
> +	[ "$(id -u)" -ne 0 ] && (echo "SKIP: need to run as root" && exit 0)
> +
> +	setup_namespaces || (echo "SKIP: namespaces not supported" && exit 0)
> +	setup_veth || (echo "SKIP: veth not supported" && exit 0)

You use this style ('<command> || (...)' or '<command> && (...)') a lot
and it does not actually exit the script. You can verify by adding
'return 1' to either function.

> +
> +	case ${tunnel_type} in
> +	"vti6")
> +		setup_vti6 && (echo "SKIP: vti6 not supported" && exit 0)
> +		setup_xfrm && (echo "SKIP: xfrm not supported" && exit 0)
> +		;;
> +	*)
> +		;;
> +	esac
> +}
> +
> +cleanup() {
> +	ip netns del ${NS_A} 2 > /dev/null
> +	ip netns del ${NS_B} 2 > /dev/null
> +}
> +
> +mtu() {
> +	ns_cmd="${1}"
> +	dev="${2}"
> +	mtu="${3}"
> +
> +	${ns_cmd} ip link set dev ${dev} mtu ${mtu}
> +}
> +
> +route_get_dst_exception() {
> +	dst="${1}"
> +
> +	${ns_a} ip -6 route get "${dst}" | tail -n1 | tr -s ' '
> +}
> +
> +route_get_dst_pmtu_from_exception() {
> +	dst="${1}"
> +
> +	exception="$(route_get_dst_exception ${dst})"
> +	next=0
> +	for i in ${exception}; do
> +		[ ${next} -eq 1 ] && echo "${i}" && return
> +		[ "${i}" = "mtu" ] && next=1
> +	done
> +}
> +
> +test_pmtu_vti6_exception() {
> +	setup vti6
> +
> +	# Create route exception by exceeding link layer MTU
> +	mtu "${ns_a}" vti_a 5000
> +	mtu "${ns_b}" vti_b 5000
> +	${ns_a} ping6 -q -i 0.1 -w 2 -s 60000 ${vti6_b_addr} > /dev/null
> +
> +	# Check that exception was created
> +	exception="$(route_get_dst_exception ${vti6_b_addr})"
> +	case ${exception} in
> +		" cache expires "*) : ;;
> +		*) echo "FAIL: Tunnel exceeding link layer MTU didn't create route exception"; exit 1 ;;

I see errors on this check and did not have time to resolve why.
    bash -x pmtu.sh
shows
+ exception='fd00:2::b from :: dev vti_a src fd00:2::a metric 0 expires
598sec mtu 1426 pref medium'

You are expecting 'cache expires' so perhaps a difference in iproute2
output?

> +	esac
> +
> +	# Decrease tunnel MTU, check for PMTU decrease in route exception
> +	mtu "${ns_a}" vti_a 3000
> +	if [ "$(route_get_dst_pmtu_from_exception ${vti6_b_addr})" -ne 3000 ]; then
> +		echo "FAIL: Decreasing tunnel MTU didn't decrease route exception PMTU"
> +		exit 1
> +	fi
> +
> +	# Increase tunnel MTU, check for PMTU increase in route exception
> +	mtu "${ns_a}" vti_a 9000
> +	if [ "$(route_get_dst_pmtu_from_exception ${vti6_b_addr})" -ne 9000 ]; then
> +		echo "FAIL: Increasing tunnel MTU didn't increase route exception PMTU"
> +		exit 1
> +	fi
> +
> +	echo "PASS"
> +}
> +
> +trap cleanup EXIT
> +
> +test_pmtu_vti6_exception
> +
> +exit 0
> 


Also, I see a lot of messages on console:

[11057.340816] ip6_tunnel: vti_a xmit: Local address not yet configured!

This is a good test case to add given the layers involved.

On Mon, 5 Mar 2018 22:26:42 -0700
David Ahern <dsahern@gmail.com> wrote:

> On 3/5/18 3:45 PM, Stefano Brivio wrote:
> > diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> > new file mode 100755
> > index 000000000000..eb186ca3e5e4
> > --- /dev/null
> > +++ b/tools/testing/selftests/net/pmtu.sh
> > @@ -0,0 +1,159 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Check that route PMTU values match expectations
> > +#
> > +# Tests currently implemented:
> > +#
> > +# - test_pmtu_vti6_exception
> > +#	Set up vti6 tunnel on top of veth, with xfrm states and policies, in two
> > +#	namespaces with matching endpoints. Check that route exception is
> > +#	created by exceeding link layer MTU with ping to other endpoint. Then
> > +#	decrease and increase MTU of tunnel, checking that route exception PMTU
> > +#	changes accordingly
> > +
> > +NS_A="ns-$(mktemp -u XXXXXX)"
> > +NS_B="ns-$(mktemp -u XXXXXX)"
> > +ns_a="ip netns exec ${NS_A}"
> > +ns_b="ip netns exec ${NS_B}"
> > +
> > +veth6_a_addr="fd00:1::a"
> > +veth6_b_addr="fd00:1::b"
> > +veth6_mask="64"
> > +
> > +vti6_a_addr="fd00:2::a"
> > +vti6_b_addr="fd00:2::b"
> > +vti6_mask="64"
> > +
> > +setup_namespaces() {
> > +	ip netns add ${NS_A} || return 1
> > +	ip netns add ${NS_B}  
> 
> for basic config commands that should work every encasing in
> set -e
> ...
> set +e
> 
> simplifies error handling. IMO, it is relevant for netns and veth config
> commands. Not so much for the xfrm commands which need to load modules
> or depend on features that need config options.

Still I would prefer in that case to handle the error explicitly and be
verbose about what went wrong, because also netns and veth might be
missing. Sure, USER_NS is in 'config', but one might actually not use
that file, or try to run this as a single test.

> > +
> > +	return 0
> > +}
> > +
> > +setup_veth() {
> > +	${ns_a} ip link add veth_a type veth peer name veth_b || return 1
> > +	${ns_a} ip link set veth_b netns ${NS_B}
> > +	
> > +	${ns_a} ip link set veth_a up
> > +	${ns_b} ip link set veth_b up
> > +
> > +	${ns_a} ip addr add ${veth6_a_addr}/${veth6_mask} dev veth_a
> > +	${ns_b} ip addr add ${veth6_b_addr}/${veth6_mask} dev veth_b
> > +
> > +	return 0
> > +}
> > +
> > +setup_vti6() {
> > +	${ns_a} ip link add vti_a type vti6 local ${veth6_a_addr} remote ${veth6_b_addr} key 10 || return 1
> > +	${ns_b} ip link add vti_b type vti6 local ${veth6_b_addr} remote ${veth6_a_addr} key 10
> > +
> > +	${ns_a} ip link set vti_a up
> > +	${ns_b} ip link set vti_b up
> > +
> > +	${ns_a} ip addr add ${vti6_a_addr}/${vti6_mask} dev vti_a
> > +	${ns_b} ip addr add ${vti6_b_addr}/${vti6_mask} dev vti_b
> > +
> > +	return 0
> > +}
> > +
> > +setup_xfrm() {
> > +	${ns_a} ip -6 xfrm state add src ${veth6_a_addr} dst ${veth6_b_addr} spi 0x1000 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel || return 1
> > +	${ns_a} ip -6 xfrm state add src ${veth6_b_addr} dst ${veth6_a_addr} spi 0x1001 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel
> > +	${ns_a} ip -6 xfrm policy add dir out mark 10 tmpl src ${veth6_a_addr} dst ${veth6_b_addr} proto esp mode tunnel
> > +	${ns_a} ip -6 xfrm policy add dir in mark 10 tmpl src ${veth6_b_addr} dst ${veth6_a_addr} proto esp mode tunnel
> > +
> > +	${ns_b} ip -6 xfrm state add src ${veth6_a_addr} dst ${veth6_b_addr} spi 0x1000 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel
> > +	${ns_b} ip -6 xfrm state add src ${veth6_b_addr} dst ${veth6_a_addr} spi 0x1001 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel
> > +	${ns_b} ip -6 xfrm policy add dir out mark 10 tmpl src ${veth6_b_addr} dst ${veth6_a_addr} proto esp mode tunnel
> > +	${ns_b} ip -6 xfrm policy add dir in mark 10 tmpl src ${veth6_a_addr} dst ${veth6_b_addr} proto esp mode tunnel
> > +
> > +	return 0
> > +}
> > +
> > +setup() {
> > +	tunnel_type="$1"
> > +
> > +	[ "$(id -u)" -ne 0 ] && (echo "SKIP: need to run as root" && exit 0)
> > +
> > +	setup_namespaces || (echo "SKIP: namespaces not supported" && exit 0)
> > +	setup_veth || (echo "SKIP: veth not supported" && exit 0)  
> 
> You use this style ('<command> || (...)' or '<command> && (...)') a lot
> and it does not actually exit the script. You can verify by adding
> 'return 1' to either function.

I forgot that runs in a subshell, will fix in v2 by reversing the
return values, so that I can just have function && echo ... && exit 0,
which is not ambiguous on any shell.

> > +
> > +	case ${tunnel_type} in
> > +	"vti6")
> > +		setup_vti6 && (echo "SKIP: vti6 not supported" && exit 0)
> > +		setup_xfrm && (echo "SKIP: xfrm not supported" && exit 0)
> > +		;;
> > +	*)
> > +		;;
> > +	esac
> > +}
> > +
> > +cleanup() {
> > +	ip netns del ${NS_A} 2 > /dev/null
> > +	ip netns del ${NS_B} 2 > /dev/null
> > +}
> > +
> > +mtu() {
> > +	ns_cmd="${1}"
> > +	dev="${2}"
> > +	mtu="${3}"
> > +
> > +	${ns_cmd} ip link set dev ${dev} mtu ${mtu}
> > +}
> > +
> > +route_get_dst_exception() {
> > +	dst="${1}"
> > +
> > +	${ns_a} ip -6 route get "${dst}" | tail -n1 | tr -s ' '
> > +}
> > +
> > +route_get_dst_pmtu_from_exception() {
> > +	dst="${1}"
> > +
> > +	exception="$(route_get_dst_exception ${dst})"
> > +	next=0
> > +	for i in ${exception}; do
> > +		[ ${next} -eq 1 ] && echo "${i}" && return
> > +		[ "${i}" = "mtu" ] && next=1
> > +	done
> > +}
> > +
> > +test_pmtu_vti6_exception() {
> > +	setup vti6
> > +
> > +	# Create route exception by exceeding link layer MTU
> > +	mtu "${ns_a}" vti_a 5000
> > +	mtu "${ns_b}" vti_b 5000
> > +	${ns_a} ping6 -q -i 0.1 -w 2 -s 60000 ${vti6_b_addr} > /dev/null
> > +
> > +	# Check that exception was created
> > +	exception="$(route_get_dst_exception ${vti6_b_addr})"
> > +	case ${exception} in
> > +		" cache expires "*) : ;;
> > +		*) echo "FAIL: Tunnel exceeding link layer MTU didn't create route exception"; exit 1 ;;  
> 
> I see errors on this check and did not have time to resolve why.
>     bash -x pmtu.sh
> shows
> + exception='fd00:2::b from :: dev vti_a src fd00:2::a metric 0 expires
> 598sec mtu 1426 pref medium'
> 
> You are expecting 'cache expires' so perhaps a difference in iproute2
> output?

It seems so. I'll make this more robust in v2, by just checking for
"mtu <n>" in whatever position, without piping to tail and tr. After
all, I'm looking for a PMTU exception here, not any exception.

> > +	esac
> > +
> > +	# Decrease tunnel MTU, check for PMTU decrease in route exception
> > +	mtu "${ns_a}" vti_a 3000
> > +	if [ "$(route_get_dst_pmtu_from_exception ${vti6_b_addr})" -ne 3000 ]; then
> > +		echo "FAIL: Decreasing tunnel MTU didn't decrease route exception PMTU"
> > +		exit 1
> > +	fi
> > +
> > +	# Increase tunnel MTU, check for PMTU increase in route exception
> > +	mtu "${ns_a}" vti_a 9000
> > +	if [ "$(route_get_dst_pmtu_from_exception ${vti6_b_addr})" -ne 9000 ]; then
> > +		echo "FAIL: Increasing tunnel MTU didn't increase route exception PMTU"
> > +		exit 1
> > +	fi
> > +
> > +	echo "PASS"
> > +}
> > +
> > +trap cleanup EXIT
> > +
> > +test_pmtu_vti6_exception
> > +
> > +exit 0
> >   
> 
> Also, I see a lot of messages on console:
> 
> [11057.340816] ip6_tunnel: vti_a xmit: Local address not yet configured!

This is because the address on vti_a is not configured immediately as
soon as I add it. I'm going to add a 'sleep 1' after configuring.

> This is a good test case to add given the layers involved.

Thanks for your review, I'll post v2 soon.