Add basic infrastructure for handling ovpn interfaces.

Tested-by: Donald Hunter <donald.hunter@gmail.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 Documentation/netlink/specs/rt_link.yaml |  16 +++++
 drivers/net/ovpn/Makefile                |   1 +
 drivers/net/ovpn/io.c                    |  22 ++++++
 drivers/net/ovpn/io.h                    |  24 +++++++
 drivers/net/ovpn/main.c                  | 114 +++++++++++++++++++++++++++++--
 drivers/net/ovpn/ovpnpriv.h              |   7 ++
 drivers/net/ovpn/proto.h                 |  38 +++++++++++
 include/uapi/linux/if_link.h             |  15 ++++
 8 files changed, 232 insertions(+), 5 deletions(-)

diff --git a/Documentation/netlink/specs/rt_link.yaml b/Documentation/netlink/specs/rt_link.yaml
index 31238455f8e9d29531884cad4951391fa47ccfaf..a50d9d7d882e7e4f9de29b2a4e7acc602972f6b3 100644
--- a/Documentation/netlink/specs/rt_link.yaml
+++ b/Documentation/netlink/specs/rt_link.yaml
@@ -938,6 +938,12 @@ definitions:
     entries:
       - name: none
       - name: default
+  -
+    name: ovpn-mode
+    type: enum
+    entries:
+      - p2p
+      - mp
 
 attribute-sets:
   -
@@ -2272,6 +2278,13 @@ attribute-sets:
       -
         name: tailroom
         type: u16
+  -
+    name: linkinfo-ovpn-attrs
+    attributes:
+      -
+        name: mode
+        type: u8
+        enum: ovpn-mode
 
 sub-messages:
   -
@@ -2322,6 +2335,9 @@ sub-messages:
       -
         value: netkit
         attribute-set: linkinfo-netkit-attrs
+      -
+        value: ovpn
+        attribute-set: linkinfo-ovpn-attrs
   -
     name: linkinfo-member-data-msg
     formats:
diff --git a/drivers/net/ovpn/Makefile b/drivers/net/ovpn/Makefile
index 75ac62bba02937bc49cb2a0dec5ca3cc31a8ee00..0e5f686672fb5052cee5a2c28797b70859514a7f 100644
--- a/drivers/net/ovpn/Makefile
+++ b/drivers/net/ovpn/Makefile
@@ -8,5 +8,6 @@
 
 obj-$(CONFIG_OVPN) := ovpn.o
 ovpn-y += main.o
+ovpn-y += io.o
 ovpn-y += netlink.o
 ovpn-y += netlink-gen.o
diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
new file mode 100644
index 0000000000000000000000000000000000000000..4b71c38165d7adbb1a2d1a64d27a13b7f76cfbfe
--- /dev/null
+++ b/drivers/net/ovpn/io.c
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2019-2025 OpenVPN, Inc.
+ *
+ *  Author:	James Yonan <james@openvpn.net>
+ *		Antonio Quartulli <antonio@openvpn.net>
+ */
+
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+
+#include "io.h"
+
+/* Send user data to the network
+ */
+netdev_tx_t ovpn_net_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	skb_tx_error(skb);
+	kfree_skb(skb);
+	return NET_XMIT_DROP;
+}
diff --git a/drivers/net/ovpn/io.h b/drivers/net/ovpn/io.h
new file mode 100644
index 0000000000000000000000000000000000000000..afea5f81f5628dcb9afda9a78974bbf6f2101c13
--- /dev/null
+++ b/drivers/net/ovpn/io.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* OpenVPN data channel offload
+ *
+ *  Copyright (C) 2019-2025 OpenVPN, Inc.
+ *
+ *  Author:	James Yonan <james@openvpn.net>
+ *		Antonio Quartulli <antonio@openvpn.net>
+ */
+
+#ifndef _NET_OVPN_OVPN_H_
+#define _NET_OVPN_OVPN_H_
+
+/* DATA_V2 header size with AEAD encryption */
+#define OVPN_HEAD_ROOM (OVPN_OPCODE_SIZE + OVPN_NONCE_WIRE_SIZE +	   \
+			16 /* AEAD TAG length */ +			   \
+			max(sizeof(struct udphdr), sizeof(struct tcphdr)) +\
+			max(sizeof(struct ipv6hdr), sizeof(struct iphdr)))
+
+/* max padding required by encryption */
+#define OVPN_MAX_PADDING 16
+
+netdev_tx_t ovpn_net_xmit(struct sk_buff *skb, struct net_device *dev);
+
+#endif /* _NET_OVPN_OVPN_H_ */
diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
index 28133e7e15e74b8a4a937ed03f70d9f83d7a14c8..e71183e6f42cd801861caaec9eb0f6828b64cda9 100644
--- a/drivers/net/ovpn/main.c
+++ b/drivers/net/ovpn/main.c
@@ -10,14 +10,42 @@
 #include <linux/genetlink.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
