Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 0/6] pull request: ovpn 2026-10-12
@ 2026-10-12 13:37 Antonio Quartulli
  2026-10-12 13:37 ` [PATCH net-next 1/6] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc Antonio Quartulli
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Antonio Quartulli @ 2026-10-12 13:37 UTC (permalink / raw)
  To: netdev
  Cc: Antonio Quartulli, Sabrina Dubroca, Ralf Lici, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, David S. Miller, Eric Dumazet

Hi all!

Here is v4 of the ovpn batch for net-next.

It is all cleanup and selftest work this time: no functional change to
the driver. The selftests get TCP_NODELAY on ovpn-cli's TCP sockets and
proper synchronization against the server-side key setup. Ralf is added
to MAINTAINERS as ovpn reviewer.

Changes since v3:
* dropped "ovpn: send peer object along with PEER_DEL_NTF" (6/7 in v3).
  Sashiko pointed out that the peers multicast group carries no
  capability flag, so any process able to open a generic netlink socket
  can bind to it and would receive the full peer object. Putting that
  object on the group needs the matching policy change to go with it,
  and that deserves its own discussion, so both will be sent as a
  separate series.
* 4/6: ovpn-cli still sets TCP_NODELAY by default, matching what openvpn
  does since 2.7.6, but it can now be turned off with
  OVPN_TCP_NODELAY=0. The new test-tcp-nagle.sh runs the TCP suite that
  way, so peers running 2.7.0 to 2.7.5 do not lose coverage. Commit
  message reworded to say which userspace version this mirrors.
* 5/6: the background subshell that installs the server-side keys now
  writes to a log of its own, dumped when the wait fails or times out.
  It inherited a temporary file that ovpn_cmd_run() had already removed,
  so a failed new_key produced no output and only showed up later as a
  ping failure.

Patches 1 to 3 are unchanged from v3.

Please pull or let me know of any issue!

Thanks a lot,
	Antonio

Antonio Quartulli (2):
  selftests: ovpn: wait for the TCP server key setup before traffic
  MAINTAINERS: ovpn: add Ralf Lici as reviewer

Karl Mehltretter (1):
  ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc

Marco Baffo (2):
  ovpn: remove redundant peer NULL checks in crypto post functions
  selftests: ovpn: enable TCP_NODELAY on TCP sockets

Ralf Lici (1):
  ovpn: remove unused work field from struct ovpn_socket

 MAINTAINERS                                   |  1 +
 drivers/net/ovpn/bind.c                       |  2 +-
 drivers/net/ovpn/io.c                         |  6 +-
 drivers/net/ovpn/socket.h                     |  2 -
 tools/testing/selftests/net/ovpn/Makefile     |  1 +
 tools/testing/selftests/net/ovpn/common.sh    | 81 +++++++++++++++----
 tools/testing/selftests/net/ovpn/ovpn-cli.c   | 39 +++++++++
 .../selftests/net/ovpn/test-close-socket.sh   |  2 +
 .../selftests/net/ovpn/test-tcp-nagle.sh      | 12 +++
 tools/testing/selftests/net/ovpn/test.sh      | 56 ++++++-------
 10 files changed, 153 insertions(+), 49 deletions(-)
 create mode 100755 tools/testing/selftests/net/ovpn/test-tcp-nagle.sh

-- 
2.55.0

The following changes since commit d8674294aefef02266c4d47ad10131f1bffbe534:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2026-10-08 22:21:56 -0700)

are available in the Git repository at:

  https://github.com/OpenVPN/ovpn-net-next.git tags/ovpn-net-next-20261012

for you to fetch changes up to b2ed202907d654337d72825d743a833009010740:

  MAINTAINERS: ovpn: add Ralf Lici as reviewer (2026-10-12 15:16:49 +0200)

----------------------------------------------------------------
Included changes:
* add Ralf Lici as ovpn reviewer
* wait for the server-side key setup before generating TCP traffic in
  selftests
* enable TCP_NODELAY on TCP sockets in selftests
* drop redundant peer NULL checks in the crypto post functions
* remove the unused work field from struct ovpn_socket
* fix the documented return value of ovpn_bind_from_sockaddr()

----------------------------------------------------------------
Antonio Quartulli (2):
      selftests: ovpn: wait for the TCP server key setup before traffic
      MAINTAINERS: ovpn: add Ralf Lici as reviewer

Karl Mehltretter (1):
      ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc

Marco Baffo (2):
      ovpn: remove redundant peer NULL checks in crypto post functions
      selftests: ovpn: enable TCP_NODELAY on TCP sockets

Ralf Lici (1):
      ovpn: remove unused work field from struct ovpn_socket

 MAINTAINERS                                        |  1 +
 drivers/net/ovpn/bind.c                            |  2 +-
 drivers/net/ovpn/io.c                              |  6 +-
 drivers/net/ovpn/socket.h                          |  2 -
 tools/testing/selftests/net/ovpn/Makefile          |  1 +
 tools/testing/selftests/net/ovpn/common.sh         | 81 ++++++++++++++++++----
 tools/testing/selftests/net/ovpn/ovpn-cli.c        | 39 +++++++++++
 .../selftests/net/ovpn/test-close-socket.sh        |  2 +
 tools/testing/selftests/net/ovpn/test-tcp-nagle.sh | 12 ++++
 tools/testing/selftests/net/ovpn/test.sh           | 56 +++++++--------
 10 files changed, 153 insertions(+), 49 deletions(-)
 create mode 100755 tools/testing/selftests/net/ovpn/test-tcp-nagle.sh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net-next 1/6] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc
  2026-10-12 13:37 [PATCH net-next 0/6] pull request: ovpn 2026-10-12 Antonio Quartulli
@ 2026-10-12 13:37 ` Antonio Quartulli
  2026-10-12 13:37 ` [PATCH net-next 2/6] ovpn: remove unused work field from struct ovpn_socket Antonio Quartulli
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Antonio Quartulli @ 2026-10-12 13:37 UTC (permalink / raw)
  To: netdev
  Cc: Karl Mehltretter, Sabrina Dubroca, Ralf Lici, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, David S. Miller, Eric Dumazet,
	Antonio Quartulli

From: Karl Mehltretter <kmehltretter@gmail.com>

ovpn_bind_from_sockaddr() returns an ERR_PTR() on failure, never NULL,
but its kernel-doc says "NULL otherwise". Say ERR_PTR().

The wrong text came in with commit 80747caef33d ("ovpn: introduce the
ovpn_peer object").

Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/bind.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ovpn/bind.c b/drivers/net/ovpn/bind.c
index e42b60cd04a9..62a5ccfd3502 100644
--- a/drivers/net/ovpn/bind.c
+++ b/drivers/net/ovpn/bind.c
@@ -18,7 +18,7 @@
  * ovpn_bind_from_sockaddr - retrieve binding matching sockaddr
  * @ss: the sockaddr to match
  *
- * Return: the bind matching the passed sockaddr if found, NULL otherwise
+ * Return: the new bind for the passed sockaddr, an ERR_PTR() on failure
  */
 struct ovpn_bind *ovpn_bind_from_sockaddr(const struct sockaddr_storage *ss)
 {
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 2/6] ovpn: remove unused work field from struct ovpn_socket
  2026-10-12 13:37 [PATCH net-next 0/6] pull request: ovpn 2026-10-12 Antonio Quartulli
  2026-10-12 13:37 ` [PATCH net-next 1/6] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc Antonio Quartulli
