* [PATCH net-next 0/5] pull request: ovpn 2026-09-22
@ 2026-09-22 6:08 Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 1/5] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc Antonio Quartulli
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Antonio Quartulli @ 2026-09-22 6:08 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 a small batch of ovpn changes for net-next, collected over the
past weeks. Most of it went through sashiko pre-review on openvpn-devel.
The bulk is cleanup: an unused work field goes away from struct
ovpn_socket, the redundant peer NULL checks in the crypto completion
helpers are dropped, and the documented return value of
ovpn_bind_from_sockaddr() is corrected.
On top of that, PEER_DEL_NTF now carries a peer snapshot in the same
format returned by PEER_GET. Userspace needs the final counters of a
disconnecting peer and so far had to issue a PEER_GET for every
notification; when many peers drop at once those extra transactions
overlap with notification processing on the same socket and can surface
transient netlink errors (peers disappear in the meantime). Carrying
the counters in the notification removes that round trip and errors.
Finally, the selftests enable TCP_NODELAY on ovpn-cli's TCP sockets, to
match what userspace does by default. Since TCP does not preserve record
boundaries, the capture-based peer ID check is restricted to UDP.
Note that the fixes I sent for net on Sep 21st ("pull request: fixes for
ovpn 2026-09-21") touch four of the same files, but the two series do
not conflict: I verified that they merge cleanly in either order and
that the resulting tree is identical both ways. The two pull requests
can therefore be merged in any order.
Please pull or let me know of any issue!
Thanks a lot,
Antonio
The following changes since commit 114bd09838ac6954baaf110aebc4d5503057f88a:
net: hip04: use 16-bit byte order for HI13X1 TX fields (2026-09-21 17:30:19 -0700)
are available in the Git repository at:
https://github.com/OpenVPN/ovpn-net-next.git ovpn-net-next-20260922
for you to fetch changes up to bdeec4b3a279a6ace7a0ad506b04df266d9bd43d:
ovpn: send peer object along with PEER_DEL_NTF (2026-09-22 02:37:40 +0200)
----------------------------------------------------------------
Included changes:
* include the peer object in the PEER_DEL notification
* 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()
----------------------------------------------------------------
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 (2):
ovpn: remove unused work field from struct ovpn_socket
ovpn: send peer object along with PEER_DEL_NTF
drivers/net/ovpn/bind.c | 2 +-
drivers/net/ovpn/io.c | 6 +--
drivers/net/ovpn/netlink.c | 60 +++++++++++++---------
drivers/net/ovpn/socket.h | 2 -
tools/testing/selftests/net/ovpn/common.sh | 59 +++++++++++++++------
.../selftests/net/ovpn/json/peer0-float.json | 12 ++---
.../selftests/net/ovpn/json/peer0-symm-float.json | 10 +++-
.../selftests/net/ovpn/json/peer0-symm.json | 7 ++-
tools/testing/selftests/net/ovpn/json/peer0.json | 12 ++---
.../selftests/net/ovpn/json/peer1-symm.json | 2 +-
tools/testing/selftests/net/ovpn/json/peer1.json | 2 +-
.../selftests/net/ovpn/json/peer2-symm.json | 2 +-
tools/testing/selftests/net/ovpn/json/peer2.json | 2 +-
.../selftests/net/ovpn/json/peer3-symm.json | 2 +-
tools/testing/selftests/net/ovpn/json/peer3.json | 2 +-
.../selftests/net/ovpn/json/peer4-symm.json | 2 +-
tools/testing/selftests/net/ovpn/json/peer4.json | 2 +-
.../selftests/net/ovpn/json/peer5-symm.json | 2 +-
tools/testing/selftests/net/ovpn/json/peer5.json | 2 +-
.../selftests/net/ovpn/json/peer6-symm.json | 2 +-
tools/testing/selftests/net/ovpn/json/peer6.json | 2 +-
tools/testing/selftests/net/ovpn/ovpn-cli.c | 22 ++++++++
tools/testing/selftests/net/ovpn/test.sh | 54 +++++++++----------
23 files changed, 171 insertions(+), 99 deletions(-)
mode change 120000 => 100644 tools/testing/selftests/net/ovpn/json/peer0-symm-float.json
mode change 120000 => 100644 tools/testing/selftests/net/ovpn/json/peer0-symm.json
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next 1/5] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc
2026-09-22 6:08 [PATCH net-next 0/5] pull request: ovpn 2026-09-22 Antonio Quartulli
@ 2026-09-22 6:08 ` Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 2/5] ovpn: remove unused work field from struct ovpn_socket Antonio Quartulli
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Antonio Quartulli @ 2026-09-22 6:08 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] 9+ messages in thread
* [PATCH net-next 2/5] ovpn: remove unused work field from struct ovpn_socket
2026-09-22 6:08 [PATCH net-next 0/5] pull request: ovpn 2026-09-22 Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 1/5] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc Antonio Quartulli
@ 2026-09-22 6:08 ` Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 3/5] ovpn: remove redundant peer NULL checks in crypto post functions Antonio Quartulli
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Antonio Quartulli @ 2026-09-22 6:08 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] 9+ messages in thread
* [PATCH net-next 3/5] ovpn: remove redundant peer NULL checks in crypto post functions
2026-09-22 6:08 [PATCH net-next 0/5] pull request: ovpn 2026-09-22 Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 1/5] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 2/5] ovpn: remove unused work field from struct ovpn_socket Antonio Quartulli
@ 2026-09-22 6:08 ` Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 4/5] selftests: ovpn: enable TCP_NODELAY on TCP sockets Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 5/5] ovpn: send peer object along with PEER_DEL_NTF Antonio Quartulli
4 siblings, 0 replies; 9+ messages in thread
From: Antonio Quartulli @ 2026-09-22 6:08 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] 9+ messages in thread
* [PATCH net-next 4/5] selftests: ovpn: enable TCP_NODELAY on TCP sockets
2026-09-22 6:08 [PATCH net-next 0/5] pull request: ovpn 2026-09-22 Antonio Quartulli
` (2 preceding siblings ...)
2026-09-22 6:08 ` [PATCH net-next 3/5] ovpn: remove redundant peer NULL checks in crypto post functions Antonio Quartulli
@ 2026-09-22 6:08 ` Antonio Quartulli
2026-09-23 6:49 ` netdev-bot+sashiko
2026-09-22 6:08 ` [PATCH net-next 5/5] ovpn: send peer object along with PEER_DEL_NTF Antonio Quartulli
4 siblings, 1 reply; 9+ messages in thread
From: Antonio Quartulli @ 2026-09-22 6:08 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>
Userspace now enables TCP_NODELAY by default. Enable it for
ovpn-cli's TCP sockets too.
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.
Signed-off-by: Marco Baffo <marco@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
tools/testing/selftests/net/ovpn/common.sh | 20 +++-----
tools/testing/selftests/net/ovpn/ovpn-cli.c | 22 +++++++++
tools/testing/selftests/net/ovpn/test.sh | 54 ++++++++++-----------
3 files changed, 56 insertions(+), 40 deletions(-)
diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
index 2d844eb3aa6e..de96d333ee2b 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -178,20 +178,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 f4effa7580c0..6b458a654a33 100644
--- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
+++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
@@ -470,6 +470,18 @@ static int ovpn_parse_key_direction(const char *dir, struct ovpn_ctx *ctx)
return 0;
}
+static int ovpn_tcp_nodelay(int socket)
+{
+ int opt = 1;
+ int ret;
+
+ 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 +618,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 +641,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.sh b/tools/testing/selftests/net/ovpn/test.sh
index 9b5610837032..d744c1a97d5f 100755
--- a/tools/testing/selftests/net/ovpn/test.sh
+++ b/tools/testing/selftests/net/ovpn/test.sh
@@ -67,35 +67,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))
@@ -103,8 +101,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] 9+ messages in thread
* [PATCH net-next 5/5] ovpn: send peer object along with PEER_DEL_NTF
2026-09-22 6:08 [PATCH net-next 0/5] pull request: ovpn 2026-09-22 Antonio Quartulli
` (3 preceding siblings ...)
2026-09-22 6:08 ` [PATCH net-next 4/5] selftests: ovpn: enable TCP_NODELAY on TCP sockets Antonio Quartulli
@ 2026-09-22 6:08 ` Antonio Quartulli
2026-09-23 6:49 ` netdev-bot+sashiko
4 siblings, 1 reply; 9+ messages in thread
From: Antonio Quartulli @ 2026-09-22 6:08 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>
OpenVPN userspace needs the final statistics of a disconnecting peer.
Today this is done by issuing a PEER_GET request after receiving
PEER_DEL_NTF. When several peers disconnect at the same time, those
extra request/reply transactions overlap with notification processing on
the same netlink socket and can make userspace hit transient netlink
errors such as NLE_BUSY or NLE_NOMEM.
Include a peer snapshot directly in PEER_DEL_NTF, using the same peer
object format returned by PEER_GET. This lets userspace consume the last
known counters from the notification itself, without issuing one netlink
request per deleted peer.
Update the ovpn selftest notification fixtures to validate the new
payload while normalizing timing-dependent counters.
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
drivers/net/ovpn/netlink.c | 60 ++++++++++++-------
tools/testing/selftests/net/ovpn/common.sh | 39 +++++++++++-
.../selftests/net/ovpn/json/peer0-float.json | 12 ++--
.../net/ovpn/json/peer0-symm-float.json | 10 +++-
.../selftests/net/ovpn/json/peer0-symm.json | 7 ++-
.../selftests/net/ovpn/json/peer0.json | 12 ++--
.../selftests/net/ovpn/json/peer1-symm.json | 2 +-
.../selftests/net/ovpn/json/peer1.json | 2 +-
.../selftests/net/ovpn/json/peer2-symm.json | 2 +-
.../selftests/net/ovpn/json/peer2.json | 2 +-
.../selftests/net/ovpn/json/peer3-symm.json | 2 +-
.../selftests/net/ovpn/json/peer3.json | 2 +-
.../selftests/net/ovpn/json/peer4-symm.json | 2 +-
.../selftests/net/ovpn/json/peer4.json | 2 +-
.../selftests/net/ovpn/json/peer5-symm.json | 2 +-
.../selftests/net/ovpn/json/peer5.json | 2 +-
.../selftests/net/ovpn/json/peer6-symm.json | 2 +-
.../selftests/net/ovpn/json/peer6.json | 2 +-
18 files changed, 112 insertions(+), 52 deletions(-)
mode change 120000 => 100644 tools/testing/selftests/net/ovpn/json/peer0-symm-float.json
mode change 120000 => 100644 tools/testing/selftests/net/ovpn/json/peer0-symm.json
diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c
index 4dad85294198..aaca0612d6f4 100644
--- a/drivers/net/ovpn/netlink.c
+++ b/drivers/net/ovpn/netlink.c
@@ -546,27 +546,15 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
return 0;
}
-static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info,
- const struct ovpn_peer *peer, u32 portid, u32 seq,
- int flags)
+static int ovpn_nl_fill_peer(struct sk_buff *skb, const struct genl_info *info,
+ const struct ovpn_peer *peer)
{
const struct ovpn_bind *bind;
struct ovpn_socket *sock;
int ret = -EMSGSIZE;
- struct nlattr *attr;
__be16 local_port;
- void *hdr;
int id;
- hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags,
- OVPN_CMD_PEER_GET);
- if (!hdr)
- return -ENOBUFS;
-
- attr = nla_nest_start(skb, OVPN_A_PEER);
- if (!attr)
- goto err;
-
rcu_read_lock();
sock = rcu_dereference(peer->sock);
if (!sock) {
@@ -574,7 +562,7 @@ static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info,
goto err_unlock;
}
- if (!net_eq(genl_info_net(info), sock_net(sock->sk))) {
+ if (info && !net_eq(genl_info_net(info), sock_net(sock->sk))) {
id = peernet2id_alloc(genl_info_net(info),
sock_net(sock->sk),
GFP_ATOMIC);
@@ -585,26 +573,26 @@ static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info,
rcu_read_unlock();
if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id))
- goto err;
+ return -EMSGSIZE;
if (nla_put_u32(skb, OVPN_A_PEER_TX_ID, peer->tx_id))
- goto err;
+ return -EMSGSIZE;
if (peer->vpn_addrs.ipv4.s_addr != htonl(INADDR_ANY))
if (nla_put_in_addr(skb, OVPN_A_PEER_VPN_IPV4,
peer->vpn_addrs.ipv4.s_addr))
- goto err;
+ return -EMSGSIZE;
if (!ipv6_addr_equal(&peer->vpn_addrs.ipv6, &in6addr_any))
if (nla_put_in6_addr(skb, OVPN_A_PEER_VPN_IPV6,
&peer->vpn_addrs.ipv6))
- goto err;
+ return -EMSGSIZE;
if (nla_put_u32(skb, OVPN_A_PEER_KEEPALIVE_INTERVAL,
peer->keepalive_interval) ||
nla_put_u32(skb, OVPN_A_PEER_KEEPALIVE_TIMEOUT,
peer->keepalive_timeout))
- goto err;
+ return -EMSGSIZE;
rcu_read_lock();
bind = rcu_dereference(peer->bind);
@@ -652,14 +640,39 @@ static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info,
atomic64_read(&peer->link_stats.tx.bytes)) ||
nla_put_uint(skb, OVPN_A_PEER_LINK_TX_PACKETS,
atomic64_read(&peer->link_stats.tx.packets)))
+ return -EMSGSIZE;
+
+ return 0;
+err_unlock:
+ rcu_read_unlock();
+ return ret;
+}
+
+static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info,
+ const struct ovpn_peer *peer, u32 portid, u32 seq,
+ int flags)
+{
+ struct nlattr *attr;
+ int ret = -EMSGSIZE;
+ void *hdr;
+
+ hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags,
+ OVPN_CMD_PEER_GET);
+ if (!hdr)
+ return -ENOBUFS;
+
+ attr = nla_nest_start(skb, OVPN_A_PEER);
+ if (!attr)
+ goto err;
+
+ ret = ovpn_nl_fill_peer(skb, info, peer);
+ if (ret < 0)
goto err;
nla_nest_end(skb, attr);
genlmsg_end(skb, hdr);
return 0;
-err_unlock:
- rcu_read_unlock();
err:
genlmsg_cancel(skb, hdr);
return ret;
@@ -1193,7 +1206,8 @@ int ovpn_nl_peer_del_notify(struct ovpn_peer *peer)
if (nla_put_u32(msg, OVPN_A_PEER_DEL_REASON, peer->delete_reason))
goto err_cancel_msg;
- if (nla_put_u32(msg, OVPN_A_PEER_ID, peer->id))
+ ret = ovpn_nl_fill_peer(msg, NULL, peer);
+ if (ret < 0)
goto err_cancel_msg;
nla_nest_end(msg, attr);
diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
index de96d333ee2b..611d528c7de8 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -19,9 +19,42 @@ OVPN_VERBOSE=${OVPN_VERBOSE:-0}
export OVPN_ID_OFFSET=$(( 9 * (OVPN_SYMMETRIC_ID == 0) ))
-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)[]'
+# Peer delete notifications include traffic counters whose values depend on
+# timing. zero_attr() sets a counter to zero only when that counter is present,
+# so missing stats still fail the comparison. zero_peer_stats is just the list
+# of counters to normalize. normalize_peer_del_ntf applies that to peer-del-ntf
+# messages and drops transport endpoint details, while leaving other
+# notifications unchanged.
+OVPN_JQ_FILTER='
+ def zero_attr(key):
+ if has(key) then .[key] = 0 else . end;
+
+ def zero_peer_stats:
+ zero_attr("vpn-rx-bytes") |
+ zero_attr("vpn-rx-packets") |
+ zero_attr("vpn-tx-bytes") |
+ zero_attr("vpn-tx-packets") |
+ zero_attr("link-rx-bytes") |
+ zero_attr("link-rx-packets") |
+ zero_attr("link-tx-bytes") |
+ zero_attr("link-tx-packets");
+
+ def normalize_peer_del_ntf:
+ if .name == "peer-del-ntf" then
+ .msg.peer |= (
+ del(.["remote-ipv4"], .["remote-ipv6"],
+ .["remote-ipv6-scope-id"], .["remote-port"],
+ .["local-ipv4"], .["local-ipv6"],
+ .["local-port"]) |
+ zero_peer_stats
+ )
+ else . end;
+
+ map(if type == "array" then .[] else . end) |
+ map(del(.msg.ifindex)) |
+ map(normalize_peer_del_ntf) |
+ sort_by(.msg.peer.id)[]'
+
OVPN_LAN_IP="11.11.11.11"
declare -A OVPN_TMP_JSONS=()
diff --git a/tools/testing/selftests/net/ovpn/json/peer0-float.json b/tools/testing/selftests/net/ovpn/json/peer0-float.json
index 682fa58ad4ea..9711f2cf9cf6 100644
--- a/tools/testing/selftests/net/ovpn/json/peer0-float.json
+++ b/tools/testing/selftests/net/ovpn/json/peer0-float.json
@@ -1,9 +1,9 @@
{"name": "peer-float-ntf", "msg": {"ifindex": 0, "peer": {"id": 1, "remote-ipv4": "10.10.1.3", "remote-port": 1}}}
{"name": "peer-float-ntf", "msg": {"ifindex": 0, "peer": {"id": 2, "remote-ipv4": "10.10.2.3", "remote-port": 1}}}
{"name": "peer-float-ntf", "msg": {"ifindex": 0, "peer": {"id": 3, "remote-ipv4": "10.10.3.3", "remote-port": 1}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 1}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 2}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 3}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 4}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 5}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 6}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 1, "tx-id": 10, "vpn-ipv4": "5.5.5.2", "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 2, "tx-id": 11, "vpn-ipv4": "5.5.5.3", "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 3, "tx-id": 12, "vpn-ipv4": "5.5.5.4", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 4, "tx-id": 13, "vpn-ipv4": "5.5.5.5", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 5, "tx-id": 14, "vpn-ipv4": "5.5.5.6", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 6, "tx-id": 15, "vpn-ipv4": "5.5.5.7", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer0-symm-float.json b/tools/testing/selftests/net/ovpn/json/peer0-symm-float.json
deleted file mode 120000
index e31a5bd59863..000000000000
--- a/tools/testing/selftests/net/ovpn/json/peer0-symm-float.json
+++ /dev/null
@@ -1 +0,0 @@
-peer0-float.json
\ No newline at end of file
diff --git a/tools/testing/selftests/net/ovpn/json/peer0-symm-float.json b/tools/testing/selftests/net/ovpn/json/peer0-symm-float.json
new file mode 100644
index 000000000000..c94dcc81c80e
--- /dev/null
+++ b/tools/testing/selftests/net/ovpn/json/peer0-symm-float.json
@@ -0,0 +1,9 @@
+{"name": "peer-float-ntf", "msg": {"ifindex": 0, "peer": {"id": 1, "remote-ipv4": "10.10.1.3", "remote-port": 1}}}
+{"name": "peer-float-ntf", "msg": {"ifindex": 0, "peer": {"id": 2, "remote-ipv4": "10.10.2.3", "remote-port": 1}}}
+{"name": "peer-float-ntf", "msg": {"ifindex": 0, "peer": {"id": 3, "remote-ipv4": "10.10.3.3", "remote-port": 1}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 1, "tx-id": 1, "vpn-ipv4": "5.5.5.2", "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 2, "tx-id": 2, "vpn-ipv4": "5.5.5.3", "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 3, "tx-id": 3, "vpn-ipv4": "5.5.5.4", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 4, "tx-id": 4, "vpn-ipv4": "5.5.5.5", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 5, "tx-id": 5, "vpn-ipv4": "5.5.5.6", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 6, "tx-id": 6, "vpn-ipv4": "5.5.5.7", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer0-symm.json b/tools/testing/selftests/net/ovpn/json/peer0-symm.json
deleted file mode 120000
index 57a163048eed..000000000000
--- a/tools/testing/selftests/net/ovpn/json/peer0-symm.json
+++ /dev/null
@@ -1 +0,0 @@
-peer0.json
\ No newline at end of file
diff --git a/tools/testing/selftests/net/ovpn/json/peer0-symm.json b/tools/testing/selftests/net/ovpn/json/peer0-symm.json
new file mode 100644
index 000000000000..2899913ea064
--- /dev/null
+++ b/tools/testing/selftests/net/ovpn/json/peer0-symm.json
@@ -0,0 +1,6 @@
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 1, "tx-id": 1, "vpn-ipv4": "5.5.5.2", "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 2, "tx-id": 2, "vpn-ipv4": "5.5.5.3", "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 3, "tx-id": 3, "vpn-ipv4": "5.5.5.4", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 4, "tx-id": 4, "vpn-ipv4": "5.5.5.5", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 5, "tx-id": 5, "vpn-ipv4": "5.5.5.6", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 6, "tx-id": 6, "vpn-ipv4": "5.5.5.7", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer0.json b/tools/testing/selftests/net/ovpn/json/peer0.json
index 7c46a33d5ecd..fff2a86b41ab 100644
--- a/tools/testing/selftests/net/ovpn/json/peer0.json
+++ b/tools/testing/selftests/net/ovpn/json/peer0.json
@@ -1,6 +1,6 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 1}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 2}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 3}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 4}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 5}}}
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 6}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 1, "tx-id": 10, "vpn-ipv4": "5.5.5.2", "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 2, "tx-id": 11, "vpn-ipv4": "5.5.5.3", "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 3, "tx-id": 12, "vpn-ipv4": "5.5.5.4", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 4, "tx-id": 13, "vpn-ipv4": "5.5.5.5", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 5, "tx-id": 14, "vpn-ipv4": "5.5.5.6", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 6, "tx-id": 15, "vpn-ipv4": "5.5.5.7", "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer1-symm.json b/tools/testing/selftests/net/ovpn/json/peer1-symm.json
index 5da4ea9d51fb..41b358e1be51 100644
--- a/tools/testing/selftests/net/ovpn/json/peer1-symm.json
+++ b/tools/testing/selftests/net/ovpn/json/peer1-symm.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 1}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 1, "tx-id": 1, "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer1.json b/tools/testing/selftests/net/ovpn/json/peer1.json
index 1009d26dc14a..6332709d2a88 100644
--- a/tools/testing/selftests/net/ovpn/json/peer1.json
+++ b/tools/testing/selftests/net/ovpn/json/peer1.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 10}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 10, "tx-id": 1, "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer2-symm.json b/tools/testing/selftests/net/ovpn/json/peer2-symm.json
index 8f6db4f8c2ac..b1840bf979e8 100644
--- a/tools/testing/selftests/net/ovpn/json/peer2-symm.json
+++ b/tools/testing/selftests/net/ovpn/json/peer2-symm.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 2}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 2, "tx-id": 2, "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer2.json b/tools/testing/selftests/net/ovpn/json/peer2.json
index 44e9fad2b622..431427e4eb53 100644
--- a/tools/testing/selftests/net/ovpn/json/peer2.json
+++ b/tools/testing/selftests/net/ovpn/json/peer2.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 11}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 11, "tx-id": 2, "keepalive-interval": 60, "keepalive-timeout": 120, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer3-symm.json b/tools/testing/selftests/net/ovpn/json/peer3-symm.json
index bdabd6fa2e64..f9ab2a15dd9f 100644
--- a/tools/testing/selftests/net/ovpn/json/peer3-symm.json
+++ b/tools/testing/selftests/net/ovpn/json/peer3-symm.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 3}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 3, "tx-id": 3, "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer3.json b/tools/testing/selftests/net/ovpn/json/peer3.json
index d4be8ba130ae..4f9522a664ee 100644
--- a/tools/testing/selftests/net/ovpn/json/peer3.json
+++ b/tools/testing/selftests/net/ovpn/json/peer3.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 12}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 12, "tx-id": 3, "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer4-symm.json b/tools/testing/selftests/net/ovpn/json/peer4-symm.json
index c3734bb9251b..b41ab838a304 100644
--- a/tools/testing/selftests/net/ovpn/json/peer4-symm.json
+++ b/tools/testing/selftests/net/ovpn/json/peer4-symm.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 4}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 4, "tx-id": 4, "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer4.json b/tools/testing/selftests/net/ovpn/json/peer4.json
index 67d27e2d48ac..7d6b3c8af0e4 100644
--- a/tools/testing/selftests/net/ovpn/json/peer4.json
+++ b/tools/testing/selftests/net/ovpn/json/peer4.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 13}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 13, "tx-id": 4, "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer5-symm.json b/tools/testing/selftests/net/ovpn/json/peer5-symm.json
index 46c4a348299d..962def1ec16f 100644
--- a/tools/testing/selftests/net/ovpn/json/peer5-symm.json
+++ b/tools/testing/selftests/net/ovpn/json/peer5-symm.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 5}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 5, "tx-id": 5, "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer5.json b/tools/testing/selftests/net/ovpn/json/peer5.json
index ecd9bd0b2f37..963dc74cdaa6 100644
--- a/tools/testing/selftests/net/ovpn/json/peer5.json
+++ b/tools/testing/selftests/net/ovpn/json/peer5.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 14}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 14, "tx-id": 5, "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer6-symm.json b/tools/testing/selftests/net/ovpn/json/peer6-symm.json
index aa30f2cff625..3ff352fae60f 100644
--- a/tools/testing/selftests/net/ovpn/json/peer6-symm.json
+++ b/tools/testing/selftests/net/ovpn/json/peer6-symm.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 6}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 6, "tx-id": 6, "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
diff --git a/tools/testing/selftests/net/ovpn/json/peer6.json b/tools/testing/selftests/net/ovpn/json/peer6.json
index 7fded29c5804..0db187f3af18 100644
--- a/tools/testing/selftests/net/ovpn/json/peer6.json
+++ b/tools/testing/selftests/net/ovpn/json/peer6.json
@@ -1 +1 @@
-{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 15}}}
+{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 15, "tx-id": 6, "keepalive-interval": 3, "keepalive-timeout": 3, "vpn-rx-bytes": 0, "vpn-rx-packets": 0, "vpn-tx-bytes": 0, "vpn-tx-packets": 0, "link-rx-bytes": 0, "link-rx-packets": 0, "link-tx-bytes": 0, "link-tx-packets": 0}}}
--
2.55.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 4/5] selftests: ovpn: enable TCP_NODELAY on TCP sockets
2026-09-22 6:08 ` [PATCH net-next 4/5] selftests: ovpn: enable TCP_NODELAY on TCP sockets Antonio Quartulli
@ 2026-09-23 6:49 ` netdev-bot+sashiko
2026-09-25 13:26 ` Antonio Quartulli
0 siblings, 1 reply; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 6:49 UTC (permalink / raw)
To: antonio; +Cc: netdev, ralf, sd, kuba, pabeni, andrew+netdev, davem, edumazet
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 2 · 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] New helper ovpn_tcp_nodelay() in
tools/testing/selftests/net/ovpn/ovpn-cli.c references the TCP_NODELAY…
- [Medium] In TCP mode the server-side data keys are installed by a
backgrounded subshell in ovpn_add_peer() ('(… ovpn-cli listen tun0 1…
--
selftests: ovpn: enable TCP_NODELAY on TCP sockets
This enables TCP_NODELAY on ovpn-cli's TCP sockets (both the accepted
server side and the connected client side) and restricts the
tcpdump-based peer ID capture check in the test script to UDP, since TCP
does not preserve record boundaries.
Two questions below.
[ ... ]
> diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c
> index f4effa7580c0f..6b458a654a337 100644
> --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
> +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
> @@ -470,6 +470,18 @@ static int ovpn_parse_key_direction(const char *dir, struct ovpn_ctx *ctx)
> return 0;
> }
>
> +static int ovpn_tcp_nodelay(int socket)
> +{
> + int opt = 1;
> + int ret;
> +
> + ret = setsockopt(socket, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(opt));
[Severity: Medium]
Does ovpn-cli still compile with this? TCP_NODELAY is used here for the
first time in this file, but the include block at the top of ovpn-cli.c is:
#include <stdint.h>
#include <stdio.h>
#include <inttypes.h>
#include <stdbool.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
#include <arpa/inet.h>
#include <net/if.h>
#include <netinet/in.h>
#include <time.h>
...
#include <sys/socket.h>
netinet/in.h provides IPPROTO_TCP but not TCP_NODELAY, so unless one of the
libnl or mbedtls headers happens to pull in netinet/tcp.h on the build host,
this fails with "'TCP_NODELAY' undeclared (first use in this function)".
Other net selftests using the same setsockopt() call include both headers
explicitly, for example fin_ack_lat.c:
#include <netinet/in.h>
#include <netinet/tcp.h>
and txtimestamp.c does the same.
Since ovpn-cli is listed in TEST_GEN_FILES in
tools/testing/selftests/net/ovpn/Makefile and every ovpn TEST_PROGS script
invokes it, would a failure here take out the whole ovpn suite? Would
adding #include <netinet/tcp.h> be the fix? It is still missing at the end
of the series (bc56ca6a91b2).
> + if (ret < 0)
> + perror("setsockopt for TCP_NODELAY");
> +
> + return ret;
> +}
> +
[ ... ]
> diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
> index 9b5610837032f..d744c1a97d5fc 100755
> --- a/tools/testing/selftests/net/ovpn/test.sh
> +++ b/tools/testing/selftests/net/ovpn/test.sh
> @@ -67,35 +67,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
> + ovpn_cmd_ok "send baseline traffic to peer ${p}" \
> + ip netns exec ovpn_peer0 \
> + ping -qfc 100 -w 3 5.5.5.$((p + 1))
[Severity: Medium]
Can moving "sleep 0.3" inside the UDP-only branch make the TCP variants
flaky? It was the only delay between peer setup and the first ping, and in
TCP mode the server-side data keys are installed by a backgrounded subshell
in ovpn_add_peer() whose PID is discarded:
tools/testing/selftests/net/ovpn/common.sh:ovpn_add_peer() {
...
(ip netns exec "${server_ns}" ${OVPN_CLI} listen tun0 \
1 ${M_ID} ${OVPN_TCP_PEERS_FILE} && {
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
done
}) &
...
}
The "listen" command returns as soon as all peers are accepted, because
CMD_LISTEN ends in ovpn_waitbg() -> daemon(1, 1), so the new_key loop runs
concurrently with the foreground script. With the sleep now gone for TCP,
ovpn_run_basic_traffic() starts "ping -qfc 100 -w 3 5.5.5.2" with no
synchronization against that loop.
If "new_key tun0 1" has not landed yet, server to client packets for peer 1
are dropped for lack of a key, and with -c 100 plus the -w 3 deadline ping
exits non-zero, so ovpn_cmd_ok reports "run baseline data traffic" as
failed.
Would recording the background setup PID and waiting for it (or polling
get_key) before generating traffic be more robust than relying on the
incidental sleep?
> @@ -103,8 +101,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
> }
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922060852.2266148-2-antonio%40openvpn.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 5/5] ovpn: send peer object along with PEER_DEL_NTF
2026-09-22 6:08 ` [PATCH net-next 5/5] ovpn: send peer object along with PEER_DEL_NTF Antonio Quartulli
@ 2026-09-23 6:49 ` netdev-bot+sashiko
0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 6:49 UTC (permalink / raw)
To: antonio; +Cc: netdev, ralf, sd, kuba, pabeni, andrew+netdev, davem, edumazet
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 2 · Low: 1
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] The commit message's premise is factually wrong: it states
that final peer statistics are obtained "today ... by issuing a…
- [Medium] Information disclosure across a privilege boundary:
PEER_DEL_NTF is multicast to the ovpn "peers" generic-netlink group,…
- [Low] The selftest does not exercise the behaviour the patch adds.
--
ovpn: send peer object along with PEER_DEL_NTF
This factors the peer serialization out of ovpn_nl_send_peer() into a new
ovpn_nl_fill_peer() helper and calls it from ovpn_nl_peer_del_notify(), so
PEER_DEL_NTF now carries the same peer object that PEER_GET returns. The
ovpn selftest notification fixtures are updated, and the jq filter
normalizes the timing-dependent traffic counters.
A few questions below.
> OpenVPN userspace needs the final statistics of a disconnecting peer.
> Today this is done by issuing a PEER_GET request after receiving
> PEER_DEL_NTF. When several peers disconnect at the same time, those
> extra request/reply transactions overlap with notification processing on
> the same netlink socket and can make userspace hit transient netlink
> errors such as NLE_BUSY or NLE_NOMEM.
[Severity: Medium]
Is the described existing workflow actually possible? In
ovpn_peer_remove() the peer is unlinked from the lookup structures before
the notification is emitted:
drivers/net/ovpn/peer.c:ovpn_peer_remove() {
...
hlist_del_init_rcu(&peer->hash_entry_id);
...
peer->delete_reason = reason;
ovpn_nl_peer_del_notify(peer);
}
A PEER_GET issued in response to the notification then goes through
ovpn_peer_get_by_id(), which walks exactly those structures:
drivers/net/ovpn/netlink.c:ovpn_nl_peer_get_doit() {
...
peer = ovpn_peer_get_by_id(ovpn, peer_id);
if (!peer) {
NL_SET_ERR_MSG_FMT_MOD(info->extack,
"cannot find peer with id %u", peer_id);
return -ENOENT;
}
}
That looks like it fails with -ENOENT for every deleted peer, including a
single disconnect on an otherwise idle netlink socket, so socket contention
would not be what makes the sequence fail. Would it be more accurate to
describe this patch as making the final counters available at all, rather
than as removing redundant request/reply traffic?
> diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c
> index 4dad852941982..aaca0612d6f4c 100644
> --- a/drivers/net/ovpn/netlink.c
> +++ b/drivers/net/ovpn/netlink.c
> @@ -546,27 +546,15 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
> return 0;
> }
>
> -static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info,
> - const struct ovpn_peer *peer, u32 portid, u32 seq,
> - int flags)
> +static int ovpn_nl_fill_peer(struct sk_buff *skb, const struct genl_info *info,
> + const struct ovpn_peer *peer)
> {
[ ... ]
> @@ -1193,7 +1206,8 @@ int ovpn_nl_peer_del_notify(struct ovpn_peer *peer)
> if (nla_put_u32(msg, OVPN_A_PEER_DEL_REASON, peer->delete_reason))
> goto err_cancel_msg;
>
> - if (nla_put_u32(msg, OVPN_A_PEER_ID, peer->id))
> + ret = ovpn_nl_fill_peer(msg, NULL, peer);
> + if (ret < 0)
> goto err_cancel_msg;
>
> nla_nest_end(msg, attr);
[Severity: Medium]
Does this widen what unprivileged readers can observe?
ovpn_nl_peer_del_notify() ends with:
genlmsg_multicast_netns(&ovpn_nl_family, sock_net(sock->sk), msg, 0,
OVPN_NLGRP_PEERS, GFP_ATOMIC);
and the destination group is declared without any capability flag:
drivers/net/ovpn/netlink-gen.c:
static const struct genl_multicast_group ovpn_nl_mcgrps[] = {
[OVPN_NLGRP_PEERS] = { "peers", },
};
genl_bind() only rejects the subscription when a flag is set:
net/netlink/genetlink.c:genl_bind() {
...
if ((grp->flags & GENL_MCAST_CAP_NET_ADMIN) &&
!ns_capable(net->user_ns, CAP_NET_ADMIN))
ret = -EPERM;
}
So any process in the netns can join "peers". Before this change the
notification carried ifindex, peer id and delete reason; with
ovpn_nl_fill_peer(msg, NULL, peer) it now also carries
OVPN_A_PEER_VPN_IPV4/IPV6, OVPN_A_PEER_LOCAL_IPV4/IPV6,
OVPN_A_PEER_LOCAL_PORT, the keepalive parameters and all eight VPN/link
byte and packet counters. Every OVPN_CMD_PEER_* op, including both
PEER_GET do and dump, carries GENL_ADMIN_PERM in ovpn_nl_ops, so the same
object is gated on CAP_NET_ADMIN when fetched directly.
Since the notification layout becomes uAPI once released, would it make
sense to mark the group GENL_MCAST_CAP_NET_ADMIN while the family is still
new, or to emit only the subset userspace needs?
> diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
> index de96d333ee2b3..611d528c7de88 100644
> --- a/tools/testing/selftests/net/ovpn/common.sh
> +++ b/tools/testing/selftests/net/ovpn/common.sh
> @@ -19,9 +19,42 @@ OVPN_VERBOSE=${OVPN_VERBOSE:-0}
>
> export OVPN_ID_OFFSET=$(( 9 * (OVPN_SYMMETRIC_ID == 0) ))
>
> -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)[]'
> +# Peer delete notifications include traffic counters whose values depend on
> +# timing. zero_attr() sets a counter to zero only when that counter is present,
> +# so missing stats still fail the comparison. zero_peer_stats is just the list
> +# of counters to normalize. normalize_peer_del_ntf applies that to peer-del-ntf
> +# messages and drops transport endpoint details, while leaving other
> +# notifications unchanged.
> +OVPN_JQ_FILTER='
> + def zero_attr(key):
> + if has(key) then .[key] = 0 else . end;
> +
> + def zero_peer_stats:
> + zero_attr("vpn-rx-bytes") |
> + zero_attr("vpn-rx-packets") |
> + zero_attr("vpn-tx-bytes") |
> + zero_attr("vpn-tx-packets") |
> + zero_attr("link-rx-bytes") |
> + zero_attr("link-rx-packets") |
> + zero_attr("link-tx-bytes") |
> + zero_attr("link-tx-packets");
[Severity: Low]
This isn't a bug, but does the updated selftest still assert anything about
the counter values the patch adds?
zero_peer_stats rewrites all eight counters to 0, and ovpn_compare_ntfs()
applies OVPN_JQ_FILTER to both the fixture and the captured stream, while
the fixtures hard-code 0 for peers that exchanged ping and iperf traffic
earlier in test.sh:
tools/testing/selftests/net/ovpn/json/peer0.json:
..."vpn-rx-bytes": 0, "vpn-rx-packets": 0, ..., "link-tx-packets": 0...
With the `if has(key)` guard the only property checked is that the
attributes are present, so a kernel emitting constant or bogus values from
ovpn_nl_fill_peer() would still pass. Could a weaker value-level check be
added instead, for example requiring nonzero counters for the peers that
carried traffic, or comparing against a PEER_GET snapshot taken before the
deletion?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922060852.2266148-2-antonio%40openvpn.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 4/5] selftests: ovpn: enable TCP_NODELAY on TCP sockets
2026-09-23 6:49 ` netdev-bot+sashiko
@ 2026-09-25 13:26 ` Antonio Quartulli
0 siblings, 0 replies; 9+ messages in thread
From: Antonio Quartulli @ 2026-09-25 13:26 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: netdev, ralf, sd, kuba, pabeni, andrew+netdev, davem, edumazet
Hi,
a few of Sashiko's finding are correct and worth investigating faster.
I'll respin this PR after having addressed the concerns.
Thanks,
pw-bot: cr
On 23/09/2026 08:49, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 2 · 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] New helper ovpn_tcp_nodelay() in
> tools/testing/selftests/net/ovpn/ovpn-cli.c references the TCP_NODELAY…
> - [Medium] In TCP mode the server-side data keys are installed by a
> backgrounded subshell in ovpn_add_peer() ('(… ovpn-cli listen tun0 1…
>
> --
>
> selftests: ovpn: enable TCP_NODELAY on TCP sockets
>
> This enables TCP_NODELAY on ovpn-cli's TCP sockets (both the accepted
> server side and the connected client side) and restricts the
> tcpdump-based peer ID capture check in the test script to UDP, since TCP
> does not preserve record boundaries.
>
> Two questions below.
>
> [ ... ]
>
>> diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c
>> index f4effa7580c0f..6b458a654a337 100644
>> --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
>> +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
>> @@ -470,6 +470,18 @@ static int ovpn_parse_key_direction(const char *dir, struct ovpn_ctx *ctx)
>> return 0;
>> }
>>
>> +static int ovpn_tcp_nodelay(int socket)
>> +{
>> + int opt = 1;
>> + int ret;
>> +
>> + ret = setsockopt(socket, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(opt));
>
> [Severity: Medium]
> Does ovpn-cli still compile with this? TCP_NODELAY is used here for the
> first time in this file, but the include block at the top of ovpn-cli.c is:
>
> #include <stdint.h>
> #include <stdio.h>
> #include <inttypes.h>
> #include <stdbool.h>
> #include <string.h>
> #include <errno.h>
> #include <unistd.h>
> #include <arpa/inet.h>
> #include <net/if.h>
> #include <netinet/in.h>
> #include <time.h>
> ...
> #include <sys/socket.h>
>
> netinet/in.h provides IPPROTO_TCP but not TCP_NODELAY, so unless one of the
> libnl or mbedtls headers happens to pull in netinet/tcp.h on the build host,
> this fails with "'TCP_NODELAY' undeclared (first use in this function)".
>
> Other net selftests using the same setsockopt() call include both headers
> explicitly, for example fin_ack_lat.c:
>
> #include <netinet/in.h>
> #include <netinet/tcp.h>
>
> and txtimestamp.c does the same.
>
> Since ovpn-cli is listed in TEST_GEN_FILES in
> tools/testing/selftests/net/ovpn/Makefile and every ovpn TEST_PROGS script
> invokes it, would a failure here take out the whole ovpn suite? Would
> adding #include <netinet/tcp.h> be the fix? It is still missing at the end
> of the series (bc56ca6a91b2).
>
>> + if (ret < 0)
>> + perror("setsockopt for TCP_NODELAY");
>> +
>> + return ret;
>> +}
>> +
>
> [ ... ]
>
>> diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
>> index 9b5610837032f..d744c1a97d5fc 100755
>> --- a/tools/testing/selftests/net/ovpn/test.sh
>> +++ b/tools/testing/selftests/net/ovpn/test.sh
>> @@ -67,35 +67,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
>> + ovpn_cmd_ok "send baseline traffic to peer ${p}" \
>> + ip netns exec ovpn_peer0 \
>> + ping -qfc 100 -w 3 5.5.5.$((p + 1))
>
> [Severity: Medium]
> Can moving "sleep 0.3" inside the UDP-only branch make the TCP variants
> flaky? It was the only delay between peer setup and the first ping, and in
> TCP mode the server-side data keys are installed by a backgrounded subshell
> in ovpn_add_peer() whose PID is discarded:
>
> tools/testing/selftests/net/ovpn/common.sh:ovpn_add_peer() {
> ...
> (ip netns exec "${server_ns}" ${OVPN_CLI} listen tun0 \
> 1 ${M_ID} ${OVPN_TCP_PEERS_FILE} && {
> 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
> done
> }) &
> ...
> }
>
> The "listen" command returns as soon as all peers are accepted, because
> CMD_LISTEN ends in ovpn_waitbg() -> daemon(1, 1), so the new_key loop runs
> concurrently with the foreground script. With the sleep now gone for TCP,
> ovpn_run_basic_traffic() starts "ping -qfc 100 -w 3 5.5.5.2" with no
> synchronization against that loop.
>
> If "new_key tun0 1" has not landed yet, server to client packets for peer 1
> are dropped for lack of a key, and with -c 100 plus the -w 3 deadline ping
> exits non-zero, so ovpn_cmd_ok reports "run baseline data traffic" as
> failed.
>
> Would recording the background setup PID and waiting for it (or polling
> get_key) before generating traffic be more robust than relying on the
> incidental sleep?
>
>> @@ -103,8 +101,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
>> }
>
--
Antonio Quartulli
OpenVPN Inc.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-09-25 13:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-22 6:08 [PATCH net-next 0/5] pull request: ovpn 2026-09-22 Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 1/5] ovpn: Fix the return value in ovpn_bind_from_sockaddr() kernel-doc Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 2/5] ovpn: remove unused work field from struct ovpn_socket Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 3/5] ovpn: remove redundant peer NULL checks in crypto post functions Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 4/5] selftests: ovpn: enable TCP_NODELAY on TCP sockets Antonio Quartulli
2026-09-23 6:49 ` netdev-bot+sashiko
2026-09-25 13:26 ` Antonio Quartulli
2026-09-22 6:08 ` [PATCH net-next 5/5] ovpn: send peer object along with PEER_DEL_NTF Antonio Quartulli
2026-09-23 6:49 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox