[PATCH net-next 0/3] selftests: add more PMTU tests

[PATCH net-next 0/3] selftests: add more PMTU tests

[Sabrina Dubroca]

The current selftests for PMTU cover VTI tunnels, but there's nothing
about the generation and handling of PMTU exceptions by intermediate
routers. This series adds and improves existing helpers, then adds
IPv4 and IPv6 selftests with a setup involving an intermediate router.

Joint work with Stefano Brivio.

Sabrina Dubroca (2):
  selftests: pmtu: extend MTU parsing helper to locked MTU
  selftests: pmtu: add basic IPv4 and IPv6 PMTU tests

Stefano Brivio (1):
  selftests: pmtu: Introduce check_pmtu_value()

 tools/testing/selftests/net/pmtu.sh | 258 ++++++++++++++++++++++++----
 1 file changed, 227 insertions(+), 31 deletions(-)

-- 
2.19.0

[PATCH net-next 1/3] selftests: pmtu: Introduce check_pmtu_value()

[Sabrina Dubroca]

From: Stefano Brivio <sbrivio@redhat.com>

Introduce and use a function that checks PMTU values against
expected values and logs error messages, to remove some clutter.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 tools/testing/selftests/net/pmtu.sh | 49 +++++++++++++----------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 0ab9423d009f..6c38a7637744 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -229,6 +229,19 @@ route_get_dst_pmtu_from_exception() {
 	mtu_parse "$(route_get_dst_exception "${ns_cmd}" ${dst})"
 }
 
+check_pmtu_value() {
+	expected="${1}"
+	value="${2}"
+	event="${3}"
+
+	[ "${expected}" = "any" ] && [ -n "${value}" ] && return 0
+	[ "${value}" = "${expected}" ] && return 0
+	[ -z "${value}" ] &&    err "  PMTU exception wasn't created after ${event}" && return 1
+	[ -z "${expected}" ] && err "  PMTU exception shouldn't exist after ${event}" && return 1
+	err "  found PMTU exception with incorrect MTU ${value}, expected ${expected}, after ${event}"
+	return 1
+}
+
 test_pmtu_vti4_exception() {
 	setup namespaces veth vti4 xfrm4 || return 2
 
@@ -248,24 +261,13 @@ test_pmtu_vti4_exception() {
 	# exception is created
 	${ns_a} ping -q -M want -i 0.1 -w 2 -s ${ping_payload} ${vti4_b_addr} > /dev/null
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti4_b_addr})"
-	if [ "${pmtu}" != "" ]; then
-		err "  unexpected exception created with PMTU ${pmtu} for IP payload length ${esp_payload_rfc4106}"
-		return 1
-	fi
+	check_pmtu_value "" "${pmtu}" "sending packet smaller than PMTU (IP payload length ${esp_payload_rfc4106})" || return 1
 
 	# Now exceed link layer MTU by one byte, check that exception is created
+	# with the right PMTU value
 	${ns_a} ping -q -M want -i 0.1 -w 2 -s $((ping_payload + 1)) ${vti4_b_addr} > /dev/null
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti4_b_addr})"
-	if [ "${pmtu}" = "" ]; then
-		err "  exception not created for IP payload length $((esp_payload_rfc4106 + 1))"
-		return 1
-	fi
-
-	# ...with the right PMTU value
-	if [ ${pmtu} -ne ${esp_payload_rfc4106} ]; then
-		err "  wrong PMTU ${pmtu} in exception, expected: ${esp_payload_rfc4106}"
-		return 1
-	fi
+	check_pmtu_value "${esp_payload_rfc4106}" "${pmtu}" "exceeding PMTU (IP payload length $((esp_payload_rfc4106 + 1)))"
 }
 
 test_pmtu_vti6_exception() {
@@ -280,25 +282,18 @@ test_pmtu_vti6_exception() {
 	${ns_a} ${ping6} -q -i 0.1 -w 2 -s 60000 ${vti6_b_addr} > /dev/null
 
 	# Check that exception was created
-	if [ "$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})" = "" ]; then
-		err "  tunnel exceeding link layer MTU didn't create route exception"
-		return 1
-	fi
+	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})"
+	check_pmtu_value any "${pmtu}" "creating tunnel exceeding link layer MTU" || return 1
 
 	# Decrease tunnel MTU, check for PMTU decrease in route exception
 	mtu "${ns_a}" vti6_a 3000
-
-	if [ "$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})" -ne 3000 ]; then
-		err "  decreasing tunnel MTU didn't decrease route exception PMTU"
-		fail=1
-	fi
+	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})"
+	check_pmtu_value "3000" "${pmtu}" "decreasing tunnel MTU" || fail=1
 
 	# Increase tunnel MTU, check for PMTU increase in route exception
 	mtu "${ns_a}" vti6_a 9000