@ 2026-10-12 13:37 ` Antonio Quartulli
  2026-10-12 13:38 ` [PATCH net-next 3/6] ovpn: remove redundant peer NULL checks in crypto post functions Antonio Quartulli
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Antonio Quartulli @ 2026-10-12 13:37 UTC (permalink / raw)
  To: netdev
  Cc: Ralf Lici, Sabrina Dubroca, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn, David S. Miller, Eric Dumazet, Antonio Quartulli

From: Ralf Lici <ralf@mandelbit.com>

work in struct ovpn_socket was introduced but never used.

Remove it.

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/socket.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ovpn/socket.h b/drivers/net/ovpn/socket.h
index 4afcec71040d..5c87392ecd30 100644
--- a/drivers/net/ovpn/socket.h
+++ b/drivers/net/ovpn/socket.h
@@ -24,7 +24,6 @@ struct ovpn_peer;
  * @peer: unique peer transmitting over this socket (TCP only)
  * @sk: the low level sock object
  * @refcount: amount of contexts currently referencing this object
- * @work: member used to schedule release routine (it may block)
  * @tcp_tx_work: work for deferring outgoing packet processing (TCP only)
  */
 struct ovpn_socket {
@@ -38,7 +37,6 @@ struct ovpn_socket {
 
 	struct sock *sk;
 	struct kref refcount;
-	struct work_struct work;
 	struct work_struct tcp_tx_work;
 };
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 3/6] ovpn: remove redundant peer NULL checks in crypto post functions
  2026-10-12 13:37 [PATCH net-next 0/6] pull request: ovpn 2026-10-12 Antonio Quartulli
  2026-10-12 13:37 ` [PATCH net-next 1/6] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc Antonio Quartulli
  2026-10-12 13:37 ` [PATCH net-next 2/6] ovpn: remove unused work field from struct ovpn_socket Antonio Quartulli
@ 2026-10-12 13:38 ` Antonio Quartulli
  2026-10-12 13:38 ` [PATCH net-next 4/6] selftests: ovpn: enable TCP_NODELAY on TCP sockets Antonio Quartulli
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Antonio Quartulli @ 2026-10-12 13:38 UTC (permalink / raw)
  To: netdev
  Cc: Marco Baffo, Sabrina Dubroca, Ralf Lici, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, David S. Miller, Eric Dumazet,
	Antonio Quartulli

From: Marco Baffo <marco@mandelbit.com>

The AEAD helpers set the peer pointer in the skb control buffer before
any error return or crypto request submission. Both crypto paths hold
a peer reference until post-processing finishes.

Remove the redundant NULL checks before ovpn_peer_put() in
ovpn_encrypt_post() and ovpn_decrypt_post().

Signed-off-by: Marco Baffo <marco@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/io.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index 9526f8096da6..f14eb4e6c46a 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -206,8 +206,7 @@ void ovpn_decrypt_post(void *data, int ret)
 drop_nocount:
 	if (likely(ks))
 		ovpn_crypto_key_slot_put(ks);
-	if (likely(peer))
-		ovpn_peer_put(peer);
+	ovpn_peer_put(peer);
 }
 
 /* RX path entry point: decrypt packet and forward it to the device */
@@ -305,8 +304,7 @@ void ovpn_encrypt_post(void *data, int ret)
 	kfree_skb(skb);
 	if (likely(ks))
 		ovpn_crypto_key_slot_put(ks);
-	if (likely(peer))
-		ovpn_peer_put(peer);
+	ovpn_peer_put(peer);
 }
 
 static bool ovpn_encrypt_one(struct ovpn_peer *peer, struct sk_buff *skb)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 4/6] selftests: ovpn: enable TCP_NODELAY on TCP sockets
  2026-10-12 13:37 [PATCH net-next 0/6] pull request: ovpn 2026-10-12 Antonio Quartulli
                   ` (2 preceding siblings ...)
  2026-10-12 13:38 ` [PATCH net-next 3/6] ovpn: remove redundant peer NULL checks in crypto post functions Antonio Quartulli
@ 2026-10-12 13:38 ` Antonio Quartulli
  2026-10-13 14:39   ` netdev-bot+sashiko
  2026-10-12 13:38 ` [PATCH net-next 5/6] selftests: ovpn: wait for the TCP server key setup before traffic Antonio Quartulli
  2026-10-12 13:38 ` [PATCH net-next 6/6] MAINTAINERS: ovpn: add Ralf Lici as reviewer Antonio Quartulli
  5 siblings, 1 reply; 10+ messages in thread
From: Antonio Quartulli @ 2026-10-12 13:38 UTC (permalink / raw)
  To: netdev
  Cc: Marco Baffo, Sabrina Dubroca, Ralf Lici, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, David S. Miller, Eric Dumazet,
	Neal Cardwell, Antonio Quartulli

From: Marco Baffo <marco@mandelbit.com>

Since openvpn 2.7.6, userspace unconditionally sets TCP_NODELAY on its
TCP sockets. Do the same in ovpn-cli by default, so that the TCP
selftests match that configuration.

Peers running 2.7.0 to 2.7.5 still leave Nagle enabled, and
mixed-version deployments will keep doing so for a while, so the option
can be turned off with OVPN_TCP_NODELAY=0. test-tcp-nagle.sh uses it to
keep running the TCP suite in that configuration.

The TCP peer ID capture assumes that every TCP segment starts with an
ovpn length prefix followed by a data header. TCP does not preserve
record boundaries, and enabling TCP_NODELAY makes this check unreliable.
Restrict the capture-based peer ID check to UDP.

Suggested-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Marco Baffo <marco@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 tools/testing/selftests/net/ovpn/Makefile     |  1 +
 tools/testing/selftests/net/ovpn/common.sh    | 20 +++----
 tools/testing/selftests/net/ovpn/ovpn-cli.c   | 39 ++++++++++++++
 .../selftests/net/ovpn/test-tcp-nagle.sh      | 12 +++++
 tools/testing/selftests/net/ovpn/test.sh      | 54 +++++++++----------
 5 files changed, 86 insertions(+), 40 deletions(-)
 create mode 100755 tools/testing/selftests/net/ovpn/test-tcp-nagle.sh

diff --git a/tools/testing/selftests/net/ovpn/Makefile b/tools/testing/selftests/net/ovpn/Makefile
index 169f0464ac3a..35e4c9d4fc68 100644
--- a/tools/testing/selftests/net/ovpn/Makefile
+++ b/tools/testing/selftests/net/ovpn/Makefile
@@ -42,6 +42,7 @@ TEST_PROGS := \
 	test-symmetric-id-float.sh \
 	test-symmetric-id-tcp.sh \
 	test-symmetric-id.sh \
+	test-tcp-nagle.sh \
 	test-tcp.sh \
 	test.sh \
 # end of TEST_PROGS
diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
index 5e9c81e885e6..2f7851b82343 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -191,20 +191,14 @@ ovpn_setup_ns() {
 
 ovpn_build_capture_filter() {
 	# match the first four bytes of the openvpn data payload
-	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
-		# headers in the outer packet.
-		if [[ "${2}" == *:* ]]; then
-			printf "ip6 and ip6[6] = 17 and ip6[48:4] = %s" "${1}"
-		else
-			printf "ip and udp[8:4] = %s" "${1}"
-		fi
+	# 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
+	# headers in the outer packet.
+	if [[ "${2}" == *:* ]]; then
+		printf "ip6 and ip6[6] = 17 and ip6[48:4] = %s" "${1}"
 	else
-		# openvpn over TCP prepends a 2-byte packet length ahead of the
-		# DATA_V2 opcode, so skip it before matching the payload header
-		printf "ip and tcp[(((tcp[12] & 0xf0) >> 2) + 2):4] = %s" "${1}"
+		printf "ip and udp[8:4] = %s" "${1}"
 	fi
 }
 
diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c
index 3b612a8a18fe..00b80bc5157f 100644
--- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
+++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
@@ -8,6 +8,7 @@
 
 #include <stdint.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <inttypes.h>
 #include <stdbool.h>
 #include <string.h>
@@ -16,6 +17,7 @@
 #include <arpa/inet.h>
 #include <net/if.h>
 #include <netinet/in.h>
+#include <netinet/tcp.h>
 #include <time.h>
 
 #include <linux/ovpn.h>
@@ -470,6 +472,33 @@ static int ovpn_parse_key_direction(const char *dir, struct ovpn_ctx *ctx)
 	return 0;
 }
 
+/* openvpn 2.7.6 and later always set TCP_NODELAY, so mirror that by
+ * default. Peers running 2.7.0 to 2.7.5 still leave Nagle enabled, and
+ * mixed-version deployments will keep doing so for a while, so let the
+ * selftests exercise that configuration too by setting OVPN_TCP_NODELAY=0.
+ */
+static bool ovpn_tcp_nodelay_wanted(void)
+{
+	const char *env = getenv("OVPN_TCP_NODELAY");
+
+	return !env || strcmp(env, "0");
+}
+
+static int ovpn_tcp_nodelay(int socket)
+{
+	int opt = 1;
+	int ret;
+
+	if (!ovpn_tcp_nodelay_wanted())
+		return 0;
+
+	ret = setsockopt(socket, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(opt));
+	if (ret < 0)
+		perror("setsockopt for TCP_NODELAY");
+
+	return ret;
+}
+
 static int ovpn_socket(struct ovpn_ctx *ctx, sa_family_t family, int proto)
 {
 	struct sockaddr_storage local_sock = { 0 };
@@ -606,6 +635,12 @@ static int ovpn_accept(struct ovpn_ctx *ctx)
 		goto err;
 	}
 