+#include <linux/inetdevice.h>
+#include <net/ip.h>
 #include <net/rtnetlink.h>
-#include <uapi/linux/ovpn.h>
+#include <uapi/linux/if_arp.h>
 
 #include "ovpnpriv.h"
 #include "main.h"
 #include "netlink.h"
+#include "io.h"
+#include "proto.h"
+
+static int ovpn_net_open(struct net_device *dev)
+{
+	netif_tx_start_all_queues(dev);
+	return 0;
+}
+
+static int ovpn_net_stop(struct net_device *dev)
+{
+	netif_tx_stop_all_queues(dev);
+	return 0;
+}
 
 static const struct net_device_ops ovpn_netdev_ops = {
+	.ndo_open		= ovpn_net_open,
+	.ndo_stop		= ovpn_net_stop,
+	.ndo_start_xmit		= ovpn_net_xmit,
+};
+
+static const struct device_type ovpn_type = {
+	.name = OVPN_FAMILY_NAME,
+};
+
+static const struct nla_policy ovpn_policy[IFLA_OVPN_MAX + 1] = {
+	[IFLA_OVPN_MODE] = NLA_POLICY_RANGE(NLA_U8, OVPN_MODE_P2P,
+					    OVPN_MODE_MP),
 };
 
 /**
@@ -31,44 +59,120 @@ bool ovpn_dev_is_valid(const struct net_device *dev)
 	return dev->netdev_ops == &ovpn_netdev_ops;
 }
 
+static void ovpn_setup(struct net_device *dev)
+{
+	netdev_features_t feat = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
+				 NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
+				 NETIF_F_HIGHDMA;
+
+	dev->needs_free_netdev = true;
+
+	dev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
+
+	dev->netdev_ops = &ovpn_netdev_ops;
+
+	dev->hard_header_len = 0;
+	dev->addr_len = 0;
+	dev->mtu = ETH_DATA_LEN - OVPN_HEAD_ROOM;
+	dev->min_mtu = IPV4_MIN_MTU;
+	dev->max_mtu = IP_MAX_MTU - OVPN_HEAD_ROOM;
+
+	dev->type = ARPHRD_NONE;
+	dev->flags = IFF_POINTOPOINT | IFF_NOARP;
+	dev->priv_flags |= IFF_NO_QUEUE;
+
+	dev->lltx = true;
+	dev->features |= feat;
+	dev->hw_features |= feat;
+	dev->hw_enc_features |= feat;
+
+	dev->needed_headroom = ALIGN(OVPN_HEAD_ROOM, 4);
+	dev->needed_tailroom = OVPN_MAX_PADDING;
+
+	SET_NETDEV_DEVTYPE(dev, &ovpn_type);
+}
+
 static int ovpn_newlink(struct net_device *dev,
 			struct rtnl_newlink_params *params,
 			struct netlink_ext_ack *extack)
 {
-	return -EOPNOTSUPP;
+	struct ovpn_priv *ovpn = netdev_priv(dev);
+	struct nlattr **data = params->data;
+	enum ovpn_mode mode = OVPN_MODE_P2P;
+
+	if (data && data[IFLA_OVPN_MODE]) {
+		mode = nla_get_u8(data[IFLA_OVPN_MODE]);
+		netdev_dbg(dev, "setting device mode: %u\n", mode);
+	}
+
+	ovpn->dev = dev;
+	ovpn->mode = mode;
+
+	/* turn carrier explicitly off after registration, this way state is
+	 * clearly defined
+	 */
+	netif_carrier_off(dev);
+
+	return register_netdevice(dev);
+}
+
+static int ovpn_fill_info(struct sk_buff *skb, const struct net_device *dev)
+{
+	struct ovpn_priv *ovpn = netdev_priv(dev);
+
+	if (nla_put_u8(skb, IFLA_OVPN_MODE, ovpn->mode))
+		return -EMSGSIZE;
+
+	return 0;
 }
 
 static struct rtnl_link_ops ovpn_link_ops = {
 	.kind = "ovpn",
 	.netns_refund = false,
+	.priv_size = sizeof(struct ovpn_priv),
+	.setup = ovpn_setup,
+	.policy = ovpn_policy,
+	.maxtype = IFLA_OVPN_MAX,
 	.newlink = ovpn_newlink,
 	.dellink = unregister_netdevice_queue,
+	.fill_info = ovpn_fill_info,
 };
 
 static int ovpn_netdev_notifier_call(struct notifier_block *nb,
 				     unsigned long state, void *ptr)
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+	struct ovpn_priv *ovpn;
 
 	if (!ovpn_dev_is_valid(dev))
 		return NOTIFY_DONE;
 