-	if [ "$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})" -ne 9000 ]; then
-		err "  increasing tunnel MTU didn't increase route exception PMTU"
-		fail=1
-	fi
+	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})"
+	check_pmtu_value "9000" "${pmtu}" "increasing tunnel MTU" || fail=1
 
 	return ${fail}
 }
-- 
2.19.0

[PATCH net-next 2/3] selftests: pmtu: extend MTU parsing helper to locked MTU

[Sabrina Dubroca]

The mtu_parse helper introduced in commit f2c929feeccd ("selftests:
pmtu: Factor out MTU parsing helper") can only handle "mtu 1234", but
not "mtu lock 1234". Extend it, so that we can do IPv4 tests with PMTU
smaller than net.ipv4.route.min_pmtu

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 tools/testing/selftests/net/pmtu.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 6c38a7637744..03e56a27f69c 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -196,7 +196,9 @@ mtu_parse() {
 
 	next=0
 	for i in ${input}; do
+		[ ${next} -eq 1 -a "${i}" = "lock" ] && next=2 && continue
 		[ ${next} -eq 1 ] && echo "${i}" && return
+		[ ${next} -eq 2 ] && echo "lock ${i}" && return
 		[ "${i}" = "mtu" ] && next=1
 	done
 }
-- 
2.19.0

[PATCH net-next 3/3] selftests: pmtu: add basic IPv4 and IPv6 PMTU tests

[Sabrina Dubroca]

Commit d1f1b9cbf34c ("selftests: net: Introduce first PMTU test") and
follow-ups introduced some PMTU tests, but they all rely on tunneling,
and, particularly, on VTI.

These new tests use simple routing to exercise the generation and
update of PMTU exceptions in IPv4 and IPv6.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 tools/testing/selftests/net/pmtu.sh | 207 +++++++++++++++++++++++++++-
 1 file changed, 203 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 03e56a27f69c..b9cdb68df4c5 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -6,6 +6,26 @@
 #
 # Tests currently implemented:
 #
+# - pmtu_ipv4
+#	Set up two namespaces, A and B, with two paths between them over routers
+#	R1 and R2 (also implemented with namespaces), with different MTUs:
+#
+#	  segment a_r1    segment b_r1		a_r1: 2000
+#	.--------------R1--------------.	a_r2: 1500
+#	A                               B	a_r3: 2000
+#	'--------------R2--------------'	a_r4: 1400
+#	  segment a_r2    segment b_r2
+#
+#	Check that PMTU exceptions with the correct PMTU are created. Then
+#	decrease and increase the MTU of the local link for one of the paths,
+#	A to R1, checking that route exception PMTU changes accordingly over
+#	this path. Also check that locked exceptions are created when an ICMP
+#	message advertising a PMTU smaller than net.ipv4.route.min_pmtu is
+#	received
+#
+# - pmtu_ipv6
+#	Same as pmtu_ipv4, except for locked PMTU tests, using IPv6
+#
 # - pmtu_vti4_exception
 #	Set up vti tunnel on top of veth, with xfrm states and policies, in two
 #	namespaces with matching endpoints. Check that route exception is not
@@ -50,6 +70,8 @@ ksft_skip=4
 which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
 
 tests="
+	pmtu_ipv4_exception		ipv4: PMTU exceptions
+	pmtu_ipv6_exception		ipv6: PMTU exceptions
 	pmtu_vti6_exception		vti6: PMTU exceptions
 	pmtu_vti4_exception		vti4: PMTU exceptions
 	pmtu_vti4_default_mtu		vti4: default MTU assignment
@@ -60,8 +82,45 @@ tests="
 
 NS_A="ns-$(mktemp -u XXXXXX)"
 NS_B="ns-$(mktemp -u XXXXXX)"
+NS_R1="ns-$(mktemp -u XXXXXX)"
+NS_R2="ns-$(mktemp -u XXXXXX)"
 ns_a="ip netns exec ${NS_A}"
 ns_b="ip netns exec ${NS_B}"
+ns_r1="ip netns exec ${NS_R1}"
+ns_r2="ip netns exec ${NS_R2}"
+
+# Addressing and routing for tests with routers: four network segments, with
+# index SEGMENT between 1 and 4, a common prefix (PREFIX4 or PREFIX6) and an
+# identifier ID, which is 1 for hosts (A and B), 2 for routers (R1 and R2).
+# Addresses are:
+# - IPv4: PREFIX4.SEGMENT.ID (/24)
+# - IPv6: PREFIX6:SEGMENT::ID (/64)
+prefix4="192.168"
+prefix6="fd00"
+a_r1=1
+a_r2=2
+b_r1=3
+b_r2=4
+#	ns	peer	segment
+routing_addrs="
+	A	R1	${a_r1}
+	A	R2	${a_r2}
+	B	R1	${b_r1}
+	B	R2	${b_r2}
+"
+# Traffic from A to B goes through R1 by default, and through R2, if destined to
+# B's address on the b_r2 segment.
+# Traffic from B to A goes through R1.
+#	ns	destination		gateway
+routes="
+	A	default			${prefix4}.${a_r1}.2
+	A	${prefix4}.${b_r2}.1	${prefix4}.${a_r2}.2
+	B	default			${prefix4}.${b_r1}.2
+
+	A	default			${prefix6}:${a_r1}::2
+	A	${prefix6}:${b_r2}::1	${prefix6}:${a_r2}::2
+	B	default			${prefix6}:${b_r1}::2
+"
 
 veth4_a_addr="192.168.1.1"
 veth4_b_addr="192.168.1.2"
@@ -94,9 +153,15 @@ err_flush() {
 	err_buf=
 }
 
+# Find the auto-generated name for this namespace
+nsname() {
+	eval echo \$NS_$1
+}
+
 setup_namespaces() {
-	ip netns add ${NS_A} || return 1
-	ip netns add ${NS_B}
+	for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do
+		ip netns add ${n} || return 1
+	done
 }
 
 setup_veth() {
@@ -167,6 +232,49 @@ setup_xfrm6() {
 	setup_xfrm 6 ${veth6_a_addr} ${veth6_b_addr}
 }
 
+setup_routing() {
+	for i in ${NS_R1} ${NS_R2}; do
+		ip netns exec ${i} sysctl -q net/ipv4/ip_forward=1
+		ip netns exec ${i} sysctl -q net/ipv6/conf/all/forwarding=1
+	done
+
+	for i in ${routing_addrs}; do
+		[ "${ns}" = "" ]	&& ns="${i}"		&& continue
+		[ "${peer}" = "" ]	&& peer="${i}"		&& continue
+		[ "${segment}" = "" ]	&& segment="${i}"
+
+		ns_name="$(nsname ${ns})"
+		peer_name="$(nsname ${peer})"
+		if="veth_${ns}-${peer}"
+		ifpeer="veth_${peer}-${ns}"
+
+		# Create veth links
+		ip link add ${if} up netns ${ns_name} type veth peer name ${ifpeer} netns ${peer_name} || return 1
+		ip -n ${peer_name} link set dev ${ifpeer} up
+
+		# Add addresses
+		ip -n ${ns_name}   addr add ${prefix4}.${segment}.1/24  dev ${if}
+		ip -n ${ns_name}   addr add ${prefix6}:${segment}::1/64 dev ${if}
+
+		ip -n ${peer_name} addr add ${prefix4}.${segment}.2/24  dev ${ifpeer}
+		ip -n ${peer_name} addr add ${prefix6}:${segment}::2/64 dev ${ifpeer}
+
+		ns=""; peer=""; segment=""
+	done
+
+	for i in ${routes}; do
+		[ "${ns}" = "" ]	&& ns="${i}"		&& continue
+		[ "${addr}" = "" ]	&& addr="${i}"		&& continue
+		[ "${gw}" = "" ]	&& gw="${i}"
+
+		ns_name="$(nsname ${ns})"
+
+		ip -n ${ns_name} route add ${addr} via ${gw}
+
+		ns=""; addr=""; gw=""
+	done
+}
+
 setup() {
 	[ "$(id -u)" -ne 0 ] && echo "  need to run as root" && return $ksft_skip
 
@@ -178,8 +286,9 @@ setup() {
 
 cleanup() {
 	[ ${cleanup_done} -eq 1 ] && return
-	ip netns del ${NS_A} 2> /dev/null
-	ip netns del ${NS_B} 2> /dev/null
+	for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do
+		ip netns del ${n} 2> /dev/null
+	done
 	cleanup_done=1
 }
 
@@ -244,6 +353,96 @@ check_pmtu_value() {
 	return 1
 }
 
+test_pmtu_ipvX() {
+	family=${1}
+
+	setup namespaces routing || return 2
+
+	if [ ${family} -eq 4 ]; then
+		ping=ping
+		dst1="${prefix4}.${b_r1}.1"
+		dst2="${prefix4}.${b_r2}.1"
+	else
+		ping=${ping6}
+		dst1="${prefix6}:${b_r1}::1"
+		dst2="${prefix6}:${b_r2}::1"
+	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
+	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1800 ${dst1} > /dev/null
+	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1800 ${dst2} > /dev/null
+
+	# 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
+
+	# Decrease local MTU below PMTU, check for PMTU decrease in route exception
+	mtu "${ns_a}"  veth_A-R1 1300
+	mtu "${ns_r1}" veth_R1-A 1300
+	pmtu_1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})"
+	check_pmtu_value "1300" "${pmtu_1}" "decreasing local MTU" || return 1
+	# Second exception shouldn't be modified
+	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+	check_pmtu_value "1500" "${pmtu_2}" "changing local MTU on a link not on this path" || return 1
+
+	# Increase MTU, check for PMTU increase in route exception
+	mtu "${ns_a}"  veth_A-R1 1700
+	mtu "${ns_r1}" veth_R1-A 1700
+	pmtu_1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})"
+	check_pmtu_value "1700" "${pmtu_1}" "increasing local MTU" || return 1
+	# Second exception shouldn't be modified
+	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+	check_pmtu_value "1500" "${pmtu_2}" "changing local MTU on a link not on this path" || return 1
+
+	# Skip PMTU locking tests for IPv6
+	[ $family -eq 6 ] && return 0
+
+	# Decrease remote MTU on path via R2, get new exception
+	mtu "${ns_r2}" veth_R2-B 400
+	mtu "${ns_b}"  veth_B-R2 400
+	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1400 ${dst2} > /dev/null
+	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+	check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1
+
+	# Decrease local MTU below PMTU
+	mtu "${ns_a}"  veth_A-R2 500
+	mtu "${ns_r2}" veth_R2-A 500
+	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+	check_pmtu_value "500" "${pmtu_2}" "decreasing local MTU" || return 1
+
+	# Increase local MTU
+	mtu "${ns_a}"  veth_A-R2 1500
+	mtu "${ns_r2}" veth_R2-A 1500
+	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+	check_pmtu_value "1500" "${pmtu_2}" "increasing local MTU" || return 1
+
+	# Get new exception
+	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1400 ${dst2} > /dev/null
+	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+	check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1
+}
+
+test_pmtu_ipv4_exception() {
+	test_pmtu_ipvX 4
+}
+
+test_pmtu_ipv6_exception() {
+	test_pmtu_ipvX 6
+}
+
 test_pmtu_vti4_exception() {
 	setup namespaces veth vti4 xfrm4 || return 2
 
-- 
2.19.0

Re: [PATCH net-next 1/3] selftests: pmtu: Introduce check_pmtu_value()

[David Ahern]

On 10/8/18 6:37 AM, Sabrina Dubroca wrote:
> From: Stefano Brivio <sbrivio@redhat.com>
> 
> Introduce and use a function that checks PMTU values against
> expected values and logs error messages, to remove some clutter.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  tools/testing/selftests/net/pmtu.sh | 49 +++++++++++++----------------
>  1 file changed, 22 insertions(+), 27 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@gmail.com>

Re: [PATCH net-next 2/3] selftests: pmtu: extend MTU parsing helper to locked MTU

[David Ahern]

On 10/8/18 6:37 AM, Sabrina Dubroca wrote:
> The mtu_parse helper introduced in commit f2c929feeccd ("selftests:
> pmtu: Factor out MTU parsing helper") can only handle "mtu 1234", but
> not "mtu lock 1234". Extend it, so that we can do IPv4 tests with PMTU
> smaller than net.ipv4.route.min_pmtu
> 
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  tools/testing/selftests/net/pmtu.sh | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: David Ahern <dsahern@gmail.com>

Re: [PATCH net-next 3/3] selftests: pmtu: add basic IPv4 and IPv6 PMTU tests

[David Ahern]

On 10/8/18 6:37 AM, Sabrina Dubroca wrote:
> Commit d1f1b9cbf34c ("selftests: net: Introduce first PMTU test") and
> follow-ups introduced some PMTU tests, but they all rely on tunneling,
> and, particularly, on VTI.
> 
> These new tests use simple routing to exercise the generation and
> update of PMTU exceptions in IPv4 and IPv6.
> 
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  tools/testing/selftests/net/pmtu.sh | 207 +++++++++++++++++++++++++++-
>  1 file changed, 203 insertions(+), 4 deletions(-)
> 

Thanks for adding more pmtu tests.

Reviewed-by: David Ahern <dsahern@gmail.com>

Re: [PATCH net-next 0/3] selftests: add more PMTU tests

[David Miller]

From: Sabrina Dubroca <sd@queasysnail.net>
Date: Mon,  8 Oct 2018 14:37:02 +0200

> The current selftests for PMTU cover VTI tunnels, but there's nothing
> about the generation and handling of PMTU exceptions by intermediate
> routers. This series adds and improves existing helpers, then adds
> IPv4 and IPv6 selftests with a setup involving an intermediate router.
> 
> Joint work with Stefano Brivio.

This is really great stuff to see.

Series applied, thanks.