+	if (ovpn_tcp_nodelay(ret) < 0) {
+		close(ret);
+		ret = -1;
+		goto err;
+	}
+
 	return ret;
 err:
 	close(ctx->socket);
@@ -623,6 +658,10 @@ static int ovpn_connect(struct ovpn_ctx *ovpn)
 		return -1;
 	}
 
+	ret = ovpn_tcp_nodelay(s);
+	if (ret < 0)
+		goto err;
+
 	switch (ovpn->remote.in4.sin_family) {
 	case AF_INET:
 		socklen = sizeof(struct sockaddr_in);
diff --git a/tools/testing/selftests/net/ovpn/test-tcp-nagle.sh b/tools/testing/selftests/net/ovpn/test-tcp-nagle.sh
new file mode 100755
index 000000000000..2d94d8e14d3d
--- /dev/null
+++ b/tools/testing/selftests/net/ovpn/test-tcp-nagle.sh
@@ -0,0 +1,12 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2025 OpenVPN, Inc.
+#
+#  Author:	Antonio Quartulli <antonio@openvpn.net>
+
+# Peers running openvpn 2.7.0 to 2.7.5 leave Nagle enabled on their TCP
+# sockets. Keep exercising that configuration so it does not silently break.
+OVPN_PROTO="TCP"
+export OVPN_TCP_NODELAY=0
+
+source test.sh
diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
index 392109d5e14e..e2e6ffdd29bb 100755
--- a/tools/testing/selftests/net/ovpn/test.sh
+++ b/tools/testing/selftests/net/ovpn/test.sh
@@ -137,35 +137,33 @@ ovpn_run_basic_traffic() {
 	local 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
+			# The first part of the data packet header consists of:
+			# - 5 bits for opcode ("9" for DATA_V2)
+			# - 3 bits for key-id ("0" at this point)
+			# - 3 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=$(awk "NR == ${p} {print \$3}" \
 				"${OVPN_UDP_PEERS_FILE}")
+			peer_ns="ovpn_peer${p}"
+
+			timeout ${tcpdump_timeout} ip netns exec "${peer_ns}" \
+				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_ns}" \
+				tcpdump --immediate-mode -p -ni veth${p} -c 1 \
+				"$(ovpn_build_capture_filter "${header2}" "${raddr}")" \
+				>/dev/null 2>&1 &
+			tcpdump_pid2=$!
+
+			sleep 0.3
 		fi
-		peer_ns="ovpn_peer${p}"
-
-		timeout ${tcpdump_timeout} ip netns exec "${peer_ns}" \
-			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_ns}" \
-			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 ovpn_peer0 \
 			ping -qfc 100 -w 3 5.5.5.$((p + 1))
@@ -173,8 +171,10 @@ ovpn_run_basic_traffic() {
 			ip netns exec ovpn_peer0 \
 			ping -qfc 100 -s 3000 -w 3 5.5.5.$((p + 1))
 
-		wait "${tcpdump_pid1}" || return 1
-		wait "${tcpdump_pid2}" || return 1
+		if [ "${OVPN_PROTO}" == "UDP" ]; then
+			wait "${tcpdump_pid1}" || return 1
+			wait "${tcpdump_pid2}" || return 1
+		fi
 	done
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 5/6] selftests: ovpn: wait for the TCP server key setup before traffic
  2026-10-12 13:37 [PATCH net-next 0/6] pull request: ovpn 2026-10-12 Antonio Quartulli
                   ` (3 preceding siblings ...)
  2026-10-12 13:38 ` [PATCH net-next 4/6] selftests: ovpn: enable TCP_NODELAY on TCP sockets Antonio Quartulli
