Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 2/5] selftests: ovpn: fail notification check on mismatch
From: Antonio Quartulli @ 2026-04-12 22:11 UTC (permalink / raw)
  To: netdev
  Cc: ralf, Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
	David S. Miller, Eric Dumazet, Antonio Quartulli
In-Reply-To: <20260412221121.410365-1-antonio@openvpn.net>

From: Ralf Lici <ralf@mandelbit.com>

compare_ntfs doesn't fail when expected and received notification
streams diverge.

Fix this bug by trackink the diff exit status explicitly and return it
to the caller so notification mismatches propagate as test failures.

Fixes: 77de28cd7cf1 ("selftests: ovpn: add notification parsing and matching")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 tools/testing/selftests/net/ovpn/common.sh | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
index 4c08f756e63a..336a2a14285f 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -140,23 +140,34 @@ add_peer() {
 }
 
 compare_ntfs() {
+	local diff_rc=0
+	local diff_file
+
 	if [ ${#tmp_jsons[@]} -gt 0 ]; then
 		suffix=""
 		[ "${SYMMETRIC_ID}" -eq 1 ] && suffix="${suffix}-symm"
 		[ "$FLOAT" == 1 ] && suffix="${suffix}-float"
 		expected="json/peer${1}${suffix}.json"
 		received="${tmp_jsons[$1]}"
+		diff_file=$(mktemp)
 
 		kill -TERM ${listener_pids[$1]} || true
 		wait ${listener_pids[$1]} || true
 		printf "Checking notifications for peer ${1}... "
 		if diff <(jq -s "${JQ_FILTER}" ${expected}) \
-			<(jq -s "${JQ_FILTER}" ${received}); then
+			<(jq -s "${JQ_FILTER}" ${received}) >"${diff_file}" 2>&1; then
 			echo "OK"
+		else
+			diff_rc=$?
+			echo "failed"
+			cat "${diff_file}"
 		fi
 
+		rm -f "${diff_file}" || true
 		rm -f ${received} || true
 	fi
+
+	return "${diff_rc}"
 }
 
 cleanup() {
-- 
2.52.0


^ permalink raw reply related

* [PATCH net-next 3/5] selftests: ovpn: flatten slurped notification JSON before filtering
From: Antonio Quartulli @ 2026-04-12 22:11 UTC (permalink / raw)
  To: netdev
  Cc: ralf, Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
	David S. Miller, Eric Dumazet, Antonio Quartulli
In-Reply-To: <20260412221121.410365-1-antonio@openvpn.net>

From: Ralf Lici <ralf@mandelbit.com>

Notification comparison uses jq -s, which slurps all inputs into an
array. Some inputs can be arrays themselves, and applying the .msg.peer
filter directly on those entries triggers jq type errors.

Expand any array-valued JSON items returned by jq -s before selecting
.msg.peer, so the filter handles both normal notification objects and []
entries without type errors.

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 tools/testing/selftests/net/ovpn/common.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
index 336a2a14285f..dd562cc41b95 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -15,7 +15,8 @@ SYMMETRIC_ID=${SYMMETRIC_ID:-0}
 
 export ID_OFFSET=$(( 9 * (SYMMETRIC_ID == 0) ))
 
-JQ_FILTER='map(select(.msg.peer | has("remote-ipv6") | not)) |
+JQ_FILTER='map(if type == "array" then .[] else . end) |
+	map(select(.msg.peer | has("remote-ipv6") | not)) |
 	map(del(.msg.ifindex)) | sort_by(.msg.peer.id)[]'
 LAN_IP="11.11.11.11"
 
-- 
2.52.0


^ permalink raw reply related

* [PATCH net-next 4/5] selftests: ovpn: add namespace to helpers and shared variables
From: Antonio Quartulli @ 2026-04-12 22:11 UTC (permalink / raw)
  To: netdev
  Cc: ralf, Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
	David S. Miller, Eric Dumazet, Antonio Quartulli
In-Reply-To: <20260412221121.410365-1-antonio@openvpn.net>

From: Ralf Lici <ralf@mandelbit.com>

Rename common helper entry points and all shared globals in the ovpn
selftests to ovpn_ or OVPN_ names so test scripts and wrappers use a
single explicit namespace. This is a mechanical refactor only, behavior
is unchanged.

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 tools/testing/selftests/net/ovpn/common.sh    | 110 +++++++++---------
 .../selftests/net/ovpn/test-chachapoly.sh     |   2 +-
 .../net/ovpn/test-close-socket-tcp.sh         |   2 +-
 .../selftests/net/ovpn/test-close-socket.sh   |  20 ++--
 .../testing/selftests/net/ovpn/test-float.sh  |   2 +-
 tools/testing/selftests/net/ovpn/test-mark.sh |  16 +--
 .../net/ovpn/test-symmetric-id-float.sh       |   4 +-
 .../net/ovpn/test-symmetric-id-tcp.sh         |   4 +-
 .../selftests/net/ovpn/test-symmetric-id.sh   |   2 +-
 tools/testing/selftests/net/ovpn/test-tcp.sh  |   2 +-
 tools/testing/selftests/net/ovpn/test.sh      |  80 ++++++-------
 11 files changed, 122 insertions(+), 122 deletions(-)

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
index dd562cc41b95..2b3f2e5c8cc9 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -4,35 +4,35 @@
 #
 #  Author:	Antonio Quartulli <antonio@openvpn.net>
 
-UDP_PEERS_FILE=${UDP_PEERS_FILE:-udp_peers.txt}
-TCP_PEERS_FILE=${TCP_PEERS_FILE:-tcp_peers.txt}
+OVPN_UDP_PEERS_FILE=${OVPN_UDP_PEERS_FILE:-udp_peers.txt}
+OVPN_TCP_PEERS_FILE=${OVPN_TCP_PEERS_FILE:-tcp_peers.txt}
 OVPN_CLI=${OVPN_CLI:-./ovpn-cli}
-YNL_CLI=${YNL_CLI:-../../../../net/ynl/pyynl/cli.py}
-ALG=${ALG:-aes}
-PROTO=${PROTO:-UDP}
-FLOAT=${FLOAT:-0}
-SYMMETRIC_ID=${SYMMETRIC_ID:-0}
+OVPN_YNL_CLI=${OVPN_YNL_CLI:-../../../../net/ynl/pyynl/cli.py}
+OVPN_ALG=${OVPN_ALG:-aes}
+OVPN_PROTO=${OVPN_PROTO:-UDP}
+OVPN_FLOAT=${OVPN_FLOAT:-0}
+OVPN_SYMMETRIC_ID=${OVPN_SYMMETRIC_ID:-0}
 
-export ID_OFFSET=$(( 9 * (SYMMETRIC_ID == 0) ))
+export OVPN_ID_OFFSET=$(( 9 * (OVPN_SYMMETRIC_ID == 0) ))
 
-JQ_FILTER='map(if type == "array" then .[] else . end) |
+OVPN_JQ_FILTER='map(if type == "array" then .[] else . end) |
 	map(select(.msg.peer | has("remote-ipv6") | not)) |
 	map(del(.msg.ifindex)) | sort_by(.msg.peer.id)[]'
-LAN_IP="11.11.11.11"
+OVPN_LAN_IP="11.11.11.11"
 
-declare -A tmp_jsons=()
-declare -A listener_pids=()
+declare -A OVPN_TMP_JSONS=()
+declare -A OVPN_LISTENER_PIDS=()
 
-create_ns() {
+ovpn_create_ns() {
 	ip netns add peer${1}
 }
 
-setup_ns() {
+ovpn_setup_ns() {
 	MODE="P2P"
 
 	if [ ${1} -eq 0 ]; then
 		MODE="MP"
-		for p in $(seq 1 ${NUM_PEERS}); do
+		for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 			ip link add veth${p} netns peer0 type veth peer name veth${p} netns peer${p}
 
 			ip -n peer0 addr add 10.10.${p}.1/24 dev veth${p}
@@ -48,9 +48,9 @@ setup_ns() {
 	ip netns exec peer${1} ${OVPN_CLI} new_iface tun${1} $MODE
 	ip -n peer${1} addr add ${2} dev tun${1}
 	# add a secondary IP to peer 1, to test a LAN behind a client
-	if [ ${1} -eq 1 -a -n "${LAN_IP}" ]; then
-		ip -n peer${1} addr add ${LAN_IP} dev tun${1}
-		ip -n peer0 route add ${LAN_IP} via $(echo ${2} |sed -e s'!/.*!!') dev tun0
+	if [ ${1} -eq 1 -a -n "${OVPN_LAN_IP}" ]; then
+		ip -n peer${1} addr add ${OVPN_LAN_IP} dev tun${1}
+		ip -n peer0 route add ${OVPN_LAN_IP} via $(echo ${2} |sed -e s'!/.*!!') dev tun0
 	fi
 	if [ -n "${3}" ]; then
 		ip -n peer${1} link set mtu ${3} dev tun${1}
@@ -58,9 +58,9 @@ setup_ns() {
 	ip -n peer${1} link set tun${1} up
 }
 
-build_capture_filter() {
+ovpn_build_capture_filter() {
 	# match the first four bytes of the openvpn data payload
-	if [ "${PROTO}" == "UDP" ]; then
+	if [ "${OVPN_PROTO}" == "UDP" ]; then
 		# For UDP, libpcap transport indexing only works for IPv4, so
 		# use an explicit IPv4 or IPv6 expression based on the peer
 		# address. The IPv6 branch assumes there are no extension
@@ -77,61 +77,61 @@ build_capture_filter() {
 	fi
 }
 
-setup_listener() {
+ovpn_setup_listener() {
 	file=$(mktemp)
-	PYTHONUNBUFFERED=1 ip netns exec peer${p} ${YNL_CLI} --family ovpn \
+	PYTHONUNBUFFERED=1 ip netns exec peer${p} ${OVPN_YNL_CLI} --family ovpn \
 		--subscribe peers --output-json --duration 40 > ${file} &
-	listener_pids[$1]=$!
-	tmp_jsons[$1]="${file}"
+	OVPN_LISTENER_PIDS[$1]=$!
+	OVPN_TMP_JSONS[$1]="${file}"
 }
 
-add_peer() {
+ovpn_add_peer() {
 	labels=("ASYMM" "SYMM")
-	M_ID=${labels[SYMMETRIC_ID]}
+	M_ID=${labels[OVPN_SYMMETRIC_ID]}
 
-	if [ "${PROTO}" == "UDP" ]; then
+	if [ "${OVPN_PROTO}" == "UDP" ]; then
 		if [ ${1} -eq 0 ]; then
 			ip netns exec peer0 ${OVPN_CLI} new_multi_peer tun0 1 \
-				${M_ID} ${UDP_PEERS_FILE}
+				${M_ID} ${OVPN_UDP_PEERS_FILE}
 
-			for p in $(seq 1 ${NUM_PEERS}); do
-				ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 1 0 ${ALG} 0 \
-					data64.key
+			for p in $(seq 1 ${OVPN_NUM_PEERS}); do
+				ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 1 0 ${OVPN_ALG} \
+					0 data64.key
 			done
 		else
-			if [ "${SYMMETRIC_ID}" -eq 1 ]; then
+			if [ "${OVPN_SYMMETRIC_ID}" -eq 1 ]; then
 				PEER_ID=${1}
 				TX_ID="none"
 			else
 				PEER_ID=$(awk "NR == ${1} {print \$2}" \
-					${UDP_PEERS_FILE})
+					${OVPN_UDP_PEERS_FILE})
 				TX_ID=${1}
 			fi
-			RADDR=$(awk "NR == ${1} {print \$3}" ${UDP_PEERS_FILE})
-			RPORT=$(awk "NR == ${1} {print \$4}" ${UDP_PEERS_FILE})
-			LPORT=$(awk "NR == ${1} {print \$6}" ${UDP_PEERS_FILE})
+			RADDR=$(awk "NR == ${1} {print \$3}" ${OVPN_UDP_PEERS_FILE})
+			RPORT=$(awk "NR == ${1} {print \$4}" ${OVPN_UDP_PEERS_FILE})
+			LPORT=$(awk "NR == ${1} {print \$6}" ${OVPN_UDP_PEERS_FILE})
 			ip netns exec peer${1} ${OVPN_CLI} new_peer tun${1} \
 				${PEER_ID} ${TX_ID} ${LPORT} ${RADDR} ${RPORT}
 			ip netns exec peer${1} ${OVPN_CLI} new_key tun${1} \
-				${PEER_ID} 1 0 ${ALG} 1 data64.key
+				${PEER_ID} 1 0 ${OVPN_ALG} 1 data64.key
 		fi
 	else
 		if [ ${1} -eq 0 ]; then
 			(ip netns exec peer0 ${OVPN_CLI} listen tun0 1 ${M_ID} \
-				${TCP_PEERS_FILE} && {
-				for p in $(seq 1 ${NUM_PEERS}); do
+				${OVPN_TCP_PEERS_FILE} && {
+				for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 					ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 1 0 \
-						${ALG} 0 data64.key
+						${OVPN_ALG} 0 data64.key
 				done
 			}) &
 			sleep 5
 		else
-			if [ "${SYMMETRIC_ID}" -eq 1 ]; then
+			if [ "${OVPN_SYMMETRIC_ID}" -eq 1 ]; then
 				PEER_ID=${1}
 				TX_ID="none"
 			else
 				PEER_ID=$(awk "NR == ${1} {print \$2}" \
-					${TCP_PEERS_FILE})
+					${OVPN_TCP_PEERS_FILE})
 				TX_ID=${1}
 			fi
 			ip netns exec peer${1} ${OVPN_CLI} connect tun${1} \
@@ -140,23 +140,23 @@ add_peer() {
 	fi
 }
 
-compare_ntfs() {
+ovpn_compare_ntfs() {
 	local diff_rc=0
 	local diff_file
 
-	if [ ${#tmp_jsons[@]} -gt 0 ]; then
+	if [ ${#OVPN_TMP_JSONS[@]} -gt 0 ]; then
 		suffix=""
-		[ "${SYMMETRIC_ID}" -eq 1 ] && suffix="${suffix}-symm"
-		[ "$FLOAT" == 1 ] && suffix="${suffix}-float"
+		[ "${OVPN_SYMMETRIC_ID}" -eq 1 ] && suffix="${suffix}-symm"
+		[ "$OVPN_FLOAT" == 1 ] && suffix="${suffix}-float"
 		expected="json/peer${1}${suffix}.json"
-		received="${tmp_jsons[$1]}"
+		received="${OVPN_TMP_JSONS[$1]}"
 		diff_file=$(mktemp)
 
-		kill -TERM ${listener_pids[$1]} || true
-		wait ${listener_pids[$1]} || true
+		kill -TERM ${OVPN_LISTENER_PIDS[$1]} || true
+		wait ${OVPN_LISTENER_PIDS[$1]} || true
 		printf "Checking notifications for peer ${1}... "
-		if diff <(jq -s "${JQ_FILTER}" ${expected}) \
-			<(jq -s "${JQ_FILTER}" ${received}) >"${diff_file}" 2>&1; then
+		if diff <(jq -s "${OVPN_JQ_FILTER}" ${expected}) \
+			<(jq -s "${OVPN_JQ_FILTER}" ${received}) >"${diff_file}" 2>&1; then
 			echo "OK"
 		else
 			diff_rc=$?
@@ -171,7 +171,7 @@ compare_ntfs() {
 	return "${diff_rc}"
 }
 
-cleanup() {
+ovpn_cleanup() {
 	# some ovpn-cli processes sleep in background so they need manual poking
 	killall $(basename ${OVPN_CLI}) 2>/dev/null || true
 
@@ -188,8 +188,8 @@ cleanup() {
 	done
 }
 
-if [ "${PROTO}" == "UDP" ]; then
-	NUM_PEERS=${NUM_PEERS:-$(wc -l ${UDP_PEERS_FILE} | awk '{print $1}')}
+if [ "${OVPN_PROTO}" == "UDP" ]; then
+	OVPN_NUM_PEERS=${OVPN_NUM_PEERS:-$(wc -l ${OVPN_UDP_PEERS_FILE} | awk '{print $1}')}
 else
-	NUM_PEERS=${NUM_PEERS:-$(wc -l ${TCP_PEERS_FILE} | awk '{print $1}')}
+	OVPN_NUM_PEERS=${OVPN_NUM_PEERS:-$(wc -l ${OVPN_TCP_PEERS_FILE} | awk '{print $1}')}
 fi
diff --git a/tools/testing/selftests/net/ovpn/test-chachapoly.sh b/tools/testing/selftests/net/ovpn/test-chachapoly.sh
index 32504079a2b8..cd3d94355d58 100755
--- a/tools/testing/selftests/net/ovpn/test-chachapoly.sh
+++ b/tools/testing/selftests/net/ovpn/test-chachapoly.sh
@@ -4,6 +4,6 @@
 #
 #  Author:	Antonio Quartulli <antonio@openvpn.net>
 
-ALG="chachapoly"
+OVPN_ALG="chachapoly"
 
 source test.sh
diff --git a/tools/testing/selftests/net/ovpn/test-close-socket-tcp.sh b/tools/testing/selftests/net/ovpn/test-close-socket-tcp.sh
index 093d44772ffd..392d269bada5 100755
--- a/tools/testing/selftests/net/ovpn/test-close-socket-tcp.sh
+++ b/tools/testing/selftests/net/ovpn/test-close-socket-tcp.sh
@@ -4,6 +4,6 @@
 #
 #  Author:	Antonio Quartulli <antonio@openvpn.net>
 
-PROTO="TCP"
+OVPN_PROTO="TCP"
 
 source test-close-socket.sh
diff --git a/tools/testing/selftests/net/ovpn/test-close-socket.sh b/tools/testing/selftests/net/ovpn/test-close-socket.sh
index 0d09df14fe8e..4f0367c60fda 100755
--- a/tools/testing/selftests/net/ovpn/test-close-socket.sh
+++ b/tools/testing/selftests/net/ovpn/test-close-socket.sh
@@ -9,30 +9,30 @@ set -e
 
 source ./common.sh
 
-cleanup
+ovpn_cleanup
 
 modprobe -q ovpn || true
 
-for p in $(seq 0 ${NUM_PEERS}); do
-	create_ns ${p}
+for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+	ovpn_create_ns ${p}
 done
 
-for p in $(seq 0 ${NUM_PEERS}); do
-	setup_ns ${p} 5.5.5.$((${p} + 1))/24
+for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+	ovpn_setup_ns ${p} 5.5.5.$((${p} + 1))/24
 done
 
-for p in $(seq 0 ${NUM_PEERS}); do
-	add_peer ${p}
+for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+	ovpn_add_peer ${p}
 done
 
-for p in $(seq 1 ${NUM_PEERS}); do
+for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 	ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 60 120
 	ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} $((${p}+9)) 60 120
 done
 
 sleep 1
 
-for p in $(seq 1 ${NUM_PEERS}); do
+for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 	ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((${p} + 1))
 done
 
@@ -40,6 +40,6 @@ ip netns exec peer0 iperf3 -1 -s &
 sleep 1
 ip netns exec peer1 iperf3 -Z -t 3 -c 5.5.5.1
 
-cleanup
+ovpn_cleanup
 
 modprobe -r ovpn || true
diff --git a/tools/testing/selftests/net/ovpn/test-float.sh b/tools/testing/selftests/net/ovpn/test-float.sh
index ba5d725e18b0..91f8e113718e 100755
--- a/tools/testing/selftests/net/ovpn/test-float.sh
+++ b/tools/testing/selftests/net/ovpn/test-float.sh
@@ -4,6 +4,6 @@
 #
 #  Author:	Antonio Quartulli <antonio@openvpn.net>
 
-FLOAT="1"
+OVPN_FLOAT="1"
 
 source test.sh
diff --git a/tools/testing/selftests/net/ovpn/test-mark.sh b/tools/testing/selftests/net/ovpn/test-mark.sh
index 8534428ed3eb..951baf2ad736 100755
--- a/tools/testing/selftests/net/ovpn/test-mark.sh
+++ b/tools/testing/selftests/net/ovpn/test-mark.sh
@@ -12,29 +12,29 @@ MARK=1056
 
 source ./common.sh
 
-cleanup
+ovpn_cleanup
 
 modprobe -q ovpn || true
 
-for p in $(seq 0 "${NUM_PEERS}"); do
-	create_ns "${p}"
+for p in $(seq 0 "${OVPN_NUM_PEERS}"); do
+	ovpn_create_ns "${p}"
 done
 
 for p in $(seq 0 3); do
-	setup_ns "${p}" 5.5.5.$((p + 1))/24
+	ovpn_setup_ns "${p}" 5.5.5.$((p + 1))/24
 done
 
 # add peer0 with mark
 ip netns exec peer0 "${OVPN_CLI}" new_multi_peer tun0 1 ASYMM \
-	"${UDP_PEERS_FILE}" \
+	"${OVPN_UDP_PEERS_FILE}" \
 	${MARK}
 for p in $(seq 1 3); do
-	ip netns exec peer0 "${OVPN_CLI}" new_key tun0 "${p}" 1 0 "${ALG}" 0 \
+	ip netns exec peer0 "${OVPN_CLI}" new_key tun0 "${p}" 1 0 "${OVPN_ALG}" 0 \
 		data64.key
 done
 
 for p in $(seq 1 3); do
-	add_peer "${p}"
+	ovpn_add_peer "${p}"
 done
 
 for p in $(seq 1 3); do
@@ -91,6 +91,6 @@ for p in $(seq 1 3); do
 	ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((p + 1))
 done
 
-cleanup
+ovpn_cleanup
 
 modprobe -r ovpn || true
diff --git a/tools/testing/selftests/net/ovpn/test-symmetric-id-float.sh b/tools/testing/selftests/net/ovpn/test-symmetric-id-float.sh
index b3711a81b463..75296fe72c39 100755
--- a/tools/testing/selftests/net/ovpn/test-symmetric-id-float.sh
+++ b/tools/testing/selftests/net/ovpn/test-symmetric-id-float.sh
@@ -5,7 +5,7 @@
 #	Author:	Ralf Lici <ralf@mandelbit.com>
 #		Antonio Quartulli <antonio@openvpn.net>
 
-SYMMETRIC_ID="1"
-FLOAT="1"
+OVPN_SYMMETRIC_ID="1"
+OVPN_FLOAT="1"
 
 source test.sh
diff --git a/tools/testing/selftests/net/ovpn/test-symmetric-id-tcp.sh b/tools/testing/selftests/net/ovpn/test-symmetric-id-tcp.sh
index 188cafb67b2f..680a465c49d2 100755
--- a/tools/testing/selftests/net/ovpn/test-symmetric-id-tcp.sh
+++ b/tools/testing/selftests/net/ovpn/test-symmetric-id-tcp.sh
@@ -5,7 +5,7 @@
 #	Author:	Ralf Lici <ralf@mandelbit.com>
 #		Antonio Quartulli <antonio@openvpn.net>
 
-PROTO="TCP"
-SYMMETRIC_ID=1
+OVPN_PROTO="TCP"
+OVPN_SYMMETRIC_ID=1
 
 source test.sh
diff --git a/tools/testing/selftests/net/ovpn/test-symmetric-id.sh b/tools/testing/selftests/net/ovpn/test-symmetric-id.sh
index 35b119c72e4f..a2e2808959d9 100755
--- a/tools/testing/selftests/net/ovpn/test-symmetric-id.sh
+++ b/tools/testing/selftests/net/ovpn/test-symmetric-id.sh
@@ -5,6 +5,6 @@
 #	Author:	Ralf Lici <ralf@mandelbit.com>
 #		Antonio Quartulli <antonio@openvpn.net>
 
-SYMMETRIC_ID="1"
+OVPN_SYMMETRIC_ID="1"
 
 source test.sh
diff --git a/tools/testing/selftests/net/ovpn/test-tcp.sh b/tools/testing/selftests/net/ovpn/test-tcp.sh
index ba3f1f315a34..27cc6e7b98bc 100755
--- a/tools/testing/selftests/net/ovpn/test-tcp.sh
+++ b/tools/testing/selftests/net/ovpn/test-tcp.sh
@@ -4,6 +4,6 @@
 #
 #  Author:	Antonio Quartulli <antonio@openvpn.net>
 
-PROTO="TCP"
+OVPN_PROTO="TCP"
 
 source test.sh
diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
index b60e94a4094e..3a826d070742 100755
--- a/tools/testing/selftests/net/ovpn/test.sh
+++ b/tools/testing/selftests/net/ovpn/test.sh
@@ -9,36 +9,36 @@ set -e
 
 source ./common.sh
 
-cleanup
+ovpn_cleanup
 
 modprobe -q ovpn || true
 
-for p in $(seq 0 ${NUM_PEERS}); do
-	create_ns ${p}
+for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+	ovpn_create_ns ${p}
 done
 
-for p in $(seq 0 ${NUM_PEERS}); do
-	setup_listener ${p}
+for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+	ovpn_setup_listener ${p}
 done
 
-for p in $(seq 0 ${NUM_PEERS}); do
-	setup_ns ${p} 5.5.5.$((${p} + 1))/24 ${MTU}
+for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+	ovpn_setup_ns ${p} 5.5.5.$((${p} + 1))/24 ${MTU}
 done
 
-for p in $(seq 0 ${NUM_PEERS}); do
-	add_peer ${p}
+for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+	ovpn_add_peer ${p}
 done
 
-for p in $(seq 1 ${NUM_PEERS}); do
+for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 	ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 60 120
 	ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
-		$((${p}+ID_OFFSET)) 60 120
+		$((${p}+OVPN_ID_OFFSET)) 60 120
 done
 
 sleep 1
 
 TCPDUMP_TIMEOUT="1.5s"
-for p in $(seq 1 ${NUM_PEERS}); do
+for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 	# The first part of the data packet header consists of:
 	# - TCP only: 2 bytes for the packet length
 	# - 5 bits for opcode ("9" for DATA_V2)
@@ -47,20 +47,20 @@ for p in $(seq 1 ${NUM_PEERS}); do
 	#     - with asymmetric ID: "${p}" one way and "${p} + 9" the other way
 	#     - with symmetric ID: "${p}" both ways
 	HEADER1=$(printf "0x4800000%x" ${p})
-	HEADER2=$(printf "0x4800000%x" $((${p} + ID_OFFSET)))
+	HEADER2=$(printf "0x4800000%x" $((${p} + OVPN_ID_OFFSET)))
 	RADDR=""
-	if [ "${PROTO}" == "UDP" ]; then
-		RADDR=$(awk "NR == ${p} {print \$3}" ${UDP_PEERS_FILE})
+	if [ "${OVPN_PROTO}" == "UDP" ]; then
+		RADDR=$(awk "NR == ${p} {print \$3}" ${OVPN_UDP_PEERS_FILE})
 	fi
 
 	timeout ${TCPDUMP_TIMEOUT} ip netns exec peer${p} \
 		tcpdump --immediate-mode -p -ni veth${p} -c 1 \
-		"$(build_capture_filter "${HEADER1}" "${RADDR}")" \
+		"$(ovpn_build_capture_filter "${HEADER1}" "${RADDR}")" \
 		>/dev/null 2>&1 &
 	TCPDUMP_PID1=$!
 	timeout ${TCPDUMP_TIMEOUT} ip netns exec peer${p} \
 		tcpdump --immediate-mode -p -ni veth${p} -c 1 \
-		"$(build_capture_filter "${HEADER2}" "${RADDR}")" \
+		"$(ovpn_build_capture_filter "${HEADER2}" "${RADDR}")" \
 		>/dev/null 2>&1 &
 	TCPDUMP_PID2=$!
 
@@ -73,15 +73,15 @@ for p in $(seq 1 ${NUM_PEERS}); do
 done
 
 # ping LAN behind client 1
-ip netns exec peer0 ping -qfc 500 -w 3 ${LAN_IP}
+ip netns exec peer0 ping -qfc 500 -w 3 ${OVPN_LAN_IP}
 
-if [ "$FLOAT" == "1" ]; then
+if [ "$OVPN_FLOAT" == "1" ]; then
 	# make clients float..
-	for p in $(seq 1 ${NUM_PEERS}); do
+	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 		ip -n peer${p} addr del 10.10.${p}.2/24 dev veth${p}
 		ip -n peer${p} addr add 10.10.${p}.3/24 dev veth${p}
 	done
-	for p in $(seq 1 ${NUM_PEERS}); do
+	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 		ip netns exec peer${p} ping -qfc 500 -w 3 5.5.5.1
 	done
 fi
@@ -91,13 +91,13 @@ sleep 1
 ip netns exec peer1 iperf3 -Z -t 3 -c 5.5.5.1
 
 echo "Adding secondary key and then swap:"
-for p in $(seq 1 ${NUM_PEERS}); do
-	ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 2 1 ${ALG} 0 \
+for p in $(seq 1 ${OVPN_NUM_PEERS}); do
+	ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 2 1 ${OVPN_ALG} 0 \
 		data64.key
 	ip netns exec peer${p} ${OVPN_CLI} new_key tun${p} \
-		$((${p} + ID_OFFSET)) 2 1 ${ALG} 1 data64.key
+		$((${p} + OVPN_ID_OFFSET)) 2 1 ${OVPN_ALG} 1 data64.key
 	ip netns exec peer${p} ${OVPN_CLI} swap_keys tun${p} \
-		$((${p} + ID_OFFSET))
+		$((${p} + OVPN_ID_OFFSET))
 done
 
 sleep 1
@@ -114,14 +114,14 @@ ip netns exec peer0 ${OVPN_CLI} get_peer tun0 20 || true
 
 echo "Deleting peer 1:"
 ip netns exec peer0 ${OVPN_CLI} del_peer tun0 1
-ip netns exec peer1 ${OVPN_CLI} del_peer tun1 $((1 + ID_OFFSET))
+ip netns exec peer1 ${OVPN_CLI} del_peer tun1 $((1 + OVPN_ID_OFFSET))
 
 echo "Querying keys:"
-for p in $(seq 2 ${NUM_PEERS}); do
+for p in $(seq 2 ${OVPN_NUM_PEERS}); do
 	ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} \
-		$((${p} + ID_OFFSET)) 1
+		$((${p} + OVPN_ID_OFFSET)) 1
 	ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} \
-		$((${p} + ID_OFFSET)) 2
+		$((${p} + OVPN_ID_OFFSET)) 2
 done
 
 echo "Deleting peer while sending traffic:"
@@ -130,36 +130,36 @@ sleep 2
 ip netns exec peer0 ${OVPN_CLI} del_peer tun0 2
 # following command fails in TCP mode
 # (both ends get conn reset when one peer disconnects)
-ip netns exec peer2 ${OVPN_CLI} del_peer tun2 $((2 + ID_OFFSET)) || true
+ip netns exec peer2 ${OVPN_CLI} del_peer tun2 $((2 + OVPN_ID_OFFSET)) || true
 
 echo "Deleting keys:"
-for p in $(seq 3 ${NUM_PEERS}); do
+for p in $(seq 3 ${OVPN_NUM_PEERS}); do
 	ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} \
-		$((${p} + ID_OFFSET)) 1
+		$((${p} + OVPN_ID_OFFSET)) 1
 	ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} \
-		$((${p} + ID_OFFSET)) 2
+		$((${p} + OVPN_ID_OFFSET)) 2
 done
 
 echo "Setting timeout to 3s MP:"
-for p in $(seq 3 ${NUM_PEERS}); do
+for p in $(seq 3 ${OVPN_NUM_PEERS}); do
 	ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 3 3 || true
 	ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
-		$((${p} + ID_OFFSET)) 0 0
+		$((${p} + OVPN_ID_OFFSET)) 0 0
 done
 # wait for peers to timeout
 sleep 5
 
 echo "Setting timeout to 3s P2P:"
-for p in $(seq 3 ${NUM_PEERS}); do
+for p in $(seq 3 ${OVPN_NUM_PEERS}); do
 	ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
-		$((${p} + ID_OFFSET)) 3 3
+		$((${p} + OVPN_ID_OFFSET)) 3 3
 done
 sleep 5
 
-for p in $(seq 0 ${NUM_PEERS}); do
-	compare_ntfs ${p}
+for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+	ovpn_compare_ntfs ${p}
 done
 
-cleanup
+ovpn_cleanup
 
 modprobe -r ovpn || true
-- 
2.52.0


^ permalink raw reply related

* [PATCH net-next 5/5] selftests: ovpn: align command flow with TAP
From: Antonio Quartulli @ 2026-04-12 22:11 UTC (permalink / raw)
  To: netdev
  Cc: ralf, Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
	David S. Miller, Eric Dumazet, Antonio Quartulli
In-Reply-To: <20260412221121.410365-1-antonio@openvpn.net>

From: Ralf Lici <ralf@mandelbit.com>

Restructure ovpn selftests into using the TAP infrastructure: split each
test in stages, execute stage bodies with fail-fast semantics, and emit
KTAP pass/fail for each stage.

Centralize behavior control in common.sh and makes the scripts use
dedicated wrappers for required-success, expected-failure, and non-fatal
commands. Also add the OVPN_VERBOSE mode that exposes captured command
output for debugging.
This way tests won't hang anymore in case of failure when executed
within the CI machinery.

This change also makes default OVPN_CLI and YNL_CLI resolution
independent from the caller CWD by anchoring both to COMMON_DIR, so
behavior is stable across direct execution and run_tests-style
execution.

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 tools/testing/selftests/net/ovpn/common.sh    | 168 ++++++-
 .../selftests/net/ovpn/test-close-socket.sh   |  81 +++-
 tools/testing/selftests/net/ovpn/test-mark.sh | 219 +++++----
 tools/testing/selftests/net/ovpn/test.sh      | 415 ++++++++++++------
 4 files changed, 624 insertions(+), 259 deletions(-)

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
index 2b3f2e5c8cc9..ec6fea37ceb3 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -4,14 +4,18 @@
 #
 #  Author:	Antonio Quartulli <antonio@openvpn.net>
 
+OVPN_COMMON_DIR=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
+source "$OVPN_COMMON_DIR/../../kselftest/ktap_helpers.sh"
+
 OVPN_UDP_PEERS_FILE=${OVPN_UDP_PEERS_FILE:-udp_peers.txt}
 OVPN_TCP_PEERS_FILE=${OVPN_TCP_PEERS_FILE:-tcp_peers.txt}
-OVPN_CLI=${OVPN_CLI:-./ovpn-cli}
-OVPN_YNL_CLI=${OVPN_YNL_CLI:-../../../../net/ynl/pyynl/cli.py}
+OVPN_CLI=${OVPN_CLI:-${OVPN_COMMON_DIR}/ovpn-cli}
+OVPN_YNL_CLI=${OVPN_YNL_CLI:-${OVPN_COMMON_DIR}/../../../../net/ynl/pyynl/cli.py}
 OVPN_ALG=${OVPN_ALG:-aes}
 OVPN_PROTO=${OVPN_PROTO:-UDP}
 OVPN_FLOAT=${OVPN_FLOAT:-0}
 OVPN_SYMMETRIC_ID=${OVPN_SYMMETRIC_ID:-0}
+OVPN_VERBOSE=${OVPN_VERBOSE:-0}
 
 export OVPN_ID_OFFSET=$(( 9 * (OVPN_SYMMETRIC_ID == 0) ))
 
@@ -22,6 +26,110 @@ OVPN_LAN_IP="11.11.11.11"
 
 declare -A OVPN_TMP_JSONS=()
 declare -A OVPN_LISTENER_PIDS=()
+OVPN_CURRENT_STAGE=""
+
+ovpn_is_verbose() {
+	[[ "${OVPN_VERBOSE}" == "1" ]]
+}
+
+ovpn_log() {
+	ovpn_is_verbose || return 0
+	printf '%s\n' "$*"
+}
+
+ovpn_print_cmd_output() {
+	local output_file="$1"
+	local line
+
+	[[ -s "${output_file}" ]] || return 0
+
+	while IFS= read -r line; do
+		ovpn_log "${line}"
+	done < "${output_file}"
+}
+
+ovpn_cmd_run() {
+	local mode="$1"
+	local label="$2"
+	local output_file
+	local rc
+	local ret=0
+
+	shift 2
+
+	output_file=$(mktemp)
+	if "$@" >"${output_file}" 2>&1; then
+		rc=0
+	else
+		rc=$?
+	fi
+
+	case "${mode}" in
+	ok)
+		if [[ "${rc}" -ne 0 ]]; then
+			cat "${output_file}"
+			printf '%s\n' "${label}: command failed with rc=${rc}: $*"
+			ret="${rc}"
+		fi
+		;;
+	mayfail)
+		;;
+	fail)
+		[[ "${rc}" -eq 0 ]] && ret=1
+		;;
+	esac
+
+	if ovpn_is_verbose && [[ "${rc}" -eq 0 || "${mode}" != "ok" ]]; then
+		ovpn_print_cmd_output "${output_file}"
+	fi
+
+	rm -f "${output_file}"
+	return "${ret}"
+}
+
+ovpn_cmd_ok() {
+	ovpn_cmd_run ok "$@"
+}
+
+ovpn_cmd_mayfail() {
+	ovpn_cmd_run mayfail "$@"
+}
+
+ovpn_cmd_fail() {
+	ovpn_cmd_run fail "$@"
+}
+
+ovpn_run_bg() {
+	local pid_var="$1"
+
+	shift
+	if ovpn_is_verbose; then
+		"$@" &
+	else
+		"$@" >/dev/null 2>&1 &
+	fi
+
+	printf -v "${pid_var}" '%s' "$!"
+}
+
+ovpn_run_stage() {
+	local label="$1"
+
+	shift
+	OVPN_CURRENT_STAGE="${label}"
+	"$@"
+	OVPN_CURRENT_STAGE=""
+	ktap_test_pass "${label}"
+}
+
+ovpn_stage_err() {
+	# ERR trap is global under set -eE: only report failures that happen
+	# while ovpn_run_stage() is actively executing a stage body.
+	if [[ -n "${OVPN_CURRENT_STAGE}" ]]; then
+		ktap_test_fail "${OVPN_CURRENT_STAGE}"
+		OVPN_CURRENT_STAGE=""
+	fi
+}
 
 ovpn_create_ns() {
 	ip netns add peer${1}
@@ -78,11 +186,14 @@ ovpn_build_capture_filter() {
 }
 
 ovpn_setup_listener() {
+	local peer="$1"
+	local file
+
 	file=$(mktemp)
-	PYTHONUNBUFFERED=1 ip netns exec peer${p} ${OVPN_YNL_CLI} --family ovpn \
-		--subscribe peers --output-json --duration 40 > ${file} &
-	OVPN_LISTENER_PIDS[$1]=$!
-	OVPN_TMP_JSONS[$1]="${file}"
+	PYTHONUNBUFFERED=1 ip netns exec "peer${peer}" "${OVPN_YNL_CLI}" --family ovpn \
+		--subscribe peers --output-json --duration 40 > "${file}" 2>/dev/null &
+	OVPN_LISTENER_PIDS["${peer}"]=$!
+	OVPN_TMP_JSONS["${peer}"]="${file}"
 }
 
 ovpn_add_peer() {
@@ -152,8 +263,8 @@ ovpn_compare_ntfs() {
 		received="${OVPN_TMP_JSONS[$1]}"
 		diff_file=$(mktemp)
 
-		kill -TERM ${OVPN_LISTENER_PIDS[$1]} || true
-		wait ${OVPN_LISTENER_PIDS[$1]} || true
+		kill -TERM ${OVPN_LISTENER_PIDS[$1]} 2>/dev/null || true
+		wait ${OVPN_LISTENER_PIDS[$1]} 2>/dev/null || true
 		printf "Checking notifications for peer ${1}... "
 		if diff <(jq -s "${OVPN_JQ_FILTER}" ${expected}) \
 			<(jq -s "${OVPN_JQ_FILTER}" ${received}) >"${diff_file}" 2>&1; then
@@ -171,20 +282,49 @@ ovpn_compare_ntfs() {
 	return "${diff_rc}"
 }
 
+ovpn_stop_listener() {
+	local peer="$1"
+	local pid="${OVPN_LISTENER_PIDS[$peer]:-}"
+	local json="${OVPN_TMP_JSONS[$peer]:-}"
+
+	if [[ -n "${pid}" ]]; then
+		kill -TERM "${pid}" 2>/dev/null || true
+		wait "${pid}" 2>/dev/null || true
+		unset "OVPN_LISTENER_PIDS[$peer]"
+	fi
+
+	if [[ -n "${json}" ]]; then
+		rm -f "${json}" || true
+		unset "OVPN_TMP_JSONS[$peer]"
+	fi
+}
+
+ovpn_cleanup_peer_ns() {
+	local peer="$1"
+
+	if ! ip netns list | grep -qx "${peer}"; then
+		return 0
+	fi
+
+	ip -n "${peer}" link set tun${peer#peer} down 2>/dev/null || true
+	ip netns exec "${peer}" ${OVPN_CLI} del_iface tun${peer#peer} \
+		1>/dev/null 2>&1 || true
+	ip netns del "${peer}" 2>/dev/null || true
+}
+
 ovpn_cleanup() {
 	# some ovpn-cli processes sleep in background so they need manual poking
-	killall $(basename ${OVPN_CLI}) 2>/dev/null || true
+	killall "$(basename "${OVPN_CLI}")" 2>/dev/null || true
 
-	# netns peer0 is deleted without erasing ifaces first
-	for p in $(seq 1 10); do
-		ip -n peer${p} link set tun${p} down 2>/dev/null || true
-		ip netns exec peer${p} ${OVPN_CLI} del_iface tun${p} 2>/dev/null || true
+	for peer in "${!OVPN_LISTENER_PIDS[@]}"; do
+		ovpn_stop_listener "${peer}" 2>/dev/null
 	done
+
 	for p in $(seq 1 10); do
 		ip -n peer0 link del veth${p} 2>/dev/null || true
 	done
 	for p in $(seq 0 10); do
-		ip netns del peer${p} 2>/dev/null || true
+		ovpn_cleanup_peer_ns "peer${p}" 2>/dev/null
 	done
 }
 
diff --git a/tools/testing/selftests/net/ovpn/test-close-socket.sh b/tools/testing/selftests/net/ovpn/test-close-socket.sh
index 4f0367c60fda..e97affe72738 100755
--- a/tools/testing/selftests/net/ovpn/test-close-socket.sh
+++ b/tools/testing/selftests/net/ovpn/test-close-socket.sh
@@ -9,37 +9,72 @@ set -e
 
 source ./common.sh
 
-ovpn_cleanup
+ovpn_test_finished=0
 
-modprobe -q ovpn || true
+ovpn_test_exit() {
+	ovpn_cleanup
+	modprobe -r ovpn || true
+
+	if [ "${ovpn_test_finished}" -eq 0 ]; then
+		ktap_print_totals
+	fi
+}
+
+ovpn_prepare_network() {
+	local p
+
+	for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "create namespace peer${p}" ovpn_create_ns "${p}"
+	done
 
-for p in $(seq 0 ${OVPN_NUM_PEERS}); do
-	ovpn_create_ns ${p}
-done
+	for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "configure peer${p} namespace" ovpn_setup_ns "${p}" \
+			5.5.5.$((p + 1))/24
+	done
 
-for p in $(seq 0 ${OVPN_NUM_PEERS}); do
-	ovpn_setup_ns ${p} 5.5.5.$((${p} + 1))/24
-done
+	for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "register peer${p} in overlay" ovpn_add_peer "${p}"
+	done
 
-for p in $(seq 0 ${OVPN_NUM_PEERS}); do
-	ovpn_add_peer ${p}
-done
+	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "set peer0 timeout for peer ${p}" \
+			ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 60 120
+		ovpn_cmd_ok "set peer${p} timeout for peer ${p}" \
+			ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
+				$((p + OVPN_ID_OFFSET)) 60 120
+	done
+}
 
-for p in $(seq 1 ${OVPN_NUM_PEERS}); do
-	ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 60 120
-	ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} $((${p}+9)) 60 120
-done
+ovpn_run_ping_traffic() {
+	local p
 
-sleep 1
+	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "send ping traffic to peer ${p}" \
+			ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((p + 1))
+	done
+}
 
-for p in $(seq 1 ${OVPN_NUM_PEERS}); do
-	ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((${p} + 1))
-done
+ovpn_run_iperf() {
+	local iperf_pid
 
-ip netns exec peer0 iperf3 -1 -s &
-sleep 1
-ip netns exec peer1 iperf3 -Z -t 3 -c 5.5.5.1
+	ovpn_run_bg iperf_pid ip netns exec peer0 iperf3 -1 -s
+	sleep 1
+	ovpn_cmd_ok "run iperf throughput flow" \
+		ip netns exec peer1 iperf3 -Z -t 3 -c 5.5.5.1
+	wait "${iperf_pid}" || return 1
+}
+
+trap ovpn_test_exit EXIT
+
+ktap_print_header
+ktap_set_plan 3
 
 ovpn_cleanup
+modprobe -q ovpn || true
+
+ovpn_run_stage "setup network topology" ovpn_prepare_network
+ovpn_run_stage "run ping traffic" ovpn_run_ping_traffic
+ovpn_run_stage "run iperf throughput" ovpn_run_iperf
 
-modprobe -r ovpn || true
+ovpn_test_finished=1
+ktap_finished
diff --git a/tools/testing/selftests/net/ovpn/test-mark.sh b/tools/testing/selftests/net/ovpn/test-mark.sh
index 951baf2ad736..ba2443fbd49f 100755
--- a/tools/testing/selftests/net/ovpn/test-mark.sh
+++ b/tools/testing/selftests/net/ovpn/test-mark.sh
@@ -6,91 +6,152 @@
 #		Antonio Quartulli <antonio@openvpn.net>
 
 #set -x
-set -e
+set -eE
 
 MARK=1056
+MARK_DROP_COUNTER=0
 
 source ./common.sh
 
-ovpn_cleanup
+ovpn_test_finished=0
+
+ovpn_test_exit() {
+	ovpn_cleanup
+	modprobe -r ovpn || true
+
+	if [ "${ovpn_test_finished}" -eq 0 ]; then
+		ktap_print_totals
+	fi
+}
+
+ovpn_mark_prepare_network() {
+	local p
+
+	for p in $(seq 0 "${OVPN_NUM_PEERS}"); do
+		ovpn_cmd_ok "create namespace peer${p}" ovpn_create_ns "${p}"
+	done
+
+	for p in $(seq 0 3); do
+		ovpn_cmd_ok "configure peer${p} namespace" ovpn_setup_ns "${p}" \
+			5.5.5.$((p + 1))/24
+	done
+
+	ovpn_cmd_ok "create server-side multi-peer with fwmark" \
+		ip netns exec peer0 "${OVPN_CLI}" new_multi_peer tun0 1 ASYMM \
+			"${OVPN_UDP_PEERS_FILE}" "${MARK}"
+	for p in $(seq 1 3); do
+		ovpn_cmd_ok "install server key for peer ${p}" \
+			ip netns exec peer0 "${OVPN_CLI}" new_key tun0 "${p}" \
+				1 0 "${OVPN_ALG}" 0 data64.key
+	done
+
+	for p in $(seq 1 3); do
+		ovpn_cmd_ok "register peer${p} in overlay" ovpn_add_peer "${p}"
+	done
+
+	for p in $(seq 1 3); do
+		ovpn_cmd_ok "set peer0 timeout for peer ${p}" \
+			ip netns exec peer0 "${OVPN_CLI}" set_peer tun0 "${p}" 60 120
+		ovpn_cmd_ok "set peer${p} timeout for peer ${p}" \
+			ip netns exec peer"${p}" "${OVPN_CLI}" set_peer tun"${p}" \
+				$((p + OVPN_ID_OFFSET)) 60 120
+	done
+}
+
+ovpn_mark_run_baseline_traffic() {
+	local p
+
+	for p in $(seq 1 3); do
+		ovpn_cmd_ok "send baseline traffic to peer ${p}" \
+			ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((p + 1))
+	done
+}
+
+ovpn_mark_add_drop_rule() {
+	ovpn_log "Adding an nftables drop rule based on mark value ${MARK}"
+
+	ovpn_cmd_ok "flush nft ruleset" ip netns exec peer0 nft flush ruleset
+	ovpn_cmd_ok "create nft filter table" ip netns exec peer0 nft "add table inet filter"
+	ovpn_cmd_ok "create nft filter output chain" \
+		ip netns exec peer0 nft "add chain inet filter output { type filter hook output \
+			priority 0; policy accept; }"
+	ovpn_cmd_ok "add nft drop rule for mark ${MARK}" \
+		ip netns exec peer0 nft add rule inet filter output \
+			meta mark == "${MARK}" \
+			counter drop
+
+	MARK_DROP_COUNTER=$(ip netns exec peer0 nft list chain inet filter output \
+		| sed -n 's/.*packets \([0-9]*\).*/\1/p')
+	if [ -z "${MARK_DROP_COUNTER}" ]; then
+		printf '%s\n' "unable to read nft drop counter"
+		return 1
+	fi
+}
+
+ovpn_mark_verify_drop_traffic() {
+	local p
+	local ping_output
+	local lost_packets
+	local total_count
+
+	for p in $(seq 1 3); do
+		if ping_output=$(ip netns exec peer0 ping -qfc 500 -w 1 \
+			5.5.5.$((p + 1)) 2>&1); then
+			printf '%s\n' "expected ping to peer ${p} to fail after nft drop rule"
+			return 1
+		fi
+		ovpn_log "${ping_output}"
+		lost_packets=$(echo "${ping_output}" | awk '/packets transmitted/ { print $1 }')
+		if [ -z "${lost_packets}" ]; then
+			printf '%s\n' "unable to parse lost packets for peer ${p}"
+			return 1
+		fi
+		MARK_DROP_COUNTER=$((MARK_DROP_COUNTER + lost_packets))
+	done
+
+	total_count=$(ip netns exec peer0 nft list chain inet filter output \
+		| sed -n 's/.*packets \([0-9]*\).*/\1/p')
+	if [ -z "${total_count}" ]; then
+		printf '%s\n' "unable to read final nft drop counter"
+		return 1
+	fi
+	if [ "${MARK_DROP_COUNTER}" -ne "${total_count}" ]; then
+		printf '%s\n' "expected ${MARK_DROP_COUNTER} drops, got ${total_count}"
+		return 1
+	fi
+}
+
+ovpn_mark_remove_drop_rule() {
+	ovpn_log "Removing the drop rule"
+
+	ovpn_cmd_ok "flush nft ruleset" ip netns exec peer0 nft flush ruleset
+}
+
+ovpn_mark_verify_traffic_recovery() {
+	local p
+
+	sleep 1
+	for p in $(seq 1 3); do
+		ovpn_cmd_ok "send recovery traffic to peer ${p}" \
+			ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((p + 1))
+	done
+}
+
+trap ovpn_test_exit EXIT
+trap ovpn_stage_err ERR
+
+ktap_print_header
+ktap_set_plan 6
 
+ovpn_cleanup
 modprobe -q ovpn || true
 
-for p in $(seq 0 "${OVPN_NUM_PEERS}"); do
-	ovpn_create_ns "${p}"
-done
-
-for p in $(seq 0 3); do
-	ovpn_setup_ns "${p}" 5.5.5.$((p + 1))/24
-done
-
-# add peer0 with mark
-ip netns exec peer0 "${OVPN_CLI}" new_multi_peer tun0 1 ASYMM \
-	"${OVPN_UDP_PEERS_FILE}" \
-	${MARK}
-for p in $(seq 1 3); do
-	ip netns exec peer0 "${OVPN_CLI}" new_key tun0 "${p}" 1 0 "${OVPN_ALG}" 0 \
-		data64.key
-done
-
-for p in $(seq 1 3); do
-	ovpn_add_peer "${p}"
-done
-
-for p in $(seq 1 3); do
-	ip netns exec peer0 "${OVPN_CLI}" set_peer tun0 "${p}" 60 120
-	ip netns exec peer"${p}" "${OVPN_CLI}" set_peer tun"${p}" \
-		$((p + 9)) 60 120
-done
-
-sleep 1
-
-for p in $(seq 1 3); do
-	ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((p + 1))
-done
-
-echo "Adding an nftables drop rule based on mark value ${MARK}"
-ip netns exec peer0 nft flush ruleset
-ip netns exec peer0 nft 'add table inet filter'
-ip netns exec peer0 nft 'add chain inet filter output {
-	type filter hook output priority 0;
-	policy accept;
-}'
-ip netns exec peer0 nft add rule inet filter output \
-	meta mark == ${MARK} \
-	counter drop
-
-DROP_COUNTER=$(ip netns exec peer0 nft list chain inet filter output \
-	| sed -n 's/.*packets \([0-9]*\).*/\1/p')
-sleep 1
-
-# ping should fail
-for p in $(seq 1 3); do
-	PING_OUTPUT=$(ip netns exec peer0 ping \
-		-qfc 500 -w 1 5.5.5.$((p + 1)) 2>&1) && exit 1
-	echo "${PING_OUTPUT}"
-	LOST_PACKETS=$(echo "$PING_OUTPUT" \
-		| awk '/packets transmitted/ { print $1 }')
-	# increment the drop counter by the amount of lost packets
-	DROP_COUNTER=$((DROP_COUNTER + LOST_PACKETS))
-done
-
-# check if the final nft counter matches our counter
-TOTAL_COUNT=$(ip netns exec peer0 nft list chain inet filter output \
-	| sed -n 's/.*packets \([0-9]*\).*/\1/p')
-if [ "${DROP_COUNTER}" -ne "${TOTAL_COUNT}" ]; then
-	echo "Expected ${TOTAL_COUNT} drops, got ${DROP_COUNTER}"
-	exit 1
-fi
-
-echo "Removing the drop rule"
-ip netns exec peer0 nft flush ruleset
-sleep 1
-
-for p in $(seq 1 3); do
-	ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((p + 1))
-done
-
-ovpn_cleanup
+ovpn_run_stage "setup marked network topology" ovpn_mark_prepare_network
+ovpn_run_stage "run baseline traffic" ovpn_mark_run_baseline_traffic
+ovpn_run_stage "install nft mark drop rule" ovpn_mark_add_drop_rule
+ovpn_run_stage "drop marked traffic and count packets" ovpn_mark_verify_drop_traffic
+ovpn_run_stage "remove nft drop rule" ovpn_mark_remove_drop_rule
+ovpn_run_stage "traffic recovers after drop removal" ovpn_mark_verify_traffic_recovery
 
-modprobe -r ovpn || true
+ovpn_test_finished=1
+ktap_finished
diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
index 3a826d070742..4e5e874fdcee 100755
--- a/tools/testing/selftests/net/ovpn/test.sh
+++ b/tools/testing/selftests/net/ovpn/test.sh
@@ -5,161 +5,290 @@
 #  Author:	Antonio Quartulli <antonio@openvpn.net>
 
 #set -x
-set -e
+set -eE
 
 source ./common.sh
 
-ovpn_cleanup
+ovpn_test_finished=0
 
-modprobe -q ovpn || true
+ovpn_test_exit() {
+	ovpn_cleanup
+	modprobe -r ovpn || true
 
-for p in $(seq 0 ${OVPN_NUM_PEERS}); do
-	ovpn_create_ns ${p}
-done
-
-for p in $(seq 0 ${OVPN_NUM_PEERS}); do
-	ovpn_setup_listener ${p}
-done
-
-for p in $(seq 0 ${OVPN_NUM_PEERS}); do
-	ovpn_setup_ns ${p} 5.5.5.$((${p} + 1))/24 ${MTU}
-done
-
-for p in $(seq 0 ${OVPN_NUM_PEERS}); do
-	ovpn_add_peer ${p}
-done
-
-for p in $(seq 1 ${OVPN_NUM_PEERS}); do
-	ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 60 120
-	ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
-		$((${p}+OVPN_ID_OFFSET)) 60 120
-done
-
-sleep 1
-
-TCPDUMP_TIMEOUT="1.5s"
-for p in $(seq 1 ${OVPN_NUM_PEERS}); do
-	# The first part of the data packet header consists of:
-	# - TCP only: 2 bytes for the packet length
-	# - 5 bits for opcode ("9" for DATA_V2)
-	# - 3 bits for key-id ("0" at this point)
-	# - 12 bytes for peer-id:
-	#     - with asymmetric ID: "${p}" one way and "${p} + 9" the other way
-	#     - with symmetric ID: "${p}" both ways
-	HEADER1=$(printf "0x4800000%x" ${p})
-	HEADER2=$(printf "0x4800000%x" $((${p} + OVPN_ID_OFFSET)))
-	RADDR=""
-	if [ "${OVPN_PROTO}" == "UDP" ]; then
-		RADDR=$(awk "NR == ${p} {print \$3}" ${OVPN_UDP_PEERS_FILE})
+	if [ "${ovpn_test_finished}" -eq 0 ]; then
+		ktap_print_totals
 	fi
+}
+
+ovpn_prepare_network() {
+	local p
+
+	for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "create namespace peer${p}" ovpn_create_ns "${p}"
+	done
+
+	for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "start notification listener peer${p}" \
+			ovpn_setup_listener "${p}"
+	done
+
+	for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "configure peer${p} namespace" ovpn_setup_ns "${p}" \
+			5.5.5.$((p + 1))/24 "${MTU}"
+	done
+
+	for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "register peer${p} in overlay" ovpn_add_peer "${p}"
+	done
 
-	timeout ${TCPDUMP_TIMEOUT} ip netns exec peer${p} \
-		tcpdump --immediate-mode -p -ni veth${p} -c 1 \
-		"$(ovpn_build_capture_filter "${HEADER1}" "${RADDR}")" \
-		>/dev/null 2>&1 &
-	TCPDUMP_PID1=$!
-	timeout ${TCPDUMP_TIMEOUT} ip netns exec peer${p} \
-		tcpdump --immediate-mode -p -ni veth${p} -c 1 \
-		"$(ovpn_build_capture_filter "${HEADER2}" "${RADDR}")" \
-		>/dev/null 2>&1 &
-	TCPDUMP_PID2=$!
-
-	sleep 0.3
-	ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((${p} + 1))
-	ip netns exec peer0 ping -qfc 500 -s 3000 -w 3 5.5.5.$((${p} + 1))
-
-	wait ${TCPDUMP_PID1}
-	wait ${TCPDUMP_PID2}
-done
-
-# ping LAN behind client 1
-ip netns exec peer0 ping -qfc 500 -w 3 ${OVPN_LAN_IP}
-
-if [ "$OVPN_FLOAT" == "1" ]; then
-	# make clients float..
 	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
-		ip -n peer${p} addr del 10.10.${p}.2/24 dev veth${p}
-		ip -n peer${p} addr add 10.10.${p}.3/24 dev veth${p}
+		ovpn_cmd_ok "set peer0 timeout for peer ${p}" \
+			ip netns exec peer0 ${OVPN_CLI} set_peer tun0 \
+				${p} 60 120
+		ovpn_cmd_ok "set peer${p} timeout for peer ${p}" \
+			ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
+				$((p + OVPN_ID_OFFSET)) 60 120
 	done
+}
+
+ovpn_run_basic_traffic() {
+	local p
+	local header1
+	local header2
+	local raddr
+	local tcpdump_pid1
+	local tcpdump_pid2
+	local tcpdump_timeout="1.5s"
+
 	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
-		ip netns exec peer${p} ping -qfc 500 -w 3 5.5.5.1
+		# The first part of the data packet header consists of:
+		# - TCP only: 2 bytes for the packet length
+		# - 5 bits for opcode ("9" for DATA_V2)
+		# - 3 bits for key-id ("0" at this point)
+		# - 12 bytes for peer-id:
+		#     - with asymmetric ID: "${p}" one way and "${p} + 9" the other way
+		#     - with symmetric ID: "${p}" both ways
+		header1=$(printf "0x4800000%x" ${p})
+		header2=$(printf "0x4800000%x" $((p + OVPN_ID_OFFSET)))
+		raddr=""
+		if [ "${OVPN_PROTO}" == "UDP" ]; then
+			raddr=$(awk "NR == ${p} {print \$3}" "${OVPN_UDP_PEERS_FILE}")
+		fi
+
+		timeout ${tcpdump_timeout} ip netns exec peer${p} \
+			tcpdump --immediate-mode -p -ni veth${p} -c 1 \
+			"$(ovpn_build_capture_filter "${header1}" "${raddr}")" \
+			>/dev/null 2>&1 &
+		tcpdump_pid1=$!
+		timeout ${tcpdump_timeout} ip netns exec peer${p} \
+			tcpdump --immediate-mode -p -ni veth${p} -c 1 \
+			"$(ovpn_build_capture_filter "${header2}" "${raddr}")" \
+			>/dev/null 2>&1 &
+		tcpdump_pid2=$!
+
+		sleep 0.3
+		ovpn_cmd_ok "send baseline traffic to peer ${p}" \
+			ip netns exec peer0 \
+			ping -qfc 500 -w 3 5.5.5.$((p + 1))
+		ovpn_cmd_ok "send large-payload traffic to peer ${p}" \
+			ip netns exec peer0 \
+			ping -qfc 500 -s 3000 -w 3 5.5.5.$((p + 1))
+
+		wait "${tcpdump_pid1}" || return 1
+		wait "${tcpdump_pid2}" || return 1
 	done
-fi
+}
+
+ovpn_run_lan_traffic() {
+	ovpn_cmd_ok "ping LAN behind peer1" \
+		ip netns exec peer0 ping -qfc 500 -w 3 "${OVPN_LAN_IP}"
+}
+
+ovpn_run_float_mode() {
+	local p
+
+	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "float: remove old transport address on peer${p}" \
+			ip -n peer${p} addr del 10.10.${p}.2/24 dev veth${p}
+		ovpn_cmd_ok "float: add new transport address on peer${p}" \
+			ip -n peer${p} addr add 10.10.${p}.3/24 dev veth${p}
+	done
+	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "ping tunnel after float peer ${p}" \
+			ip netns exec peer${p} ping -qfc 500 -w 3 5.5.5.1
+	done
+}
+
+ovpn_run_iperf() {
+	local iperf_pid
+
+	ovpn_run_bg iperf_pid ip netns exec peer0 iperf3 -1 -s
+	sleep 1
+
+	ovpn_cmd_ok "run iperf throughput flow" \
+		ip netns exec peer1 iperf3 -Z -t 3 -c 5.5.5.1
+	wait "${iperf_pid}" || return 1
+}
+
+ovpn_run_key_rollover() {
+	local p
+
+	ovpn_log "Adding secondary key and then swap:"
+
+	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "add secondary key on peer0 for peer ${p}" \
+			ip netns exec peer0 ${OVPN_CLI} new_key tun0 \
+				${p} 2 1 ${OVPN_ALG} 0 data64.key
+		ovpn_cmd_ok "add secondary key on peer${p} for peer ${p}" \
+			ip netns exec peer${p} ${OVPN_CLI} new_key tun${p} \
+				$((p + OVPN_ID_OFFSET)) 2 1 ${OVPN_ALG} 1 data64.key
+		ovpn_cmd_ok "swap keys on peer${p}" \
+			ip netns exec peer${p} ${OVPN_CLI} swap_keys tun${p} \
+				$((p + OVPN_ID_OFFSET))
+	done
+}
+
+ovpn_run_queries() {
+	ovpn_log "Querying all peers:"
+
+	ovpn_cmd_ok "query all peers from peer0" \
+		ip netns exec peer0 ${OVPN_CLI} get_peer tun0
+	ovpn_cmd_ok "query all peers from peer1" \
+		ip netns exec peer1 ${OVPN_CLI} get_peer tun1
 
-ip netns exec peer0 iperf3 -1 -s &
-sleep 1
-ip netns exec peer1 iperf3 -Z -t 3 -c 5.5.5.1
-
-echo "Adding secondary key and then swap:"
-for p in $(seq 1 ${OVPN_NUM_PEERS}); do
-	ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 2 1 ${OVPN_ALG} 0 \
-		data64.key
-	ip netns exec peer${p} ${OVPN_CLI} new_key tun${p} \
-		$((${p} + OVPN_ID_OFFSET)) 2 1 ${OVPN_ALG} 1 data64.key
-	ip netns exec peer${p} ${OVPN_CLI} swap_keys tun${p} \
-		$((${p} + OVPN_ID_OFFSET))
-done
-
-sleep 1
-
-echo "Querying all peers:"
-ip netns exec peer0 ${OVPN_CLI} get_peer tun0
-ip netns exec peer1 ${OVPN_CLI} get_peer tun1
-
-echo "Querying peer 1:"
-ip netns exec peer0 ${OVPN_CLI} get_peer tun0 1
-
-echo "Querying non-existent peer 20:"
-ip netns exec peer0 ${OVPN_CLI} get_peer tun0 20 || true
-
-echo "Deleting peer 1:"
-ip netns exec peer0 ${OVPN_CLI} del_peer tun0 1
-ip netns exec peer1 ${OVPN_CLI} del_peer tun1 $((1 + OVPN_ID_OFFSET))
-
-echo "Querying keys:"
-for p in $(seq 2 ${OVPN_NUM_PEERS}); do
-	ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} \
-		$((${p} + OVPN_ID_OFFSET)) 1
-	ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} \
-		$((${p} + OVPN_ID_OFFSET)) 2
-done
-
-echo "Deleting peer while sending traffic:"
-(ip netns exec peer2 ping -qf -w 4 5.5.5.1)&
-sleep 2
-ip netns exec peer0 ${OVPN_CLI} del_peer tun0 2
-# following command fails in TCP mode
-# (both ends get conn reset when one peer disconnects)
-ip netns exec peer2 ${OVPN_CLI} del_peer tun2 $((2 + OVPN_ID_OFFSET)) || true
-
-echo "Deleting keys:"
-for p in $(seq 3 ${OVPN_NUM_PEERS}); do
-	ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} \
-		$((${p} + OVPN_ID_OFFSET)) 1
-	ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} \
-		$((${p} + OVPN_ID_OFFSET)) 2
-done
-
-echo "Setting timeout to 3s MP:"
-for p in $(seq 3 ${OVPN_NUM_PEERS}); do
-	ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 3 3 || true
-	ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
-		$((${p} + OVPN_ID_OFFSET)) 0 0
-done
-# wait for peers to timeout
-sleep 5
-
-echo "Setting timeout to 3s P2P:"
-for p in $(seq 3 ${OVPN_NUM_PEERS}); do
-	ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
-		$((${p} + OVPN_ID_OFFSET)) 3 3
-done
-sleep 5
-
-for p in $(seq 0 ${OVPN_NUM_PEERS}); do
-	ovpn_compare_ntfs ${p}
-done
+	ovpn_log "Querying peer 1:"
+
+	ovpn_cmd_ok "query peer 1 from peer0" \
+		ip netns exec peer0 ${OVPN_CLI} get_peer tun0 1
+}
+
+ovpn_query_peer_missing() {
+	ovpn_log "Querying non-existent peer 20:"
+
+	ovpn_cmd_fail "query missing peer 20 on peer0" \
+		ip netns exec peer0 ${OVPN_CLI} get_peer tun0 20
+}
+
+ovpn_run_peer_cleanup() {
+	local p
+
+	ovpn_log "Deleting peer 1:"
+
+	ovpn_cmd_ok "delete peer1 on peer0" \
+		ip netns exec peer0 ${OVPN_CLI} del_peer tun0 1
+	ovpn_cmd_ok "delete peer1 on peer1" \
+		ip netns exec peer1 ${OVPN_CLI} del_peer tun1 $((1 + OVPN_ID_OFFSET))
+
+	ovpn_log "Querying keys:"
+
+	for p in $(seq 2 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "query peer${p} key 1" \
+			ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} \
+				$((p + OVPN_ID_OFFSET)) 1
+		ovpn_cmd_ok "query peer${p} key 2" \
+			ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} \
+				$((p + OVPN_ID_OFFSET)) 2
+	done
+}
+
+ovpn_run_traffic_delete_peer() {
+	local ping_pid
+
+	ovpn_log "Deleting peer while sending traffic:"
+
+	ovpn_run_bg ping_pid ip netns exec peer2 ping -qf -w 4 5.5.5.1
+	sleep 2
+	ovpn_cmd_ok "delete peer0 peer 2" \
+		ip netns exec peer0 ${OVPN_CLI} del_peer tun0 2
+
+	if [ "${OVPN_PROTO}" == "TCP" ]; then
+		# In TCP mode this command is expected to fail for both peers.
+		ovpn_cmd_mayfail "delete peer2 peer 2 (TCP non-fatal)" \
+			ip netns exec peer2 ${OVPN_CLI} del_peer tun2 $((2 + OVPN_ID_OFFSET))
+	else
+		ovpn_cmd_ok "delete peer2 peer 2" ip netns exec peer2 \
+			${OVPN_CLI} del_peer tun2 $((2 + OVPN_ID_OFFSET))
+	fi
+
+	wait "${ping_pid}" || true
+}
+
+ovpn_run_key_cleanup() {
+	local p
+
+	ovpn_log "Deleting keys:"
+
+	for p in $(seq 3 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "delete key 1 for peer${p}" \
+			ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} \
+				$((p + OVPN_ID_OFFSET)) 1
+		ovpn_cmd_ok "delete key 2 for peer${p}" \
+			ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} \
+				$((p + OVPN_ID_OFFSET)) 2
+	done
+}
+
+ovpn_run_timeouts() {
+	local p
+
+	ovpn_log "Setting timeout to 3s MP:"
+
+	for p in $(seq 3 ${OVPN_NUM_PEERS}); do
+		# Non-fatal: this may fail in some protocol modes.
+		ovpn_cmd_mayfail "set peer0 timeout for peer ${p} (non-fatal)" \
+			ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 3 3
+		ovpn_cmd_ok "disable timeout on peer${p} while peer0 adjusts state" \
+			ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
+				$((p + OVPN_ID_OFFSET)) 0 0
+	done
+	# wait for peers to timeout
+	sleep 5
+
+	ovpn_log "Setting timeout to 3s P2P:"
+
+	for p in $(seq 3 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "set peer${p} P2P timeout" \
+			ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} \
+				$((p + OVPN_ID_OFFSET)) 3 3
+	done
+	sleep 5
+}
+
+ovpn_run_notifications() {
+	local p
+
+	for p in $(seq 0 ${OVPN_NUM_PEERS}); do
+		ovpn_cmd_ok "validate listener output for peer ${p}" \
+			ovpn_compare_ntfs "${p}"
+	done
+}
+
+trap ovpn_test_exit EXIT
+trap ovpn_stage_err ERR
+
+ktap_print_header
+if [ "${OVPN_FLOAT}" == "1" ]; then
+	ktap_set_plan 13
+else
+	ktap_set_plan 12
+fi
 
 ovpn_cleanup
+modprobe -q ovpn || true
+
+ovpn_run_stage "setup network topology" ovpn_prepare_network
+ovpn_run_stage "run baseline data traffic" ovpn_run_basic_traffic
+ovpn_run_stage "run LAN traffic behind peer1" ovpn_run_lan_traffic
+[ "${OVPN_FLOAT}" == "1" ] && ovpn_run_stage "run floating peer checks" ovpn_run_float_mode
+ovpn_run_stage "run iperf throughput" ovpn_run_iperf
+ovpn_run_stage "run key rollout" ovpn_run_key_rollover
+ovpn_run_stage "query peers" ovpn_run_queries
+ovpn_run_stage "query missing peer fails" ovpn_query_peer_missing
+ovpn_run_stage "peer lifecycle and key queries" ovpn_run_peer_cleanup
+ovpn_run_stage "delete peer while traffic" ovpn_run_traffic_delete_peer
+ovpn_run_stage "delete stale keys" ovpn_run_key_cleanup
+ovpn_run_stage "check timeout behavior" ovpn_run_timeouts
+ovpn_run_stage "validate notification output" ovpn_run_notifications
 
-modprobe -r ovpn || true
+ovpn_test_finished=1
+ktap_finished
-- 
2.52.0


^ permalink raw reply related

* Re: [PATCH v2 net-next 00/15] ip6mr: No RTNL for RTNL_FAMILY_IP6MR rtnetlink.
From: Kuniyuki Iwashima @ 2026-04-12 22:14 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S . Miller, David Ahern, Eric Dumazet, Paolo Abeni,
	Simon Horman, Kuniyuki Iwashima, netdev
In-Reply-To: <20260412141114.143d0858@kernel.org>

On Sun, Apr 12, 2026 at 2:11 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sun, 12 Apr 2026 13:50:59 -0700 Kuniyuki Iwashima wrote:
> > > Hitting a bunch of:
> > >
> > >   SKIP      no netlink MFC interface
> > >
> > > on the new test here. Do we need to add something to .../config ?
> >
> > No, I used SKIP() intentionally becuase only IPv4 has the MFC
> > netlink interface and IPv6 does not have the corresponding one.
> >
> > Should I just return 0 in this case instead of SKIP() ?
>
> XFAIL

Will use XFAIL_ADD().

Thanks !

^ permalink raw reply

* Re: [PATCH net-next] ppp: tear down bridge before clearing pch->chan
From: Jakub Kicinski @ 2026-04-12 22:27 UTC (permalink / raw)
  To: qingfang.deng
  Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni,
	yuehaibing, kuniyu, kees, bigeasy, linux-ppp, netdev,
	linux-kernel, tparkin, jchapman, gnault
In-Reply-To: <20260410093859.69237-1-qingfang.deng@linux.dev>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ppp: tear down bridge before clearing pch->chan

This commit moves ppp_unbridge_channels() before pch->chan is cleared
and removes the NULL check for pchb->chan in ppp_channel_bridge_input(),
relying on synchronize_rcu() inside ppp_unbridge_channels() for safety.

> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index b097d1b38ac9c..3a609d48a4248 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
> @@ -2285,17 +2285,11 @@ static bool ppp_channel_bridge_input(struct channel *pch, struct sk_buff *skb)
>  		goto out_rcu;
>  
>  	spin_lock_bh(&pchb->downl);
> -	if (!pchb->chan) {
> -		/* channel got unregistered */
> -		kfree_skb(skb);
> -		goto outl;
> -	}
>  
>  	skb_scrub_packet(skb, !net_eq(pch->chan_net, pchb->chan_net));
>  	if (!pchb->chan->ops->start_xmit(pchb->chan, skb))

Could removing this check lead to a NULL pointer dereference if both channels
in a bridge (A and B) are unregistered concurrently?

If Thread 1 unregisters A and calls ppp_unbridge_channels(A), it clears
both A->bridge and B->bridge to NULL, then blocks on synchronize_rcu().

If Thread 2 unregisters B and calls ppp_unbridge_channels(B), it finds
B->bridge is already NULL and returns -EINVAL early, bypassing its own
synchronize_rcu() call.

Thread 2 would then proceed to set B->chan = NULL while an RCU reader
(Thread 3) might still be active here, leading to a crash when it tries
to access pchb->chan->ops->start_xmit.

[ ... ]

> @@ -2997,6 +2991,8 @@ ppp_unregister_channel(struct ppp_channel *chan)
>  	 * the channel's start_xmit or ioctl routine before we proceed.
>  	 */
>  	ppp_disconnect_channel(pch);
> +	ppp_unbridge_channels(pch);
> +

Does moving ppp_unbridge_channels() here introduce a re-bridging race window?

At this point, the channel is still in the pn->all_channels list. After
ppp_unbridge_channels(pch) completes, pch->bridge is NULL.

Could a concurrent PPPIOCBRIDGECHAN ioctl from another channel find this
channel via ppp_find_channel() and successfully establish a new bridge
with it, since ppp_bridge_channels() only verifies that pch->bridge is NULL?

If so, ppp_unregister_channel() would then unconditionally set pch->chan = NULL
and remove it from the list without tearing down the newly formed bridge.
This regression could crash the peer channel when it processes a packet.

>  	down_write(&pch->chan_sem);
>  	spin_lock_bh(&pch->downl);
>  	pch->chan = NULL;
-- 
pw-bot: cr

^ permalink raw reply

* [PATCH net v3] netfilter: nft_set_rbtree: fix use count leak on transaction abort
From: Marko Jevtic @ 2026-04-12 22:28 UTC (permalink / raw)
  To: pablo, fw, netfilter-devel
  Cc: phil, coreteam, davem, edumazet, kuba, pabeni, horms, netdev,
	linux-kernel

nft_rbtree_abort() does not handle elements moved to the expired list
by inline GC during __nft_rbtree_insert(). When inline GC encounters
expired elements during overlap detection, it calls
nft_rbtree_gc_elem_move() which deactivates element data (decrementing
chain/object use counts), removes the element from the rbtree, and
queues it for deferred freeing. On commit, these elements are freed
via nft_rbtree_gc_queue(). On abort, however, the expired list is
ignored entirely.

This leaves use counts permanently decremented after abort.

This restores transactional semantics by ensuring that inline GC side
effects are fully rolled back on abort:

- Introduce a separate tx_gc list for elements collected during insert
  (transaction-scoped), distinct from the existing expired list used
  by commit-time gc_scan (commit-scoped). This prevents abort from
  touching committed expired elements left over from a prior gc_queue
  OOM.

- On commit: splice tx_gc into expired after publishing the new binary
  search blob, then drain via gc_queue as before.

- On abort: iterate tx_gc, re-activate element data (restoring use
  counts), and re-insert into the rbtree. Elements remain expired and
  will be properly collected on the next successful commit.

- Extract nft_rbtree_node_insert() helper from __nft_rbtree_insert()
  to share the tree insertion logic with the abort restore path.

- Add WARN_ON_ONCE in commit early-return path to catch any violation
  of the invariant that tx_gc is empty when no tree changes occurred.

- Reset start_rbe_cookie on abort so insertion state from a failed
  transaction does not persist.

Fixes: f6c383b8c31a ("netfilter: nf_tables: adapt set backend to use GC transaction API")
Signed-off-by: Marko Jevtic <marko.jevtic@codereflect.io>
---
v3:
- add Fixes tag
- narrow the changelog to the abort-side use-count accounting bug

v2:
- introduce a transaction-scoped tx_gc list for insert-time GC
- restore tx_gc entries on abort and splice them to expired on commit
- export nft_setelem_data_activate() and factor out nft_rbtree_node_insert()

 include/net/netfilter/nf_tables.h |  3 +
 net/netfilter/nf_tables_api.c     |  4 +-
 net/netfilter/nft_set_rbtree.c    | 96 ++++++++++++++++++++++---------
 3 files changed, 74 insertions(+), 29 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index ec8a8ec9c..f8c912332 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1910,6 +1910,9 @@ struct nft_trans_gc *nft_trans_gc_catchall_async(struct nft_trans_gc *gc,
 						 unsigned int gc_seq);
 struct nft_trans_gc *nft_trans_gc_catchall_sync(struct nft_trans_gc *gc);
 
+void nft_setelem_data_activate(const struct net *net,
+				 const struct nft_set *set,
+				 struct nft_elem_priv *elem_priv);
 void nft_setelem_data_deactivate(const struct net *net,
 				 const struct nft_set *set,
 				 struct nft_elem_priv *elem_priv);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8c42247a1..8e783db3f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5837,7 +5837,7 @@ static void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
 	}
 }
 
-static void nft_setelem_data_activate(const struct net *net,
-				      const struct nft_set *set,
-				      struct nft_elem_priv *elem_priv);
+void nft_setelem_data_activate(const struct net *net,
+			       const struct nft_set *set,
+			       struct nft_elem_priv *elem_priv);
 
@@ -7656,7 +7656,7 @@ static int nft_setelem_active_next(const struct net *net,
 	return nft_set_elem_active(ext, genmask);
 }
 
-static void nft_setelem_data_activate(const struct net *net,
-				      const struct nft_set *set,
-				      struct nft_elem_priv *elem_priv)
+void nft_setelem_data_activate(const struct net *net,
+			       const struct nft_set *set,
+			       struct nft_elem_priv *elem_priv)
 {
diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 737c339de..e1f76d6ef 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -36,6 +36,7 @@ struct nft_rbtree {
 	unsigned long		start_rbe_cookie;
 	unsigned long		last_gc;
 	struct list_head	expired;
+	struct list_head	tx_gc;
 	u64			last_tstamp;
 };
 
@@ -194,14 +195,14 @@ nft_rbtree_get(const struct net *net, const struct nft_set *set,
 
 static void nft_rbtree_gc_elem_move(struct net *net, struct nft_set *set,
 				    struct nft_rbtree *priv,
-				    struct nft_rbtree_elem *rbe)
+				    struct nft_rbtree_elem *rbe,
+				    struct list_head *target_list)
 {
 	lockdep_assert_held_write(&priv->lock);
 	nft_setelem_data_deactivate(net, set, &rbe->priv);
 	rb_erase(&rbe->node, &priv->root);
 
-	/* collected later on in commit callback */
-	list_add(&rbe->list, &priv->expired);
+	list_add(&rbe->list, target_list);
 }
 
 static const struct nft_rbtree_elem *
@@ -229,10 +230,10 @@ nft_rbtree_gc_elem(const struct nft_set *__set, struct nft_rbtree *priv,
 	rbe_prev = NULL;
 	if (prev) {
 		rbe_prev = rb_entry(prev, struct nft_rbtree_elem, node);
-		nft_rbtree_gc_elem_move(net, set, priv, rbe_prev);
+		nft_rbtree_gc_elem_move(net, set, priv, rbe_prev, &priv->tx_gc);
 	}
 
-	nft_rbtree_gc_elem_move(net, set, priv, rbe);
+	nft_rbtree_gc_elem_move(net, set, priv, rbe, &priv->tx_gc);
 
 	return rbe_prev;
 }
@@ -335,6 +336,35 @@ static bool nft_rbtree_insert_same_interval(const struct net *net,
 	return false;
 }
 
+static void nft_rbtree_node_insert(const struct nft_set *set,
+				   struct nft_rbtree *priv,
+				   struct nft_rbtree_elem *new)
+{
+	struct nft_rbtree_elem *rbe;
+	struct rb_node *parent, **p;
+	int d;
+
+	lockdep_assert_held_write(&priv->lock);
+
+	parent = NULL;
+	p = &priv->root.rb_node;
+	while (*p) {
+		parent = *p;
+		rbe = rb_entry(parent, struct nft_rbtree_elem, node);
+		d = nft_rbtree_cmp(set, rbe, new);
+		if (d < 0)
+			p = &parent->rb_left;
+		else if (d > 0)
+			p = &parent->rb_right;
+		else if (nft_rbtree_interval_end(rbe))
+			p = &parent->rb_left;
+		else
+			p = &parent->rb_right;
+	}
+	rb_link_node_rcu(&new->node, parent, p);
+	rb_insert_color(&new->node, &priv->root);
+}
+
 static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
 			       struct nft_rbtree_elem *new,
 			       struct nft_elem_priv **elem_priv, u64 tstamp)
@@ -516,25 +546,7 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
 		return -ENOTEMPTY;
 
 	/* Accepted element: pick insertion point depending on key value */
-	parent = NULL;
-	p = &priv->root.rb_node;
-	while (*p != NULL) {
-		parent = *p;
-		rbe = rb_entry(parent, struct nft_rbtree_elem, node);
-		d = nft_rbtree_cmp(set, rbe, new);
-
-		if (d < 0)
-			p = &parent->rb_left;
-		else if (d > 0)
-			p = &parent->rb_right;
-		else if (nft_rbtree_interval_end(rbe))
-			p = &parent->rb_left;
-		else
-			p = &parent->rb_right;
-	}
-
-	rb_link_node_rcu(&new->node, parent, p);
-	rb_insert_color(&new->node, &priv->root);
+	nft_rbtree_node_insert(set, priv, new);
 	return 0;
 }
 
@@ -920,11 +932,11 @@ static void nft_rbtree_gc_scan(struct nft_set *set)
 		 */
 		write_lock_bh(&priv->lock);
 		if (rbe_end) {
-			nft_rbtree_gc_elem_move(net, set, priv, rbe_end);
+			nft_rbtree_gc_elem_move(net, set, priv, rbe_end, &priv->expired);
 			rbe_end = NULL;
 		}
 
-		nft_rbtree_gc_elem_move(net, set, priv, rbe);
+		nft_rbtree_gc_elem_move(net, set, priv, rbe, &priv->expired);
 		write_unlock_bh(&priv->lock);
 	}
 
@@ -974,6 +986,7 @@ static int nft_rbtree_init(const struct nft_set *set,
 	rwlock_init(&priv->lock);
 	priv->root = RB_ROOT;
 	INIT_LIST_HEAD(&priv->expired);
+	INIT_LIST_HEAD(&priv->tx_gc);
 
 	priv->array = NULL;
 	priv->array_next = NULL;
@@ -1000,6 +1013,11 @@ static void nft_rbtree_destroy(const struct nft_ctx *ctx,
 		nf_tables_set_elem_destroy(ctx, set, &rbe->priv);
 	}
 
+	list_for_each_entry_safe(rbe, next, &priv->tx_gc, list) {
+		list_del(&rbe->list);
+		nf_tables_set_elem_destroy(ctx, set, &rbe->priv);
+	}
+
 	while ((node = priv->root.rb_node) != NULL) {
 		rb_erase(node, &priv->root);
 		rbe = rb_entry(node, struct nft_rbtree_elem, node);
@@ -1047,8 +1065,10 @@ static void nft_rbtree_commit(struct nft_set *set)
 	struct rb_node *node;
 
 	/* No changes, skip, eg. elements updates only. */
-	if (!priv->array_next)
+	if (!priv->array_next) {
+		WARN_ON_ONCE(!list_empty(&priv->tx_gc));
 		return;
+	}
 
 	/* GC can be performed if the binary search blob is going
 	 * to be rebuilt.  It has to be done in two phases: first
@@ -1116,13 +1136,35 @@ static void nft_rbtree_commit(struct nft_set *set)
 	/* New blob is public, queue collected entries for freeing.
 	 * call_rcu ensures elements stay around until readers are done.
 	 */
+	list_splice_tail_init(&priv->tx_gc, &priv->expired);
 	nft_rbtree_gc_queue(set);
 }
 
 static void nft_rbtree_abort(const struct nft_set *set)
 {
 	struct nft_rbtree *priv = nft_set_priv(set);
+	struct nft_rbtree_elem *rbe, *tmp;
 	struct nft_array *array_next;
+	struct net *net;
+
+	/* Restore elements that inline GC moved to the tx_gc list during
+	 * insert: their data was deactivated (use counts decremented) but
+	 * the transaction was aborted, so re-activate and re-insert to
+	 * undo GC side effects and restore transactional rollback semantics.
+	 */
+	if (!list_empty(&priv->tx_gc)) {
+		net = read_pnet(&set->net);
+
+		write_lock_bh(&priv->lock);
+		list_for_each_entry_safe(rbe, tmp, &priv->tx_gc, list) {
+			list_del_init(&rbe->list);
+			nft_setelem_data_activate(net, set, &rbe->priv);
+			nft_rbtree_node_insert(set, priv, rbe);
+		}
+		write_unlock_bh(&priv->lock);
+	}
+
+	priv->start_rbe_cookie = 0;
 
 	if (!priv->array_next)
 		return;
-- 
2.43.0

^ permalink raw reply related

* Re: [PATCH net v2] net: ethernet: mtk_eth_soc: initialize PPE per-tag-layer MTU registers
From: patchwork-bot+netdevbpf @ 2026-04-12 22:30 UTC (permalink / raw)
  To: Daniel Golle
  Cc: nbd, lorenzo, andrew+netdev, davem, edumazet, kuba, pabeni,
	matthias.bgg, angelogioacchino.delregno, pablo, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <ec995ab8ce8be423267a1cc093147a74d2eb9d82.1775789829.git.daniel@makrotopia.org>

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 10 Apr 2026 03:57:52 +0100 you wrote:
> The PPE enforces output frame size limits via per-tag-layer VLAN_MTU
> registers that the driver never initializes. The hardware defaults do
> not account for PPPoE overhead, causing the PPE to punt encapsulated
> frames back to the CPU instead of forwarding them.
> 
> Initialize the registers at PPE start and on MTU changes using the
> maximum GMAC MTU. This is a conservative approximation -- the actual
> per-PPE requirement depends on egress path, but using the global
> maximum ensures the limits are never too small.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: ethernet: mtk_eth_soc: initialize PPE per-tag-layer MTU registers
    https://git.kernel.org/netdev/net/c/2dddb34dd0d0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [PATCH net-next v2 1/2] pppox: remove sk_pppox() helper
From: patchwork-bot+netdevbpf @ 2026-04-12 22:30 UTC (permalink / raw)
  To: Qingfang Deng
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, kees, gnault,
	ericwouds, dawid.osuchowski, netdev, linux-kernel, linux-ppp
In-Reply-To: <20260410054954.114031-1-qingfang.deng@linux.dev>

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 10 Apr 2026 13:49:49 +0800 you wrote:
> The sk member can be directly accessed from struct pppox_sock without
> relying on type casting. Remove the sk_pppox() helper and update all
> call sites to use po->sk directly.
> 
> Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
> ---
> Changes in v2: none
>  Link to v1: https://lore.kernel.org/r/20260408015138.280687-1-qingfang.deng@linux.dev
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/2] pppox: remove sk_pppox() helper
    https://git.kernel.org/netdev/net-next/c/105369d627b9
  - [net-next,v2,2/2] pppox: convert pppox_sk() to use container_of()
    https://git.kernel.org/netdev/net-next/c/6bc78039a77a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [PATCH net v3] netfilter: nft_set_rbtree: fix use count leak on transaction abort
From: Florian Westphal @ 2026-04-12 22:31 UTC (permalink / raw)
  To: Marko Jevtic
  Cc: pablo, netfilter-devel, phil, coreteam, davem, edumazet, kuba,
	pabeni, horms, netdev, linux-kernel
In-Reply-To: <20260412222801.34965-1-marko.jevtic@codereflect.io>

Marko Jevtic <marko.jevtic@codereflect.io> wrote:
> nft_rbtree_abort() does not handle elements moved to the expired list
> by inline GC during __nft_rbtree_insert(). When inline GC encounters
> expired elements during overlap detection, it calls
> nft_rbtree_gc_elem_move() which deactivates element data (decrementing
> chain/object use counts), removes the element from the rbtree, and
> queues it for deferred freeing. On commit, these elements are freed
> via nft_rbtree_gc_queue(). On abort, however, the expired list is
> ignored entirely.
> 
> This leaves use counts permanently decremented after abort.

I have not seen a reason/answer why this needs to be rolled back.
GC is an implementation detail, its not part of the transaction.

It could also be done from work queue, for example, not just from insert
or commit.

I see no reason to change the existing approach.

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] IPA v5.2 support
From: patchwork-bot+netdevbpf @ 2026-04-12 22:40 UTC (permalink / raw)
  To: Luca Weiss
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, elder, ~postmarketos/upstreaming, phone-devel,
	linux-arm-msm, netdev, devicetree, linux-kernel,
	krzysztof.kozlowski, horms
In-Reply-To: <20260410-ipa-v5-2-v2-0-778422a05060@fairphone.com>

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 10 Apr 2026 09:40:06 +0200 you wrote:
> Add support for IPA v5.2 which can be found in the Milos SoC.
> 
> Note: This series has been split up into two, one for net(-next), one
> for the qcom dts bits.
> 
> Changes in v2:
> - Split the series, drop applied IPA fixes, mark as net-next
> - Pick up tags
> - Link to v1: https://patch.msgid.link/20260403-milos-ipa-v1-0-01e9e4e03d3e@fairphone.com
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/2] dt-bindings: net: qcom,ipa: add Milos compatible
    https://git.kernel.org/netdev/net-next/c/d471d70cc964
  - [net-next,v2,2/2] net: ipa: add IPA v5.2 configuration data
    https://git.kernel.org/netdev/net-next/c/4bf38bac1b2e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [PATCH v3 0/2] net: fix skb_ext BUILD_BUG_ON failures with GCOV
From: patchwork-bot+netdevbpf @ 2026-04-12 22:40 UTC (permalink / raw)
  To: Konstantin Khorenko
  Cc: davem, edumazet, kuba, pabeni, horms, linux, arnd, oberpar,
	zaslonko, netdev, linux-kernel, ptikhomirov, vasileios.almpanis
In-Reply-To: <20260410162150.3105738-1-khorenko@virtuozzo.com>

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 10 Apr 2026 19:21:48 +0300 you wrote:
> This mini-series fixes build failures in net/core/skbuff.c when the
> kernel is built with CONFIG_GCOV_PROFILE_ALL=y.
> 
> This is part of a larger effort to add -fprofile-update=atomic to
> global CFLAGS_GCOV (posted earlier as a combined series):
>   https://lore.kernel.org/lkml/20260401142020.1434243-1-khorenko@virtuozzo.com/T/#t
> 
> [...]

Here is the summary with links:
  - [v3,1/2] net: fix skb_ext_total_length() BUILD_BUG_ON with CONFIG_GCOV_PROFILE_ALL
    https://git.kernel.org/netdev/net-next/c/c0b4382c86e3
  - [v3,2/2] net: add noinline __init __no_profile to skb_extensions_init() for GCOV compatibility
    https://git.kernel.org/netdev/net-next/c/29b1ee8788c5

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [PATCH bpf v4 0/2] bpf: fix short IPv4/IPv6 handling in test_run_skb
From: patchwork-bot+netdevbpf @ 2026-04-12 22:50 UTC (permalink / raw)
  To: sun jian
  Cc: ast, daniel, andrii, davem, edumazet, kuba, pabeni, shuah,
	martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
	sdf, haoluo, jolsa, horms, syzbot+619b9ef527f510a57cfc, bpf,
	netdev, linux-kselftest, linux-kernel
In-Reply-To: <20260408034623.180320-1-sun.jian.kdev@gmail.com>

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Wed,  8 Apr 2026 11:46:21 +0800 you wrote:
> bpf_prog_test_run_skb() may access IPv4/IPv6 network headers based on
> skb->protocol even when the provided test input only contains an
> Ethernet header.
> 
> Fix it by rejecting such short IPv4/IPv6 inputs before accessing the
> L3 headers, and add a selftest that exercises the reported
> bpf_skb_adjust_room() path on ETH_HLEN-sized IPv4/IPv6 EtherType
> inputs.
> 
> [...]

Here is the summary with links:
  - [bpf,v4,1/2] bpf: reject short IPv4/IPv6 inputs in bpf_prog_test_run_skb
    https://git.kernel.org/bpf/bpf-next/c/12bec2bd4b76
  - [bpf,v4,2/2] selftests/bpf: cover short IPv4/IPv6 inputs with adjust_room
    https://git.kernel.org/bpf/bpf-next/c/f1cc94665df9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [PATCH net-next v2 1/2] keys, dns: drop unused upayload->data NUL terminator
From: Thorsten Blum @ 2026-04-12 23:04 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	Tim Bird, netdev, linux-kernel
In-Reply-To: <20260412141004.22c6686c@kernel.org>

On Sun, Apr 12, 2026 at 02:10:04PM -0700, Jakub Kicinski wrote:
> On Fri, 10 Apr 2026 00:57:02 +0200 Thorsten Blum wrote:
> > In dns_resolver_preparse(), do not NUL-terminate ->data and allocate one
> > byte less. The NUL terminator is never used and only ->datalen bytes are
> > accessed.
> 
> I can't see where this is used at all.
> Please write better commit messages, there's no way this 1 byte
> is worth the amount of time I wasted trying to review this :/

The point of patch 1/2 is not the removed NUL terminator itself, but to
prepare for patch 2/2, which adds __counted_by() and requires ->datalen
to match the number of elements in ->data.

Currently, that is not the case because ->data includes an extra NUL
despite never being used as a C string. Removing the unused terminator
makes the length match the allocation size and allows adding the
__counted_by() annotation.

I can fold this into the __counted_by() patch if you prefer.

^ permalink raw reply

* [PATCH net-next v3] r8169: Use napi_schedule_irqoff()
From: Matt Vollrath @ 2026-04-12 23:29 UTC (permalink / raw)
  To: netdev
  Cc: Matt Vollrath, edumazet, pabeni, hkallweit1, kuba, andrew+netdev,
	nic_swsd

napi_schedule() masks hard interrupts while doing its work, which is
redundant when called from an interrupt handler where hard interrupts
are already masked. Use napi_schedule_irqoff() instead to bypass this
redundant masking. This is an optimization.

This is a partial reversion of a previous fix:
Commit 2734a24e6e5d ("r8169: fix issue with forced threading in combination with shared interrupts")
was applied in 2020 to work around an issue with forced threading.
IRQ handlers were run without interrupts masked, and RX interrupts could
be missed in the race, causing delays.

This was fixed in 2021 by masking interrupts in forced thread context:
Commit 81e2073c175b ("genirq: Disable interrupts for force threaded handlers")

Compatibility with PREEMPT_RT also came in 2021:
Commit 8380c81d5c4f ("net: Treat __napi_schedule_irqoff() as __napi_schedule() on PREEMPT_RT")

Tested on a Lenovo RTL8168h/8111h.

Signed-off-by: Matt Vollrath <tactii@gmail.com>
Reviewed-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 791277e750ba..4c0ad0de3410 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4873,7 +4873,7 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 		phy_mac_interrupt(tp->phydev);
 
 	rtl_irq_disable(tp);
-	napi_schedule(&tp->napi);
+	napi_schedule_irqoff(&tp->napi);
 out:
 	rtl_ack_events(tp, status);
 
-- 
2.43.0

Changes:
v3:
* Describe the history of this schedule call
v2:
* CC the maintainers, make the CI board green


^ permalink raw reply related

* Re: [PATCH net-next v2 1/2] keys, dns: drop unused upayload->data NUL terminator
From: Jakub Kicinski @ 2026-04-13  0:05 UTC (permalink / raw)
  To: Thorsten Blum
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	Tim Bird, netdev, linux-kernel
In-Reply-To: <adwlFgKPdW4zDVb_@linux.dev>

On Mon, 13 Apr 2026 01:04:54 +0200 Thorsten Blum wrote:
> On Sun, Apr 12, 2026 at 02:10:04PM -0700, Jakub Kicinski wrote:
> > On Fri, 10 Apr 2026 00:57:02 +0200 Thorsten Blum wrote:  
> > > In dns_resolver_preparse(), do not NUL-terminate ->data and allocate one
> > > byte less. The NUL terminator is never used and only ->datalen bytes are
> > > accessed.  
> > 
> > I can't see where this is used at all.
> > Please write better commit messages, there's no way this 1 byte
> > is worth the amount of time I wasted trying to review this :/  
> 
> The point of patch 1/2 is not the removed NUL terminator itself, but to
> prepare for patch 2/2, which adds __counted_by() and requires ->datalen
> to match the number of elements in ->data.
> 
> Currently, that is not the case because ->data includes an extra NUL
> despite never being used as a C string. Removing the unused terminator
> makes the length match the allocation size and allows adding the
> __counted_by() annotation.
> 
> I can fold this into the __counted_by() patch if you prefer.

I understand that part, but I don't get where the data from which 
the terminating character is removed, is used. Only other access
I saw was freeing it, the rest of the callback seem to looking
at the error, not the data..

^ permalink raw reply

* Re: [PATCH v2] nfc: hci: fix OOB heap read on short HCP frames
From: Ashutosh Desai @ 2026-04-13  0:06 UTC (permalink / raw)
  To: kuba; +Cc: netdev, edumazet, davem, pabeni, horms, linux-kernel
In-Reply-To: <20260412134218.34cbe88d@kernel.org>

On Sun, 12 Apr 2026 13:42:18 -0700 Jakub Kicinski wrote:
> As Eric mentioned elsewhere - he did not suggest any of this,
> merely reviewed your submission.

Agree, that tag was incorrect on my part. Will remove it in the
next version.

> How did a broken packet get enqueued in the first place?

You are right to point that out. nfc_hci_recv_from_llc() already
gates the queue with pskb_may_pull(), so a short skb cannot reach
nfc_hci_msg_rx_work() to begin with. The same holds for the nci
path. Those two checks are redundant and will be dropped in v3.

^ permalink raw reply

* Re: [PATCH net-next v3] r8169: Use napi_schedule_irqoff()
From: Jakub Kicinski @ 2026-04-13  0:06 UTC (permalink / raw)
  To: Matt Vollrath
  Cc: netdev, edumazet, pabeni, hkallweit1, andrew+netdev, nic_swsd
In-Reply-To: <20260412232914.31463-1-tactii@gmail.com>

On Sun, 12 Apr 2026 19:29:14 -0400 Matt Vollrath wrote:
> napi_schedule() masks hard interrupts while doing its work, which is
> redundant when called from an interrupt handler where hard interrupts
> are already masked. Use napi_schedule_irqoff() instead to bypass this
> redundant masking. This is an optimization.

Linus tagged final v7.0, net-next is closed. See for more information:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html

^ permalink raw reply

* Re: [PATCH net-next v9 00/10] net: phy_port: SFP modules representation and phy_port listing
From: Russell King (Oracle) @ 2026-04-13  0:29 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Maxime Chevallier, Paolo Abeni, davem, Andrew Lunn, Eric Dumazet,
	Heiner Kallweit, netdev, linux-kernel, thomas.petazzoni,
	Christophe Leroy, Herve Codina, Florian Fainelli, Vladimir Oltean,
	Köry Maincent, Marek Behún, Oleksij Rempel,
	Nicolò Veronese, Simon Horman, mwojtas, Romain Gantois,
	Daniel Golle, Dimitri Fedrau
In-Reply-To: <20260412142732.5dec7ebe@kernel.org>

On Sun, Apr 12, 2026 at 02:27:32PM -0700, Jakub Kicinski wrote:
> On Thu, 9 Apr 2026 10:40:13 +0200 Maxime Chevallier wrote:
> > Let's see if the PHY crew have things to say on the overall approach :)
> 
> Not a word from them. I suspect we need call a meeting or just apply
> this after the merge window..

Sorry, no opportunity has presented itself yet to review this, and
won't do for a few more days due to appointments.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* Re: [PATCH net-next v2 1/2] keys, dns: drop unused upayload->data NUL terminator
From: Thorsten Blum @ 2026-04-13  0:31 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	Tim Bird, netdev, linux-kernel
In-Reply-To: <20260412170508.1f33a371@kernel.org>

On Sun, Apr 12, 2026 at 05:05:08PM -0700, Jakub Kicinski wrote:
> On Mon, 13 Apr 2026 01:04:54 +0200 Thorsten Blum wrote:
> > On Sun, Apr 12, 2026 at 02:10:04PM -0700, Jakub Kicinski wrote:
> > > On Fri, 10 Apr 2026 00:57:02 +0200 Thorsten Blum wrote:  
> > > > In dns_resolver_preparse(), do not NUL-terminate ->data and allocate one
> > > > byte less. The NUL terminator is never used and only ->datalen bytes are
> > > > accessed.  
> > > 
> > > I can't see where this is used at all.
> > > Please write better commit messages, there's no way this 1 byte
> > > is worth the amount of time I wasted trying to review this :/  
> > 
> > The point of patch 1/2 is not the removed NUL terminator itself, but to
> > prepare for patch 2/2, which adds __counted_by() and requires ->datalen
> > to match the number of elements in ->data.
> > 
> > Currently, that is not the case because ->data includes an extra NUL
> > despite never being used as a C string. Removing the unused terminator
> > makes the length match the allocation size and allows adding the
> > __counted_by() annotation.
> > 
> > I can fold this into the __counted_by() patch if you prefer.
> 
> I understand that part, but I don't get where the data from which 
> the terminating character is removed, is used. Only other access
> I saw was freeing it, the rest of the callback seem to looking
> at the error, not the data..

->data and ->datalen are used in multiple places.

For example, in dns_query() in net/dns_resolver/dns_query.c:

	upayload = user_key_payload_locked(rkey);
	len = upayload->datalen;

	if (_result) {
		ret = -ENOMEM;
		*_result = kmemdup_nul(upayload->data, len, GFP_KERNEL);
		if (!*_result)
			goto put;
	}

In cifs_set_cifscreds() in fs/smb/client/connect.c:

	/* find first : in payload */
	payload = upayload->data;
	delim = strnchr(payload, upayload->datalen, ':');

^ permalink raw reply

* Re: [PATCH net-next] net: stmmac: enable RPS and RBU interrupts
From: Sam Edwards @ 2026-04-13  1:42 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Maxime Chevallier, Andrew Lunn, Alexandre Torgue, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, linux-arm-kernel,
	linux-stm32, netdev, Paolo Abeni
In-Reply-To: <aduq7Lvkfrz971Rb@shell.armlinux.org.uk>

On Sun, Apr 12, 2026 at 7:23 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> As the dwmac 5.0 core receive path seems to lock up after the first
> RBU, I never see more than one of those at a time.
>
> Right now, I consider this pretty much unsolvable - I've spent quite
> some time looking at it and trying various approaches, nothing seems
> to fix it. However, adding dma_rmb() in the descriptor cleanup/refill
> paths does seem to improve the situation a little with the 480Mbps
> case, because I think it means that we're reading the descriptors in
> a more timely manner after the hardware has updated them.

Hey Russell,

I'd like to repro this but I currently can't boot net-next. My issue
is the same as [1], and the patch to fix it [2] isn't yet committed
anywhere apparently.

This prevents my Jetson Xavier NX from starting at all (and after
enough attempts, corrupts eMMC); I'm surprised you're not suffering
the same effects. But because this bug lives in the IOMMU subsystem
(and it has somewhat inconsistent effects), perhaps this is just a
different way it manifests? Could you confirm whether your dwmac hang
happens with IOMMU disabled, and/or with [1] reverted or [2] applied?

I'm using a defconfig build and a fairly minimal cmdline (just
console=, root=, and rootwait).

Cheers,
Sam

[1] https://lore.kernel.org/all/8800a38b-8515-4bbe-af15-0dae81274bf7@nvidia.com/
[2] https://lore.kernel.org/all/0-v1-664d3acaabb9+78b-iommu_gather_always_jgg@nvidia.com/

^ permalink raw reply

* Re: [net-next v38] mctp pcc: Implement MCTP over PCC Transport
From: Jeremy Kerr @ 2026-04-13  2:15 UTC (permalink / raw)
  To: Jakub Kicinski, admiyo
  Cc: matt, andrew+netdev, davem, edumazet, pabeni, netdev,
	linux-kernel, sudeep.holla, Jonathan.Cameron, lihuisong
In-Reply-To: <20260410032441.1844450-1-kuba@kernel.org>

Hi Adam,

> > +       memcpy_toio(outbox->chan->shmem,  skb->data, skb->len);
> 
> Is it possible to read out of bounds here if the skb is fragmented?
> 
> The skb->data pointer only points to the linear portion of the packet, while
> skb->len represents the total packet length including page fragments.
> skb_cow_head() does not linearize the packet, so a call to skb_linearize()
> might be needed before copying.

I assume that we should only be seeing linear skbs here, as the driver
does not advertise NETIF_F_FRAGLIST or NETIF_F_SG.

(that said, this could support fragmented skbs quite easily, but that
would be more suitable for a follow-up change)

Cheers,


Jeremy

^ permalink raw reply

* [PATCH net,v2 1/1] net: stmmac: Update default_an_inband before passing value to phylink_config
From: KhaiWenTan @ 2026-04-13  2:03 UTC (permalink / raw)
  To: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32,
	alexandre.torgue, rmk+kernel, maxime.chevallier, ovidiu.panait.rb,
	vladimir.oltean
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
	yoong.siang.song, hong.aun.looi, khai.wen.tan, KhaiWenTan

get_interfaces() will update both the plat->phy_interfaces and
mdio_bus_data->default_an_inband based on reading a SERDES register. As
get_interfaces() will be called after default_an_inband had already been
read, dwmac-intel regressed as a result with incorrect default_an_inband
value in phylink_config.

Therefore, we moved the priv->plat->get_interfaces() to be executed first
before assigning mdio_bus_data->default_an_inband to
config->default_an_inband to ensure default_an_inband is in correct value.

Fixes: d3836052fe09 ("net: stmmac: intel: convert speed_mode_2500() to get_interfaces()")
Signed-off-by: KhaiWenTan <khai.wen.tan@linux.intel.com>
---
v2:
  - update commit message for better understanding (Russell King)
  - corrected the blamed commit (Russell King)
v1: https://patchwork.kernel.org/project/netdevbpf/patch/20260410020735.327590-1-khai.wen.tan@linux.intel.com/
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 13d3cac056be..c92054648a7e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1345,10 +1345,6 @@ static int stmmac_phylink_setup(struct stmmac_priv *priv)
 	priv->tx_lpi_clk_stop = priv->plat->flags &
 				STMMAC_FLAG_EN_TX_LPI_CLOCKGATING;
 
-	mdio_bus_data = priv->plat->mdio_bus_data;
-	if (mdio_bus_data)
-		config->default_an_inband = mdio_bus_data->default_an_inband;
-
 	/* Get the PHY interface modes (at the PHY end of the link) that
 	 * are supported by the platform.
 	 */
@@ -1356,6 +1352,10 @@ static int stmmac_phylink_setup(struct stmmac_priv *priv)
 		priv->plat->get_interfaces(priv, priv->plat->bsp_priv,
 					   config->supported_interfaces);
 
+	mdio_bus_data = priv->plat->mdio_bus_data;
+	if (mdio_bus_data)
+		config->default_an_inband = mdio_bus_data->default_an_inband;
+
 	/* Set the platform/firmware specified interface mode if the
 	 * supported interfaces have not already been provided using
 	 * phy_interface as a last resort.
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3] nfc: hci: fix out-of-bounds read in HCP header parsing
From: Ashutosh Desai @ 2026-04-13  2:43 UTC (permalink / raw)
  To: netdev; +Cc: kuba, edumazet, davem, pabeni, horms, linux-kernel,
	Ashutosh Desai
In-Reply-To: <20260413000627.3273477-1-ashutoshdesai993@gmail.com>

nfc_hci_recv_from_llc() and nci_hci_data_received_cb() cast skb->data
to struct hcp_packet and read the message header byte without checking
that enough data is present in the linear sk_buff area. A malicious NFC
peer can send a 1-byte HCP frame that passes through the SHDLC layer
and reaches these functions, causing an out-of-bounds heap read.

Fix this by adding pskb_may_pull() before each cast to ensure the full
2-byte HCP header is pulled into the linear area before it is accessed.

Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
v3: drop redundant pskb_may_pull checks from msg_rx_work functions,
    remove incorrect Suggested-by tag
v2: switch skb->len check to pskb_may_pull

 net/nfc/hci/core.c | 5 +++++
 net/nfc/nci/hci.c  | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
index 0d33c81a1..cd9cf6c94 100644
--- a/net/nfc/hci/core.c
+++ b/net/nfc/hci/core.c
@@ -904,6 +904,11 @@ static void nfc_hci_recv_from_llc(struct nfc_hci_dev *hdev, struct sk_buff *skb)
 	 * unblock waiting cmd context. Otherwise, enqueue to dispatch
 	 * in separate context where handler can also execute command.
 	 */
+	if (!pskb_may_pull(hcp_skb, NFC_HCI_HCP_HEADER_LEN)) {
+		kfree_skb(hcp_skb);
+		return;
+	}
+
 	packet = (struct hcp_packet *)hcp_skb->data;
 	type = HCP_MSG_GET_TYPE(packet->message.header);
 	if (type == NFC_HCI_HCP_RESPONSE) {
diff --git a/net/nfc/nci/hci.c b/net/nfc/nci/hci.c
index 40ae8e5a7..6e633da25 100644
--- a/net/nfc/nci/hci.c
+++ b/net/nfc/nci/hci.c
@@ -482,6 +482,11 @@ void nci_hci_data_received_cb(void *context,
 	 * unblock waiting cmd context. Otherwise, enqueue to dispatch
 	 * in separate context where handler can also execute command.
 	 */
+	if (!pskb_may_pull(hcp_skb, NCI_HCI_HCP_HEADER_LEN)) {
+		kfree_skb(hcp_skb);
+		return;
+	}
+
 	packet = (struct nci_hcp_packet *)hcp_skb->data;
 	type = NCI_HCP_MSG_GET_TYPE(packet->message.header);
 	if (type == NCI_HCI_HCP_RESPONSE) {
-- 
2.34.1


^ permalink raw reply related

* RE: [PATCH net 1/1] tipc: validate Gap ACK blocks in STATE message
From: Tung Quang Nguyen @ 2026-04-13  3:06 UTC (permalink / raw)
  To: Ren Wei
  Cc: jmaloy@redhat.com, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, horms@kernel.org,
	yifanwucs@gmail.com, tomapufckgml@gmail.com, yuantan098@gmail.com,
	bird@lzu.edu.cn, enjou1224z@gmail.com, caoruide123@gmail.com,
	netdev@vger.kernel.org
In-Reply-To: <1316452e465e9a96fce44ec15130a14f3872149f.1775809727.git.caoruide123@gmail.com>

>Subject: [PATCH net 1/1] tipc: validate Gap ACK blocks in STATE message
>
>From: Ruide Cao <caoruide123@gmail.com>
>
>tipc_get_gap_ack_blks() reads len, ugack_cnt and bgack_cnt directly from
>msg_data(hdr) before verifying that a STATE message actually contains the
>fixed Gap ACK block header in its logical data area.
>
>A peer that negotiates TIPC_GAP_ACK_BLOCK can send a short STATE message
>with a declared TIPC payload shorter than struct tipc_gap_ack_blks and still
>append a few physical bytes after the header. The helper then trusts those
>bytes as Gap ACK metadata, and the forged bgack_cnt/len values can drive the
>broadcast receive path into kmemdup() beyond the skb boundary.
Can you explain how that peer can alter the STATE message ? If it can, what concrete values are used  and on what fields of the STATE messages ?
>
>Fix this by rejecting Gap ACK parsing unless the logical STATE payload is large
>enough to cover the fixed header, and by rejecting declared Gap ACK lengths
>that are smaller than the fixed header or larger than the logical payload.
>Return 0 for invalid lengths so malformed Gap ACK data is not treated as a
>valid payload offset, and drop unicast STATE messages that advertise Gap ACK
>support but still yield an invalid Gap ACK length. This keeps malformed Gap
>ACK data ignored without misaligning monitor payload parsing.
>
>Fixes: d7626b5acff9 ("tipc: introduce Gap ACK blocks for broadcast link")
>Cc: stable@kernel.org
>Reported-by: Yifan Wu <yifanwucs@gmail.com>
>Reported-by: Juefei Pu <tomapufckgml@gmail.com>
>Co-developed-by: Yuan Tan <yuantan098@gmail.com>
>Signed-off-by: Yuan Tan <yuantan098@gmail.com>
>Suggested-by: Xin Liu <bird@lzu.edu.cn>
>Tested-by: Ren Wei <enjou1224z@gmail.com>
>Signed-off-by: Ruide Cao <caoruide123@gmail.com>
>Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
>---
> net/tipc/link.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
>diff --git a/net/tipc/link.c b/net/tipc/link.c index 49dfc098d89b..44678d98939a
>100644
>--- a/net/tipc/link.c
>+++ b/net/tipc/link.c
>@@ -1415,12 +1415,22 @@ u16 tipc_get_gap_ack_blks(struct
>tipc_gap_ack_blks **ga, struct tipc_link *l,
> 			  struct tipc_msg *hdr, bool uc)
> {
> 	struct tipc_gap_ack_blks *p;
>-	u16 sz = 0;
>+	u16 sz = 0, dlen = msg_data_sz(hdr);
>
> 	/* Does peer support the Gap ACK blocks feature? */
> 	if (l->peer_caps & TIPC_GAP_ACK_BLOCK) {
>+		u16 min_sz = struct_size(p, gacks, 0);
>+
>+		if (dlen < min_sz)
>+			goto ignore;
This checking is redundant because with existing sanity checking, the invalid gap ACK blocks will not be used to release acked messages in transmit queue.
>+
> 		p = (struct tipc_gap_ack_blks *)msg_data(hdr);
> 		sz = ntohs(p->len);
>+		if (sz < min_sz || sz > dlen) {
>+			sz = 0;
>+			goto ignore;
>+		}
This checking is redundant. Existing sanity checking is good enough.
>+
> 		/* Sanity check */
> 		if (sz == struct_size(p, gacks, size_add(p->ugack_cnt, p-
>>bgack_cnt))) {
> 			/* Good, check if the desired type exists */ @@ -
>1434,6 +1444,8 @@ u16 tipc_get_gap_ack_blks(struct tipc_gap_ack_blks **ga,
>struct tipc_link *l,
> 			}
> 		}
> 	}
>+
>+ignore:
> 	/* Other cases: ignore! */
> 	p = NULL;
>
>@@ -2270,7 +2282,7 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct
>sk_buff *skb,
> 	case STATE_MSG:
> 		/* Validate Gap ACK blocks, drop if invalid */
> 		glen = tipc_get_gap_ack_blks(&ga, l, hdr, true);
>-		if (glen > dlen)
>+		if (glen > dlen || ((l->peer_caps & TIPC_GAP_ACK_BLOCK) &&
>!glen))
This checking is redundant. Existing sanity checking is good enough.
> 			break;
>
> 		l->rcv_nxt_state = msg_seqno(hdr) + 1;
>--
>2.34.1
>


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox