[PATCH net-next v1 0/6] selftests: rds: Log collection, TAP compliance and cleanups

[PATCH net-next v1 0/6] selftests: rds: Log collection, TAP compliance and cleanups

[Allison Henderson]

This series is a set of bug fixes and improvements for the rds
selftests.

Patch 1 bumps the kselftest timeout from 400s to 800s. The original
limit was developed against a lean config, but the kselftest harness
counts boot time and gcov log collection against the limit, so a
default config with gcov enabled needs more headroom. Patch 2 removes
an unused "-g" entry in run.sh's USAGE string, and patch 3 adds a -t
flag to run.sh so the timeout can be overridden if needed.

Patch 4 fixes log collection under vng. The vng guest inherits the
host's systemd config, so tmp and debugfs may not be mounted by
default; without them tcpdump can't write pcaps and gcov data is
silently dropped. The patch mounts each filesystem when it isn't
already mounted, and also specifies the --root folder so that gcov
can still find the kernel source when it is run from the ksft
test directory.

Patch 5 hoists pcap collection into a helper and calls it from the
timeout signal handler so dumps are preserved when a test times out.
Patch 6 makes the test output TAP compliant so the kselftest runner
parses results correctly.

Questions, comments and feedback appreciated!

Thanks everyone!
Allison

Allison Henderson (6):
  selftests: rds: Increase selftest timeout
  selftests: rds: Update USAGE string for run.sh
  selftests: rds: Add timeout flag to run.sh
  selftests: rds: Fix gcov and pcap collection
  selftests: rds: Collect pcaps on timeout
  selftests: rds: Make rds selftests TAP compliant

 tools/testing/selftests/net/rds/run.sh   | 42 ++++++++++----
 tools/testing/selftests/net/rds/settings |  2 +-
 tools/testing/selftests/net/rds/test.py  | 70 +++++++++++++++---------
 3 files changed, 76 insertions(+), 38 deletions(-)

-- 
2.25.1

[PATCH net-next v1 1/6] selftests: rds: Increase selftest timeout

[Allison Henderson]

The 400s time out was originally developed under a leaner
kernel config that booted much faster than a default config.
Boot up is included as part of the over all test runtime, as
well as any log collection done when the test is complete.
A slower config combined with the gcov enabled test means
we'll need more time to accommodate the boot up and log
collection.  So, bump time out to 800s.

Signed-off-by: Allison Henderson <achender@kernel.org>
---
 tools/testing/selftests/net/rds/settings | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/rds/settings b/tools/testing/selftests/net/rds/settings
index d2009a64589c..8cb41e6a83cc 100644
--- a/tools/testing/selftests/net/rds/settings
+++ b/tools/testing/selftests/net/rds/settings
@@ -1 +1 @@
-timeout=400
+timeout=800
-- 
2.25.1

[PATCH net-next v1 2/6] selftests: rds: Update USAGE string for run.sh

[Allison Henderson]

The run.sh script does not have a -g flag.  Update USAGE string with
correct flags.  Aslo fix typo packet_duplcate -> packet_duplicate

Signed-off-by: Allison Henderson <achender@kernel.org>
---
 tools/testing/selftests/net/rds/run.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/rds/run.sh b/tools/testing/selftests/net/rds/run.sh
index 897d17d1b8db..73a9b986b0ef 100755
--- a/tools/testing/selftests/net/rds/run.sh
+++ b/tools/testing/selftests/net/rds/run.sh
@@ -171,7 +171,7 @@ while getopts "d:l:c:u:" opt; do
       ;;
     :)
       echo "USAGE: run.sh [-d logdir] [-l packet_loss] [-c packet_corruption]" \
-           "[-u packet_duplcate] [-g]"
+           "[-u packet_duplicate]"
       exit 1
       ;;
     ?)
-- 
2.25.1

[PATCH net-next v1 3/6] selftests: rds: Add timeout flag to run.sh

[Allison Henderson]

Add a -t flag to run.sh to optionally override the default
timeout.  The --timeout flag is already supported in test.py,
so just add the shorthand -t flag

Signed-off-by: Allison Henderson <achender@kernel.org>
---
 tools/testing/selftests/net/rds/run.sh  | 11 ++++++++---
 tools/testing/selftests/net/rds/test.py |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/rds/run.sh b/tools/testing/selftests/net/rds/run.sh