@ 2026-10-12 13:38 ` Antonio Quartulli
  2026-10-13 14:39   ` netdev-bot+sashiko
  2026-10-12 13:38 ` [PATCH net-next 6/6] MAINTAINERS: ovpn: add Ralf Lici as reviewer Antonio Quartulli
  5 siblings, 1 reply; 10+ messages in thread
From: Antonio Quartulli @ 2026-10-12 13:38 UTC (permalink / raw)
  To: netdev
  Cc: Antonio Quartulli, Sabrina Dubroca, Ralf Lici, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, David S. Miller, Eric Dumazet

In TCP mode ovpn_add_peer() starts the server side from a background
subshell that first listens for the incoming connections and then
installs the data key of every peer. The subshell PID is discarded, so
nothing synchronizes the test against the key installation.

The sleep 5 that follows does not cover it: it elapses while "listen" is
still waiting for connections, and the peers only connect in the
subsequent ovpn_add_peer() iterations, so the key loop starts well after
that sleep has expired. Until now the test happened to work because of
the unrelated delays that ran in between.

Record the PID of the background subshell and wait for it once all the
peers have been registered, which is the earliest point at which
"listen" can have returned. A peer that was not given a key yet would
otherwise drop the server-to-client packets and make the first ping
fail.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 tools/testing/selftests/net/ovpn/common.sh    | 61 ++++++++++++++++++-
 .../selftests/net/ovpn/test-close-socket.sh   |  2 +
 tools/testing/selftests/net/ovpn/test.sh      |  2 +
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
index 2f7851b82343..da732c64ffea 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -256,14 +256,19 @@ ovpn_add_peer() {
 		fi
 	else
 		if [ ${1} -eq 0 ]; then
+			OVPN_TCP_KEYS_LOG=$(mktemp)
 			(ip netns exec "${server_ns}" ${OVPN_CLI} listen tun0 \
 				1 ${M_ID} ${OVPN_TCP_PEERS_FILE} && {
+				rc=0
 				for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 					ip netns exec "${server_ns}" \
 						${OVPN_CLI} new_key tun0 ${p} \
-						1 0 ${OVPN_ALG} 0 data64.key
+						1 0 ${OVPN_ALG} 0 data64.key \
+						|| rc=1
 				done
-			}) &
+				exit ${rc}
+			}) >"${OVPN_TCP_KEYS_LOG}" 2>&1 &
+			OVPN_TCP_KEYS_PID=$!
 			sleep 5
 		else
 			peer_ns="ovpn_peer${1}"
@@ -281,6 +286,58 @@ ovpn_add_peer() {
 	fi
 }
 
+# In TCP mode the server accepts the peers and installs their data keys from a
+# background subshell. "listen" returns once a connection has been accepted for
+# every entry of the peers file, so the key installation runs after the last
+# ovpn_add_peer() call. Wait for that subshell before generating any traffic,
+# otherwise the first packets may race the key installation and get dropped for
+# lack of a key.
+#
+# The wait is bounded: both the accept loop and the subsequent receive in
+# ovpn-cli are unbounded, so a peer that never connects (for instance when
+# OVPN_NUM_PEERS is smaller than the number of entries in the peers file) would
+# otherwise hang the whole test instead of failing.
+OVPN_TCP_KEYS_TIMEOUT=${OVPN_TCP_KEYS_TIMEOUT:-30}
+
+ovpn_wait_tcp_keys() {
+	local deadline rc
+
+	[ -n "${OVPN_TCP_KEYS_PID:-}" ] || return 0
+
+	deadline=$((SECONDS + OVPN_TCP_KEYS_TIMEOUT))
+	while kill -0 "${OVPN_TCP_KEYS_PID}" 2>/dev/null; do
+		if [ "${SECONDS}" -ge "${deadline}" ]; then
+			printf '%s\n' \
+				"server-side key setup still running after ${OVPN_TCP_KEYS_TIMEOUT}s"
+			kill "${OVPN_TCP_KEYS_PID}" 2>/dev/null
+			wait "${OVPN_TCP_KEYS_PID}" 2>/dev/null
+			ovpn_dump_tcp_keys_log
+			return 1
+		fi
+		sleep 0.2
+	done
+
+	wait "${OVPN_TCP_KEYS_PID}"
+	rc=$?
+
+	# ovpn_cmd_run() has already removed the temporary file this subshell
+	# inherited, so without this its output would be lost and a failed
+	# new_key would only show up later as a ping failure.
+	if [ "${rc}" -ne 0 ]; then
+		ovpn_dump_tcp_keys_log
+	fi
+	rm -f "${OVPN_TCP_KEYS_LOG}"
+
+	return "${rc}"
+}
+
+ovpn_dump_tcp_keys_log() {
+	[ -s "${OVPN_TCP_KEYS_LOG:-}" ] || return 0
+
+	printf '%s\n' "--- server-side key setup output ---"
+	cat "${OVPN_TCP_KEYS_LOG}"
+}
+
 ovpn_compare_ntfs() {
 	local diff_rc=0
 	local diff_file
diff --git a/tools/testing/selftests/net/ovpn/test-close-socket.sh b/tools/testing/selftests/net/ovpn/test-close-socket.sh
index ec9a51bbf3c9..142dc14e2606 100755
--- a/tools/testing/selftests/net/ovpn/test-close-socket.sh
+++ b/tools/testing/selftests/net/ovpn/test-close-socket.sh
@@ -37,6 +37,8 @@ ovpn_prepare_network() {
 		ovpn_cmd_ok "register peer${p} in overlay" ovpn_add_peer "${p}"
 	done
 
+	ovpn_cmd_ok "wait for server-side key setup" ovpn_wait_tcp_keys
+
 	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 		peer_ns="ovpn_peer${p}"
 		ovpn_cmd_ok "set peer0 timeout for peer ${p}" \
diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
index e2e6ffdd29bb..0762fa83e4b5 100755
--- a/tools/testing/selftests/net/ovpn/test.sh
+++ b/tools/testing/selftests/net/ovpn/test.sh
@@ -45,6 +45,8 @@ ovpn_prepare_network() {
 		ovpn_cmd_ok "register peer${p} in overlay" ovpn_add_peer "${p}"
 	done
 
+	ovpn_cmd_ok "wait for server-side key setup" ovpn_wait_tcp_keys
+
 	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 		peer_ns="ovpn_peer${p}"
 		ovpn_cmd_ok "set peer0 timeout for peer ${p}" \
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 6/6] MAINTAINERS: ovpn: add Ralf Lici as reviewer
  2026-10-12 13:37 [PATCH net-next 0/6] pull request: ovpn 2026-10-12 Antonio Quartulli
                   ` (4 preceding siblings ...)
  2026-10-12 13:38 ` [PATCH net-next 5/6] selftests: ovpn: wait for the TCP server key setup before traffic Antonio Quartulli
@ 2026-10-12 13:38 ` Antonio Quartulli
  5 siblings, 0 replies; 10+ messages in thread
From: Antonio Quartulli @ 2026-10-12 13:38 UTC (permalink / raw)
  To: netdev
  Cc: Antonio Quartulli, Sabrina Dubroca, Ralf Lici, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, David S. Miller, Eric Dumazet

Ralf has authored 39 of the ovpn commits merged so far, second only to
the maintainer, and is already involved in the review of most changes
posted to the list. Add him as designated reviewer so that he is CCed
on incoming patches.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Ralf Lici <ralf@mandelbit.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 509b35b5a0e7..9e5fa06b2b51 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20537,6 +20537,7 @@ F:	drivers/irqchip/irq-or1k-*
 
 OPENVPN DATA CHANNEL OFFLOAD
 M:	Antonio Quartulli <antonio@openvpn.net>
+R:	Ralf Lici <ralf@mandelbit.com>
 R:	Sabrina Dubroca <sd@queasysnail.net>
 L:	openvpn-devel@lists.sourceforge.net (subscribers-only)
 L:	netdev@vger.kernel.org
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 4/6] selftests: ovpn: enable TCP_NODELAY on TCP sockets
  2026-10-12 13:38 ` [PATCH net-next 4/6] selftests: ovpn: enable TCP_NODELAY on TCP sockets Antonio Quartulli
@ 2026-10-13 14:39   ` netdev-bot+sashiko
  2026-10-13 20:17     ` Antonio Quartulli
  0 siblings, 1 reply; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13 14:39 UTC (permalink / raw)
  To: antonio; +Cc: netdev, sd, ralf, kuba, pabeni, andrew+netdev, davem, edumazet

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] After this patch, no TCP selftest checks the TX peer-ID that
  goes out on the wire.

--

selftests: ovpn: enable TCP_NODELAY on TCP sockets

This commit makes ovpn-cli set TCP_NODELAY on its TCP sockets by default.
Setting OVPN_TCP_NODELAY=0 turns it off, and the new test-tcp-nagle.sh
uses that to keep running the TCP suite with Nagle enabled. It also limits
the tcpdump-based peer ID check in ovpn_run_basic_traffic() to UDP.

> diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
> index 392109d5e14e2..e2e6ffdd29bb0 100755
> --- a/tools/testing/selftests/net/ovpn/test.sh
> +++ b/tools/testing/selftests/net/ovpn/test.sh
> @@ -137,35 +137,33 @@ ovpn_run_basic_traffic() {
>  	local tcpdump_timeout="1.5s"
>  
>  	for p in $(seq 1 ${OVPN_NUM_PEERS}); do

[ ... ]

>  		if [ "${OVPN_PROTO}" == "UDP" ]; then

[ ... ]

> @@ -173,8 +171,10 @@ ovpn_run_basic_traffic() {
>  			ip netns exec ovpn_peer0 \
>  			ping -qfc 100 -s 3000 -w 3 5.5.5.$((p + 1))
>  
> -		wait "${tcpdump_pid1}" || return 1
> -		wait "${tcpdump_pid2}" || return 1
> +		if [ "${OVPN_PROTO}" == "UDP" ]; then
> +			wait "${tcpdump_pid1}" || return 1
> +			wait "${tcpdump_pid2}" || return 1
> +		fi
>  	done
>  }

[Severity: Medium]
This patch removes the TCP branch of ovpn_build_capture_filter() and makes
these waits UDP-only. After that, does any TCP selftest still check the
peer-id that goes out on the wire?

The capture was added by commit 367f4b163a8c ("selftests: ovpn: check
asymmetric peer-id"). Its purpose was to check the peer-id in the data
packet headers for every connection, TCP included.

On the kernel side, the TCP receive path never reads the peer-id from the
packet. It takes the peer that is bound to the socket:

drivers/net/ovpn/tcp.c:ovpn_tcp_rcv() {
    ...
	ovpn_recv(peer, skb);
    ...
}

So if a TCP peer had the wrong tx_id here, traffic would still flow:

drivers/net/ovpn/crypto_aead.c:ovpn_aead_encrypt() {
    ...
	op = ovpn_opcode_compose(OVPN_DATA_V2, ks->key_id, peer->tx_id);
    ...
}

The JSON expectations in the selftest directory don't compare tx-id
either. With this change, test-tcp.sh, test-symmetric-id-tcp.sh and the new
test-tcp-nagle.sh would all pass with an incorrect TCP tx_id.

The later patch in the series ("selftests: ovpn: wait for the TCP server
key setup before traffic") does not bring the TCP check back.

Could these TCP variants keep some form of peer-id check that does not
depend on TCP segment boundaries?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012133809.2477063-1-antonio%40openvpn.net

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 5/6] selftests: ovpn: wait for the TCP server key setup before traffic
  2026-10-12 13:38 ` [PATCH net-next 5/6] selftests: ovpn: wait for the TCP server key setup before traffic Antonio Quartulli
@ 2026-10-13 14:39   ` netdev-bot+sashiko
  0 siblings, 0 replies; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13 14:39 UTC (permalink / raw)
  To: antonio; +Cc: netdev, sd, ralf, kuba, pabeni, andrew+netdev, davem, edumazet

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit fixes a real ordering race but has no Fixes: tag.
- [Low] In TCP mode, ovpn_add_peer() creates a temporary file with
  `OVPN_TCP_KEYS_LOG=$(mktemp)` and redirects the background subshell…
- [Low] ovpn_wait_tcp_keys() decides whether the background subshell is
  still running with `while kill -0 "${OVPN_TCP_KEYS_PID}"`.

--

selftests: ovpn: wait for the TCP server key setup before traffic

This patch saves the PID of the background TCP server subshell that
ovpn_add_peer() starts. That subshell runs listen and then new_key for
every peer, and its output now goes to a temporary log file. A new
ovpn_wait_tcp_keys() polls the subshell with a 30s bound, dumps the log
on failure, and is called from test.sh and test-close-socket.sh once all
peers are registered.

> The sleep 5 that follows does not cover it: it elapses while "listen" is
> still waiting for connections, and the peers only connect in the
> subsequent ovpn_add_peer() iterations, so the key loop starts well after
> that sleep has expired. Until now the test happened to work because of
> the unrelated delays that ran in between.

[ ... ]

> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

[Severity: Low]
This fixes a race in the test, but there is no Fixes: tag. Should one be
added?

The background listen/new_key subshell followed by sleep 5 in
ovpn_add_peer() comes from:

959bc330a439 ("testing/selftests: add test tool and scripts for ovpn module")

Commit 7c29665a3a3c later rewrote the listen and new_key lines but kept
the same structure.

Documentation/process/maintainer-netdev.rst says "for fixes the Fixes:
tag is required, regardless of the tree". The exception there for tests
only covers tests that come with a code fix.

Also, the previous patch in this series ("selftests: ovpn: enable
TCP_NODELAY on TCP sockets") removed the two tcpdump captures and the
sleep 0.3 before the first TCP ping. That may be one of the "unrelated
delays" mentioned above.

> diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
> index 2f7851b823436..da732c64ffea6 100644
> --- a/tools/testing/selftests/net/ovpn/common.sh
> +++ b/tools/testing/selftests/net/ovpn/common.sh
> @@ -256,14 +256,19 @@ ovpn_add_peer() {
>  		fi
>  	else
>  		if [ ${1} -eq 0 ]; then
> +			OVPN_TCP_KEYS_LOG=$(mktemp)
>  			(ip netns exec "${server_ns}" ${OVPN_CLI} listen tun0 \
>  				1 ${M_ID} ${OVPN_TCP_PEERS_FILE} && {
> +				rc=0
>  				for p in $(seq 1 ${OVPN_NUM_PEERS}); do
>  					ip netns exec "${server_ns}" \
>  						${OVPN_CLI} new_key tun0 ${p} \
> -						1 0 ${OVPN_ALG} 0 data64.key
> +						1 0 ${OVPN_ALG} 0 data64.key \
> +						|| rc=1
>  				done
> -			}) &
> +				exit ${rc}
> +			}) >"${OVPN_TCP_KEYS_LOG}" 2>&1 &
> +			OVPN_TCP_KEYS_PID=$!
>  			sleep 5
>  		else
>  			peer_ns="ovpn_peer${1}"

[ ... ]

> +ovpn_wait_tcp_keys() {
> +	local deadline rc
> +
> +	[ -n "${OVPN_TCP_KEYS_PID:-}" ] || return 0
> +
> +	deadline=$((SECONDS + OVPN_TCP_KEYS_TIMEOUT))
> +	while kill -0 "${OVPN_TCP_KEYS_PID}" 2>/dev/null; do
> +		if [ "${SECONDS}" -ge "${deadline}" ]; then
> +			printf '%s\n' \
> +				"server-side key setup still running after ${OVPN_TCP_KEYS_TIMEOUT}s"
> +			kill "${OVPN_TCP_KEYS_PID}" 2>/dev/null

[Severity: Low]
Can kill -0 and kill here end up hitting an unrelated process?

Both use the raw number saved by OVPN_TCP_KEYS_PID=$! in ovpn_add_peer().
Bash reaps background children itself, both from its SIGCHLD handling
and while it waits for the foreground sleep 0.2. It keeps the exit status
so the later wait still works.

Once the child is reaped, its PID is free in the kernel and can be given
to another task. If that happens before the next kill -0:

  - the loop treats the unrelated process as the key setup job and spins
    until the 30s deadline
  - kill then sends SIGTERM to that process
  - setup is reported as failed even though the keys were installed

alloc_pid() hands out PIDs cyclically, so this needs a PID wraparound
within one poll interval. That makes it very unlikely in practice, but
the check is not tied to the job's lifetime.

Could the liveness check and the kill be tied to the background job
itself rather than to the numeric PID?

> +			wait "${OVPN_TCP_KEYS_PID}" 2>/dev/null
> +			ovpn_dump_tcp_keys_log
> +			return 1

[Severity: Low]
Does this timeout path leak the OVPN_TCP_KEYS_LOG file that mktemp
creates in ovpn_add_peer()?

The only removal is the rm -f "${OVPN_TCP_KEYS_LOG}" further down. It
only runs when the subshell exits on its own, and this return 1 skips it.

The file is also left behind if a later ovpn_add_peer() call fails before
ovpn_wait_tcp_keys() is reached, for example when connect fails. Under
set -eE that path goes through the EXIT trap to ovpn_cleanup(), which
never touches OVPN_TCP_KEYS_LOG:

ovpn_cleanup() {
	local peer

	# some ovpn-cli processes sleep in background so they need manual poking
	killall "$(basename "${OVPN_CLI}")" 2>/dev/null || true

	for peer in "${!OVPN_LISTENER_PIDS[@]}"; do
		ovpn_stop_listener "${peer}" 2>/dev/null
	done
	...
}

The existing OVPN_TMP_JSONS temp files are already removed from
ovpn_cleanup() through ovpn_stop_listener(). Should OVPN_TCP_KEYS_LOG be
removed from ovpn_cleanup() too?

> +		fi
> +		sleep 0.2
> +	done

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012133809.2477063-1-antonio%40openvpn.net

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 4/6] selftests: ovpn: enable TCP_NODELAY on TCP sockets
  2026-10-13 14:39   ` netdev-bot+sashiko
