[PATCH 0/7] Fixes and workarounds for tests, Coverity warnings

[PATCH 0/7] Fixes and workarounds for tests, Coverity warnings

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

With this series, I'm almost able to run the full test suite together
with the new command dispatch mechanism.

I still hit frequent failures in the passt_tcp performance test, so
I'm not pushing this out at the moment, but as it's taking me a while,
I'd rather share this earlier.

Included are also two fixes for harmless (but ugly) issues discovered
by Coverity.

Stefano Brivio (7):
  test/perf: Always use /sbin/sysctl in tcp test
  test/perf: Switch performance test duration to 10 seconds instead of
    30
  tap: Check return value of accept4() before calling getsockopt()
  conf, tcp, udp: Arrays for ports need 2^16 values, not 2^16-8
  test/lib: Restore IFS while executing directives in def blocks
  test/lib: Wait on iperf3 clients to be done, then send SIGINT to
    servers
  test/perf: Disable periodic throughput reports to avoid vhost hang

 conf.c              |  2 +-
 tap.c               |  6 ++++--
 tcp.h               |  4 ++--
 test/lib/test       | 20 ++++++++++----------
 test/perf/passt_tcp | 10 +++++-----
 test/perf/passt_udp |  4 ++--
 test/perf/pasta_tcp | 10 +++++-----
 test/perf/pasta_udp |  4 ++--
 udp.h               |  4 ++--
 9 files changed, 33 insertions(+), 31 deletions(-)

-- 
2.35.1

[PATCH 1/7] test/perf: Always use /sbin/sysctl in tcp test

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
 test/perf/passt_tcp | 6 +++---
 test/perf/pasta_tcp | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/test/perf/passt_tcp b/test/perf/passt_tcp
index f0cfa1b..bee4f9e 100644
--- a/test/perf/passt_tcp
+++ b/test/perf/passt_tcp
@@ -30,9 +30,9 @@ guest	/sbin/sysctl -w net.ipv4.tcp_rmem="4096 131072 268435456"
 guest	/sbin/sysctl -w net.ipv4.tcp_wmem="4096 131072 268435456"
 guest	/sbin/sysctl -w net.ipv4.tcp_timestamps=0
 
-ns	sysctl -w net.ipv4.tcp_rmem="4096 524288 134217728"
-ns	sysctl -w net.ipv4.tcp_wmem="4096 524288 134217728"
-ns	sysctl -w net.ipv4.tcp_timestamps=0
+ns	/sbin/sysctl -w net.ipv4.tcp_rmem="4096 524288 134217728"
+ns	/sbin/sysctl -w net.ipv4.tcp_wmem="4096 524288 134217728"
+ns	/sbin/sysctl -w net.ipv4.tcp_timestamps=0
 
 gout	GW ip -j -4 route show|jq -rM '.[] | select(.dst == "default").gateway'
 gout	GW6 ip -j -6 route show|jq -rM '.[] | select(.dst == "default").gateway'
diff --git a/test/perf/pasta_tcp b/test/perf/pasta_tcp
index 602ce52..cc21075 100644
--- a/test/perf/pasta_tcp
+++ b/test/perf/pasta_tcp
@@ -16,9 +16,9 @@ nstools	/sbin/sysctl nproc ip seq sleep iperf3 tcp_rr tcp_crr jq sed
 
 test	pasta: throughput and latency (local connections)
 
-ns	sysctl -w net.ipv4.tcp_rmem="131072 524288 134217728"
-ns	sysctl -w net.ipv4.tcp_wmem="131072 524288 134217728"
-ns	sysctl -w net.ipv4.tcp_timestamps=0
+ns	/sbin/sysctl -w net.ipv4.tcp_rmem="131072 524288 134217728"
+ns	/sbin/sysctl -w net.ipv4.tcp_wmem="131072 524288 134217728"
+ns	/sbin/sysctl -w net.ipv4.tcp_timestamps=0
 
 
 set	THREADS 2
-- 
@@ -16,9 +16,9 @@ nstools	/sbin/sysctl nproc ip seq sleep iperf3 tcp_rr tcp_crr jq sed
 
 test	pasta: throughput and latency (local connections)
 
-ns	sysctl -w net.ipv4.tcp_rmem="131072 524288 134217728"
-ns	sysctl -w net.ipv4.tcp_wmem="131072 524288 134217728"
-ns	sysctl -w net.ipv4.tcp_timestamps=0
+ns	/sbin/sysctl -w net.ipv4.tcp_rmem="131072 524288 134217728"
+ns	/sbin/sysctl -w net.ipv4.tcp_wmem="131072 524288 134217728"
+ns	/sbin/sysctl -w net.ipv4.tcp_timestamps=0
 
 
 set	THREADS 2
-- 
2.35.1

[PATCH 2/7] test/perf: Switch performance test duration to 10 seconds instead of 30

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]

It looks like the workaround for the virtio_net TX hang issue is
working less reliably with the new command dispatch mechanism, I'm
not sure why. Switch to 10 seconds, at least for the moment.

Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
 test/perf/passt_tcp | 2 +-
 test/perf/passt_udp | 2 +-
 test/perf/pasta_tcp | 2 +-
 test/perf/pasta_udp | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/perf/passt_tcp b/test/perf/passt_tcp
index bee4f9e..5f0aa3a 100644
--- a/test/perf/passt_tcp
+++ b/test/perf/passt_tcp
@@ -44,7 +44,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
 
 set	THREADS 1
 set	STREAMS 8
