[PATCH net-next 0/8] pull request: ovpn 2025-11-11

[PATCH net-next 0/8] pull request: ovpn 2025-11-11

[Antonio Quartulli]

Hello netdev team!

After a bit of silence, here is a PR including a bunch of small
features for ovpn.
See the tag content (below) for the highlights.

Please pull or let me know of any issue!

Thanks a lot.
Antonio,


The following changes since commit 21f43f4a2b57cfeddf4e722d0969a85dd6185dcb:

  Merge branch 'devlink-eswitch-inactive-mode' (2025-11-11 13:17:57 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 84623a8944c866aef670023d163cb6ae708e4386:

  ovpn: use bound address in UDP when available (2025-11-11 22:18:13 +0100)

----------------------------------------------------------------
This batch includes the following features:
* use proper len constant when declaring array in CMD_KEY_SWAP parser
* use bitops API rather than manual operations in pktid.c
* send peer float event notification (netlink)
* exclude link-local v6 addresses from RPF check
* add support for asymmetric peer IDs (TX vs RX ID)
* improve memory allocations layout in encrypt/decrypt
* honour socket bound device and source address in routing lookup

----------------------------------------------------------------
Qingfang Deng (1):
      ovpn: pktid: use bitops.h API

Ralf Lici (6):
      ovpn: notify userspace on client float event
      ovpn: Allow IPv6 link-local addresses through RPF check
      ovpn: add support for asymmetric peer IDs
      ovpn: consolidate crypto allocations in one chunk
      ovpn: use bound device in UDP when available
      ovpn: use bound address in UDP when available

Sabrina Dubroca (1):
      ovpn: use correct array size to parse nested attributes in ovpn_nl_key_swap_doit

 Documentation/netlink/specs/ovpn.yaml       |  23 ++++-
 drivers/net/ovpn/crypto_aead.c              | 153 +++++++++++++++++++++-------
 drivers/net/ovpn/io.c                       |   8 +-
 drivers/net/ovpn/netlink-gen.c              |  13 ++-
 drivers/net/ovpn/netlink-gen.h              |   6 +-
 drivers/net/ovpn/netlink.c                  |  97 +++++++++++++++++-
 drivers/net/ovpn/netlink.h                  |   2 +
 drivers/net/ovpn/peer.c                     |  13 +++
 drivers/net/ovpn/peer.h                     |   4 +-
 drivers/net/ovpn/pktid.c                    |  11 +-
 drivers/net/ovpn/pktid.h                    |   2 +-
 drivers/net/ovpn/skb.h                      |  13 ++-
 drivers/net/ovpn/udp.c                      |  10 +-
 include/uapi/linux/ovpn.h                   |   2 +
 tools/testing/selftests/net/ovpn/ovpn-cli.c |   3 +
 15 files changed, 294 insertions(+), 66 deletions(-)

[PATCH net-next 1/8] ovpn: use correct array size to parse nested attributes in ovpn_nl_key_swap_doit

[Antonio Quartulli]

From: Sabrina Dubroca <sd@queasysnail.net>

In ovpn_nl_key_swap_doit, the attributes array used to parse the
OVPN_A_KEYCONF uses OVPN_A_PEER_MAX instead of
OVPN_A_KEYCONF_MAX. Note that this does not cause any bug, since
currently OVPN_A_KEYCONF_MAX < OVPN_A_PEER_MAX.

Fixes: 203e2bf55990 ("ovpn: implement key add/get/del/swap via netlink")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c
index c7f382437630..fed0e46b32a3 100644
--- a/drivers/net/ovpn/netlink.c
+++ b/drivers/net/ovpn/netlink.c
@@ -1061,8 +1061,8 @@ int ovpn_nl_key_get_doit(struct sk_buff *skb, struct genl_info *info)
 
 int ovpn_nl_key_swap_doit(struct sk_buff *skb, struct genl_info *info)
 {
+	struct nlattr *attrs[OVPN_A_KEYCONF_MAX + 1];
 	struct ovpn_priv *ovpn = info->user_ptr[0];
-	struct nlattr *attrs[OVPN_A_PEER_MAX + 1];
 	struct ovpn_peer *peer;
 	u32 peer_id;
 	int ret;
-- 
2.51.0

Re: [PATCH net-next 1/8] ovpn: use correct array size to parse nested attributes in ovpn_nl_key_swap_doit

[Jakub Kicinski]

On Tue, 11 Nov 2025 22:47:34 +0100 Antonio Quartulli wrote:
> Fixes: 203e2bf55990 ("ovpn: implement key add/get/del/swap via netlink")

Since (IIUC) you'll respin - please drop the Fixes tag here.
You can say something like

The bad line was added in commit 203e2bf55990 ("ovpn: implement key
add/get/del/swap via netlink")

But real Fixes tags are for fixes

Re: [PATCH net-next 1/8] ovpn: use correct array size to parse nested attributes in ovpn_nl_key_swap_doit

[Antonio Quartulli]

On 14/11/2025 03:26, Jakub Kicinski wrote:
> On Tue, 11 Nov 2025 22:47:34 +0100 Antonio Quartulli wrote:
>> Fixes: 203e2bf55990 ("ovpn: implement key add/get/del/swap via netlink")
> 
> Since (IIUC) you'll respin - please drop the Fixes tag here.
> You can say something like
> 
> The bad line was added in commit 203e2bf55990 ("ovpn: implement key
> add/get/del/swap via netlink")
> 
> But real Fixes tags are for fixes

Will drop the tag.

Thanks,


-- 
Antonio Quartulli
OpenVPN Inc.

[PATCH net-next 2/8] ovpn: pktid: use bitops.h API

[Antonio Quartulli]

From: Qingfang Deng <dqfext@gmail.com>

Use bitops.h for replay window to simplify code.

Signed-off-by: Qingfang Deng <dqfext@gmail.com>
[antonio@openvpn.net: extended commit message]
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/pktid.c | 11 ++++-------
 drivers/net/ovpn/pktid.h |  2 +-
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ovpn/pktid.c b/drivers/net/ovpn/pktid.c
index 2f29049897e3..f1c243b84463 100644
--- a/drivers/net/ovpn/pktid.c
+++ b/drivers/net/ovpn/pktid.c
@@ -65,7 +65,7 @@ int ovpn_pktid_recv(struct ovpn_pktid_recv *pr, u32 pkt_id, u32 pkt_time)
 	if (likely(pkt_id == pr->id + 1)) {
 		/* well-formed ID sequence (incremented by 1) */
 		pr->base = REPLAY_INDEX(pr->base, -1);
-		pr->history[pr->base / 8] |= (1 << (pr->base % 8));
+		__set_bit(pr->base, pr->history);
 		if (pr->extent < REPLAY_WINDOW_SIZE)
 			++pr->extent;
 		pr->id = pkt_id;
@@ -77,14 +77,14 @@ int ovpn_pktid_recv(struct ovpn_pktid_recv *pr, u32 pkt_id, u32 pkt_time)
 			unsigned int i;
 
 			pr->base = REPLAY_INDEX(pr->base, -delta);
-			pr->history[pr->base / 8] |= (1 << (pr->base % 8));
+			__set_bit(pr->base, pr->history);
 			pr->extent += delta;
 			if (pr->extent > REPLAY_WINDOW_SIZE)
 				pr->extent = REPLAY_WINDOW_SIZE;
 			for (i = 1; i < delta; ++i) {
 				unsigned int newb = REPLAY_INDEX(pr->base, i);
 
-				pr->history[newb / 8] &= ~BIT(newb % 8);
+				__clear_bit(newb, pr->history);
 			}
 		} else {
 			pr->base = 0;
@@ -103,14 +103,11 @@ int ovpn_pktid_recv(struct ovpn_pktid_recv *pr, u32 pkt_id, u32 pkt_time)
 			if (pkt_id > pr->id_floor) {
 				const unsigned int ri = REPLAY_INDEX(pr->base,
 								     delta);
-				u8 *p = &pr->history[ri / 8];
-				const u8 mask = (1 << (ri % 8));
 
-				if (*p & mask) {
+				if (__test_and_set_bit(ri, pr->history)) {
 					ret = -EINVAL;
 					goto out;
 				}
-				*p |= mask;
 			} else {
 				ret = -EINVAL;
 				goto out;
diff --git a/drivers/net/ovpn/pktid.h b/drivers/net/ovpn/pktid.h
index 0262d026d15e..21845f353bc8 100644
--- a/drivers/net/ovpn/pktid.h
+++ b/drivers/net/ovpn/pktid.h
@@ -34,7 +34,7 @@ struct ovpn_pktid_xmit {
  */
 struct ovpn_pktid_recv {
 	/* "sliding window" bitmask of recent packet IDs received */
-	u8 history[REPLAY_WINDOW_BYTES];
+	DECLARE_BITMAP(history, REPLAY_WINDOW_SIZE);
 	/* bit position of deque base in history */
 	unsigned int base;
 	/* extent (in bits) of deque in history */
-- 
2.51.0

Re: [PATCH net-next 2/8] ovpn: pktid: use bitops.h API

[Sabrina Dubroca]

2025-11-11, 22:47:35 +0100, Antonio Quartulli wrote:
> From: Qingfang Deng <dqfext@gmail.com>
> 
> Use bitops.h for replay window to simplify code.
> 
> Signed-off-by: Qingfang Deng <dqfext@gmail.com>
> [antonio@openvpn.net: extended commit message]
> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
> ---
>  drivers/net/ovpn/pktid.c | 11 ++++-------
>  drivers/net/ovpn/pktid.h |  2 +-
>  2 files changed, 5 insertions(+), 8 deletions(-)

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina

[PATCH net-next 3/8] ovpn: notify userspace on client float event

[Antonio Quartulli]

From: Ralf Lici <ralf@mandelbit.com>

Send a netlink notification when a client updates its remote UDP
endpoint. The notification includes the new IP address, port, and scope
ID (for IPv6).

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 Documentation/netlink/specs/ovpn.yaml       |  6 ++
 drivers/net/ovpn/netlink.c                  | 81 +++++++++++++++++++++
 drivers/net/ovpn/netlink.h                  |  2 +
 drivers/net/ovpn/peer.c                     |  2 +
 include/uapi/linux/ovpn.h                   |  1 +
 tools/testing/selftests/net/ovpn/ovpn-cli.c |  3 +
 6 files changed, 95 insertions(+)

diff --git a/Documentation/netlink/specs/ovpn.yaml b/Documentation/netlink/specs/ovpn.yaml
index 1b91045cee2e..0d0c028bf96f 100644
--- a/Documentation/netlink/specs/ovpn.yaml
+++ b/Documentation/netlink/specs/ovpn.yaml
@@ -502,6 +502,12 @@ operations:
             - ifindex
             - keyconf
 
+    -
+      name: peer-float-ntf
+      doc: Notification about a peer floating (changing its remote UDP endpoint)
+      notify: peer-get
+      mcgrp: peers
+
 mcast-groups:
   list:
     -
diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c
index fed0e46b32a3..c68f09a8c385 100644
--- a/drivers/net/ovpn/netlink.c
+++ b/drivers/net/ovpn/netlink.c
@@ -1203,6 +1203,87 @@ int ovpn_nl_peer_del_notify(struct ovpn_peer *peer)
 	return ret;
 }
 
+/**
+ * ovpn_nl_float_peer_notify - notify userspace about peer floating
+ * @peer: the floated peer
+ * @ss: sockaddr representing the new remote endpoint
+ *
+ * Return: 0 on success or a negative error code otherwise
+ */
+int ovpn_nl_peer_float_notify(struct ovpn_peer *peer,
+			      const struct sockaddr_storage *ss)
+{
+	struct ovpn_socket *sock;
+	struct sockaddr_in6 *sa6;
+	struct sockaddr_in *sa;
+	struct sk_buff *msg;
+	struct nlattr *attr;
+	int ret = -EMSGSIZE;
+	void *hdr;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!msg)
+		return -ENOMEM;
+
+	hdr = genlmsg_put(msg, 0, 0, &ovpn_nl_family, 0,
+			  OVPN_CMD_PEER_FLOAT_NTF);
+	if (!hdr) {
+		ret = -ENOBUFS;
+		goto err_free_msg;
+	}
+
+	if (nla_put_u32(msg, OVPN_A_IFINDEX, peer->ovpn->dev->ifindex))
+		goto err_cancel_msg;
+
+	attr = nla_nest_start(msg, OVPN_A_PEER);
+	if (!attr)
+		goto err_cancel_msg;
+
+	if (nla_put_u32(msg, OVPN_A_PEER_ID, peer->id))
+		goto err_cancel_msg;
+
+	if (ss->ss_family == AF_INET) {
+		sa = (struct sockaddr_in *)ss;
+		if (nla_put_in_addr(msg, OVPN_A_PEER_REMOTE_IPV4,
+				    sa->sin_addr.s_addr) ||
+		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT, sa->sin_port))
+			goto err_cancel_msg;
+	} else if (ss->ss_family == AF_INET6) {
+		sa6 = (struct sockaddr_in6 *)ss;
+		if (nla_put_in6_addr(msg, OVPN_A_PEER_REMOTE_IPV6,
+				     &sa6->sin6_addr) ||
+		    nla_put_u32(msg, OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID,
+				sa6->sin6_scope_id) ||
+		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT, sa6->sin6_port))
+			goto err_cancel_msg;
+	} else {
+		goto err_cancel_msg;
+	}
+
+	nla_nest_end(msg, attr);
+	genlmsg_end(msg, hdr);
+
+	rcu_read_lock();
+	sock = rcu_dereference(peer->sock);
+	if (!sock) {
+		ret = -EINVAL;
+		goto err_unlock;
+	}
+	genlmsg_multicast_netns(&ovpn_nl_family, sock_net(sock->sk), msg,
+				0, OVPN_NLGRP_PEERS, GFP_ATOMIC);
+	rcu_read_unlock();
+
+	return 0;
+
+err_unlock:
+	rcu_read_unlock();
+err_cancel_msg:
+	genlmsg_cancel(msg, hdr);
+err_free_msg:
+	nlmsg_free(msg);
+	return ret;
+}
+
 /**
  * ovpn_nl_key_swap_notify - notify userspace peer's key must be renewed
  * @peer: the peer whose key needs to be renewed
diff --git a/drivers/net/ovpn/netlink.h b/drivers/net/ovpn/netlink.h
index 8615dfc3c472..11ee7c681885 100644
--- a/drivers/net/ovpn/netlink.h
+++ b/drivers/net/ovpn/netlink.h
@@ -13,6 +13,8 @@ int ovpn_nl_register(void);
 void ovpn_nl_unregister(void);
 
 int ovpn_nl_peer_del_notify(struct ovpn_peer *peer);
+int ovpn_nl_peer_float_notify(struct ovpn_peer *peer,
+			      const struct sockaddr_storage *ss);
 int ovpn_nl_key_swap_notify(struct ovpn_peer *peer, u8 key_id);
 
 #endif /* _NET_OVPN_NETLINK_H_ */
diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index 4bfcab0c8652..9ad50f1ac2c3 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -287,6 +287,8 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb)
 
 	spin_unlock_bh(&peer->lock);
 
+	ovpn_nl_peer_float_notify(peer, &ss);
+
 	/* rehashing is required only in MP mode as P2P has one peer
 	 * only and thus there is no hashtable
 	 */
diff --git a/include/uapi/linux/ovpn.h b/include/uapi/linux/ovpn.h
index 680d1522dc87..b3c9ff0a6849 100644
--- a/include/uapi/linux/ovpn.h
+++ b/include/uapi/linux/ovpn.h
@@ -99,6 +99,7 @@ enum {
 	OVPN_CMD_KEY_SWAP,
 	OVPN_CMD_KEY_SWAP_NTF,
 	OVPN_CMD_KEY_DEL,
+	OVPN_CMD_PEER_FLOAT_NTF,
 
 	__OVPN_CMD_MAX,
 	OVPN_CMD_MAX = (__OVPN_CMD_MAX - 1)
diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c
index 0a5226196a2e..064453d16fdd 100644
--- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
+++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
@@ -1516,6 +1516,9 @@ static int ovpn_handle_msg(struct nl_msg *msg, void *arg)
 	case OVPN_CMD_PEER_DEL_NTF:
 		fprintf(stdout, "received CMD_PEER_DEL_NTF\n");
 		break;
+	case OVPN_CMD_PEER_FLOAT_NTF:
+		fprintf(stdout, "received CMD_PEER_FLOAT_NTF\n");
+		break;
 	case OVPN_CMD_KEY_SWAP_NTF:
 		fprintf(stdout, "received CMD_KEY_SWAP_NTF\n");
 		break;
-- 
2.51.0

Re: [PATCH net-next 3/8] ovpn: notify userspace on client float event

[Jakub Kicinski]

On Tue, 11 Nov 2025 22:47:36 +0100 Antonio Quartulli wrote:
> +	if (ss->ss_family == AF_INET) {
> +		sa = (struct sockaddr_in *)ss;
> +		if (nla_put_in_addr(msg, OVPN_A_PEER_REMOTE_IPV4,
> +				    sa->sin_addr.s_addr) ||
> +		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT, sa->sin_port))
> +			goto err_cancel_msg;
> +	} else if (ss->ss_family == AF_INET6) {
> +		sa6 = (struct sockaddr_in6 *)ss;
> +		if (nla_put_in6_addr(msg, OVPN_A_PEER_REMOTE_IPV6,
> +				     &sa6->sin6_addr) ||
> +		    nla_put_u32(msg, OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID,
> +				sa6->sin6_scope_id) ||
> +		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT, sa6->sin6_port))
> +			goto err_cancel_msg;
> +	} else {

presumably on this branch ret should be set to something?

> +		goto err_cancel_msg;
> +	}

Re: [PATCH net-next 3/8] ovpn: notify userspace on client float event

[Ralf Lici]

On Thu, 2025-11-13 at 18:21 -0800, Jakub Kicinski wrote:
> On Tue, 11 Nov 2025 22:47:36 +0100 Antonio Quartulli wrote:
> > +	if (ss->ss_family == AF_INET) {
> > +		sa = (struct sockaddr_in *)ss;
> > +		if (nla_put_in_addr(msg, OVPN_A_PEER_REMOTE_IPV4,
> > +				    sa->sin_addr.s_addr) ||
> > +		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT, sa-
> > >sin_port))
> > +			goto err_cancel_msg;
> > +	} else if (ss->ss_family == AF_INET6) {
> > +		sa6 = (struct sockaddr_in6 *)ss;
> > +		if (nla_put_in6_addr(msg, OVPN_A_PEER_REMOTE_IPV6,
> > +				     &sa6->sin6_addr) ||
> > +		    nla_put_u32(msg,
> > OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID,
> > +				sa6->sin6_scope_id) ||
> > +		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT,
> > sa6->sin6_port))
> > +			goto err_cancel_msg;
> > +	} else {
> 
> presumably on this branch ret should be set to something?

You're right, otherwise it would return -EMSGSIZE which is not what we
want here.

-- 
Ralf Lici
Mandelbit Srl

Re: [PATCH net-next 3/8] ovpn: notify userspace on client float event

[Sabrina Dubroca]

2025-11-14, 10:26:31 +0100, Ralf Lici wrote:
> On Thu, 2025-11-13 at 18:21 -0800, Jakub Kicinski wrote:
> > On Tue, 11 Nov 2025 22:47:36 +0100 Antonio Quartulli wrote:
> > > +	if (ss->ss_family == AF_INET) {
> > > +		sa = (struct sockaddr_in *)ss;
> > > +		if (nla_put_in_addr(msg, OVPN_A_PEER_REMOTE_IPV4,
> > > +				    sa->sin_addr.s_addr) ||
> > > +		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT, sa-
> > > >sin_port))
> > > +			goto err_cancel_msg;
> > > +	} else if (ss->ss_family == AF_INET6) {
> > > +		sa6 = (struct sockaddr_in6 *)ss;
> > > +		if (nla_put_in6_addr(msg, OVPN_A_PEER_REMOTE_IPV6,
> > > +				     &sa6->sin6_addr) ||
> > > +		    nla_put_u32(msg,
> > > OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID,
> > > +				sa6->sin6_scope_id) ||
> > > +		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT,
> > > sa6->sin6_port))
> > > +			goto err_cancel_msg;
> > > +	} else {
> > 
> > presumably on this branch ret should be set to something?
> 
> You're right, otherwise it would return -EMSGSIZE which is not what we
> want here.

But that should never happen with the current code, since
ovpn_nl_peer_float_notify is only called by ovpn_peer_endpoints_update
when salen != 0, and in that case we can only have ss_family = AF_INET
or ss_family = AF_INET6? (and otherwise it'd be an unitialized value
from ovpn_peer_endpoints_update)

(no objection to making ovpn_nl_peer_float_notify handle that
situation better in case it grows some other callers/contexts)

-- 
Sabrina

Re: [PATCH net-next 3/8] ovpn: notify userspace on client float event

[Ralf Lici]

On Fri, 2025-11-14 at 15:22 +0100, Sabrina Dubroca wrote:
> 2025-11-14, 10:26:31 +0100, Ralf Lici wrote:
> > On Thu, 2025-11-13 at 18:21 -0800, Jakub Kicinski wrote:
> > > On Tue, 11 Nov 2025 22:47:36 +0100 Antonio Quartulli wrote:
> > > > +	if (ss->ss_family == AF_INET) {
> > > > +		sa = (struct sockaddr_in *)ss;
> > > > +		if (nla_put_in_addr(msg,
> > > > OVPN_A_PEER_REMOTE_IPV4,
> > > > +				    sa->sin_addr.s_addr) ||
> > > > +		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT,
> > > > sa-
> > > > > sin_port))
> > > > +			goto err_cancel_msg;
> > > > +	} else if (ss->ss_family == AF_INET6) {
> > > > +		sa6 = (struct sockaddr_in6 *)ss;
> > > > +		if (nla_put_in6_addr(msg,
> > > > OVPN_A_PEER_REMOTE_IPV6,
> > > > +				     &sa6->sin6_addr) ||
> > > > +		    nla_put_u32(msg,
> > > > OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID,
> > > > +				sa6->sin6_scope_id) ||
> > > > +		    nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT,
> > > > sa6->sin6_port))
> > > > +			goto err_cancel_msg;
> > > > +	} else {
> > > 
> > > presumably on this branch ret should be set to something?
> > 
> > You're right, otherwise it would return -EMSGSIZE which is not what
> > we
> > want here.
> 
> But that should never happen with the current code, since
> ovpn_nl_peer_float_notify is only called by ovpn_peer_endpoints_update
> when salen != 0, and in that case we can only have ss_family = AF_INET
> or ss_family = AF_INET6? (and otherwise it'd be an unitialized value
> from ovpn_peer_endpoints_update)
> 
> (no objection to making ovpn_nl_peer_float_notify handle that
> situation better in case it grows some other callers/contexts)

True, with the current code we can't reach that branch. But as you
noted, if the calling context evolves, we're already covered and IMHO
the cost of handling this case right now is negligible.

-- 
Ralf Lici
Mandelbit Srl

[PATCH net-next 4/8] ovpn: Allow IPv6 link-local addresses through RPF check

[Antonio Quartulli]

From: Ralf Lici <ralf@mandelbit.com>

IPv6 link-local addresses are not globally routable and are therefore
absent in the unicast routing table. This causes legitimate packets with
link-local source addresses to fail standard RPF checks within ovpn.

Introduce an exception to explicitly allow such packets as link-local
addresses are essential for core IPv6 link-level operations like NDP,
which must function correctly within the virtual tunnel interface.

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/peer.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index 9ad50f1ac2c3..8fb6e43ecff7 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -882,6 +882,13 @@ bool ovpn_peer_check_by_src(struct ovpn_priv *ovpn, struct sk_buff *skb,
 		rcu_read_unlock();
 		break;
 	case htons(ETH_P_IPV6):
+		/* Link-local addresses are not globally routable and thus
+		 * would always fail a standard RPF lookup. Allow them as
+		 * they are essential for IPv6 link operations (e.g. NDP)
+		 */
+		if (ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)
+			return true;
+
 		addr6 = ovpn_nexthop_from_rt6(ovpn, ipv6_hdr(skb)->saddr);
 		rcu_read_lock();
 		match = (peer == ovpn_peer_get_by_vpn_addr6(ovpn, &addr6));
-- 
2.51.0

Re: [PATCH net-next 4/8] ovpn: Allow IPv6 link-local addresses through RPF check

[Sabrina Dubroca]

2025-11-11, 22:47:37 +0100, Antonio Quartulli wrote:
> From: Ralf Lici <ralf@mandelbit.com>
> 
> IPv6 link-local addresses are not globally routable and are therefore
> absent in the unicast routing table. This causes legitimate packets with
> link-local source addresses to fail standard RPF checks within ovpn.
> 
> Introduce an exception to explicitly allow such packets as link-local
> addresses are essential for core IPv6 link-level operations like NDP,
> which must function correctly within the virtual tunnel interface.

Does this fix an existing bug, or does it only become a problem for
some of the new features in that series (multipeer-to-multipeer?)? If
this is a problem for existing use-cases, there should be a Fixes tag.

-- 
Sabrina

Re: [PATCH net-next 4/8] ovpn: Allow IPv6 link-local addresses through RPF check

[Antonio Quartulli]

On 14/11/2025 17:06, Sabrina Dubroca wrote:
> 2025-11-11, 22:47:37 +0100, Antonio Quartulli wrote:
>> From: Ralf Lici <ralf@mandelbit.com>
>>
>> IPv6 link-local addresses are not globally routable and are therefore
>> absent in the unicast routing table. This causes legitimate packets with
>> link-local source addresses to fail standard RPF checks within ovpn.
>>
>> Introduce an exception to explicitly allow such packets as link-local
>> addresses are essential for core IPv6 link-level operations like NDP,
>> which must function correctly within the virtual tunnel interface.
> 
> Does this fix an existing bug, or does it only become a problem for
> some of the new features in that series (multipeer-to-multipeer?)? If
> this is a problem for existing use-cases, there should be a Fixes tag.
> 

Actually, after having spent more time on this patch, we realized that 
this patch is not really needed, because we can't truly route packets to 
addresses that are not known to ovpn (we wouldn't know which peer to 
send them to).

Hence this is a change that we originally thought to be needed, but 
further tests proved what I said above.

I'll drop this patch from the next PR.

Regards,


-- 
Antonio Quartulli
OpenVPN Inc.

[PATCH net-next 5/8] ovpn: add support for asymmetric peer IDs

[Antonio Quartulli]

From: Ralf Lici <ralf@mandelbit.com>

In order to support the multipeer architecture, upon connection setup
each side of a tunnel advertises a unique ID that the other side must
include in packets sent to them. Therefore when transmitting a packet, a
peer inserts the recipient's advertised ID for that specific tunnel into
the peer ID field. When receiving a packet, a peer expects to find its
own unique receive ID for that specific tunnel in the peer ID field.

Add support for the TX peer ID and embed it into transmitting packets.
If no TX peer ID is specified, fallback to using the same peer ID both
for RX and TX in order to be compatible with the non-multipeer compliant
peers.

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 Documentation/netlink/specs/ovpn.yaml | 17 ++++++++++++++++-
 drivers/net/ovpn/crypto_aead.c        |  2 +-
 drivers/net/ovpn/netlink-gen.c        | 13 ++++++++++---
 drivers/net/ovpn/netlink-gen.h        |  6 +++---
 drivers/net/ovpn/netlink.c            | 14 ++++++++++++--
 drivers/net/ovpn/peer.c               |  4 ++++
 drivers/net/ovpn/peer.h               |  4 +++-
 include/uapi/linux/ovpn.h             |  1 +
 8 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/Documentation/netlink/specs/ovpn.yaml b/Documentation/netlink/specs/ovpn.yaml
index 0d0c028bf96f..b0c782e59a32 100644
--- a/Documentation/netlink/specs/ovpn.yaml
+++ b/Documentation/netlink/specs/ovpn.yaml
@@ -43,7 +43,8 @@ attribute-sets:
         type: u32
         doc: >-
           The unique ID of the peer in the device context. To be used to
-          identify peers during operations for a specific device
+          identify peers during operations for a specific device.
+          Also used to match packets received from this peer.
         checks:
           max: 0xFFFFFF
       -
@@ -160,6 +161,16 @@ attribute-sets:
         name: link-tx-packets
         type: uint
         doc: Number of packets transmitted at the transport level
+      -
+        name: tx-id
+        type: u32
+        doc: >-
+          The ID value used when transmitting packets to this peer. This
+          way outgoing packets can have a different ID than incoming ones.
+          Useful in multipeer-to-multipeer connections, where each peer
+          will advertise the tx-id to be used on the link.
+        checks:
+          max: 0xFFFFFF
   -
     name: peer-new-input
     subset-of: peer
@@ -188,6 +199,8 @@ attribute-sets:
         name: keepalive-interval
       -
         name: keepalive-timeout
+      -
+        name: tx-id
   -
     name: peer-set-input
     subset-of: peer
@@ -214,6 +227,8 @@ attribute-sets:
         name: keepalive-interval
       -
         name: keepalive-timeout
+      -
+        name: tx-id
   -
     name: peer-del-input
     subset-of: peer
diff --git a/drivers/net/ovpn/crypto_aead.c b/drivers/net/ovpn/crypto_aead.c
index 2cca759feffa..cb6cdf8ec317 100644
--- a/drivers/net/ovpn/crypto_aead.c
+++ b/drivers/net/ovpn/crypto_aead.c
@@ -122,7 +122,7 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 	memcpy(skb->data, iv, OVPN_NONCE_WIRE_SIZE);
 
 	/* add packet op as head of additional data */
-	op = ovpn_opcode_compose(OVPN_DATA_V2, ks->key_id, peer->id);
+	op = ovpn_opcode_compose(OVPN_DATA_V2, ks->key_id, peer->tx_id);
 	__skb_push(skb, OVPN_OPCODE_SIZE);
 	BUILD_BUG_ON(sizeof(op) != OVPN_OPCODE_SIZE);
 	*((__force __be32 *)skb->data) = htonl(op);
diff --git a/drivers/net/ovpn/netlink-gen.c b/drivers/net/ovpn/netlink-gen.c
index 14298188c5f1..81b2dd946f33 100644
--- a/drivers/net/ovpn/netlink-gen.c
+++ b/drivers/net/ovpn/netlink-gen.c
@@ -15,6 +15,10 @@ static const struct netlink_range_validation ovpn_a_peer_id_range = {
 	.max	= 16777215ULL,
 };
 
+static const struct netlink_range_validation ovpn_a_peer_tx_id_range = {
+	.max	= 16777215ULL,
+};
+
 static const struct netlink_range_validation ovpn_a_keyconf_peer_id_range = {
 	.max	= 16777215ULL,
 };
@@ -50,7 +54,7 @@ const struct nla_policy ovpn_keydir_nl_policy[OVPN_A_KEYDIR_NONCE_TAIL + 1] = {
 	[OVPN_A_KEYDIR_NONCE_TAIL] = NLA_POLICY_EXACT_LEN(OVPN_NONCE_TAIL_SIZE),
 };
 
-const struct nla_policy ovpn_peer_nl_policy[OVPN_A_PEER_LINK_TX_PACKETS + 1] = {
+const struct nla_policy ovpn_peer_nl_policy[OVPN_A_PEER_TX_ID + 1] = {
 	[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
 	[OVPN_A_PEER_REMOTE_IPV4] = { .type = NLA_BE32, },
 	[OVPN_A_PEER_REMOTE_IPV6] = NLA_POLICY_EXACT_LEN(16),
@@ -74,13 +78,14 @@ const struct nla_policy ovpn_peer_nl_policy[OVPN_A_PEER_LINK_TX_PACKETS + 1] = {
 	[OVPN_A_PEER_LINK_TX_BYTES] = { .type = NLA_UINT, },
 	[OVPN_A_PEER_LINK_RX_PACKETS] = { .type = NLA_UINT, },
 	[OVPN_A_PEER_LINK_TX_PACKETS] = { .type = NLA_UINT, },
+	[OVPN_A_PEER_TX_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_tx_id_range),
 };
 
 const struct nla_policy ovpn_peer_del_input_nl_policy[OVPN_A_PEER_ID + 1] = {
 	[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
 };
 
-const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1] = {
+const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_TX_ID + 1] = {
 	[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
 	[OVPN_A_PEER_REMOTE_IPV4] = { .type = NLA_BE32, },
 	[OVPN_A_PEER_REMOTE_IPV6] = NLA_POLICY_EXACT_LEN(16),
@@ -93,9 +98,10 @@ const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIME
 	[OVPN_A_PEER_LOCAL_IPV6] = NLA_POLICY_EXACT_LEN(16),
 	[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32, },
 	[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32, },
+	[OVPN_A_PEER_TX_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_tx_id_range),
 };
 
-const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1] = {
+const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_TX_ID + 1] = {
 	[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
 	[OVPN_A_PEER_REMOTE_IPV4] = { .type = NLA_BE32, },
 	[OVPN_A_PEER_REMOTE_IPV6] = NLA_POLICY_EXACT_LEN(16),
@@ -107,6 +113,7 @@ const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIME
 	[OVPN_A_PEER_LOCAL_IPV6] = NLA_POLICY_EXACT_LEN(16),
 	[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32, },
 	[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32, },
+	[OVPN_A_PEER_TX_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_tx_id_range),
 };
 
 /* OVPN_CMD_PEER_NEW - do */
diff --git a/drivers/net/ovpn/netlink-gen.h b/drivers/net/ovpn/netlink-gen.h
index 220b5b2fdd4f..a66cc1268a43 100644
--- a/drivers/net/ovpn/netlink-gen.h
+++ b/drivers/net/ovpn/netlink-gen.h
@@ -17,10 +17,10 @@ extern const struct nla_policy ovpn_keyconf_del_input_nl_policy[OVPN_A_KEYCONF_S
 extern const struct nla_policy ovpn_keyconf_get_nl_policy[OVPN_A_KEYCONF_CIPHER_ALG + 1];
 extern const struct nla_policy ovpn_keyconf_swap_input_nl_policy[OVPN_A_KEYCONF_PEER_ID + 1];
 extern const struct nla_policy ovpn_keydir_nl_policy[OVPN_A_KEYDIR_NONCE_TAIL + 1];
-extern const struct nla_policy ovpn_peer_nl_policy[OVPN_A_PEER_LINK_TX_PACKETS + 1];
+extern const struct nla_policy ovpn_peer_nl_policy[OVPN_A_PEER_TX_ID + 1];
 extern const struct nla_policy ovpn_peer_del_input_nl_policy[OVPN_A_PEER_ID + 1];
-extern const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1];
-extern const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1];
+extern const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_TX_ID + 1];
+extern const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_TX_ID + 1];
 
 int ovpn_nl_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 		     struct genl_info *info);
diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c
index c68f09a8c385..d9b332758f55 100644
--- a/drivers/net/ovpn/netlink.c
+++ b/drivers/net/ovpn/netlink.c
@@ -305,6 +305,12 @@ static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
 		dst_cache_reset(&peer->dst_cache);
 	}
 
+	/* In a multipeer-to-multipeer setup we may have asymmetric peer IDs,
+	 * that is peer->id might be different from peer->tx_id.
+	 */
+	if (attrs[OVPN_A_PEER_TX_ID])
+		peer->tx_id = nla_get_u32(attrs[OVPN_A_PEER_TX_ID]);
+
 	if (attrs[OVPN_A_PEER_VPN_IPV4]) {
 		rehash = true;
 		peer->vpn_addrs.ipv4.s_addr =
@@ -326,8 +332,8 @@ static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
 	}
 
 	netdev_dbg(peer->ovpn->dev,
-		   "modify peer id=%u endpoint=%pIScp VPN-IPv4=%pI4 VPN-IPv6=%pI6c\n",
-		   peer->id, &ss,
+		   "modify peer id=%u tx_id=%u endpoint=%pIScp VPN-IPv4=%pI4 VPN-IPv6=%pI6c\n",
+		   peer->id, peer->tx_id, &ss,
 		   &peer->vpn_addrs.ipv4.s_addr, &peer->vpn_addrs.ipv6);
 
 	spin_unlock_bh(&peer->lock);
@@ -373,6 +379,7 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	peer_id = nla_get_u32(attrs[OVPN_A_PEER_ID]);
+
 	peer = ovpn_peer_new(ovpn, peer_id);
 	if (IS_ERR(peer)) {
 		NL_SET_ERR_MSG_FMT_MOD(info->extack,
@@ -572,6 +579,9 @@ static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info,
 	if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id))
 		goto err;
 
+	if (nla_put_u32(skb, OVPN_A_PEER_TX_ID, peer->tx_id))
+		goto err;
+
 	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))
diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index 8fb6e43ecff7..0657b1a6835c 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -99,7 +99,11 @@ struct ovpn_peer *ovpn_peer_new(struct ovpn_priv *ovpn, u32 id)
 	if (!peer)
 		return ERR_PTR(-ENOMEM);
 
+	/* in the default case TX and RX IDs are the same.
+	 * the user may set a different TX ID via netlink
+	 */
 	peer->id = id;
+	peer->tx_id = id;
 	peer->ovpn = ovpn;
 
 	peer->vpn_addrs.ipv4.s_addr = htonl(INADDR_ANY);
diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h
index a1423f2b09e0..328401570cba 100644
--- a/drivers/net/ovpn/peer.h
+++ b/drivers/net/ovpn/peer.h
@@ -21,7 +21,8 @@
  * struct ovpn_peer - the main remote peer object
  * @ovpn: main openvpn instance this peer belongs to
  * @dev_tracker: reference tracker for associated dev
- * @id: unique identifier
+ * @id: unique identifier, used to match incoming packets
+ * @tx_id: identifier to be used in TX packets
  * @vpn_addrs: IP addresses assigned over the tunnel
  * @vpn_addrs.ipv4: IPv4 assigned to peer on the tunnel
  * @vpn_addrs.ipv6: IPv6 assigned to peer on the tunnel