@ 2026-10-13 20:17     ` Antonio Quartulli
  0 siblings, 0 replies; 10+ messages in thread
From: Antonio Quartulli @ 2026-10-13 20:17 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: netdev, sd, ralf, kuba, pabeni, andrew+netdev, davem, edumazet

Hi,

On 13/10/2026 16:39, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] After this patch, no TCP selftest checks the TX peer-ID that
>    goes out on the wire.
> 
> --
> 
> selftests: ovpn: enable TCP_NODELAY on TCP sockets
> 
> This commit makes ovpn-cli set TCP_NODELAY on its TCP sockets by default.
> Setting OVPN_TCP_NODELAY=0 turns it off, and the new test-tcp-nagle.sh
> uses that to keep running the TCP suite with Nagle enabled. It also limits
> the tcpdump-based peer ID check in ovpn_run_basic_traffic() to UDP.
> 
>> diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
>> index 392109d5e14e2..e2e6ffdd29bb0 100755
>> --- a/tools/testing/selftests/net/ovpn/test.sh
>> +++ b/tools/testing/selftests/net/ovpn/test.sh
>> @@ -137,35 +137,33 @@ ovpn_run_basic_traffic() {
>>   	local tcpdump_timeout="1.5s"
>>   
>>   	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
> 
> [ ... ]
> 
>>   		if [ "${OVPN_PROTO}" == "UDP" ]; then
> 
> [ ... ]
> 
>> @@ -173,8 +171,10 @@ ovpn_run_basic_traffic() {
>>   			ip netns exec ovpn_peer0 \
>>   			ping -qfc 100 -s 3000 -w 3 5.5.5.$((p + 1))
>>   
>> -		wait "${tcpdump_pid1}" || return 1
>> -		wait "${tcpdump_pid2}" || return 1
>> +		if [ "${OVPN_PROTO}" == "UDP" ]; then
>> +			wait "${tcpdump_pid1}" || return 1
>> +			wait "${tcpdump_pid2}" || return 1
>> +		fi
>>   	done
>>   }
> 
> [Severity: Medium]
> This patch removes the TCP branch of ovpn_build_capture_filter() and makes
> these waits UDP-only. After that, does any TCP selftest still check the
> peer-id that goes out on the wire?
> 
> The capture was added by commit 367f4b163a8c ("selftests: ovpn: check
> asymmetric peer-id"). Its purpose was to check the peer-id in the data
> packet headers for every connection, TCP included.
> 
> On the kernel side, the TCP receive path never reads the peer-id from the
> packet. It takes the peer that is bound to the socket:
> 
> drivers/net/ovpn/tcp.c:ovpn_tcp_rcv() {
>      ...
> 	ovpn_recv(peer, skb);
>      ...
> }
> 
> So if a TCP peer had the wrong tx_id here, traffic would still flow:
> 
> drivers/net/ovpn/crypto_aead.c:ovpn_aead_encrypt() {
>      ...
> 	op = ovpn_opcode_compose(OVPN_DATA_V2, ks->key_id, peer->tx_id);
>      ...
> }
> 
> The JSON expectations in the selftest directory don't compare tx-id
> either. With this change, test-tcp.sh, test-symmetric-id-tcp.sh and the new
> test-tcp-nagle.sh would all pass with an incorrect TCP tx_id.
> 
> The later patch in the series ("selftests: ovpn: wait for the TCP server
> key setup before traffic") does not bring the TCP check back.
> 
> Could these TCP variants keep some form of peer-id check that does not
> depend on TCP segment boundaries?
> 

Testing tx-peer-id with TCP_NODELAY enabled is extremely challenging 
because we'd need to (almost) re-implement an OpenVPN TCP stream parser 
in the selftest.

Since the rest of the selftests already ensures that packets are well 
formed and that the ovpn TCP state machine works as expected, we decided 
to simply give up on this specific on-wire test.
We don't think to be losing coverage by doing so.

Hence this decision was deliberate.

Regards,



-- 
Antonio Quartulli
OpenVPN Inc.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-10-13 20:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 13:37 [PATCH net-next 0/6] pull request: ovpn 2026-10-12 Antonio Quartulli
2026-10-12 13:37 ` [PATCH net-next 1/6] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc Antonio Quartulli
2026-10-12 13:37 ` [PATCH net-next 2/6] ovpn: remove unused work field from struct ovpn_socket Antonio Quartulli
2026-10-12 13:38 ` [PATCH net-next 3/6] ovpn: remove redundant peer NULL checks in crypto post functions Antonio Quartulli
2026-10-12 13:38 ` [PATCH net-next 4/6] selftests: ovpn: enable TCP_NODELAY on TCP sockets Antonio Quartulli
2026-10-13 14:39   ` netdev-bot+sashiko
2026-10-13 20:17     ` Antonio Quartulli
2026-10-12 13:38 ` [PATCH net-next 5/6] selftests: ovpn: wait for the TCP server key setup before traffic Antonio Quartulli
2026-10-13 14:39   ` netdev-bot+sashiko
2026-10-12 13:38 ` [PATCH net-next 6/6] MAINTAINERS: ovpn: add Ralf Lici as reviewer Antonio Quartulli

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