-set	TIME 30
+set	TIME 10
 hout	OMIT echo __TIME__ / 6 | bc -l
 set	OPTS -Z -P __STREAMS__ -l 1M -i1 -O__OMIT__ --pacing-timer 1000000
 
diff --git a/test/perf/passt_udp b/test/perf/passt_udp
index 80731d1..6bd86ff 100644
--- a/test/perf/passt_udp
+++ b/test/perf/passt_udp
@@ -37,7 +37,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
 
 set	THREADS 4
 set	STREAMS 1
-set	TIME 30
+set	TIME 10
 set	OPTS -u -i1 -P __STREAMS__ --pacing-timer 1000
 
 info	Throughput in Gbps, latency in µs, __THREADS__ threads at __FREQ__ GHz, one stream each
diff --git a/test/perf/pasta_tcp b/test/perf/pasta_tcp
index cc21075..44c5e54 100644
--- a/test/perf/pasta_tcp
+++ b/test/perf/pasta_tcp
@@ -23,7 +23,7 @@ ns	/sbin/sysctl -w net.ipv4.tcp_timestamps=0
 
 set	THREADS 2
 set	STREAMS 2
-set	TIME 30
+set	TIME 10
 hout	OMIT echo __TIME__ / 6 | bc -l
 set	OPTS -Z -w 4M -l 1M -P __STREAMS__ -i1 -O__OMIT__ --pacing-timer 10000
 
diff --git a/test/perf/pasta_udp b/test/perf/pasta_udp
index ae898b1..abb88b0 100644
--- a/test/perf/pasta_udp
+++ b/test/perf/pasta_udp
@@ -22,7 +22,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
 
 set	THREADS 1
 set	STREAMS 4
-set	TIME 30
+set	TIME 10
 set	OPTS -u -i1 -P __STREAMS__
 
 info	Throughput in Gbps, latency in µs, one thread at __FREQ__ GHz, __STREAMS__ streams
-- 
@@ -22,7 +22,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
 
 set	THREADS 1
 set	STREAMS 4
-set	TIME 30
+set	TIME 10
 set	OPTS -u -i1 -P __STREAMS__
 
 info	Throughput in Gbps, latency in µs, one thread at __FREQ__ GHz, __STREAMS__ streams
-- 
2.35.1

[PATCH 3/7] tap: Check return value of accept4() before calling getsockopt()

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

Reported by Coverity (CWE-119):

	Negative value used as argument to a function expecting a
	positive value (for example, size of buffer or allocation)

and harmless, because getsockopt() would return -EBADF if the
socket is -1, so we wouldn't print anything.

Check if accept4() returns a valid socket before calling getsockopt()
on it.

Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
 tap.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tap.c b/tap.c
index 3231da7..4d7422f 100644
--- a/tap.c
+++ b/tap.c
@@ -872,11 +872,13 @@ static void tap_sock_unix_new(struct ctx *c)
 		int discard = accept4(c->fd_tap_listen, NULL, NULL,
 				      SOCK_NONBLOCK);
 
+		if (discard == -1)
+			return;
+
 		if (!getsockopt(discard, SOL_SOCKET, SO_PEERCRED, &ucred, &len))
 			info("discarding connection from PID %i", ucred.pid);
 
-		if (discard != -1)
-			close(discard);
+		close(discard);
 
 		return;
 	}
-- 
@@ -872,11 +872,13 @@ static void tap_sock_unix_new(struct ctx *c)
 		int discard = accept4(c->fd_tap_listen, NULL, NULL,
 				      SOCK_NONBLOCK);
 
+		if (discard == -1)
+			return;
+
 		if (!getsockopt(discard, SOL_SOCKET, SO_PEERCRED, &ucred, &len))
 			info("discarding connection from PID %i", ucred.pid);
 
-		if (discard != -1)
-			close(discard);
+		close(discard);
 
 		return;
 	}
-- 
2.35.1

[PATCH 4/7] conf, tcp, udp: Arrays for ports need 2^16 values, not 2^16-8

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 1964 bytes --]

Reported by David but also by Coverity (CWE-119):

	In conf_ports: Out-of-bounds access to a buffer

...not in practice, because the allocation size is rounded up
anyway, but not nice either.

Reported-by: David Gibson <david(a)gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
 conf.c | 2 +-
 tcp.h  | 4 ++--
 udp.h  | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/conf.c b/conf.c