@@ -64,6 +65,7 @@ struct ovpn_peer {
 	struct ovpn_priv *ovpn;
 	netdevice_tracker dev_tracker;
 	u32 id;
+	u32 tx_id;
 	struct {
 		struct in_addr ipv4;
 		struct in6_addr ipv6;
diff --git a/include/uapi/linux/ovpn.h b/include/uapi/linux/ovpn.h
index b3c9ff0a6849..28cf97a86a18 100644
--- a/include/uapi/linux/ovpn.h
+++ b/include/uapi/linux/ovpn.h
@@ -54,6 +54,7 @@ enum {
 	OVPN_A_PEER_LINK_TX_BYTES,
 	OVPN_A_PEER_LINK_RX_PACKETS,
 	OVPN_A_PEER_LINK_TX_PACKETS,
+	OVPN_A_PEER_TX_ID,
 
 	__OVPN_A_PEER_MAX,
 	OVPN_A_PEER_MAX = (__OVPN_A_PEER_MAX - 1)
-- 
2.51.0

Re: [PATCH net-next 5/8] ovpn: add support for asymmetric peer IDs

[Sabrina Dubroca]

2025-11-11, 22:47:38 +0100, Antonio Quartulli wrote:
> From: Ralf Lici <ralf@mandelbit.com>
> 
> In order to support the multipeer architecture, upon connection setup
> each side of a tunnel advertises a unique ID that the other side must
> include in packets sent to them. Therefore when transmitting a packet, a
> peer inserts the recipient's advertised ID for that specific tunnel into
> the peer ID field. When receiving a packet, a peer expects to find its
> own unique receive ID for that specific tunnel in the peer ID field.
> 
> Add support for the TX peer ID and embed it into transmitting packets.
> If no TX peer ID is specified, fallback to using the same peer ID both
> for RX and TX in order to be compatible with the non-multipeer compliant
> peers.
> 
> Signed-off-by: Ralf Lici <ralf@mandelbit.com>
> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
> ---
>  Documentation/netlink/specs/ovpn.yaml | 17 ++++++++++++++++-
>  drivers/net/ovpn/crypto_aead.c        |  2 +-
>  drivers/net/ovpn/netlink-gen.c        | 13 ++++++++++---
>  drivers/net/ovpn/netlink-gen.h        |  6 +++---
>  drivers/net/ovpn/netlink.c            | 14 ++++++++++++--
>  drivers/net/ovpn/peer.c               |  4 ++++
>  drivers/net/ovpn/peer.h               |  4 +++-
>  include/uapi/linux/ovpn.h             |  1 +
>  8 files changed, 50 insertions(+), 11 deletions(-)

The patch looks ok, but shouldn't there be a selftest for this
feature, and a few others in this series (bound device/address, maybe
the RPF patch as well)?

-- 
Sabrina

Re: [PATCH net-next 5/8] ovpn: add support for asymmetric peer IDs

[Antonio Quartulli]

Hi Sabrina,

On 13/11/2025 14:58, Sabrina Dubroca wrote:
> 2025-11-11, 22:47:38 +0100, Antonio Quartulli wrote:
>> From: Ralf Lici <ralf@mandelbit.com>
>>
>> In order to support the multipeer architecture, upon connection setup
>> each side of a tunnel advertises a unique ID that the other side must
>> include in packets sent to them. Therefore when transmitting a packet, a
>> peer inserts the recipient's advertised ID for that specific tunnel into
>> the peer ID field. When receiving a packet, a peer expects to find its
>> own unique receive ID for that specific tunnel in the peer ID field.
>>
>> Add support for the TX peer ID and embed it into transmitting packets.
>> If no TX peer ID is specified, fallback to using the same peer ID both
>> for RX and TX in order to be compatible with the non-multipeer compliant
>> peers.
>>
>> Signed-off-by: Ralf Lici <ralf@mandelbit.com>
>> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
>> ---
>>   Documentation/netlink/specs/ovpn.yaml | 17 ++++++++++++++++-
>>   drivers/net/ovpn/crypto_aead.c        |  2 +-
>>   drivers/net/ovpn/netlink-gen.c        | 13 ++++++++++---
>>   drivers/net/ovpn/netlink-gen.h        |  6 +++---
>>   drivers/net/ovpn/netlink.c            | 14 ++++++++++++--
>>   drivers/net/ovpn/peer.c               |  4 ++++
>>   drivers/net/ovpn/peer.h               |  4 +++-
>>   include/uapi/linux/ovpn.h             |  1 +
>>   8 files changed, 50 insertions(+), 11 deletions(-)
> 
> The patch looks ok, but shouldn't there be a selftest for this
> feature, and a few others in this series (bound device/address, maybe
> the RPF patch as well)?

selftests were indeed extended to check for this feature (and others).
However, since these extensions required some restructuring, I preferred 
to keep all selftests patches for a second PR.

It's obviously always better to have feature+test shipped together, but 
the restructuring on the selftests may require a discussion on its own, 
therefore I decided to go this way.

I hope it makes sense.

Regards,

-- 
Antonio Quartulli
OpenVPN Inc.

Re: [PATCH net-next 5/8] ovpn: add support for asymmetric peer IDs

[Sabrina Dubroca]

2025-11-13, 15:12:41 +0100, Antonio Quartulli wrote:
> Hi Sabrina,
> 
> On 13/11/2025 14:58, Sabrina Dubroca wrote:
> > 2025-11-11, 22:47:38 +0100, Antonio Quartulli wrote:
> > > From: Ralf Lici <ralf@mandelbit.com>
> > > 
> > > In order to support the multipeer architecture, upon connection setup
> > > each side of a tunnel advertises a unique ID that the other side must
> > > include in packets sent to them. Therefore when transmitting a packet, a
> > > peer inserts the recipient's advertised ID for that specific tunnel into
> > > the peer ID field. When receiving a packet, a peer expects to find its
> > > own unique receive ID for that specific tunnel in the peer ID field.
> > > 
> > > Add support for the TX peer ID and embed it into transmitting packets.
> > > If no TX peer ID is specified, fallback to using the same peer ID both
> > > for RX and TX in order to be compatible with the non-multipeer compliant
> > > peers.
> > > 
> > > Signed-off-by: Ralf Lici <ralf@mandelbit.com>
> > > Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
> > > ---
> > >   Documentation/netlink/specs/ovpn.yaml | 17 ++++++++++++++++-
> > >   drivers/net/ovpn/crypto_aead.c        |  2 +-
> > >   drivers/net/ovpn/netlink-gen.c        | 13 ++++++++++---
> > >   drivers/net/ovpn/netlink-gen.h        |  6 +++---
> > >   drivers/net/ovpn/netlink.c            | 14 ++++++++++++--
> > >   drivers/net/ovpn/peer.c               |  4 ++++
> > >   drivers/net/ovpn/peer.h               |  4 +++-
> > >   include/uapi/linux/ovpn.h             |  1 +
> > >   8 files changed, 50 insertions(+), 11 deletions(-)
> > 
> > The patch looks ok, but shouldn't there be a selftest for this
> > feature, and a few others in this series (bound device/address, maybe
> > the RPF patch as well)?
> 
> selftests were indeed extended to check for this feature (and others).
> However, since these extensions required some restructuring, I preferred to
> keep all selftests patches for a second PR.
> 
> It's obviously always better to have feature+test shipped together, but the
> restructuring on the selftests may require a discussion on its own,
> therefore I decided to go this way.
> 
> I hope it makes sense.

Ok, not ideal but I can live with that.

Then for this patch:
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina

[PATCH net-next 6/8] ovpn: consolidate crypto allocations in one chunk

[Antonio Quartulli]

From: Ralf Lici <ralf@mandelbit.com>

Currently ovpn uses three separate dynamically allocated structures to
set up cryptographic operations for both encryption and decryption. This
adds overhead to performance-critical paths and contribute to memory
fragmentation.

This commit consolidates those allocations into a single temporary blob,
similar to what esp_alloc_temp() does.

The resulting performance gain is +7.7% and +4.3% for UDP when using AES
and ChaChaPoly respectively, and +4.3% for TCP.

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/crypto_aead.c | 151 +++++++++++++++++++++++++--------
 drivers/net/ovpn/io.c          |   8 +-
 drivers/net/ovpn/skb.h         |  13 ++-
 3 files changed, 129 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ovpn/crypto_aead.c b/drivers/net/ovpn/crypto_aead.c
index cb6cdf8ec317..9ace27fc130a 100644
--- a/drivers/net/ovpn/crypto_aead.c
+++ b/drivers/net/ovpn/crypto_aead.c
@@ -36,6 +36,105 @@ static int ovpn_aead_encap_overhead(const struct ovpn_crypto_key_slot *ks)
 		crypto_aead_authsize(ks->encrypt);	/* Auth Tag */
 }
 
+/*
+ * ovpn_aead_crypto_tmp_size - compute the size of a temporary object containing
+ *			       an AEAD request structure with extra space for SG
+ *			       and IV.
+ * @tfm: the AEAD cipher handle
+ * @nfrags: the number of fragments in the skb
+ *
+ * This function calculates the size of a contiguous memory block that includes
+ * the initialization vector (IV), the AEAD request, and an array of scatterlist
+ * entries. For alignment considerations, the IV is placed first, followed by
+ * the request, and then the scatterlist.
+ * Additional alignment is applied according to the requirements of the
+ * underlying structures.
+ *
+ * Return: the size of the temporary memory that needs to be allocated
+ */
+static unsigned int ovpn_aead_crypto_tmp_size(struct crypto_aead *tfm,
+					      const unsigned int nfrags)
+{
+	unsigned int len = crypto_aead_ivsize(tfm);
+
+	if (likely(len)) {
+		/* min size for a buffer of ivsize, aligned to alignmask */
+		len += crypto_aead_alignmask(tfm) &
+		       ~(crypto_tfm_ctx_alignment() - 1);
+		/* round up to the next multiple of the crypto context’s alignment */
+		len = ALIGN(len, crypto_tfm_ctx_alignment());
+	}
+
+	/* reserve space for the AEAD request */
+	len += sizeof(struct aead_request) + crypto_aead_reqsize(tfm);
+	/* round up to the next multiple of the struct scatterlist's alignment */
+	len = ALIGN(len, __alignof__(struct scatterlist));
+
+	/* adds enough space for nfrags + 2 scatterlist entries */
+	len += sizeof(struct scatterlist) * (nfrags + 2);
+	return len;
+}
+
+/**
+ * ovpn_aead_crypto_tmp_iv - retrieve the pointer to the IV within a temporary
+ *			     buffer allocated using ovpn_aead_crypto_tmp_size
+ * @aead: the AEAD cipher handle
+ * @tmp: a pointer to the beginning of the temporary buffer
+ *
+ * This function retrieves a pointer to the initialization vector (IV) in the
+ * temporary buffer. If the AEAD cipher specifies an IV size, the pointer is
+ * adjusted using the AEAD's alignment mask to ensure proper alignment.
+ *
+ * Returns: a pointer to the IV within the temporary buffer
+ */
+static inline u8 *ovpn_aead_crypto_tmp_iv(struct crypto_aead *aead, void *tmp)
+{
+	return likely(crypto_aead_ivsize(aead)) ?
+		      PTR_ALIGN((u8 *)tmp, crypto_aead_alignmask(aead) + 1) :
+		      tmp;
+}
+
+/**
+ * ovpn_aead_crypto_tmp_req - retrieve the pointer to the AEAD request structure
+ *			      within a temporary buffer allocated using
+ *			      ovpn_aead_crypto_tmp_size
+ * @aead: the AEAD cipher handle
+ * @iv: a pointer to the initialization vector in the temporary buffer
+ *
+ * This function computes the location of the AEAD request structure that
+ * immediately follows the IV in the temporary buffer and it ensures the request
+ * is aligned to the crypto transform context alignment.
+ *
+ * Returns: a pointer to the AEAD request structure
+ */
+static inline struct aead_request *
+ovpn_aead_crypto_tmp_req(struct crypto_aead *aead, const u8 *iv)
+{
+	return (void *)PTR_ALIGN(iv + crypto_aead_ivsize(aead),
+				 crypto_tfm_ctx_alignment());
+}
+
+/**
+ * ovpn_aead_crypto_req_sg - locate the scatterlist following the AEAD request
+ *			     within a temporary buffer allocated using
+ *			     ovpn_aead_crypto_tmp_size
+ * @aead: the AEAD cipher handle
+ * @req: a pointer to the AEAD request structure in the temporary buffer
+ *
+ * This function computes the starting address of the scatterlist that is
+ * allocated immediately after the AEAD request structure. It aligns the pointer
+ * based on the alignment requirements of the scatterlist structure.
+ *
+ * Returns: a pointer to the scatterlist
+ */
+static inline struct scatterlist *
+ovpn_aead_crypto_req_sg(struct crypto_aead *aead, struct aead_request *req)
+{
+	return (void *)ALIGN((unsigned long)(req + 1) +
+			     crypto_aead_reqsize(aead),
+			     __alignof__(struct scatterlist));
+}
+
 int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 		      struct sk_buff *skb)
 {
@@ -45,6 +144,7 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 	struct scatterlist *sg;
 	int nfrags, ret;
 	u32 pktid, op;
+	void *tmp;
 	u8 *iv;
 
 	ovpn_skb_cb(skb)->peer = peer;
@@ -71,13 +171,15 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 	if (unlikely(nfrags + 2 > (MAX_SKB_FRAGS + 2)))
 		return -ENOSPC;
 
-	/* sg may be required by async crypto */
-	ovpn_skb_cb(skb)->sg = kmalloc(sizeof(*ovpn_skb_cb(skb)->sg) *
-				       (nfrags + 2), GFP_ATOMIC);
-	if (unlikely(!ovpn_skb_cb(skb)->sg))
+	/* allocate temporary memory for iv, sg and req */
+	tmp = kmalloc(ovpn_aead_crypto_tmp_size(ks->encrypt, nfrags),
+		      GFP_ATOMIC);
+	if (unlikely(!tmp))
 		return -ENOMEM;
 
-	sg = ovpn_skb_cb(skb)->sg;
+	iv = ovpn_aead_crypto_tmp_iv(ks->encrypt, tmp);
+	req = ovpn_aead_crypto_tmp_req(ks->encrypt, iv);
+	sg = ovpn_aead_crypto_req_sg(ks->encrypt, req);
 
 	/* sg table:
 	 * 0: op, wire nonce (AD, len=OVPN_OP_SIZE_V2+OVPN_NONCE_WIRE_SIZE),
@@ -105,13 +207,6 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 	if (unlikely(ret < 0))
 		return ret;
 
-	/* iv may be required by async crypto */
-	ovpn_skb_cb(skb)->iv = kmalloc(OVPN_NONCE_SIZE, GFP_ATOMIC);
-	if (unlikely(!ovpn_skb_cb(skb)->iv))
-		return -ENOMEM;
-
-	iv = ovpn_skb_cb(skb)->iv;
-
 	/* concat 4 bytes packet id and 8 bytes nonce tail into 12 bytes
 	 * nonce
 	 */
@@ -130,11 +225,7 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 	/* AEAD Additional data */
 	sg_set_buf(sg, skb->data, OVPN_AAD_SIZE);
 
-	req = aead_request_alloc(ks->encrypt, GFP_ATOMIC);
-	if (unlikely(!req))
-		return -ENOMEM;
-
-	ovpn_skb_cb(skb)->req = req;
+	ovpn_skb_cb(skb)->crypto_tmp = tmp;
 
 	/* setup async crypto operation */
 	aead_request_set_tfm(req, ks->encrypt);
@@ -156,6 +247,7 @@ int ovpn_aead_decrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 	struct aead_request *req;
 	struct sk_buff *trailer;
 	struct scatterlist *sg;
+	void *tmp;
 	u8 *iv;
 
 	payload_offset = OVPN_AAD_SIZE + tag_size;
@@ -184,13 +276,15 @@ int ovpn_aead_decrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 	if (unlikely(nfrags + 2 > (MAX_SKB_FRAGS + 2)))
 		return -ENOSPC;
 
-	/* sg may be required by async crypto */
-	ovpn_skb_cb(skb)->sg = kmalloc(sizeof(*ovpn_skb_cb(skb)->sg) *
-				       (nfrags + 2), GFP_ATOMIC);
-	if (unlikely(!ovpn_skb_cb(skb)->sg))
+	/* allocate temporary memory for iv, sg and req */
+	tmp = kmalloc(ovpn_aead_crypto_tmp_size(ks->decrypt, nfrags),
+		      GFP_ATOMIC);
+	if (unlikely(!tmp))
 		return -ENOMEM;
 
-	sg = ovpn_skb_cb(skb)->sg;
+	iv = ovpn_aead_crypto_tmp_iv(ks->decrypt, tmp);
+	req = ovpn_aead_crypto_tmp_req(ks->decrypt, iv);
+	sg = ovpn_aead_crypto_req_sg(ks->decrypt, req);
 
 	/* sg table:
 	 * 0: op, wire nonce (AD, len=OVPN_OPCODE_SIZE+OVPN_NONCE_WIRE_SIZE),
@@ -213,23 +307,12 @@ int ovpn_aead_decrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 	/* append auth_tag onto scatterlist */
 	sg_set_buf(sg + ret + 1, skb->data + OVPN_AAD_SIZE, tag_size);
 
-	/* iv may be required by async crypto */
-	ovpn_skb_cb(skb)->iv = kmalloc(OVPN_NONCE_SIZE, GFP_ATOMIC);
-	if (unlikely(!ovpn_skb_cb(skb)->iv))
-		return -ENOMEM;
-
-	iv = ovpn_skb_cb(skb)->iv;
-
 	/* copy nonce into IV buffer */
 	memcpy(iv, skb->data + OVPN_OPCODE_SIZE, OVPN_NONCE_WIRE_SIZE);
 	memcpy(iv + OVPN_NONCE_WIRE_SIZE, ks->nonce_tail_recv,
 	       OVPN_NONCE_TAIL_SIZE);
 
-	req = aead_request_alloc(ks->decrypt, GFP_ATOMIC);
-	if (unlikely(!req))
-		return -ENOMEM;
-
-	ovpn_skb_cb(skb)->req = req;
+	ovpn_skb_cb(skb)->crypto_tmp = tmp;
 
 	/* setup async crypto operation */
 	aead_request_set_tfm(req, ks->decrypt);
diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index 3e9e7f8444b3..2721ee8268b2 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -119,9 +119,7 @@ void ovpn_decrypt_post(void *data, int ret)
 	peer = ovpn_skb_cb(skb)->peer;
 
 	/* crypto is done, cleanup skb CB and its members */
-	kfree(ovpn_skb_cb(skb)->iv);
-	kfree(ovpn_skb_cb(skb)->sg);
-	aead_request_free(ovpn_skb_cb(skb)->req);
+	kfree(ovpn_skb_cb(skb)->crypto_tmp);
 
 	if (unlikely(ret < 0))
 		goto drop;
@@ -248,9 +246,7 @@ void ovpn_encrypt_post(void *data, int ret)
 	peer = ovpn_skb_cb(skb)->peer;
 
 	/* crypto is done, cleanup skb CB and its members */
-	kfree(ovpn_skb_cb(skb)->iv);
-	kfree(ovpn_skb_cb(skb)->sg);
-	aead_request_free(ovpn_skb_cb(skb)->req);
+	kfree(ovpn_skb_cb(skb)->crypto_tmp);
 
 	if (unlikely(ret == -ERANGE)) {
 		/* we ran out of IVs and we must kill the key as it can't be
diff --git a/drivers/net/ovpn/skb.h b/drivers/net/ovpn/skb.h
index 64430880f1da..4fb7ea025426 100644
--- a/drivers/net/ovpn/skb.h
+++ b/drivers/net/ovpn/skb.h
@@ -18,12 +18,19 @@
 #include <linux/socket.h>
 #include <linux/types.h>
 
+/**
+ * struct ovpn_cb - ovpn skb control block
+ * @peer: the peer this skb was received from/sent to
+ * @ks: the crypto key slot used to encrypt/decrypt this skb
+ * @crypto_tmp: pointer to temporary memory used for crypto operations
+ *		containing the IV, the scatter gather list and the aead request
+ * @payload_offset: offset in the skb where the payload starts
+ * @nosignal: whether this skb should be sent with the MSG_NOSIGNAL flag (TCP)
+ */
 struct ovpn_cb {
 	struct ovpn_peer *peer;
 	struct ovpn_crypto_key_slot *ks;
-	struct aead_request *req;
-	struct scatterlist *sg;
-	u8 *iv;
+	void *crypto_tmp;
 	unsigned int payload_offset;
 	bool nosignal;
 };
-- 
2.51.0

Re: [PATCH net-next 6/8] ovpn: consolidate crypto allocations in one chunk

[Sabrina Dubroca]

2025-11-11, 22:47:39 +0100, Antonio Quartulli wrote:
> From: Ralf Lici <ralf@mandelbit.com>
> 
> Currently ovpn uses three separate dynamically allocated structures to
> set up cryptographic operations for both encryption and decryption. This
> adds overhead to performance-critical paths and contribute to memory
> fragmentation.
> 
> This commit consolidates those allocations into a single temporary blob,
> similar to what esp_alloc_temp() does.

nit: esp_alloc_tmp (no 'e')

> The resulting performance gain is +7.7% and +4.3% for UDP when using AES
> and ChaChaPoly respectively, and +4.3% for TCP.

Nice improvement! I didn't think it would be that much.


> Signed-off-by: Ralf Lici <ralf@mandelbit.com>
> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

BTW, I didn't see any of these patches posted on the openvpn-devel
list or on netdev before this pull request. Otherwise I'd have
reviewed them earlier.


>  drivers/net/ovpn/crypto_aead.c | 151 +++++++++++++++++++++++++--------
>  drivers/net/ovpn/io.c          |   8 +-
>  drivers/net/ovpn/skb.h         |  13 ++-
>  3 files changed, 129 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/net/ovpn/crypto_aead.c b/drivers/net/ovpn/crypto_aead.c
> index cb6cdf8ec317..9ace27fc130a 100644
> --- a/drivers/net/ovpn/crypto_aead.c
> +++ b/drivers/net/ovpn/crypto_aead.c
> @@ -36,6 +36,105 @@ static int ovpn_aead_encap_overhead(const struct ovpn_crypto_key_slot *ks)
>  		crypto_aead_authsize(ks->encrypt);	/* Auth Tag */
>  }
>  
> +/*

nit: missing a 2nd * to make it kdoc?

> + * ovpn_aead_crypto_tmp_size - compute the size of a temporary object containing
> + *			       an AEAD request structure with extra space for SG
> + *			       and IV.
> + * @tfm: the AEAD cipher handle
> + * @nfrags: the number of fragments in the skb
> + *
> + * This function calculates the size of a contiguous memory block that includes
> + * the initialization vector (IV), the AEAD request, and an array of scatterlist
> + * entries. For alignment considerations, the IV is placed first, followed by
> + * the request, and then the scatterlist.
> + * Additional alignment is applied according to the requirements of the
> + * underlying structures.
> + *
> + * Return: the size of the temporary memory that needs to be allocated
> + */
> +static unsigned int ovpn_aead_crypto_tmp_size(struct crypto_aead *tfm,
> +					      const unsigned int nfrags)
> +{
> +	unsigned int len = crypto_aead_ivsize(tfm);
> +
> +	if (likely(len)) {

Is that right?

Previously iv was reserved with a constant size (OVPN_NONCE_SIZE), and
we're always going to write some data into ->iv via
ovpn_pktid_aead_write, but now we're only reserving the crypto
algorithm's IV size (which appear to be 12, ie OVPN_NONCE_SIZE, for
both chachapoly and gcm(aes), so maybe it doesn't matter).


> @@ -71,13 +171,15 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
>  	if (unlikely(nfrags + 2 > (MAX_SKB_FRAGS + 2)))
>  		return -ENOSPC;
>  
> -	/* sg may be required by async crypto */
> -	ovpn_skb_cb(skb)->sg = kmalloc(sizeof(*ovpn_skb_cb(skb)->sg) *
> -				       (nfrags + 2), GFP_ATOMIC);
> -	if (unlikely(!ovpn_skb_cb(skb)->sg))
> +	/* allocate temporary memory for iv, sg and req */
> +	tmp = kmalloc(ovpn_aead_crypto_tmp_size(ks->encrypt, nfrags),
> +		      GFP_ATOMIC);
> +	if (unlikely(!tmp))
>  		return -ENOMEM;
>  
> -	sg = ovpn_skb_cb(skb)->sg;
> +	iv = ovpn_aead_crypto_tmp_iv(ks->encrypt, tmp);
> +	req = ovpn_aead_crypto_tmp_req(ks->encrypt, iv);
> +	sg = ovpn_aead_crypto_req_sg(ks->encrypt, req);
>  
>  	/* sg table:
>  	 * 0: op, wire nonce (AD, len=OVPN_OP_SIZE_V2+OVPN_NONCE_WIRE_SIZE),
> @@ -105,13 +207,6 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
>  	if (unlikely(ret < 0))
>  		return ret;
>  
> -	/* iv may be required by async crypto */
> -	ovpn_skb_cb(skb)->iv = kmalloc(OVPN_NONCE_SIZE, GFP_ATOMIC);
> -	if (unlikely(!ovpn_skb_cb(skb)->iv))
> -		return -ENOMEM;
> -
> -	iv = ovpn_skb_cb(skb)->iv;
> -
>  	/* concat 4 bytes packet id and 8 bytes nonce tail into 12 bytes
>  	 * nonce
>  	 */
> @@ -130,11 +225,7 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
>  	/* AEAD Additional data */
>  	sg_set_buf(sg, skb->data, OVPN_AAD_SIZE);
>  
> -	req = aead_request_alloc(ks->encrypt, GFP_ATOMIC);
> -	if (unlikely(!req))
> -		return -ENOMEM;
> -
> -	ovpn_skb_cb(skb)->req = req;
> +	ovpn_skb_cb(skb)->crypto_tmp = tmp;

That should be done immediately after the allocation, so that any
failure before this (skb_to_sgvec_nomark, ovpn_pktid_xmit_next) will
not leak this blob? ovpn_aead_encrypt returns directly and lets
ovpn_encrypt_post handle the error and free the memory, but only after
 ->crypto_tmp has been set.

(same thing on the decrypt path)

-- 
Sabrina

Re: [PATCH net-next 6/8] ovpn: consolidate crypto allocations in one chunk

[Antonio Quartulli]

Hi Sabrina,

On 12/11/2025 17:29, Sabrina Dubroca wrote:
> 2025-11-11, 22:47:39 +0100, Antonio Quartulli wrote:
>> From: Ralf Lici <ralf@mandelbit.com>
>>
>> Currently ovpn uses three separate dynamically allocated structures to
>> set up cryptographic operations for both encryption and decryption. This
>> adds overhead to performance-critical paths and contribute to memory
>> fragmentation.
>>
>> This commit consolidates those allocations into a single temporary blob,
>> similar to what esp_alloc_temp() does.
> 
> nit: esp_alloc_tmp (no 'e')
> 
>> The resulting performance gain is +7.7% and +4.3% for UDP when using AES
>> and ChaChaPoly respectively, and +4.3% for TCP.
> 
> Nice improvement! I didn't think it would be that much.

Yep! Quite impressive.

> 
> 
>> Signed-off-by: Ralf Lici <ralf@mandelbit.com>
>> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
> 
> BTW, I didn't see any of these patches posted on the openvpn-devel
> list or on netdev before this pull request. Otherwise I'd have
> reviewed them earlier.

You're right. I worked with Ralf to get this patch done and I directly 
pulled it in my tree.

Next time we will make sure all patches land on the openvpn-devel 
mailing list before being staged for net-next.

This said, Ralf will get back to you on the questions below.

Thanks!

Regards,

> 
> 
>>   drivers/net/ovpn/crypto_aead.c | 151 +++++++++++++++++++++++++--------
>>   drivers/net/ovpn/io.c          |   8 +-
>>   drivers/net/ovpn/skb.h         |  13 ++-
>>   3 files changed, 129 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/net/ovpn/crypto_aead.c b/drivers/net/ovpn/crypto_aead.c
>> index cb6cdf8ec317..9ace27fc130a 100644
>> --- a/drivers/net/ovpn/crypto_aead.c
>> +++ b/drivers/net/ovpn/crypto_aead.c
>> @@ -36,6 +36,105 @@ static int ovpn_aead_encap_overhead(const struct ovpn_crypto_key_slot *ks)
>>   		crypto_aead_authsize(ks->encrypt);	/* Auth Tag */
>>   }
>>   
>> +/*
> 
> nit: missing a 2nd * to make it kdoc?
> 
>> + * ovpn_aead_crypto_tmp_size - compute the size of a temporary object containing
>> + *			       an AEAD request structure with extra space for SG
>> + *			       and IV.
>> + * @tfm: the AEAD cipher handle
>> + * @nfrags: the number of fragments in the skb
>> + *
>> + * This function calculates the size of a contiguous memory block that includes
>> + * the initialization vector (IV), the AEAD request, and an array of scatterlist
>> + * entries. For alignment considerations, the IV is placed first, followed by
>> + * the request, and then the scatterlist.
>> + * Additional alignment is applied according to the requirements of the
>> + * underlying structures.
>> + *
>> + * Return: the size of the temporary memory that needs to be allocated
>> + */
>> +static unsigned int ovpn_aead_crypto_tmp_size(struct crypto_aead *tfm,
>> +					      const unsigned int nfrags)
>> +{
>> +	unsigned int len = crypto_aead_ivsize(tfm);
>> +
>> +	if (likely(len)) {
> 
> Is that right?
> 
> Previously iv was reserved with a constant size (OVPN_NONCE_SIZE), and
> we're always going to write some data into ->iv via
> ovpn_pktid_aead_write, but now we're only reserving the crypto
> algorithm's IV size (which appear to be 12, ie OVPN_NONCE_SIZE, for
> both chachapoly and gcm(aes), so maybe it doesn't matter).
> 
> 
>> @@ -71,13 +171,15 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
>>   	if (unlikely(nfrags + 2 > (MAX_SKB_FRAGS + 2)))
>>   		return -ENOSPC;
>>   
>> -	/* sg may be required by async crypto */
>> -	ovpn_skb_cb(skb)->sg = kmalloc(sizeof(*ovpn_skb_cb(skb)->sg) *
>> -				       (nfrags + 2), GFP_ATOMIC);
>> -	if (unlikely(!ovpn_skb_cb(skb)->sg))
>> +	/* allocate temporary memory for iv, sg and req */
>> +	tmp = kmalloc(ovpn_aead_crypto_tmp_size(ks->encrypt, nfrags),
>> +		      GFP_ATOMIC);
>> +	if (unlikely(!tmp))
>>   		return -ENOMEM;
>>   
>> -	sg = ovpn_skb_cb(skb)->sg;
>> +	iv = ovpn_aead_crypto_tmp_iv(ks->encrypt, tmp);
>> +	req = ovpn_aead_crypto_tmp_req(ks->encrypt, iv);
>> +	sg = ovpn_aead_crypto_req_sg(ks->encrypt, req);
>>   
>>   	/* sg table:
>>   	 * 0: op, wire nonce (AD, len=OVPN_OP_SIZE_V2+OVPN_NONCE_WIRE_SIZE),
>> @@ -105,13 +207,6 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
>>   	if (unlikely(ret < 0))
>>   		return ret;
>>   
>> -	/* iv may be required by async crypto */
>> -	ovpn_skb_cb(skb)->iv = kmalloc(OVPN_NONCE_SIZE, GFP_ATOMIC);
>> -	if (unlikely(!ovpn_skb_cb(skb)->iv))
>> -		return -ENOMEM;
>> -
>> -	iv = ovpn_skb_cb(skb)->iv;
>> -
>>   	/* concat 4 bytes packet id and 8 bytes nonce tail into 12 bytes
>>   	 * nonce
>>   	 */
>> @@ -130,11 +225,7 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
>>   	/* AEAD Additional data */
>>   	sg_set_buf(sg, skb->data, OVPN_AAD_SIZE);
>>   
>> -	req = aead_request_alloc(ks->encrypt, GFP_ATOMIC);
>> -	if (unlikely(!req))
>> -		return -ENOMEM;
>> -
>> -	ovpn_skb_cb(skb)->req = req;
>> +	ovpn_skb_cb(skb)->crypto_tmp = tmp;
> 
> That should be done immediately after the allocation, so that any
> failure before this (skb_to_sgvec_nomark, ovpn_pktid_xmit_next) will
> not leak this blob? ovpn_aead_encrypt returns directly and lets
> ovpn_encrypt_post handle the error and free the memory, but only after
>   ->crypto_tmp has been set.
> 
> (same thing on the decrypt path)
> 

-- 
Antonio Quartulli
OpenVPN Inc.

Re: [PATCH net-next 6/8] ovpn: consolidate crypto allocations in one chunk

[Ralf Lici]

On Wed, 2025-11-12 at 17:29 +0100, Sabrina Dubroca wrote:
> 2025-11-11, 22:47:39 +0100, Antonio Quartulli wrote:
> > diff --git a/drivers/net/ovpn/crypto_aead.c
> > b/drivers/net/ovpn/crypto_aead.c
> > index cb6cdf8ec317..9ace27fc130a 100644
> > --- a/drivers/net/ovpn/crypto_aead.c
> > +++ b/drivers/net/ovpn/crypto_aead.c
> > @@ -36,6 +36,105 @@ static int ovpn_aead_encap_overhead(const struct
> > ovpn_crypto_key_slot *ks)
> >  		crypto_aead_authsize(ks->encrypt);	/* Auth Tag
> > */
> >  }
> >  
> > +/*
> 
> nit: missing a 2nd * to make it kdoc?
> 

ACK.
> 

> > + * ovpn_aead_crypto_tmp_size - compute the size of a temporary
> > object containing
> > + *			       an AEAD request structure with extra
> > space for SG
> > + *			       and IV.
> > + * @tfm: the AEAD cipher handle
> > + * @nfrags: the number of fragments in the skb
> > + *
> > + * This function calculates the size of a contiguous memory block
> > that includes
> > + * the initialization vector (IV), the AEAD request, and an array
> > of scatterlist
> > + * entries. For alignment considerations, the IV is placed first,
> > followed by
> > + * the request, and then the scatterlist.
> > + * Additional alignment is applied according to the requirements of
> > the
> > + * underlying structures.
> > + *
> > + * Return: the size of the temporary memory that needs to be
> > allocated
> > + */
> > +static unsigned int ovpn_aead_crypto_tmp_size(struct crypto_aead
> > *tfm,
> > +					      const unsigned int
> > nfrags)
> > +{
> > +	unsigned int len = crypto_aead_ivsize(tfm);
> > +
> > +	if (likely(len)) {
> 
> Is that right?
> 
> Previously iv was reserved with a constant size (OVPN_NONCE_SIZE), and
> we're always going to write some data into ->iv via
> ovpn_pktid_aead_write, but now we're only reserving the crypto
> algorithm's IV size (which appear to be 12, ie OVPN_NONCE_SIZE, for
> both chachapoly and gcm(aes), so maybe it doesn't matter).

Exactly, I checked and both gcm-aes and chachapoly return an IV size
equal to OVPN_NONCE_SIZE, as you noted. I just thought it wouldn't hurt
to make the function a bit more generic in case we ever support
algorithms without an IV in the future, knowing that OVPN_NONCE_SIZE
matches ivsize for all current cases.

Also, there's a check in ovpn_aead_init to ensure that
crypto_aead_ivsize returns the expected value, so we're covered if
anything changes unexpectedly.

> > @@ -71,13 +171,15 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer,
> > struct ovpn_crypto_key_slot *ks,
> >  	if (unlikely(nfrags + 2 > (MAX_SKB_FRAGS + 2)))
> >  		return -ENOSPC;
> >  
> > -	/* sg may be required by async crypto */
> > -	ovpn_skb_cb(skb)->sg = kmalloc(sizeof(*ovpn_skb_cb(skb)-
> > >sg) *
> > -				       (nfrags + 2), GFP_ATOMIC);
> > -	if (unlikely(!ovpn_skb_cb(skb)->sg))
> > +	/* allocate temporary memory for iv, sg and req */
> > +	tmp = kmalloc(ovpn_aead_crypto_tmp_size(ks->encrypt,
> > nfrags),
> > +		      GFP_ATOMIC);
> > +	if (unlikely(!tmp))
> >  		return -ENOMEM;
> >  
> > -	sg = ovpn_skb_cb(skb)->sg;
> > +	iv = ovpn_aead_crypto_tmp_iv(ks->encrypt, tmp);
> > +	req = ovpn_aead_crypto_tmp_req(ks->encrypt, iv);
> > +	sg = ovpn_aead_crypto_req_sg(ks->encrypt, req);
> >  
> >  	/* sg table:
> >  	 * 0: op, wire nonce (AD,
> > len=OVPN_OP_SIZE_V2+OVPN_NONCE_WIRE_SIZE),
> > @@ -105,13 +207,6 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer,
> > struct ovpn_crypto_key_slot *ks,
> >  	if (unlikely(ret < 0))
> >  		return ret;
> >  
> > -	/* iv may be required by async crypto */
> > -	ovpn_skb_cb(skb)->iv = kmalloc(OVPN_NONCE_SIZE,
> > GFP_ATOMIC);
> > -	if (unlikely(!ovpn_skb_cb(skb)->iv))
> > -		return -ENOMEM;
> > -
> > -	iv = ovpn_skb_cb(skb)->iv;
> > -
> >  	/* concat 4 bytes packet id and 8 bytes nonce tail into 12
> > bytes
> >  	 * nonce
> >  	 */
> > @@ -130,11 +225,7 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer,
> > struct ovpn_crypto_key_slot *ks,
> >  	/* AEAD Additional data */
> >  	sg_set_buf(sg, skb->data, OVPN_AAD_SIZE);
> >  
> > -	req = aead_request_alloc(ks->encrypt, GFP_ATOMIC);
> > -	if (unlikely(!req))
> > -		return -ENOMEM;
> > -
> > -	ovpn_skb_cb(skb)->req = req;
> > +	ovpn_skb_cb(skb)->crypto_tmp = tmp;
> 
> That should be done immediately after the allocation, so that any
> failure before this (skb_to_sgvec_nomark, ovpn_pktid_xmit_next) will
> not leak this blob? ovpn_aead_encrypt returns directly and lets
> ovpn_encrypt_post handle the error and free the memory, but only after
>  ->crypto_tmp has been set.
> 
> (same thing on the decrypt path)

Right, will fix both paths.

-- 
Ralf Lici
Mandelbit Srl

Re: [PATCH net-next 6/8] ovpn: consolidate crypto allocations in one chunk

[Sabrina Dubroca]

2025-11-13, 11:35:07 +0100, Ralf Lici wrote:
> On Wed, 2025-11-12 at 17:29 +0100, Sabrina Dubroca wrote:
> > 2025-11-11, 22:47:39 +0100, Antonio Quartulli wrote:
> > > +static unsigned int ovpn_aead_crypto_tmp_size(struct crypto_aead
> > > *tfm,
> > > +					      const unsigned int
> > > nfrags)
> > > +{
> > > +	unsigned int len = crypto_aead_ivsize(tfm);
> > > +
> > > +	if (likely(len)) {
> > 
> > Is that right?
> > 
> > Previously iv was reserved with a constant size (OVPN_NONCE_SIZE), and
> > we're always going to write some data into ->iv via
> > ovpn_pktid_aead_write, but now we're only reserving the crypto
> > algorithm's IV size (which appear to be 12, ie OVPN_NONCE_SIZE, for
> > both chachapoly and gcm(aes), so maybe it doesn't matter).
> 
> Exactly, I checked and both gcm-aes and chachapoly return an IV size
> equal to OVPN_NONCE_SIZE, as you noted. I just thought it wouldn't hurt
> to make the function a bit more generic in case we ever support
> algorithms without an IV in the future, knowing that OVPN_NONCE_SIZE
> matches ivsize for all current cases.

IMO there's not much to gain here, since the rest of the code
(ovpn_aead_encrypt/decrypt) isn't ready for it. It may even be more
confusing since this bit looks generic but the rest isn't, and
figuring out why the packets are not being encrypted/decrypted
correctly could be a bit painful.

> Also, there's a check in ovpn_aead_init to ensure that
> crypto_aead_ivsize returns the expected value, so we're covered if
> anything changes unexpectedly.

Ah, good.

Then I would prefer to just make ovpn_aead_crypto_tmp_size always use
OVPN_NONCE_SIZE, and maybe add a comment in ovpn_aead_init referencing
ovpn_aead_crypto_tmp_size.

Something like:

/* basic AEAD assumption
 * all current algorithms use OVPN_NONCE_SIZE.
 * ovpn_aead_crypto_tmp_size and ovpn_aead_encrypt/decrypt
 * expect this.
 */


Or a

    DEBUG_NET_WARN_ON_ONCE(OVPN_NONCE_SIZE != crypto_aead_ivsize(tfm));

in ovpn_aead_crypto_tmp_size, which would fire if the check in
ovpn_aead_init is ever removed.


> > > @@ -130,11 +225,7 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer,
> > > struct ovpn_crypto_key_slot *ks,
> > >  	/* AEAD Additional data */
> > >  	sg_set_buf(sg, skb->data, OVPN_AAD_SIZE);
> > >  
> > > -	req = aead_request_alloc(ks->encrypt, GFP_ATOMIC);
> > > -	if (unlikely(!req))
> > > -		return -ENOMEM;
> > > -
> > > -	ovpn_skb_cb(skb)->req = req;
> > > +	ovpn_skb_cb(skb)->crypto_tmp = tmp;
> > 
> > That should be done immediately after the allocation, so that any
> > failure before this (skb_to_sgvec_nomark, ovpn_pktid_xmit_next) will
> > not leak this blob? ovpn_aead_encrypt returns directly and lets
> > ovpn_encrypt_post handle the error and free the memory, but only after
> >  ->crypto_tmp has been set.
> > 
> > (same thing on the decrypt path)
> 
> Right, will fix both paths.

Thanks.

-- 
Sabrina

Re: [PATCH net-next 6/8] ovpn: consolidate crypto allocations in one chunk

[Ralf Lici]

On Thu, 2025-11-13 at 14:48 +0100, Sabrina Dubroca wrote:
> 2025-11-13, 11:35:07 +0100, Ralf Lici wrote:
> > On Wed, 2025-11-12 at 17:29 +0100, Sabrina Dubroca wrote:
> > > 2025-11-11, 22:47:39 +0100, Antonio Quartulli wrote:
> > > > +static unsigned int ovpn_aead_crypto_tmp_size(struct
> > > > crypto_aead
> > > > *tfm,
> > > > +					      const unsigned
> > > > int
> > > > nfrags)
> > > > +{
> > > > +	unsigned int len = crypto_aead_ivsize(tfm);
> > > > +
> > > > +	if (likely(len)) {
> > > 
> > > Is that right?
> > > 
> > > Previously iv was reserved with a constant size (OVPN_NONCE_SIZE),
> > > and
> > > we're always going to write some data into ->iv via
> > > ovpn_pktid_aead_write, but now we're only reserving the crypto
> > > algorithm's IV size (which appear to be 12, ie OVPN_NONCE_SIZE,
> > > for
> > > both chachapoly and gcm(aes), so maybe it doesn't matter).
> > 
> > Exactly, I checked and both gcm-aes and chachapoly return an IV size
> > equal to OVPN_NONCE_SIZE, as you noted. I just thought it wouldn't
> > hurt
> > to make the function a bit more generic in case we ever support
> > algorithms without an IV in the future, knowing that OVPN_NONCE_SIZE
> > matches ivsize for all current cases.
> 
> IMO there's not much to gain here, since the rest of the code
> (ovpn_aead_encrypt/decrypt) isn't ready for it. It may even be more
> confusing since this bit looks generic but the rest isn't, and
> figuring out why the packets are not being encrypted/decrypted
> correctly could be a bit painful.

That’s a good point, actually.

> > Also, there's a check in ovpn_aead_init to ensure that
> > crypto_aead_ivsize returns the expected value, so we're covered if
> > anything changes unexpectedly.
> 
> Ah, good.
> 
> Then I would prefer to just make ovpn_aead_crypto_tmp_size always use
> OVPN_NONCE_SIZE, and maybe add a comment in ovpn_aead_init referencing
> ovpn_aead_crypto_tmp_size.
> 
> Something like:
> 
> /* basic AEAD assumption
>  * all current algorithms use OVPN_NONCE_SIZE.
>  * ovpn_aead_crypto_tmp_size and ovpn_aead_encrypt/decrypt
>  * expect this.
>  */
> 
> 
> Or a
> 
>     DEBUG_NET_WARN_ON_ONCE(OVPN_NONCE_SIZE !=
> crypto_aead_ivsize(tfm));
> 
> in ovpn_aead_crypto_tmp_size, which would fire if the check in
> ovpn_aead_init is ever removed.

I'll go with both to clearly assert the assumption.

Thanks,

-- 
Ralf Lici
Mandelbit Srl

Re: [PATCH net-next 6/8] ovpn: consolidate crypto allocations in one chunk

[Jakub Kicinski]

On Tue, 11 Nov 2025 22:47:39 +0100 Antonio Quartulli wrote:
> +	/* adds enough space for nfrags + 2 scatterlist entries */
> +	len += sizeof(struct scatterlist) * (nfrags + 2);

nit: array_size() ?

> +	return len;
> +}
> +
> +/**
> + * ovpn_aead_crypto_tmp_iv - retrieve the pointer to the IV within a temporary
> + *			     buffer allocated using ovpn_aead_crypto_tmp_size
> + * @aead: the AEAD cipher handle
> + * @tmp: a pointer to the beginning of the temporary buffer
> + *
> + * This function retrieves a pointer to the initialization vector (IV) in the
> + * temporary buffer. If the AEAD cipher specifies an IV size, the pointer is
> + * adjusted using the AEAD's alignment mask to ensure proper alignment.
> + *
> + * Returns: a pointer to the IV within the temporary buffer
> + */
> +static inline u8 *ovpn_aead_crypto_tmp_iv(struct crypto_aead *aead, void *tmp)

nit: does the compiler really not inline this? the long standing kernel
preference is to avoid using "inline" unless it's actually making 
a different. Trivial static function will be inlined anyway.

Re: [PATCH net-next 6/8] ovpn: consolidate crypto allocations in one chunk

[Ralf Lici]

On Thu, 2025-11-13 at 18:25 -0800, Jakub Kicinski wrote:
> On Tue, 11 Nov 2025 22:47:39 +0100 Antonio Quartulli wrote:
> > +	/* adds enough space for nfrags + 2 scatterlist entries */
> > +	len += sizeof(struct scatterlist) * (nfrags + 2);
> 
> nit: array_size() ?

ACK.

> > +	return len;
> > +}
> > +
> > +/**
> > + * ovpn_aead_crypto_tmp_iv - retrieve the pointer to the IV within
> > a temporary
> > + *			     buffer allocated using
> > ovpn_aead_crypto_tmp_size
> > + * @aead: the AEAD cipher handle
> > + * @tmp: a pointer to the beginning of the temporary buffer
> > + *
> > + * This function retrieves a pointer to the initialization vector
> > (IV) in the
> > + * temporary buffer. If the AEAD cipher specifies an IV size, the
> > pointer is
> > + * adjusted using the AEAD's alignment mask to ensure proper
> > alignment.
> > + *
> > + * Returns: a pointer to the IV within the temporary buffer
> > + */
> > +static inline u8 *ovpn_aead_crypto_tmp_iv(struct crypto_aead *aead,
> > void *tmp)
> 
> nit: does the compiler really not inline this? the long standing
> kernel
> preference is to avoid using "inline" unless it's actually making 
> a different. Trivial static function will be inlined anyway.

Got it. I wasn't aware of that kernel preference and was following the
esp implementation.

Thanks.

-- 
Ralf Lici
Mandelbit Srl

[PATCH net-next 7/8] ovpn: use bound device in UDP when available

[Antonio Quartulli]

From: Ralf Lici <ralf@mandelbit.com>

Use the socket’s bound network interface if it’s explicitly specified
via the --bind-dev option in openvpn.

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/udp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
index d6a0f7a0b75d..328819f27e1e 100644
--- a/drivers/net/ovpn/udp.c
+++ b/drivers/net/ovpn/udp.c
@@ -154,6 +154,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
 		.fl4_dport = bind->remote.in4.sin_port,
 		.flowi4_proto = sk->sk_protocol,
 		.flowi4_mark = sk->sk_mark,
+		.flowi4_oif = sk->sk_bound_dev_if,
 	};
 	int ret;
 
@@ -231,7 +232,8 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
 		.fl6_dport = bind->remote.in6.sin6_port,
 		.flowi6_proto = sk->sk_protocol,
 		.flowi6_mark = sk->sk_mark,
-		.flowi6_oif = bind->remote.in6.sin6_scope_id,
+		.flowi6_oif = sk->sk_bound_dev_if ?:
+				      bind->remote.in6.sin6_scope_id,
 	};
 
 	local_bh_disable();
-- 
2.51.0

[PATCH net-next 8/8] ovpn: use bound address in UDP when available

[Antonio Quartulli]

From: Ralf Lici <ralf@mandelbit.com>

Use the socket's locally bound address if it's explicitly specified via
the --local option in openvpn.

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

diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
index 328819f27e1e..42798aca7bce 100644
--- a/drivers/net/ovpn/udp.c
+++ b/drivers/net/ovpn/udp.c
@@ -148,7 +148,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
 {
 	struct rtable *rt;
 	struct flowi4 fl = {
-		.saddr = bind->local.ipv4.s_addr,
+		.saddr = inet_sk(sk)->inet_rcv_saddr ?: bind->local.ipv4.s_addr,
 		.daddr = bind->remote.in4.sin_addr.s_addr,
 		.fl4_sport = inet_sk(sk)->inet_sport,
 		.fl4_dport = bind->remote.in4.sin_port,
@@ -226,7 +226,9 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
 	int ret;
 
 	struct flowi6 fl = {
-		.saddr = bind->local.ipv6,
+		.saddr = ipv6_addr_any(&sk->sk_v6_rcv_saddr) ?
+				 bind->local.ipv6 :
+				 sk->sk_v6_rcv_saddr,
 		.daddr = bind->remote.in6.sin6_addr,
 		.fl6_sport = inet_sk(sk)->inet_sport,
 		.fl6_dport = bind->remote.in6.sin6_port,
-- 
2.51.0