index 73a9b986b0ef..bc2e53126aab 100755
--- a/tools/testing/selftests/net/rds/run.sh
+++ b/tools/testing/selftests/net/rds/run.sh
@@ -154,8 +154,9 @@ LOG_DIR="$current_dir"/rds_logs
 PLOSS=0
 PCORRUPT=0
 PDUP=0
+TIMEOUT=$timeout
 GENERATE_GCOV_REPORT=1
-while getopts "d:l:c:u:" opt; do
+while getopts "d:l:c:u:t:" opt; do
   case ${opt} in
     d)
       LOG_DIR=${OPTARG}
@@ -166,12 +167,15 @@ while getopts "d:l:c:u:" opt; do
     c)
       PCORRUPT=${OPTARG}
       ;;
+    t)
+      TIMEOUT=${OPTARG}
+      ;;
     u)
       PDUP=${OPTARG}
       ;;
     :)
       echo "USAGE: run.sh [-d logdir] [-l packet_loss] [-c packet_corruption]" \
-           "[-u packet_duplicate]"
+           "[-u packet_duplicate] [-t timeout]"
       exit 1
       ;;
     ?)
@@ -198,7 +202,8 @@ echo running RDS tests...
 echo Traces will be logged to "$TRACE_FILE"
 rm -f "$TRACE_FILE"
 strace -T -tt -o "$TRACE_FILE" python3 "$(dirname "$0")/test.py" \
-	--timeout "$timeout" -d "$LOG_DIR" -l "$PLOSS" -c "$PCORRUPT" -u "$PDUP"
+	-t "$TIMEOUT" -d "$LOG_DIR" -l "$PLOSS" -c "$PCORRUPT" \
+	-u "$PDUP"
 
 test_rc=$?
 dmesg > "${LOG_DIR}/dmesg.out"
diff --git a/tools/testing/selftests/net/rds/test.py b/tools/testing/selftests/net/rds/test.py
index 93e23e8b256c..90f9adad18b3 100755
--- a/tools/testing/selftests/net/rds/test.py
+++ b/tools/testing/selftests/net/rds/test.py
@@ -79,7 +79,7 @@ parser = argparse.ArgumentParser(description="init script args",
                   formatter_class=argparse.ArgumentDefaultsHelpFormatter)
 parser.add_argument("-d", "--logdir", action="store",
                     help="directory to store logs", default="/tmp")