+	ovpn = netdev_priv(dev);
+
 	switch (state) {
 	case NETDEV_REGISTER:
-		/* add device to internal list for later destruction upon
-		 * unregistration
-		 */
+		ovpn->registered = true;
 		break;
 	case NETDEV_UNREGISTER:
+		/* twiddle thumbs on netns device moves */
+		if (dev->reg_state != NETREG_UNREGISTERING)
+			break;
+
 		/* can be delivered multiple times, so check registered flag,
 		 * then destroy the interface
 		 */
+		if (!ovpn->registered)
+			return NOTIFY_DONE;
+
+		netif_carrier_off(dev);
+		ovpn->registered = false;
 		break;
 	case NETDEV_POST_INIT:
 	case NETDEV_GOING_DOWN:
 	case NETDEV_DOWN:
 	case NETDEV_UP:
 	case NETDEV_PRE_UP:
+		break;
 	default:
 		return NOTIFY_DONE;
 	}
diff --git a/drivers/net/ovpn/ovpnpriv.h b/drivers/net/ovpn/ovpnpriv.h
index f9322536b06d6baa5524de57cd7d69f5ecbbd194..33c2a41edf9b3204e8aebd2679649cb7158f05f2 100644
--- a/drivers/net/ovpn/ovpnpriv.h
+++ b/drivers/net/ovpn/ovpnpriv.h
@@ -10,12 +10,19 @@
 #ifndef _NET_OVPN_OVPNSTRUCT_H_
 #define _NET_OVPN_OVPNSTRUCT_H_
 
+#include <uapi/linux/if_link.h>
+#include <uapi/linux/ovpn.h>
+
 /**
  * struct ovpn_priv - per ovpn interface state
  * @dev: the actual netdev representing the tunnel
+ * @registered: whether dev is still registered with netdev or not
+ * @mode: device operation mode (i.e. p2p, mp, ..)
  */
 struct ovpn_priv {
 	struct net_device *dev;
+	bool registered;
+	enum ovpn_mode mode;
 };
 
 #endif /* _NET_OVPN_OVPNSTRUCT_H_ */
