Re: [PATCH net-next 4/6] selftests: ovpn: enable TCP_NODELAY on TCP sockets

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.

quoted

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

[ ... ]

quoted

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

[ ... ]

quoted

@@ -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.