selftests: pmtu: fix and increase coverage
Subsystems (3)
Paolo Abeni
maintainerv2 cover selftests: pmtu: fix and increase coverage
This series includes a fixup for the pmtu.sh test script, related to IPv6 address management, and adds coverage for the recently reported and fixed PMTU exception issue
v1 -> v2:
- several script cleanups
Paolo Abeni (2):
selftests: pmtu: disable DAD in all namespaces
selftests: pmtu: add explicit tests for PMTU exceptions cleanup
tools/testing/selftests/net/pmtu.sh | 95 ++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 16 deletions(-)
Signature · 1 line
2.20.1
Paolo Abeni
maintainerPATCH v2 2/2 selftests: pmtu: add explicit tests for PMTU exceptions cleanup
Add a couple of new tests, explicitly checking that the kernel timely releases PMTU exceptions on related device removal. This is mostly a regression test vs the issue fixed by commit f5b51fe804ec ("ipv6: route: purge exception on removal")
Only 2 new test cases have been added, instead of extending all the existing ones, because the reproducer requires executing several commands and would slow down too much the tests otherwise.
v1 -> v2:
- several script cleanups, as suggested by Stefano
--- tools/testing/selftests/net/pmtu.sh | 67 ++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-)
tools/testing/selftests/net/pmtu.sh +66 -1
@@ -103,6 +103,15 @@ # and check that configured MTU is used on link creation and changes, and # that MTU is properly calculated instead when MTU is not configured from # userspace +# +# - pmtu_ipv4_exception_cleanup +# Similar to pmtu_ipv4_vxlan4_exception, but explicitly generate PMTU +# exceptions on multiple CPUs and check that the veth device tear-down +# happens in a timely manner +# +# - pmtu_ipv6_exception_cleanup +# Same as above, but use IPv6 transport from A to B + # Kselftest framework requirement - SKIP code is 4. ksft_skip=4
@@ -135,7 +144,9 @@ tests=" pmtu_vti6_default_mtu vti6: default MTU assignment pmtu_vti4_link_add_mtu vti4: MTU setting on link creation pmtu_vti6_link_add_mtu vti6: MTU setting on link creation - pmtu_vti6_link_change_mtu vti6: MTU changes on link changes" + pmtu_vti6_link_change_mtu vti6: MTU changes on link changes + pmtu_ipv4_exception_cleanup ipv4: cleanup of cached exceptions + pmtu_ipv6_exception_cleanup ipv6: cleanup of cached exceptions" NS_A="ns-$(mktemp -u XXXXXX)" NS_B="ns-$(mktemp -u XXXXXX)"
@@ -1006,6 +1017,60 @@ test_pmtu_vti6_link_change_mtu() { return ${fail} } +chk_command() { + cmd=${1} + + if ! which ${cmd} > /dev/null 2>&1; then + err " missing required command: '${cmd}'" + return 1 + fi + return 0 +} + +test_cleanup_vxlanX_exception() { + outer="${1}" + encap="vxlan" + ll_mtu=4000 + + chk_command taskset || return 2 + chk_command timeout || return 2 + cpu_list=$(grep -m 2 processor /proc/cpuinfo | cut -d ' ' -f 2) + + setup namespaces routing ${encap}${outer} || return 2 + trace "${ns_a}" ${encap}_a "${ns_b}" ${encap}_b \ + "${ns_a}" veth_A-R1 "${ns_r1}" veth_R1-A \ + "${ns_b}" veth_B-R1 "${ns_r1}" veth_R1-B + + # Create route exception by exceeding link layer MTU + mtu "${ns_a}" veth_A-R1 $((${ll_mtu} + 1000)) + mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000)) + mtu "${ns_b}" veth_B-R1 ${ll_mtu} + mtu "${ns_r1}" veth_R1-B ${ll_mtu} + + mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000)) + mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000)) + + # Fill exception cache for multiple CPUs (2) + # we can always use inner IPv4 for that + for cpu in ${cpu_list}; do + taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null + done + + if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then + err " can't delete veth device in a timely manner, PMTU dst likely leaked" + return 1 + fi + return 0 +} + +test_pmtu_ipv6_exception_cleanup() { + test_cleanup_vxlanX_exception 6 vxlan +} + +test_pmtu_ipv4_exception_cleanup() { + test_cleanup_vxlanX_exception 4 vxlan +} + usage() { echo echo "$0 [OPTIONS] [TEST]..."
Signature · 1 line
2.20.1
Paolo Abeni
maintainerPATCH v2 1/2 selftests: pmtu: disable DAD in all namespaces
In reply to Paolo AbeniOtherwise, the configured IPv6 address could be still "tentative" at test time, possibly causing tests failures.
We can also drop some sleep along the code and decrease the timeout for most commands so that the test runtime decreases.
v1 -> v2:
- fix comment (Stefano)
--- tools/testing/selftests/net/pmtu.sh | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)
tools/testing/selftests/net/pmtu.sh +13 -15
@@ -263,8 +263,6 @@ setup_fou_or_gue() { ${ns_a} ip link set ${encap}_a up ${ns_b} ip link set ${encap}_b up - - sleep 1 } setup_fou44() {
@@ -302,6 +300,10 @@ setup_gue66() { setup_namespaces() { for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do ip netns add ${n} || return 1 + + # Disable DAD, so that we don't have to wait to use the + # configured IPv6 addresses + ip netns exec ${n} sysctl -q net/ipv6/conf/default/accept_dad=0 done }
@@ -337,8 +339,6 @@ setup_vti() { ${ns_a} ip link set vti${proto}_a up ${ns_b} ip link set vti${proto}_b up - - sleep 1 } setup_vti4() {
@@ -375,8 +375,6 @@ setup_vxlan_or_geneve() { ${ns_a} ip link set ${type}_a up ${ns_b} ip link set ${type}_b up - - sleep 1 } setup_geneve4() {
@@ -588,8 +586,8 @@ test_pmtu_ipvX() { 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 + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst1} > /dev/null + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -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})"
@@ -621,7 +619,7 @@ test_pmtu_ipvX() { # 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 + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -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
@@ -638,7 +636,7 @@ test_pmtu_ipvX() { 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 + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -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 }
@@ -687,7 +685,7 @@ test_pmtu_ipvX_over_vxlanY_or_geneveY_exception() { mtu "${ns_a}" ${type}_a $((${ll_mtu} + 1000)) mtu "${ns_b}" ${type}_b $((${ll_mtu} + 1000)) - ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s $((${ll_mtu} + 500)) ${dst} > /dev/null + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst} > /dev/null # Check that exception was created pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst})"
@@ -767,7 +765,7 @@ test_pmtu_ipvX_over_fouY_or_gueY() { mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000)) mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000)) - ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s $((${ll_mtu} + 500)) ${dst} > /dev/null + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst} > /dev/null # Check that exception was created pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst})"
@@ -825,13 +823,13 @@ test_pmtu_vti4_exception() { # Send DF packet without exceeding link layer MTU, check that no # exception is created - ${ns_a} ping -q -M want -i 0.1 -w 2 -s ${ping_payload} ${tunnel4_b_addr} > /dev/null + ${ns_a} ping -q -M want -i 0.1 -w 1 -s ${ping_payload} ${tunnel4_b_addr} > /dev/null pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})" 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)) ${tunnel4_b_addr} > /dev/null + ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((ping_payload + 1)) ${tunnel4_b_addr} > /dev/null pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})" check_pmtu_value "${esp_payload_rfc4106}" "${pmtu}" "exceeding PMTU (IP payload length $((esp_payload_rfc4106 + 1)))" }
@@ -847,7 +845,7 @@ test_pmtu_vti6_exception() { mtu "${ns_b}" veth_b 4000 mtu "${ns_a}" vti6_a 5000 mtu "${ns_b}" vti6_b 5000 - ${ns_a} ${ping6} -q -i 0.1 -w 2 -s 60000 ${tunnel6_b_addr} > /dev/null + ${ns_a} ${ping6} -q -i 0.1 -w 1 -s 60000 ${tunnel6_b_addr} > /dev/null # Check that exception was created pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel6_b_addr})"
Signature · 1 line
2.20.1
Re: [PATCH net v2 2/2] selftests: pmtu: add explicit tests for PMTU exceptions cleanup
In reply to Paolo AbeniJust three minor details, feel free to ignore:
tools/testing/selftests/net/pmtu.sh test_pmtu_vti6_link_change_mtu() { · line 1020
Quoted 5 lines · On Mon, 25 Feb 2019 12:13:46 +0100 Paolo Abeni <[email protected]> wrote:
tools/testing/selftests/net/pmtu.sh +1
-0@@ -1006,6 +1017,60 @@ test_pmtu_vti6_link_change_mtu() { return ${fail} } +chk_command() {
All the other functions checking something are called 'check_*'.
Quoted 33 lines
tools/testing/selftests/net/pmtu.sh +33
-0+ cmd=${1} + + if ! which ${cmd} > /dev/null 2>&1; then + err " missing required command: '${cmd}'" + return 1 + fi + return 0 +} + +test_cleanup_vxlanX_exception() { + outer="${1}" + encap="vxlan" + ll_mtu=4000 + + chk_command taskset || return 2 + chk_command timeout || return 2 + cpu_list=$(grep -m 2 processor /proc/cpuinfo | cut -d ' ' -f 2) + + setup namespaces routing ${encap}${outer} || return 2 + trace "${ns_a}" ${encap}_a "${ns_b}" ${encap}_b \ + "${ns_a}" veth_A-R1 "${ns_r1}" veth_R1-A \ + "${ns_b}" veth_B-R1 "${ns_r1}" veth_R1-B + + # Create route exception by exceeding link layer MTU + mtu "${ns_a}" veth_A-R1 $((${ll_mtu} + 1000)) + mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000)) + mtu "${ns_b}" veth_B-R1 ${ll_mtu} + mtu "${ns_r1}" veth_R1-B ${ll_mtu} + + mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000)) + mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000)) + + # Fill exception cache for multiple CPUs (2)
tools/testing/selftests/net/pmtu.sh test_pmtu_vti6_link_change_mtu() { · line 1063
Quoted 10 lines
tools/testing/selftests/net/pmtu.sh +10
-0+ # we can always use inner IPv4 for that + for cpu in ${cpu_list}; do + taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null + done + + if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then + err " can't delete veth device in a timely manner, PMTU dst likely leaked" + return 1 + fi + return 0
No need for explicit return 0.
tools/testing/selftests/net/pmtu.sh test_pmtu_vti6_link_change_mtu() { · line 1071
Quoted 8 lines
tools/testing/selftests/net/pmtu.sh +8
-0+} + +test_pmtu_ipv6_exception_cleanup() { + test_cleanup_vxlanX_exception 6 vxlan +} + +test_pmtu_ipv4_exception_cleanup() { + test_cleanup_vxlanX_exception 4 vxlan
This function now takes just one argument.
Signature · 1 line
Stefano
Re: [PATCH net v2 2/2] selftests: pmtu: add explicit tests for PMTU exceptions cleanup
In reply to Paolo Abenitools/testing/selftests/net/pmtu.sh test_pmtu_vti6_link_change_mtu() { · line 1059
Quoted 1 line · Paolo Abeni
tools/testing/selftests/net/pmtu.sh +1
-0+ if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then
That doesn't work. "ip link del" is stuck in a way that timeout can't terminate it, so this is still going to hang. Did you actually test this? :/
Quoted 5 lines
tools/testing/selftests/net/pmtu.sh +5
-0+ err " can't delete veth device in a timely manner, PMTU dst likely leaked" + return 1 + fi + return 0 +}
Signature · 1 line
Sabrina
Re: [PATCH net v2 2/2] selftests: pmtu: add explicit tests for PMTU exceptions cleanup
Quoted 6 lines · On Mon, 25 Feb 2019 13:33:30 +0100 Sabrina Dubroca <[email protected]> wrote:
tools/testing/selftests/net/pmtu.sh test_pmtu_vti6_link_change_mtu() { · line 1059
Quoted 1 line · Paolo Abeni
tools/testing/selftests/net/pmtu.sh +1
-0+ if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; thenThat doesn't work. "ip link del" is stuck in a way that timeout can't terminate it, so this is still going to hang. Did you actually test this? :/
Indeed, upon actual testing, this hangs and the error is not reported.
Quoted 6 lines
tools/testing/selftests/net/pmtu.sh test_pmtu_vti6_link_change_mtu() { · line 1028
Quoted 6 lines
(quoted diff) +1
-0+ err " can't delete veth device in a timelymanner, PMTU dst likely leaked"
tools/testing/selftests/net/pmtu.sh +4
-0+ return 1 + fi + return 0 +}
You can use a subshell here, something like:
${ns_a} ip link del dev veth_A-R1 &
iplink_pid=$!
sleep 1
if [ "$(cat /proc/${iplink_pid}/cmdline 2>/dev/null | tr -d '\0')" = "iplinkdeldevveth_A-R1" ]; then
err " can't delete veth device in a timely manner, PMTU dst likely leaked"
return 1
fi
}and now that I tried to run as a single test and couldn't find it, I think it would also be worth it to rename the tests to something more sensible.
Right now you have test_pmtu_ipv6_exception_cleanup() calling test_cleanup_...(), I think it's confusing. Just call the test test_cleanup_ipv6_exception(), it doesn't have so much to do with PMTU anyway.
Signature · 1 line
Stefano