diff --git a/drivers/net/ovpn/proto.h b/drivers/net/ovpn/proto.h
new file mode 100644
index 0000000000000000000000000000000000000000..5f95a78bebd3702868ffeeab3ea4938e957d568c
--- /dev/null
+++ b/drivers/net/ovpn/proto.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2020-2025 OpenVPN, Inc.
+ *
+ *  Author:	Antonio Quartulli <antonio@openvpn.net>
+ *		James Yonan <james@openvpn.net>
+ */
+
+#ifndef _NET_OVPN_PROTO_H_
+#define _NET_OVPN_PROTO_H_
+
+/* When the OpenVPN protocol is ran in AEAD mode, use
+ * the OpenVPN packet ID as the AEAD nonce:
+ *
+ *    00000005 521c3b01 4308c041
+ *    [seq # ] [  nonce_tail   ]
+ *    [     12-byte full IV    ] -> OVPN_NONCE_SIZE
+ *    [4-bytes                   -> OVPN_NONCE_WIRE_SIZE
+ *    on wire]
+ */
+
+/* nonce size (96bits) as required by AEAD ciphers */
+#define OVPN_NONCE_SIZE			12
+/* last 8 bytes of AEAD nonce: provided by userspace and usually derived
+ * from key material generated during TLS handshake
+ */
+#define OVPN_NONCE_TAIL_SIZE		8
+
+/* OpenVPN nonce size reduced by 8-byte nonce tail -- this is the
+ * size of the AEAD Associated Data (AD) sent over the wire
+ * and is normally the head of the IV
+ */
+#define OVPN_NONCE_WIRE_SIZE (OVPN_NONCE_SIZE - OVPN_NONCE_TAIL_SIZE)
+
+#define OVPN_OPCODE_SIZE		4 /* DATA_V2 opcode size */
+
+#endif /* _NET_OVPN_PROTO_H_ */
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 318386cc5b0d19ed6a37734feffb450353dd9440..3ad2d5d9803479a10a6b2cfab2df98ce0f823926 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1986,4 +1986,19 @@ enum {
 
 #define IFLA_DSA_MAX	(__IFLA_DSA_MAX - 1)
 
+/* OVPN section */
+
+enum ovpn_mode {
+	OVPN_MODE_P2P,
+	OVPN_MODE_MP,
+};
+
+enum {
+	IFLA_OVPN_UNSPEC,
+	IFLA_OVPN_MODE,
+	__IFLA_OVPN_MAX,
+};
+
+#define IFLA_OVPN_MAX	(__IFLA_OVPN_MAX - 1)
+
 #endif /* _UAPI_LINUX_IF_LINK_H */

-- 
2.48.1

Hi Antonio,

On Wed, 12 Mar 2025 21:54:32 +0100, Antonio Quartulli Wrote:
> diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
> index 28133e7e15e74b8a4a937ed03f70d9f83d7a14c8..e71183e6f42cd801861caaec9eb0f6828b64cda9 100644
> --- a/drivers/net/ovpn/main.c
> +++ b/drivers/net/ovpn/main.c
> @@ -10,14 +10,42 @@
>  #include <linux/genetlink.h>
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
> +#include <linux/inetdevice.h>
> +#include <net/ip.h>
>  #include <net/rtnetlink.h>
> -#include <uapi/linux/ovpn.h>
> +#include <uapi/linux/if_arp.h>
>  
>  #include "ovpnpriv.h"
>  #include "main.h"
>  #include "netlink.h"
> +#include "io.h"
> +#include "proto.h"
> +
> +static int ovpn_net_open(struct net_device *dev)
> +{
> +	netif_tx_start_all_queues(dev);

This is not required as the virtual interface does not have a queue
(marked as IFF_NO_QUEUE).

> +	return 0;
> +}
> +
> +static int ovpn_net_stop(struct net_device *dev)
> +{
> +	netif_tx_stop_all_queues(dev);

Same as above.

> +	return 0;
> +}
>  
>  static const struct net_device_ops ovpn_netdev_ops = {
> +	.ndo_open		= ovpn_net_open,
> +	.ndo_stop		= ovpn_net_stop,
> +	.ndo_start_xmit		= ovpn_net_xmit,
> +};
> +
> +static const struct device_type ovpn_type = {
> +	.name = OVPN_FAMILY_NAME,
> +};
> +
> +static const struct nla_policy ovpn_policy[IFLA_OVPN_MAX + 1] = {
> +	[IFLA_OVPN_MODE] = NLA_POLICY_RANGE(NLA_U8, OVPN_MODE_P2P,
> +					    OVPN_MODE_MP),
>  };
>  
>  /**
> @@ -31,44 +59,120 @@ bool ovpn_dev_is_valid(const struct net_device *dev)
>  	return dev->netdev_ops == &ovpn_netdev_ops;
>  }
>  
> +static void ovpn_setup(struct net_device *dev)
> +{
> +	netdev_features_t feat = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM |

Do not advertise NETIF_F_HW_CSUM or NETIF_F_RXCSUM, as TX/RX checksum is
not handled in hardware.

> +				 NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
> +				 NETIF_F_HIGHDMA;
> +
> +	dev->needs_free_netdev = true;
> +
> +	dev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
> +
> +	dev->netdev_ops = &ovpn_netdev_ops;
> +
> +	dev->hard_header_len = 0;
> +	dev->addr_len = 0;
> +	dev->mtu = ETH_DATA_LEN - OVPN_HEAD_ROOM;
> +	dev->min_mtu = IPV4_MIN_MTU;
> +	dev->max_mtu = IP_MAX_MTU - OVPN_HEAD_ROOM;
> +
> +	dev->type = ARPHRD_NONE;
> +	dev->flags = IFF_POINTOPOINT | IFF_NOARP;
> +	dev->priv_flags |= IFF_NO_QUEUE;
> +
> +	dev->lltx = true;
> +	dev->features |= feat;
> +	dev->hw_features |= feat;
> +	dev->hw_enc_features |= feat;
> +
> +	dev->needed_headroom = ALIGN(OVPN_HEAD_ROOM, 4);
> +	dev->needed_tailroom = OVPN_MAX_PADDING;
> +
> +	SET_NETDEV_DEVTYPE(dev, &ovpn_type);
> +}
> +
>  static int ovpn_newlink(struct net_device *dev,
>  			struct rtnl_newlink_params *params,
>  			struct netlink_ext_ack *extack)
>  {
> -	return -EOPNOTSUPP;
> +	struct ovpn_priv *ovpn = netdev_priv(dev);
> +	struct nlattr **data = params->data;
> +	enum ovpn_mode mode = OVPN_MODE_P2P;
> +
> +	if (data && data[IFLA_OVPN_MODE]) {
> +		mode = nla_get_u8(data[IFLA_OVPN_MODE]);
> +		netdev_dbg(dev, "setting device mode: %u\n", mode);
> +	}
> +
> +	ovpn->dev = dev;
> +	ovpn->mode = mode;
> +
> +	/* turn carrier explicitly off after registration, this way state is
> +	 * clearly defined
> +	 */
> +	netif_carrier_off(dev);
> +
> +	return register_netdevice(dev);
> +}
> +
> +static int ovpn_fill_info(struct sk_buff *skb, const struct net_device *dev)
> +{
> +	struct ovpn_priv *ovpn = netdev_priv(dev);
> +
> +	if (nla_put_u8(skb, IFLA_OVPN_MODE, ovpn->mode))
> +		return -EMSGSIZE;
> +
> +	return 0;
>  }
>  
>  static struct rtnl_link_ops ovpn_link_ops = {
>  	.kind = "ovpn",
>  	.netns_refund = false,
> +	.priv_size = sizeof(struct ovpn_priv),
> +	.setup = ovpn_setup,
> +	.policy = ovpn_policy,
> +	.maxtype = IFLA_OVPN_MAX,
>  	.newlink = ovpn_newlink,
>  	.dellink = unregister_netdevice_queue,
> +	.fill_info = ovpn_fill_info,
>  };
>  
>  static int ovpn_netdev_notifier_call(struct notifier_block *nb,
>  				     unsigned long state, void *ptr)
>  {
>  	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> +	struct ovpn_priv *ovpn;
>  
>  	if (!ovpn_dev_is_valid(dev))
>  		return NOTIFY_DONE;
>  
> +	ovpn = netdev_priv(dev);
> +
>  	switch (state) {
>  	case NETDEV_REGISTER:
> -		/* add device to internal list for later destruction upon
> -		 * unregistration
> -		 */
> +		ovpn->registered = true;
>  		break;
>  	case NETDEV_UNREGISTER:
> +		/* twiddle thumbs on netns device moves */
> +		if (dev->reg_state != NETREG_UNREGISTERING)
> +			break;
> +
>  		/* can be delivered multiple times, so check registered flag,
>  		 * then destroy the interface
>  		 */
> +		if (!ovpn->registered)
> +			return NOTIFY_DONE;
> +
> +		netif_carrier_off(dev);
> +		ovpn->registered = false;
>  		break;
>  	case NETDEV_POST_INIT:
>  	case NETDEV_GOING_DOWN:
>  	case NETDEV_DOWN:
>  	case NETDEV_UP:
>  	case NETDEV_PRE_UP:
> +		break;
>  	default:
>  		return NOTIFY_DONE;
>  	}
> diff --git a/drivers/net/ovpn/ovpnpriv.h b/drivers/net/ovpn/ovpnpriv.h
> index f9322536b06d6baa5524de57cd7d69f5ecbbd194..33c2a41edf9b3204e8aebd2679649cb7158f05f2 100644
> --- a/drivers/net/ovpn/ovpnpriv.h
> +++ b/drivers/net/ovpn/ovpnpriv.h
> @@ -10,12 +10,19 @@
>  #ifndef _NET_OVPN_OVPNSTRUCT_H_
>  #define _NET_OVPN_OVPNSTRUCT_H_
>  
> +#include <uapi/linux/if_link.h>
> +#include <uapi/linux/ovpn.h>
> +
>  /**
>   * struct ovpn_priv - per ovpn interface state
>   * @dev: the actual netdev representing the tunnel
> + * @registered: whether dev is still registered with netdev or not
> + * @mode: device operation mode (i.e. p2p, mp, ..)
>   */
>  struct ovpn_priv {
>  	struct net_device *dev;
> +	bool registered;
> +	enum ovpn_mode mode;
>  };
>  
>  #endif /* _NET_OVPN_OVPNSTRUCT_H_ */
> diff --git a/drivers/net/ovpn/proto.h b/drivers/net/ovpn/proto.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..5f95a78bebd3702868ffeeab3ea4938e957d568c
> --- /dev/null
> +++ b/drivers/net/ovpn/proto.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*  OpenVPN data channel offload
> + *
> + *  Copyright (C) 2020-2025 OpenVPN, Inc.
> + *
> + *  Author:	Antonio Quartulli <antonio@openvpn.net>
> + *		James Yonan <james@openvpn.net>
> + */
> +
> +#ifndef _NET_OVPN_PROTO_H_
> +#define _NET_OVPN_PROTO_H_
> +
> +/* When the OpenVPN protocol is ran in AEAD mode, use
> + * the OpenVPN packet ID as the AEAD nonce:
> + *
> + *    00000005 521c3b01 4308c041
> + *    [seq # ] [  nonce_tail   ]
> + *    [     12-byte full IV    ] -> OVPN_NONCE_SIZE
> + *    [4-bytes                   -> OVPN_NONCE_WIRE_SIZE
> + *    on wire]
> + */
> +
> +/* nonce size (96bits) as required by AEAD ciphers */
> +#define OVPN_NONCE_SIZE			12
> +/* last 8 bytes of AEAD nonce: provided by userspace and usually derived
> + * from key material generated during TLS handshake
> + */
> +#define OVPN_NONCE_TAIL_SIZE		8
> +
> +/* OpenVPN nonce size reduced by 8-byte nonce tail -- this is the
> + * size of the AEAD Associated Data (AD) sent over the wire
> + * and is normally the head of the IV
> + */
> +#define OVPN_NONCE_WIRE_SIZE (OVPN_NONCE_SIZE - OVPN_NONCE_TAIL_SIZE)
> +
> +#define OVPN_OPCODE_SIZE		4 /* DATA_V2 opcode size */
> +
> +#endif /* _NET_OVPN_PROTO_H_ */
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 318386cc5b0d19ed6a37734feffb450353dd9440..3ad2d5d9803479a10a6b2cfab2df98ce0f823926 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -1986,4 +1986,19 @@ enum {
>  
>  #define IFLA_DSA_MAX	(__IFLA_DSA_MAX - 1)
>  
> +/* OVPN section */
> +
> +enum ovpn_mode {
> +	OVPN_MODE_P2P,
> +	OVPN_MODE_MP,
> +};
> +
> +enum {
> +	IFLA_OVPN_UNSPEC,
> +	IFLA_OVPN_MODE,
> +	__IFLA_OVPN_MAX,
> +};
> +
> +#define IFLA_OVPN_MAX	(__IFLA_OVPN_MAX - 1)
> +
>  #endif /* _UAPI_LINUX_IF_LINK_H */
> 

-- Qingfang

Hi Qingfang and thanks for chiming in,

On 17/03/2025 07:09, Qingfang Deng wrote:
> Hi Antonio,
> 
> On Wed, 12 Mar 2025 21:54:32 +0100, Antonio Quartulli Wrote:
>> diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
>> index 28133e7e15e74b8a4a937ed03f70d9f83d7a14c8..e71183e6f42cd801861caaec9eb0f6828b64cda9 100644
>> --- a/drivers/net/ovpn/main.c
>> +++ b/drivers/net/ovpn/main.c
>> @@ -10,14 +10,42 @@
>>   #include <linux/genetlink.h>
>>   #include <linux/module.h>
>>   #include <linux/netdevice.h>
>> +#include <linux/inetdevice.h>
>> +#include <net/ip.h>
>>   #include <net/rtnetlink.h>
>> -#include <uapi/linux/ovpn.h>
>> +#include <uapi/linux/if_arp.h>
>>   
>>   #include "ovpnpriv.h"
>>   #include "main.h"
>>   #include "netlink.h"
>> +#include "io.h"
>> +#include "proto.h"
>> +
>> +static int ovpn_net_open(struct net_device *dev)
>> +{
>> +	netif_tx_start_all_queues(dev);
> 
> This is not required as the virtual interface does not have a queue
> (marked as IFF_NO_QUEUE).

ACK

> 
>> +	return 0;
>> +}
>> +
>> +static int ovpn_net_stop(struct net_device *dev)
>> +{
>> +	netif_tx_stop_all_queues(dev);
> 
> Same as above.

ACK

> 
>> +	return 0;
>> +}
>>   
>>   static const struct net_device_ops ovpn_netdev_ops = {
>> +	.ndo_open		= ovpn_net_open,
>> +	.ndo_stop		= ovpn_net_stop,
>> +	.ndo_start_xmit		= ovpn_net_xmit,
>> +};
>> +
>> +static const struct device_type ovpn_type = {
>> +	.name = OVPN_FAMILY_NAME,
>> +};
>> +
>> +static const struct nla_policy ovpn_policy[IFLA_OVPN_MAX + 1] = {
>> +	[IFLA_OVPN_MODE] = NLA_POLICY_RANGE(NLA_U8, OVPN_MODE_P2P,
>> +					    OVPN_MODE_MP),
>>   };
>>   
>>   /**
>> @@ -31,44 +59,120 @@ bool ovpn_dev_is_valid(const struct net_device *dev)
>>   	return dev->netdev_ops == &ovpn_netdev_ops;
>>   }
>>   
>> +static void ovpn_setup(struct net_device *dev)
>> +{
>> +	netdev_features_t feat = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
> 
> Do not advertise NETIF_F_HW_CSUM or NETIF_F_RXCSUM, as TX/RX checksum is
> not handled in hardware.

The idea behind these flags was that the OpenVPN protocol will take care 
of authenticating packets, thus substituting what the CSUM would do here.
For this I wanted to avoid the stack to spend time computing the CSUM in 
software.

I believe wireguard sets those flags for the same reason.

Does it make sense to you?

> 
>> +				 NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
>> +				 NETIF_F_HIGHDMA;


Regards,

-- 
Antonio Quartulli
OpenVPN Inc.

Hi Antonio,

On Mon, Mar 17, 2025 at 5:23 PM Antonio Quartulli <antonio@openvpn.net> wrote:
> >> +static void ovpn_setup(struct net_device *dev)
> >> +{
> >> +    netdev_features_t feat = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
> >
> > Do not advertise NETIF_F_HW_CSUM or NETIF_F_RXCSUM, as TX/RX checksum is
> > not handled in hardware.
>
> The idea behind these flags was that the OpenVPN protocol will take care
> of authenticating packets, thus substituting what the CSUM would do here.
> For this I wanted to avoid the stack to spend time computing the CSUM in
> software.

For the RX part (NETIF_F_RXCSUM), you might be correct, but in patch
08 you wrote:
> /* we can't guarantee the packet wasn't corrupted before entering the
> * VPN, therefore we give other layers a chance to check that
> */
> skb->ip_summed = CHECKSUM_NONE;

So NETIF_F_RXCSUM has no effect.

For the TX part (NETIF_F_HW_CSUM) however, I believe wireguard made
the same mistake.
Your code both contains the pattern:

if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb)) // ...