index d80233c..7ecfa1e 100644
--- a/conf.c
+++ b/conf.c
@@ -127,8 +127,8 @@ static int conf_ports(struct ctx *c, char optname, const char *optarg,
 {
 	int start_src, end_src, start_dst, end_dst, exclude_only = 1, i, port;
 	char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf;
+	uint8_t *map, exclude[DIV_ROUND_UP(USHRT_MAX, 8)] = { 0 };
 	void (*remap)(in_port_t port, in_port_t delta);
-	uint8_t *map, exclude[USHRT_MAX / 8] = { 0 };
 	char buf[BUFSIZ], *sep, *spec, *p;
 	sa_family_t af = AF_UNSPEC;
 
diff --git a/tcp.h b/tcp.h
index 7b720c1..6431b75 100644
--- a/tcp.h
+++ b/tcp.h
@@ -69,9 +69,9 @@ struct tcp_ctx {
 	uint64_t hash_secret[2];
 	int conn_count;
 	int splice_conn_count;
-	uint8_t port_to_tap	[USHRT_MAX / 8];
+	uint8_t port_to_tap	[DIV_ROUND_UP(USHRT_MAX, 8)];
 	int init_detect_ports;
-	uint8_t port_to_init	[USHRT_MAX / 8];
+	uint8_t port_to_init	[DIV_ROUND_UP(USHRT_MAX, 8)];
 	int ns_detect_ports;
 	struct timespec timer_run;
 #ifdef HAS_SND_WND
diff --git a/udp.h b/udp.h
index f16fe5e..8f82842 100644
--- a/udp.h
+++ b/udp.h
@@ -53,9 +53,9 @@ union udp_epoll_ref {
  * @timer_run:		Timestamp of most recent timer run
  */
 struct udp_ctx {
-	uint8_t port_to_tap		[USHRT_MAX / 8];
+	uint8_t port_to_tap		[DIV_ROUND_UP(USHRT_MAX, 8)];
 	int init_detect_ports;
-	uint8_t port_to_init		[USHRT_MAX / 8];
+	uint8_t port_to_init		[DIV_ROUND_UP(USHRT_MAX, 8)];
 	int ns_detect_ports;
 	struct timespec timer_run;
 };
-- 
@@ -53,9 +53,9 @@ union udp_epoll_ref {
  * @timer_run:		Timestamp of most recent timer run
  */
 struct udp_ctx {
-	uint8_t port_to_tap		[USHRT_MAX / 8];
+	uint8_t port_to_tap		[DIV_ROUND_UP(USHRT_MAX, 8)];
 	int init_detect_ports;
-	uint8_t port_to_init		[USHRT_MAX / 8];
+	uint8_t port_to_init		[DIV_ROUND_UP(USHRT_MAX, 8)];
 	int ns_detect_ports;
 	struct timespec timer_run;
 };
-- 
2.35.1

[PATCH 5/7] test/lib: Restore IFS while executing directives in def blocks

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

If we don't, guest command dispatch will fail altogether, given that
we use cat(1) on the enter file, which contains spaces.

Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
 test/lib/test | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/lib/test b/test/lib/test
index 3ad178f..a9ffe83 100755
--- a/test/lib/test
+++ b/test/lib/test
@@ -312,7 +312,7 @@ test_one_line() {
 			IFS='
 '
 			for __def_line in ${__def_body}; do
-				IFS= test_one_line "${__def_line}"
+				IFS="${__ifs}" test_one_line "${__def_line}"
 			done
 			IFS="${__ifs}"
 		fi
-- 
@@ -312,7 +312,7 @@ test_one_line() {
 			IFS='
 '
 			for __def_line in ${__def_body}; do
-				IFS= test_one_line "${__def_line}"
+				IFS="${__ifs}" test_one_line "${__def_line}"
 			done
 			IFS="${__ifs}"
 		fi
-- 
2.35.1

[PATCH 6/7] test/lib: Wait on iperf3 clients to be done, then send SIGINT to servers

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]

An iperf3 client might fail to send the control message indicating
the end of the test, if the kernel buffer doesn't accept it, and exit
without having sent it, as the control socket is non-blocking. Should
this happen, the server will just wait forever for this message,
instead of terminating.

Restore some of the behaviour that went away with the
"test: Rewrite test_iperf3" patch: instead of waiting on servers to
terminate, wait on the clients. When they are done, wait 2 seconds,
and then send SIGINT to the servers, which make them still write
out the JSON report before terminating.

Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
 test/lib/test | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/test/lib/test b/test/lib/test
index a9ffe83..b69d519 100755
--- a/test/lib/test
+++ b/test/lib/test
@@ -32,13 +32,11 @@ test_iperf3() {
 	__time="${1}"; shift
 
 	pane_or_context_run_bg "${__sctx}" 				\
-		 '('							\
-		 '	for i in $(seq 0 '${__procs}'); do'		\
-		 '		iperf3 -s1J -p'${__port}' -i'${__time}	\
-		 '		   > s${i}.json &'			\
-		 '	done;'						\
-		 '	wait'						\
-		 ')'
+		 'for i in $(seq 0 '${__procs}'); do'			\
+		 '	(iperf3 -s1J -p'${__port}' -i'${__time}		\
+		 '	 > s${i}.json) &'				\
+		 '	echo $! > s${i}.pid &'				\
+		 'done'							\
 
 	sleep 1		# Wait for server to be ready
 
@@ -51,7 +49,9 @@ test_iperf3() {
 		 '	wait'						\
 		 ')'
 
-	pane_or_context_wait "${__sctx}"
+	# If client fails to deliver control message, tell server we're done
+	pane_or_context_run "${__sctx}" 				\
+		'sleep 2; kill -INT $(cat s*.pid); rm s*.pid'
 
 	__jval=".end.sum_received.bits_per_second"
 	for __opt in ${@}; do
-- 
@@ -32,13 +32,11 @@ test_iperf3() {
 	__time="${1}"; shift
 
 	pane_or_context_run_bg "${__sctx}" 				\
-		 '('							\
-		 '	for i in $(seq 0 '${__procs}'); do'		\
-		 '		iperf3 -s1J -p'${__port}' -i'${__time}	\
-		 '		   > s${i}.json &'			\
-		 '	done;'						\
-		 '	wait'						\
-		 ')'
+		 'for i in $(seq 0 '${__procs}'); do'			\
+		 '	(iperf3 -s1J -p'${__port}' -i'${__time}		\
+		 '	 > s${i}.json) &'				\
+		 '	echo $! > s${i}.pid &'				\
+		 'done'							\
 
 	sleep 1		# Wait for server to be ready
 
@@ -51,7 +49,9 @@ test_iperf3() {
 		 '	wait'						\
 		 ')'
 
-	pane_or_context_wait "${__sctx}"
+	# If client fails to deliver control message, tell server we're done
+	pane_or_context_run "${__sctx}" 				\
+		'sleep 2; kill -INT $(cat s*.pid); rm s*.pid'
 
 	__jval=".end.sum_received.bits_per_second"
 	for __opt in ${@}; do
-- 
2.35.1

[PATCH 7/7] test/perf: Disable periodic throughput reports to avoid vhost hang

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 3427 bytes --]

It appears that if we run throughput tests with one-second periodic
reports, the sending side of the vhost channel used for SSH-based
command dispatch occasionally stops working altogether. I haven't
investigated this further, all I see is that output is truncated
at some point, and doesn't resume.

If we use gzip compression (ssh -C) this happens less frequently,
but it still happens, seemingly indicating the issue is probably
related to vhost itself.

Disable periodic reports in iperf3 clients. The -i options were
actually redundant, so remove them from both test files as well as
from test_iperf3().

Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
 test/lib/test       | 2 +-
 test/perf/passt_tcp | 2 +-
 test/perf/passt_udp | 2 +-
 test/perf/pasta_tcp | 2 +-
 test/perf/pasta_udp | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/lib/test b/test/lib/test
index b69d519..d68ade4 100755
--- a/test/lib/test
+++ b/test/lib/test
@@ -44,7 +44,7 @@ test_iperf3() {
 		 '('							\
 		 '	for i in $(seq 0 '${__procs}'); do'		\
 		 '		iperf3 -c '${__dest}' -p '${__port}	\
-		 '		  -t'${__time}' -T s${i} '"${@}"' &'	\
+		 '		 -t'${__time}' -i0 -T s${i} '"${@}"' &' \
 		 '	done;'						\
 		 '	wait'						\
 		 ')'
diff --git a/test/perf/passt_tcp b/test/perf/passt_tcp
index 5f0aa3a..5ba5450 100644
--- a/test/perf/passt_tcp
+++ b/test/perf/passt_tcp
@@ -46,7 +46,7 @@ set	THREADS 1
 set	STREAMS 8
 set	TIME 10
 hout	OMIT echo __TIME__ / 6 | bc -l
-set	OPTS -Z -P __STREAMS__ -l 1M -i1 -O__OMIT__ --pacing-timer 1000000
+set	OPTS -Z -P __STREAMS__ -l 1M -O__OMIT__ --pacing-timer 1000000
 
 info	Throughput in Gbps, latency in µs, one thread at __FREQ__ GHz, __STREAMS__ streams
 report	passt tcp __THREADS__ __FREQ__
diff --git a/test/perf/passt_udp b/test/perf/passt_udp
index 6bd86ff..fd2ddc1 100644
--- a/test/perf/passt_udp
+++ b/test/perf/passt_udp
@@ -38,7 +38,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
 set	THREADS 4
 set	STREAMS 1
 set	TIME 10
-set	OPTS -u -i1 -P __STREAMS__ --pacing-timer 1000
+set	OPTS -u -P __STREAMS__ --pacing-timer 1000
 
 info	Throughput in Gbps, latency in µs, __THREADS__ threads at __FREQ__ GHz, one stream each
 
diff --git a/test/perf/pasta_tcp b/test/perf/pasta_tcp
index 44c5e54..1847c83 100644
--- a/test/perf/pasta_tcp
+++ b/test/perf/pasta_tcp
@@ -25,7 +25,7 @@ set	THREADS 2
 set	STREAMS 2
 set	TIME 10
 hout	OMIT echo __TIME__ / 6 | bc -l
-set	OPTS -Z -w 4M -l 1M -P __STREAMS__ -i1 -O__OMIT__ --pacing-timer 10000
+set	OPTS -Z -w 4M -l 1M -P __STREAMS__ -O__OMIT__ --pacing-timer 10000
 
 hout	FREQ_PROCFS (echo "scale=1"; sed -n 's/cpu MHz.*: \([0-9]*\)\..*$/(\1+10^2\/2)\/10^3/p' /proc/cpuinfo) | bc -l | head -n1
 hout	FREQ_CPUFREQ (echo "scale=1"; printf '( %i + 10^5 / 2 ) / 10^6\n' $(cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq) ) | bc -l
diff --git a/test/perf/pasta_udp b/test/perf/pasta_udp
index abb88b0..27ea724 100644
--- a/test/perf/pasta_udp
+++ b/test/perf/pasta_udp
@@ -23,7 +23,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
 set	THREADS 1
 set	STREAMS 4
 set	TIME 10
-set	OPTS -u -i1 -P __STREAMS__
+set	OPTS -u -P __STREAMS__
 
 info	Throughput in Gbps, latency in µs, one thread at __FREQ__ GHz, __STREAMS__ streams
 
-- 
@@ -23,7 +23,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
 set	THREADS 1
 set	STREAMS 4
 set	TIME 10
-set	OPTS -u -i1 -P __STREAMS__
+set	OPTS -u -P __STREAMS__
 
 info	Throughput in Gbps, latency in µs, one thread at __FREQ__ GHz, __STREAMS__ streams
 
-- 
2.35.1

Re: [PATCH 1/7] test/perf: Always use /sbin/sysctl in tcp test

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2065 bytes --]

On Wed, Sep 21, 2022 at 10:55:01PM +0200, Stefano Brivio wrote:
> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>

Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>

> ---
>  test/perf/passt_tcp | 6 +++---
>  test/perf/pasta_tcp | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/test/perf/passt_tcp b/test/perf/passt_tcp
> index f0cfa1b..bee4f9e 100644
> --- a/test/perf/passt_tcp
> +++ b/test/perf/passt_tcp
> @@ -30,9 +30,9 @@ guest	/sbin/sysctl -w net.ipv4.tcp_rmem="4096 131072 268435456"
>  guest	/sbin/sysctl -w net.ipv4.tcp_wmem="4096 131072 268435456"
>  guest	/sbin/sysctl -w net.ipv4.tcp_timestamps=0
>  
> -ns	sysctl -w net.ipv4.tcp_rmem="4096 524288 134217728"
> -ns	sysctl -w net.ipv4.tcp_wmem="4096 524288 134217728"
> -ns	sysctl -w net.ipv4.tcp_timestamps=0
> +ns	/sbin/sysctl -w net.ipv4.tcp_rmem="4096 524288 134217728"
> +ns	/sbin/sysctl -w net.ipv4.tcp_wmem="4096 524288 134217728"
> +ns	/sbin/sysctl -w net.ipv4.tcp_timestamps=0
>  
>  gout	GW ip -j -4 route show|jq -rM '.[] | select(.dst == "default").gateway'
>  gout	GW6 ip -j -6 route show|jq -rM '.[] | select(.dst == "default").gateway'
> diff --git a/test/perf/pasta_tcp b/test/perf/pasta_tcp
> index 602ce52..cc21075 100644
> --- a/test/perf/pasta_tcp
> +++ b/test/perf/pasta_tcp
> @@ -16,9 +16,9 @@ nstools	/sbin/sysctl nproc ip seq sleep iperf3 tcp_rr tcp_crr jq sed
>  
>  test	pasta: throughput and latency (local connections)
>  
> -ns	sysctl -w net.ipv4.tcp_rmem="131072 524288 134217728"
> -ns	sysctl -w net.ipv4.tcp_wmem="131072 524288 134217728"
> -ns	sysctl -w net.ipv4.tcp_timestamps=0
> +ns	/sbin/sysctl -w net.ipv4.tcp_rmem="131072 524288 134217728"
> +ns	/sbin/sysctl -w net.ipv4.tcp_wmem="131072 524288 134217728"
> +ns	/sbin/sysctl -w net.ipv4.tcp_timestamps=0
>  
>  
>  set	THREADS 2

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/7] test/perf: Switch performance test duration to 10 seconds instead of 30

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2569 bytes --]

On Wed, Sep 21, 2022 at 10:55:02PM +0200, Stefano Brivio wrote:
> It looks like the workaround for the virtio_net TX hang issue is
> working less reliably with the new command dispatch mechanism, I'm
> not sure why. Switch to 10 seconds, at least for the moment.
> 
> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>

Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>

> ---
>  test/perf/passt_tcp | 2 +-
>  test/perf/passt_udp | 2 +-
>  test/perf/pasta_tcp | 2 +-
>  test/perf/pasta_udp | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/test/perf/passt_tcp b/test/perf/passt_tcp
> index bee4f9e..5f0aa3a 100644
> --- a/test/perf/passt_tcp
> +++ b/test/perf/passt_tcp
> @@ -44,7 +44,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
>  
>  set	THREADS 1
>  set	STREAMS 8
> -set	TIME 30
> +set	TIME 10
>  hout	OMIT echo __TIME__ / 6 | bc -l
>  set	OPTS -Z -P __STREAMS__ -l 1M -i1 -O__OMIT__ --pacing-timer 1000000
>  
> diff --git a/test/perf/passt_udp b/test/perf/passt_udp
> index 80731d1..6bd86ff 100644
> --- a/test/perf/passt_udp
> +++ b/test/perf/passt_udp
> @@ -37,7 +37,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
>  
>  set	THREADS 4
>  set	STREAMS 1
> -set	TIME 30
> +set	TIME 10
>  set	OPTS -u -i1 -P __STREAMS__ --pacing-timer 1000
>  
>  info	Throughput in Gbps, latency in µs, __THREADS__ threads at __FREQ__ GHz, one stream each
> diff --git a/test/perf/pasta_tcp b/test/perf/pasta_tcp
> index cc21075..44c5e54 100644
> --- a/test/perf/pasta_tcp
> +++ b/test/perf/pasta_tcp
> @@ -23,7 +23,7 @@ ns	/sbin/sysctl -w net.ipv4.tcp_timestamps=0
>  
>  set	THREADS 2
>  set	STREAMS 2
> -set	TIME 30
> +set	TIME 10
>  hout	OMIT echo __TIME__ / 6 | bc -l
>  set	OPTS -Z -w 4M -l 1M -P __STREAMS__ -i1 -O__OMIT__ --pacing-timer 10000
>  
> diff --git a/test/perf/pasta_udp b/test/perf/pasta_udp
> index ae898b1..abb88b0 100644
> --- a/test/perf/pasta_udp
> +++ b/test/perf/pasta_udp
> @@ -22,7 +22,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
>  
>  set	THREADS 1
>  set	STREAMS 4
> -set	TIME 30
> +set	TIME 10
>  set	OPTS -u -i1 -P __STREAMS__
>  
>  info	Throughput in Gbps, latency in µs, one thread at __FREQ__ GHz, __STREAMS__ streams

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/7] tap: Check return value of accept4() before calling getsockopt()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1324 bytes --]

On Wed, Sep 21, 2022 at 10:55:03PM +0200, Stefano Brivio wrote:
> Reported by Coverity (CWE-119):
> 
> 	Negative value used as argument to a function expecting a
> 	positive value (for example, size of buffer or allocation)
> 
> and harmless, because getsockopt() would return -EBADF if the
> socket is -1, so we wouldn't print anything.
> 
> Check if accept4() returns a valid socket before calling getsockopt()
> on it.
> 
> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>

Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>

> ---
>  tap.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tap.c b/tap.c
> index 3231da7..4d7422f 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -872,11 +872,13 @@ static void tap_sock_unix_new(struct ctx *c)
>  		int discard = accept4(c->fd_tap_listen, NULL, NULL,
>  				      SOCK_NONBLOCK);
>  
> +		if (discard == -1)
> +			return;
> +
>  		if (!getsockopt(discard, SOL_SOCKET, SO_PEERCRED, &ucred, &len))
>  			info("discarding connection from PID %i", ucred.pid);
>  
> -		if (discard != -1)
> -			close(discard);
> +		close(discard);
>  
>  		return;
>  	}

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/7] conf, tcp, udp: Arrays for ports need 2^16 values, not 2^16-8

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3482 bytes --]

On Wed, Sep 21, 2022 at 10:55:04PM +0200, Stefano Brivio wrote:
> Reported by David but also by Coverity (CWE-119):
> 
> 	In conf_ports: Out-of-bounds access to a buffer
> 
> ...not in practice, because the allocation size is rounded up
> anyway, but not nice either.

So... this helps, but it's not enough.  For starters, this is still
conceptually wrong - there should be 65536 bits in the bitmap, not
65535 (USHRT_MAX).  This patch just masks it by rounding up to 65536
rather than down to 65528.

Alas, this is not the only buffer overrun caused by an off-by-one in
port numbers: the delta_to_tap etc. global arrays in tcp.c and udp.c
should also have length 65536, rather than USHRT_MAX.  So should
tcp_sock_init_lo and friends.  There are also similar problems in
icmp.c with echo request id rather than port number.

Not a buffer overrun, but the USHRT_MAX in udp_remap_to_tap() and
udp_remap_to_init() is also out by one (consider the case of
delta==0).

I have a series which makes a number of cleanups to the port mapping
handling.  It fixes this bug and adds typing stuff that should make it
harder to make similar mistakes in future.

I held off on sending, since it's currently based on a lot of my
outstanding patches.  I think it wouldn't be too hard to rebase
directly on master, should I do that so you can fix this before going
on to debug the rest of my outstanding stuff?

> 
> Reported-by: David Gibson <david(a)gibson.dropbear.id.au>
> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
> ---
>  conf.c | 2 +-
>  tcp.h  | 4 ++--
>  udp.h  | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index d80233c..7ecfa1e 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -127,8 +127,8 @@ static int conf_ports(struct ctx *c, char optname, const char *optarg,
>  {
>  	int start_src, end_src, start_dst, end_dst, exclude_only = 1, i, port;
>  	char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf;
> +	uint8_t *map, exclude[DIV_ROUND_UP(USHRT_MAX, 8)] = { 0 };
>  	void (*remap)(in_port_t port, in_port_t delta);
> -	uint8_t *map, exclude[USHRT_MAX / 8] = { 0 };
>  	char buf[BUFSIZ], *sep, *spec, *p;
>  	sa_family_t af = AF_UNSPEC;
>  
> diff --git a/tcp.h b/tcp.h
> index 7b720c1..6431b75 100644
> --- a/tcp.h
> +++ b/tcp.h
> @@ -69,9 +69,9 @@ struct tcp_ctx {
>  	uint64_t hash_secret[2];
>  	int conn_count;
>  	int splice_conn_count;
> -	uint8_t port_to_tap	[USHRT_MAX / 8];
> +	uint8_t port_to_tap	[DIV_ROUND_UP(USHRT_MAX, 8)];
>  	int init_detect_ports;
> -	uint8_t port_to_init	[USHRT_MAX / 8];
> +	uint8_t port_to_init	[DIV_ROUND_UP(USHRT_MAX, 8)];
>  	int ns_detect_ports;
>  	struct timespec timer_run;
>  #ifdef HAS_SND_WND
> diff --git a/udp.h b/udp.h
> index f16fe5e..8f82842 100644
> --- a/udp.h
> +++ b/udp.h
> @@ -53,9 +53,9 @@ union udp_epoll_ref {
>   * @timer_run:		Timestamp of most recent timer run
>   */
>  struct udp_ctx {
> -	uint8_t port_to_tap		[USHRT_MAX / 8];
> +	uint8_t port_to_tap		[DIV_ROUND_UP(USHRT_MAX, 8)];
>  	int init_detect_ports;
> -	uint8_t port_to_init		[USHRT_MAX / 8];
> +	uint8_t port_to_init		[DIV_ROUND_UP(USHRT_MAX, 8)];
>  	int ns_detect_ports;
>  	struct timespec timer_run;
>  };

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/7] test/lib: Restore IFS while executing directives in def blocks

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

On Wed, Sep 21, 2022 at 10:55:05PM +0200, Stefano Brivio wrote:
> If we don't, guest command dispatch will fail altogether, given that
> we use cat(1) on the enter file, which contains spaces.
> 
> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>

Huh.  I wonder how this was working for me.

> ---
>  test/lib/test | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/lib/test b/test/lib/test
> index 3ad178f..a9ffe83 100755
> --- a/test/lib/test
> +++ b/test/lib/test
> @@ -312,7 +312,7 @@ test_one_line() {
>  			IFS='
>  '
>  			for __def_line in ${__def_body}; do
> -				IFS= test_one_line "${__def_line}"
> +				IFS="${__ifs}" test_one_line "${__def_line}"
>  			done
>  			IFS="${__ifs}"
>  		fi

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 7/7] test/perf: Disable periodic throughput reports to avoid vhost hang

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3892 bytes --]

On Wed, Sep 21, 2022 at 10:55:07PM +0200, Stefano Brivio wrote:
> It appears that if we run throughput tests with one-second periodic
> reports, the sending side of the vhost channel used for SSH-based
> command dispatch occasionally stops working altogether. I haven't
> investigated this further, all I see is that output is truncated
> at some point, and doesn't resume.

Huh.  I don't think I ever observed this.

> If we use gzip compression (ssh -C) this happens less frequently,
> but it still happens, seemingly indicating the issue is probably
> related to vhost itself.
> 
> Disable periodic reports in iperf3 clients. The -i options were
> actually redundant, so remove them from both test files as well as
> from test_iperf3().
> 
> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
> ---
>  test/lib/test       | 2 +-
>  test/perf/passt_tcp | 2 +-
>  test/perf/passt_udp | 2 +-
>  test/perf/pasta_tcp | 2 +-
>  test/perf/pasta_udp | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/test/lib/test b/test/lib/test
> index b69d519..d68ade4 100755
> --- a/test/lib/test
> +++ b/test/lib/test
> @@ -44,7 +44,7 @@ test_iperf3() {
>  		 '('							\
>  		 '	for i in $(seq 0 '${__procs}'); do'		\
>  		 '		iperf3 -c '${__dest}' -p '${__port}	\
> -		 '		  -t'${__time}' -T s${i} '"${@}"' &'	\
> +		 '		 -t'${__time}' -i0 -T s${i} '"${@}"' &' \
>  		 '	done;'						\
>  		 '	wait'						\
>  		 ')'
> diff --git a/test/perf/passt_tcp b/test/perf/passt_tcp
> index 5f0aa3a..5ba5450 100644
> --- a/test/perf/passt_tcp
> +++ b/test/perf/passt_tcp
> @@ -46,7 +46,7 @@ set	THREADS 1
>  set	STREAMS 8
>  set	TIME 10
>  hout	OMIT echo __TIME__ / 6 | bc -l
> -set	OPTS -Z -P __STREAMS__ -l 1M -i1 -O__OMIT__ --pacing-timer 1000000
> +set	OPTS -Z -P __STREAMS__ -l 1M -O__OMIT__ --pacing-timer 1000000
>  
>  info	Throughput in Gbps, latency in µs, one thread at __FREQ__ GHz, __STREAMS__ streams
>  report	passt tcp __THREADS__ __FREQ__
> diff --git a/test/perf/passt_udp b/test/perf/passt_udp
> index 6bd86ff..fd2ddc1 100644
> --- a/test/perf/passt_udp
> +++ b/test/perf/passt_udp
> @@ -38,7 +38,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
>  set	THREADS 4
>  set	STREAMS 1
>  set	TIME 10
> -set	OPTS -u -i1 -P __STREAMS__ --pacing-timer 1000
> +set	OPTS -u -P __STREAMS__ --pacing-timer 1000
>  
>  info	Throughput in Gbps, latency in µs, __THREADS__ threads at __FREQ__ GHz, one stream each
>  
> diff --git a/test/perf/pasta_tcp b/test/perf/pasta_tcp
> index 44c5e54..1847c83 100644
> --- a/test/perf/pasta_tcp
> +++ b/test/perf/pasta_tcp
> @@ -25,7 +25,7 @@ set	THREADS 2
>  set	STREAMS 2
>  set	TIME 10
>  hout	OMIT echo __TIME__ / 6 | bc -l
> -set	OPTS -Z -w 4M -l 1M -P __STREAMS__ -i1 -O__OMIT__ --pacing-timer 10000
> +set	OPTS -Z -w 4M -l 1M -P __STREAMS__ -O__OMIT__ --pacing-timer 10000
>  
>  hout	FREQ_PROCFS (echo "scale=1"; sed -n 's/cpu MHz.*: \([0-9]*\)\..*$/(\1+10^2\/2)\/10^3/p' /proc/cpuinfo) | bc -l | head -n1
>  hout	FREQ_CPUFREQ (echo "scale=1"; printf '( %i + 10^5 / 2 ) / 10^6\n' $(cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq) ) | bc -l
> diff --git a/test/perf/pasta_udp b/test/perf/pasta_udp
> index abb88b0..27ea724 100644
> --- a/test/perf/pasta_udp
> +++ b/test/perf/pasta_udp
> @@ -23,7 +23,7 @@ hout	FREQ [ -n "__FREQ_CPUFREQ__" ] && echo __FREQ_CPUFREQ__ || echo __FREQ_PROC
>  set	THREADS 1
>  set	STREAMS 4
>  set	TIME 10
> -set	OPTS -u -i1 -P __STREAMS__
> +set	OPTS -u -P __STREAMS__
>  
>  info	Throughput in Gbps, latency in µs, one thread at __FREQ__ GHz, __STREAMS__ streams
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/7] conf, tcp, udp: Arrays for ports need 2^16 values, not 2^16-8

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 1973 bytes --]

On Thu, 22 Sep 2022 16:43:24 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> On Wed, Sep 21, 2022 at 10:55:04PM +0200, Stefano Brivio wrote:
> > Reported by David but also by Coverity (CWE-119):
> > 
> > 	In conf_ports: Out-of-bounds access to a buffer
> > 
> > ...not in practice, because the allocation size is rounded up
> > anyway, but not nice either.  
> 
> So... this helps, but it's not enough.  For starters, this is still
> conceptually wrong - there should be 65536 bits in the bitmap, not
> 65535 (USHRT_MAX).  This patch just masks it by rounding up to 65536
> rather than down to 65528.

Ah, yes, I indeed wanted to have 65536 values in the bitmap (see commit
title), and this does it, but there's no reason to make it implicit.

> Alas, this is not the only buffer overrun caused by an off-by-one in
> port numbers: the delta_to_tap etc. global arrays in tcp.c and udp.c
> should also have length 65536, rather than USHRT_MAX.  So should
> tcp_sock_init_lo and friends.  There are also similar problems in
> icmp.c with echo request id rather than port number.
> 
> Not a buffer overrun, but the USHRT_MAX in udp_remap_to_tap() and
> udp_remap_to_init() is also out by one (consider the case of
> delta==0).
> 
> I have a series which makes a number of cleanups to the port mapping
> handling.  It fixes this bug and adds typing stuff that should make it
> harder to make similar mistakes in future.
> 
> I held off on sending, since it's currently based on a lot of my
> outstanding patches.  I think it wouldn't be too hard to rebase
> directly on master, should I do that so you can fix this before going
> on to debug the rest of my outstanding stuff?

If it's not too much effort for you, yes, that would be appreciated.

I hope to be able to push out everything else later today, it's just
one glitch still occurring in test_iperf3() -- sometimes, clients and
servers terminate properly, but we don't see it for some reason.

-- 
Stefano

Re: [PATCH 5/7] test/lib: Restore IFS while executing directives in def blocks

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

On Thu, 22 Sep 2022 16:44:31 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> On Wed, Sep 21, 2022 at 10:55:05PM +0200, Stefano Brivio wrote:
> > If we don't, guest command dispatch will fail altogether, given that
> > we use cat(1) on the enter file, which contains spaces.
> > 
> > Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>  
> 
> Huh.  I wonder how this was working for me.

I thought you disabled that workaround, which should be the only 'def'
used in non-distro tests (which are still using screen grabbing)...?

-- 
Stefano

Re: [PATCH 4/7] conf, tcp, udp: Arrays for ports need 2^16 values, not 2^16-8

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

On Thu, 22 Sep 2022 10:21:50 +0200
Stefano Brivio <sbrivio(a)redhat.com> wrote:

> On Thu, 22 Sep 2022 16:43:24 +1000
> David Gibson <david(a)gibson.dropbear.id.au> wrote:
> 
> [...]
> 
> > I have a series which makes a number of cleanups to the port mapping
> > handling.  It fixes this bug and adds typing stuff that should make it
> > harder to make similar mistakes in future.
> > 
> > I held off on sending, since it's currently based on a lot of my
> > outstanding patches.  I think it wouldn't be too hard to rebase
> > directly on master, should I do that so you can fix this before going
> > on to debug the rest of my outstanding stuff?  
> 
> If it's not too much effort for you, yes, that would be appreciated.
> 
> I hope to be able to push out everything else later today, it's just
> one glitch still occurring in test_iperf3() -- sometimes, clients and
> servers terminate properly, but we don't see it for some reason.

Actually, it looks like I got tests finally passing, and I should be
pushing all the pending patches soon... perhaps you could wait before
you rebase, I guess it's more convenient.

-- 
Stefano

Re: [PATCH 4/7] conf, tcp, udp: Arrays for ports need 2^16 values, not 2^16-8

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]

On Fri, Sep 23, 2022 at 01:39:47AM +0200, Stefano Brivio wrote:
> On Thu, 22 Sep 2022 10:21:50 +0200
> Stefano Brivio <sbrivio(a)redhat.com> wrote:
> 
> > On Thu, 22 Sep 2022 16:43:24 +1000
> > David Gibson <david(a)gibson.dropbear.id.au> wrote:
> > 
> > [...]
> > 
> > > I have a series which makes a number of cleanups to the port mapping
> > > handling.  It fixes this bug and adds typing stuff that should make it
> > > harder to make similar mistakes in future.
> > > 
> > > I held off on sending, since it's currently based on a lot of my
> > > outstanding patches.  I think it wouldn't be too hard to rebase
> > > directly on master, should I do that so you can fix this before going
> > > on to debug the rest of my outstanding stuff?  
> > 
> > If it's not too much effort for you, yes, that would be appreciated.
> > 
> > I hope to be able to push out everything else later today, it's just
> > one glitch still occurring in test_iperf3() -- sometimes, clients and
> > servers terminate properly, but we don't see it for some reason.
> 
> Actually, it looks like I got tests finally passing, and I should be
> pushing all the pending patches soon... perhaps you could wait before
> you rebase, I guess it's more convenient.

Will do, thanks.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/7] test/lib: Restore IFS while executing directives in def blocks

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

On Thu, Sep 22, 2022 at 10:25:14AM +0200, Stefano Brivio wrote:
> On Thu, 22 Sep 2022 16:44:31 +1000
> David Gibson <david(a)gibson.dropbear.id.au> wrote:
> 
> > On Wed, Sep 21, 2022 at 10:55:05PM +0200, Stefano Brivio wrote:
> > > If we don't, guest command dispatch will fail altogether, given that
> > > we use cat(1) on the enter file, which contains spaces.
> > > 
> > > Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>  
> > 
> > Huh.  I wonder how this was working for me.
> 
> I thought you disabled that workaround, which should be the only 'def'
> used in non-distro tests (which are still using screen grabbing)...?

Oh, right.  I missed that this was specific to the 'def' handling.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]