-parser.add_argument('--timeout', help="timeout to terminate hung test",
+parser.add_argument('-t', '--timeout', help="timeout to terminate hung test",
                     type=int, default=0)
 parser.add_argument('-l', '--loss', help="Simulate tcp packet loss",
                     type=int, default=0)
-- 
2.25.1

[PATCH net-next v1 4/6] selftests: rds: Fix gcov and pcap collection

[Allison Henderson]

The vng guest shares the host filesystem via 9p and runs a minimal
systemd inherited from the host's /lib/systemd/system/. As a result,
which filesystems get auto-mounted in the guest depends on the host OS
and its systemd version.

The tcpdump pcaps are initially saved to /tmp because 9p does not support
chown which tcpdump requires. But whether /tmp is already a tmpfs depends
on the host's systemd configuration and may still sometimes fail if /tmp
is not mounted by default. Fix this by mounting tmpfs on /tmp in run.sh
when it is not already a separately mounted filesystem.

A similar dependency exists for gcov. debugfs is not mounted automatically
in vng guest, so the gcov data copy from /sys/kernel/debug/gcov/
silently finds nothing depending on whether debugfs is mounted by default
on the host OS. Fix this by mounting debugfs in run.sh before copying the
gcda files.

Finally, when invoked through the kselftest runner, the working directory
is the test directory rather than the kernel source root. gcovr defaults
--root to the current working directory, causing it to filter out all
coverage data for files under net/rds/ since they are not under the test
directory. Fix this by passing --root to gcovr explicitly.

Signed-off-by: Allison Henderson <achender@kernel.org>
---
 tools/testing/selftests/net/rds/run.sh | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/rds/run.sh b/tools/testing/selftests/net/rds/run.sh
index bc2e53126aab..3fc116d23410 100755
--- a/tools/testing/selftests/net/rds/run.sh
+++ b/tools/testing/selftests/net/rds/run.sh
@@ -197,6 +197,13 @@ COVR_DIR="${LOG_DIR}/coverage/"
 mkdir -p  "$LOG_DIR"
 mkdir -p "$COVR_DIR"
 
+# tcpdump saves pcaps to /tmp because it requires chown to save the
+# pcap but chown is not supported by 9p.  Mount tmpfs on /tmp if it is
+# not already a separate filesystem
+if ! mountpoint -q /tmp 2>/dev/null; then
+	mount -t tmpfs tmpfs /tmp
+fi
+
 set +e
 echo running RDS tests...
 echo Traces will be logged to "$TRACE_FILE"
@@ -210,6 +217,12 @@ dmesg > "${LOG_DIR}/dmesg.out"
 
 if [ "$GENERATE_GCOV_REPORT" -eq 1 ]; then
        echo saving coverage data...
+
+       # Ensure debugfs is mounted
+       if ! test -d /sys/kernel/debug/gcov; then
+               mount -t debugfs debugfs /sys/kernel/debug 2>/dev/null || true
+       fi
+
        (set +x; cd /sys/kernel/debug/gcov; find ./* -name '*.gcda' | \
        while read -r f
        do
@@ -218,7 +231,7 @@ if [ "$GENERATE_GCOV_REPORT" -eq 1 ]; then
 
        echo running gcovr...
        gcovr -s --html-details --gcov-executable "$GCOV_CMD" --gcov-ignore-parse-errors \
-             -o "${COVR_DIR}/gcovr" "${ksrc_dir}/net/rds/"
+             --root "${ksrc_dir}" -o "${COVR_DIR}/gcovr" "${ksrc_dir}/net/rds/"
 else
        echo "Coverage report will be skipped"
 fi
-- 
2.25.1

[PATCH net-next v1 5/6] selftests: rds: Collect pcaps on timeout

[Allison Henderson]

The timeout signal handler for the rds selftests currently just
exits when the time limit is exceeded, and forgets to collect the
network dumps.  Which can be valueable for discerning why the test
timed out in the first place.  Fix this by hoisting the network
dump collection into a helper function, and call it from the
signal handler before exiting

Signed-off-by: Allison Henderson <achender@kernel.org>
---
 tools/testing/selftests/net/rds/test.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/rds/test.py b/tools/testing/selftests/net/rds/test.py
index 90f9adad18b3..6685b960d013 100755
--- a/tools/testing/selftests/net/rds/test.py
+++ b/tools/testing/selftests/net/rds/test.py
@@ -66,11 +66,20 @@ def netns_socket(netns, *sock_args):
     u1.close()
     return socket.fromfd(fds[0], *sock_args)
 
+def collect_pcaps():
+    print("Stopping network packet captures")
+    for p, pcap_tmp, pcap, fd in tcpdump_procs:
+        p.terminate()
+        p.wait()
+        os.close(fd)
+        shutil.move(pcap_tmp, pcap)
+
 def signal_handler(_sig, _frame):
     """
     Test timed out signal handler
     """
     print('Test timed out')
+    collect_pcaps()
     sys.exit(1)
 
 #Parse out command line arguments.  We take an optional
@@ -246,13 +255,7 @@ for s in sockets:
                 pass
 
 print(f"getsockopt(): {nr_success}/{nr_error}")
-
-print("Stopping network packet captures")
-for p, pcap_tmp, pcap, fd in tcpdump_procs:
-    p.terminate()
-    p.wait()
-    os.close(fd)
-    shutil.move(pcap_tmp, pcap)
+collect_pcaps()
 
 # We're done sending and receiving stuff, now let's check if what
 # we received is what we sent.
-- 
2.25.1

[PATCH net-next v1 6/6] selftests: rds: Make rds selftests TAP compliant

[Allison Henderson]

This patch updates the rds selftests output to be TAP compliant.

Use ksft_pr() to mark debug output with a leading '# ' so that TAP
parsers treat it as commentary, and convert all informational print()
calls to use ksft_pr(). sys.exit(0) is changed to os._exit(0) to
avoid duplicate prints from the buffered TAP output. The console
output from the tcpdump subprocess is silenced, and the gcov console
output is redirected to a gcovr.log.

Finally adjust the exit path so that the hash check loop sets a
return code instead exiting directly. Then print the TAP results
and totals lines before exiting.

Signed-off-by: Allison Henderson <achender@kernel.org>
---
 tools/testing/selftests/net/rds/run.sh  | 18 +++++----
 tools/testing/selftests/net/rds/test.py | 53 ++++++++++++++++---------
 2 files changed, 44 insertions(+), 27 deletions(-)

diff --git a/tools/testing/selftests/net/rds/run.sh b/tools/testing/selftests/net/rds/run.sh
index 3fc116d23410..805cd0915585 100755
--- a/tools/testing/selftests/net/rds/run.sh
+++ b/tools/testing/selftests/net/rds/run.sh
@@ -205,8 +205,8 @@ if ! mountpoint -q /tmp 2>/dev/null; then
 fi
 
 set +e
-echo running RDS tests...
-echo Traces will be logged to "$TRACE_FILE"
+echo "# running RDS tests..."
+echo "# Traces will be logged to $TRACE_FILE"
 rm -f "$TRACE_FILE"
 strace -T -tt -o "$TRACE_FILE" python3 "$(dirname "$0")/test.py" \
 	-t "$TIMEOUT" -d "$LOG_DIR" -l "$PLOSS" -c "$PCORRUPT" \
@@ -216,7 +216,7 @@ test_rc=$?
 dmesg > "${LOG_DIR}/dmesg.out"
 
 if [ "$GENERATE_GCOV_REPORT" -eq 1 ]; then
-       echo saving coverage data...
+       echo "# saving coverage data..."
 
        # Ensure debugfs is mounted
        if ! test -d /sys/kernel/debug/gcov; then
@@ -229,17 +229,19 @@ if [ "$GENERATE_GCOV_REPORT" -eq 1 ]; then
                cat < "/sys/kernel/debug/gcov/$f" > "/$f"
        done)
 
-       echo running gcovr...
+       echo "# running gcovr..."
        gcovr -s --html-details --gcov-executable "$GCOV_CMD" --gcov-ignore-parse-errors \
-             --root "${ksrc_dir}" -o "${COVR_DIR}/gcovr" "${ksrc_dir}/net/rds/"
+             --root "${ksrc_dir}" -o "${COVR_DIR}/gcovr" "${ksrc_dir}/net/rds/" \
+             > "${LOG_DIR}/gcovr.log" 2>&1
+       echo "# gcovr log: ${LOG_DIR}/gcovr.log"
 else
-       echo "Coverage report will be skipped"
+       echo "# Coverage report will be skipped"
 fi
 
 if [ "$test_rc" -eq 0 ]; then
-	echo "PASS: Test completed successfully"
+	echo "# PASS: Test completed successfully"
 else
-	echo "FAIL: Test failed"
+	echo "# FAIL: Test failed"
 fi
 
 exit "$test_rc"
diff --git a/tools/testing/selftests/net/rds/test.py b/tools/testing/selftests/net/rds/test.py
index 6685b960d013..bef0b02c8d5c 100755
--- a/tools/testing/selftests/net/rds/test.py
+++ b/tools/testing/selftests/net/rds/test.py
@@ -18,6 +18,7 @@ import shutil
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.join(this_dir, "../"))
 from lib.py.utils import ip
+from lib.py.ksft import ksft_pr
 
 libc = ctypes.cdll.LoadLibrary('libc.so.6')
 setns = libc.setns
@@ -57,7 +58,7 @@ def netns_socket(netns, *sock_args):
         # send resulting socket to parent
         socket.send_fds(u0, [], [sock.fileno()])
 
-        sys.exit(0)
+        os._exit(0)
 
     # receive socket from child
     _, fds, _, _ = socket.recv_fds(u1, 0, 1)
@@ -67,7 +68,7 @@ def netns_socket(netns, *sock_args):
     return socket.fromfd(fds[0], *sock_args)
 
 def collect_pcaps():
-    print("Stopping network packet captures")
+    ksft_pr("Stopping network packet captures")
     for p, pcap_tmp, pcap, fd in tcpdump_procs:
         p.terminate()
         p.wait()
@@ -78,8 +79,9 @@ def signal_handler(_sig, _frame):
     """
     Test timed out signal handler
     """
-    print('Test timed out')
+    ksft_pr("Test timed out")
     collect_pcaps()
+    print("not ok 1 rds selftest")
     sys.exit(1)
 
 #Parse out command line arguments.  We take an optional
@@ -140,7 +142,8 @@ for net in [NET0, NET1]:
     fd, pcap_tmp = tempfile.mkstemp(suffix=".pcap", prefix=f"{net}-", dir="/tmp")
     p = subprocess.Popen(
         ['ip', 'netns', 'exec', net,
-         '/usr/sbin/tcpdump', '-i', 'any', '-w', pcap_tmp])
+         '/usr/sbin/tcpdump', '-i', 'any', '-w', pcap_tmp],
+         stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
     tcpdump_procs.append((p, pcap_tmp, pcap, fd))
 
 # simulate packet loss, duplication and corruption
@@ -149,6 +152,9 @@ for net, iface in [(NET0, VETH0), (NET1, VETH1)]:
          corrupt {PACKET_CORRUPTION} loss {PACKET_LOSS} duplicate  \
          {PACKET_DUPLICATE}")
 
+print("TAP version 13")
+print("1..1")
+
 # add a timeout
 if args.timeout > 0:
     signal.alarm(args.timeout)
@@ -187,7 +193,7 @@ nr_recv = 0
 
 while nr_send < NUM_PACKETS:
     # Send as much as we can without blocking
-    print("sending...", nr_send, nr_recv)
+    ksft_pr("sending...", nr_send, nr_recv)
     while nr_send < NUM_PACKETS:
         send_data = hashlib.sha256(
             f'packet {nr_send}'.encode('utf-8')).hexdigest().encode('utf-8')
@@ -209,7 +215,7 @@ while nr_send < NUM_PACKETS:
             raise
 
     # Receive as much as we can without blocking
-    print("receiving...", nr_send, nr_recv)
+    ksft_pr("receiving...", nr_send, nr_recv)
     while nr_recv < nr_send:
         for fileno, eventmask in ep.poll():
             receiver = fileno_to_socket[fileno]
@@ -231,7 +237,7 @@ while nr_send < NUM_PACKETS:
         ip(f"netns exec {net} /usr/sbin/sysctl net.rds.tcp.rds_tcp_rcvbuf=10000")
         ip(f"netns exec {net} /usr/sbin/sysctl net.rds.tcp.rds_tcp_sndbuf=10000")
 
-print("done", nr_send, nr_recv)
+ksft_pr("done", nr_send, nr_recv)
 
 # the Python socket module doesn't know these
 RDS_INFO_FIRST = 10000
@@ -254,25 +260,34 @@ for s in sockets:
                 # ignore
                 pass
 
-print(f"getsockopt(): {nr_success}/{nr_error}")
+ksft_pr(f"getsockopt(): {nr_success}/{nr_error}")
 collect_pcaps()
 
 # We're done sending and receiving stuff, now let's check if what
 # we received is what we sent.
+ret = 0
 for (sender, receiver), send_hash in send_hashes.items():
     recv_hash = recv_hashes.get((sender, receiver))
 
     if recv_hash is None:
-        print("FAIL: No data received")
-        sys.exit(1)
+        ksft_pr("FAIL: No data received")
+        ret = 1
+        break
 
     if send_hash.hexdigest() != recv_hash.hexdigest():
-        print("FAIL: Send/recv mismatch")
-        print("hash expected:", send_hash.hexdigest())
-        print("hash received:", recv_hash.hexdigest())
-        sys.exit(1)
-
-    print(f"{sender}/{receiver}: ok")
-
-print("Success")
-sys.exit(0)
+        ksft_pr("FAIL: Send/recv mismatch")
+        ksft_pr("hash expected:", send_hash.hexdigest())
+        ksft_pr("hash received:", recv_hash.hexdigest())
+        ret = 1
+        break
+
+    ksft_pr(f"{sender}/{receiver}: ok")
+
+if ret == 0:
+    ksft_pr("Success")
+    print("ok 1 rds selftest")
+else:
+    print("not ok 1 rds selftest")
+
+ksft_pr(f"Totals: pass:{1-ret} fail:{ret} skip:0")
+sys.exit(ret)
-- 
2.25.1