NETIF_F_HW_CSUM causes the upper layers to send packets with
CHECKSUM_PARTIAL, assuming hardware offload will complete the
checksum, but if skb_checksum_help(skb) is invoked, the checksum is
still computed in software. This means there's no real benefit unless
there's an actual hardware offload mechanism.

+Cc: zx2c4

>
> I believe wireguard sets those flags for the same reason.
>
> Does it make sense to you?
>
> >
> >> +                             NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
> >> +                             NETIF_F_HIGHDMA;
>
>
> Regards,
>
> --
> Antonio Quartulli
> OpenVPN Inc.
>

On 17/03/2025 10:41, Qingfang Deng wrote:
> Hi Antonio,
> 
> On Mon, Mar 17, 2025 at 5:23 PM Antonio Quartulli <antonio@openvpn.net> wrote:
>>>> +static void ovpn_setup(struct net_device *dev)
>>>> +{
>>>> +    netdev_features_t feat = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
>>>
>>> Do not advertise NETIF_F_HW_CSUM or NETIF_F_RXCSUM, as TX/RX checksum is
>>> not handled in hardware.
>>
>> The idea behind these flags was that the OpenVPN protocol will take care
>> of authenticating packets, thus substituting what the CSUM would do here.
>> For this I wanted to avoid the stack to spend time computing the CSUM in
>> software.
> 
> For the RX part (NETIF_F_RXCSUM), you might be correct, but in patch
> 08 you wrote:
>> /* we can't guarantee the packet wasn't corrupted before entering the
>> * VPN, therefore we give other layers a chance to check that
>> */
>> skb->ip_summed = CHECKSUM_NONE;

Right. This was the result after a lengthy discussion with Sabrina.
Despite authenticating what enters the tunnel, we indeed concluded it is 
better to let the stack verify that what entered was not corrupted.

> 
> So NETIF_F_RXCSUM has no effect.

Does it mean I can drop NETIF_F_RXCSUM and also the line

skb->ip_summed = CHECKSUM_NONE;

at the same time?

> 
> For the TX part (NETIF_F_HW_CSUM) however, I believe wireguard made
> the same mistake.
> Your code both contains the pattern:
> 
> if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb)) // ...
> 
> NETIF_F_HW_CSUM causes the upper layers to send packets with
> CHECKSUM_PARTIAL, assuming hardware offload will complete the
> checksum, but if skb_checksum_help(skb) is invoked, the checksum is
> still computed in software. This means there's no real benefit unless
> there's an actual hardware offload mechanism.

Got it.
Then as per your suggestion I can drop both NETIF_F_HW_CSUM and the 
if/call to skb_checksum_help().

Regards,

> 
> +Cc: zx2c4
> 
>>
>> I believe wireguard sets those flags for the same reason.
>>
>> Does it make sense to you?
>>
>>>
>>>> +                             NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
>>>> +                             NETIF_F_HIGHDMA;
>>
>>
>> Regards,
>>
>> --
>> Antonio Quartulli
>> OpenVPN Inc.
>>

-- 
Antonio Quartulli
OpenVPN Inc.

On Mon, Mar 17, 2025 at 6:00 PM Antonio Quartulli <antonio@openvpn.net> wrote:
>
> On 17/03/2025 10:41, Qingfang Deng wrote:
> > Hi Antonio,
> >
> > On Mon, Mar 17, 2025 at 5:23 PM Antonio Quartulli <antonio@openvpn.net> wrote:
> >>>> +static void ovpn_setup(struct net_device *dev)
> >>>> +{
> >>>> +    netdev_features_t feat = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
> >>>
> >>> Do not advertise NETIF_F_HW_CSUM or NETIF_F_RXCSUM, as TX/RX checksum is
> >>> not handled in hardware.
> >>
> >> The idea behind these flags was that the OpenVPN protocol will take care
> >> of authenticating packets, thus substituting what the CSUM would do here.
> >> For this I wanted to avoid the stack to spend time computing the CSUM in
> >> software.
> >
> > For the RX part (NETIF_F_RXCSUM), you might be correct, but in patch
> > 08 you wrote:
> >> /* we can't guarantee the packet wasn't corrupted before entering the
> >> * VPN, therefore we give other layers a chance to check that
> >> */
> >> skb->ip_summed = CHECKSUM_NONE;
>
> Right. This was the result after a lengthy discussion with Sabrina.
> Despite authenticating what enters the tunnel, we indeed concluded it is
> better to let the stack verify that what entered was not corrupted.
>
> >
> > So NETIF_F_RXCSUM has no effect.
>
> Does it mean I can drop NETIF_F_RXCSUM and also the line
>
> skb->ip_summed = CHECKSUM_NONE;
>
> at the same time?

I don't think so. skb->ip_summed might have been set to
CHECKSUM_UNNECESSARY on the lower layer with UDP/TCP RX checksum.

>
> >
> > For the TX part (NETIF_F_HW_CSUM) however, I believe wireguard made
> > the same mistake.
> > Your code both contains the pattern:
> >
> > if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb)) // ...
> >
> > NETIF_F_HW_CSUM causes the upper layers to send packets with
> > CHECKSUM_PARTIAL, assuming hardware offload will complete the
> > checksum, but if skb_checksum_help(skb) is invoked, the checksum is
> > still computed in software. This means there's no real benefit unless
> > there's an actual hardware offload mechanism.
>
> Got it.
> Then as per your suggestion I can drop both NETIF_F_HW_CSUM and the
> if/call to skb_checksum_help().
>
> Regards,
>
> >
> > +Cc: zx2c4
> >
> >>
> >> I believe wireguard sets those flags for the same reason.
> >>
> >> Does it make sense to you?
> >>
> >>>
> >>>> +                             NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
> >>>> +                             NETIF_F_HIGHDMA;
> >>
> >>
> >> Regards,
> >>
> >> --
> >> Antonio Quartulli
> >> OpenVPN Inc.
> >>
>
> --
> Antonio Quartulli
> OpenVPN Inc.
>

On 17/03/2025 11:10, Qingfang Deng wrote:
> On Mon, Mar 17, 2025 at 6:00 PM Antonio Quartulli <antonio@openvpn.net> wrote:
>>
>> On 17/03/2025 10:41, Qingfang Deng wrote:
>>> Hi Antonio,
>>>
>>> On Mon, Mar 17, 2025 at 5:23 PM Antonio Quartulli <antonio@openvpn.net> wrote:
>>>>>> +static void ovpn_setup(struct net_device *dev)
>>>>>> +{
>>>>>> +    netdev_features_t feat = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
>>>>>
>>>>> Do not advertise NETIF_F_HW_CSUM or NETIF_F_RXCSUM, as TX/RX checksum is
>>>>> not handled in hardware.
>>>>
>>>> The idea behind these flags was that the OpenVPN protocol will take care
>>>> of authenticating packets, thus substituting what the CSUM would do here.
>>>> For this I wanted to avoid the stack to spend time computing the CSUM in
>>>> software.
>>>
>>> For the RX part (NETIF_F_RXCSUM), you might be correct, but in patch
>>> 08 you wrote:
>>>> /* we can't guarantee the packet wasn't corrupted before entering the
>>>> * VPN, therefore we give other layers a chance to check that
>>>> */
>>>> skb->ip_summed = CHECKSUM_NONE;
>>
>> Right. This was the result after a lengthy discussion with Sabrina.
>> Despite authenticating what enters the tunnel, we indeed concluded it is
>> better to let the stack verify that what entered was not corrupted.
>>
>>>
>>> So NETIF_F_RXCSUM has no effect.
>>
>> Does it mean I can drop NETIF_F_RXCSUM and also the line
>>
>> skb->ip_summed = CHECKSUM_NONE;
>>
>> at the same time?
> 
> I don't think so. skb->ip_summed might have been set to
> CHECKSUM_UNNECESSARY on the lower layer with UDP/TCP RX checksum.

Makes sense.

Ok, thanks!

Regards,


-- 
Antonio Quartulli
OpenVPN Inc.