diff mbox series

[net-next,v2,03/22] ovpn: add basic netlink support

Message ID 20240304150914.11444-4-antonio@openvpn.net (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Introducing OpenVPN Data Channel Offload | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches (and no cover letter)
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 941 this patch: 947
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: openvpn-devel@lists.sourceforge.net
netdev/build_clang fail Errors and warnings before: 959 this patch: 965
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api warning Found: 'dev_put(' was: 0 now: 2
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 957 this patch: 963
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 81 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns WARNING: line length of 89 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc fail Errors and warnings before: 0 this patch: 6
netdev/source_inline success Was 0 now: 0

Commit Message

Antonio Quartulli March 4, 2024, 3:08 p.m. UTC
This commit introduces basic netlink support with
registration/unregistration functionalities and stub pre/post-doit.

More importantly it introduces the UAPI header file that contains
the attributes that are inteded to be used by the netlink API
implementation.

For convience, packet.h is also added containing some macros about
the OpenVPN packet format.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/Makefile     |   1 +
 drivers/net/ovpn/main.c       |  15 +++
 drivers/net/ovpn/netlink.c    | 229 ++++++++++++++++++++++++++++++++++
 drivers/net/ovpn/netlink.h    |  18 +++
 drivers/net/ovpn/ovpnstruct.h |  20 +++
 drivers/net/ovpn/packet.h     |  42 +++++++
 include/uapi/linux/ovpn.h     | 174 ++++++++++++++++++++++++++
 7 files changed, 499 insertions(+)
 create mode 100644 drivers/net/ovpn/netlink.c
 create mode 100644 drivers/net/ovpn/netlink.h
 create mode 100644 drivers/net/ovpn/ovpnstruct.h
 create mode 100644 drivers/net/ovpn/packet.h
 create mode 100644 include/uapi/linux/ovpn.h

Comments

Andrew Lunn March 4, 2024, 9:20 p.m. UTC | #1
On Mon, Mar 04, 2024 at 04:08:54PM +0100, Antonio Quartulli wrote:
> This commit introduces basic netlink support with
> registration/unregistration functionalities and stub pre/post-doit.
> 
> More importantly it introduces the UAPI header file that contains
> the attributes that are inteded to be used by the netlink API

intended.

> implementation.
> 
> For convience, packet.h is also added containing some macros about
> the OpenVPN packet format.
> 
> +/** KEYDIR policy. Can be used for configuring an encryption and a decryption key */
> +static const struct nla_policy ovpn_nl_policy_keydir[NUM_OVPN_A_KEYDIR] = {
> +	[OVPN_A_KEYDIR_CIPHER_KEY] = NLA_POLICY_MAX_LEN(U8_MAX),

I don't know netlink that well. Is this saying keys are limited to 256
bytes? How future proof is that? I'm not a crypto person, but
symmetric algorithms, e.g. AES, seem to have reasonably short keys, 32
bytes, so this seems O.K, to me.

> +	[OVPN_A_KEYDIR_NONCE_TAIL] = NLA_POLICY_EXACT_LEN(NONCE_TAIL_SIZE),
> +};
> +
> +/** KEYCONF policy */
> +static const struct nla_policy ovpn_nl_policy_keyconf[NUM_OVPN_A_KEYCONF] = {
> +	[OVPN_A_KEYCONF_SLOT] = NLA_POLICY_RANGE(NLA_U8, __OVPN_KEY_SLOT_FIRST,
> +						 NUM_OVPN_KEY_SLOT - 1),
> +	[OVPN_A_KEYCONF_KEY_ID] = { .type = NLA_U8 },

Is that 256 keys globally, or just associated to one session?

> +	[OVPN_A_KEYCONF_CIPHER_ALG] = { .type = NLA_U16 },
> +	[OVPN_A_KEYCONF_ENCRYPT_DIR] = NLA_POLICY_NESTED(ovpn_nl_policy_keydir),
> +	[OVPN_A_KEYCONF_DECRYPT_DIR] = NLA_POLICY_NESTED(ovpn_nl_policy_keydir),
> +};
> +

> +/** Generic message container policy */
> +static const struct nla_policy ovpn_nl_policy[NUM_OVPN_A] = {
> +	[OVPN_A_IFINDEX] = { .type = NLA_U32 },
> +	[OVPN_A_IFNAME] = NLA_POLICY_MAX_LEN(IFNAMSIZ),

Generally, ifnames are not passed around, only ifindex. An interface
can have multiple names:

12: enlr0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq master br0 state DOWN group default qlen 1000
    link/ether 3c:ec:ef:7e:0a:90 brd ff:ff:ff:ff:ff:ff
    altname enp183s0f2
    altname eno7

It is better to let userspace figure out the name from the index,
since the name is mostly a user space concept.

> +	[OVPN_A_MODE] = NLA_POLICY_RANGE(NLA_U8, __OVPN_MODE_FIRST,
> +					 NUM_OVPN_MODE - 1),
> +	[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_nl_policy_peer),
> +};

> +static int ovpn_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
> +			 struct genl_info *info)
> +{
> +	struct net *net = genl_info_net(info);
> +	struct net_device *dev;
> +
> +	/* the OVPN_CMD_NEW_IFACE command is different from the rest as it
> +	 * just expects an IFNAME, while all the others expect an IFINDEX
> +	 */

Could you explain that some more. In general, the name should not
matter to the kernel, udev/systemd might rename it soon after creation
etc. If it gets moved into a network namespace it might need renaming
etc.

> +enum ovpn_nl_peer_attrs {
> +	OVPN_A_PEER_UNSPEC = 0,
> +	OVPN_A_PEER_ID,
> +	OVPN_A_PEER_RX_STATS,
> +	OVPN_A_PEER_TX_STATS,

Probably answered later in the patch series: What sort of statistics
do you expect here. Don't overlap any of the normal network statistics
with this here, please use the existing kAPIs for those. Anything you
return here need to be very specific to ovpn.

> +	OVPN_A_PEER_VPN_RX_BYTES,
> +	OVPN_A_PEER_VPN_TX_BYTES,
> +	OVPN_A_PEER_VPN_RX_PACKETS,
> +	OVPN_A_PEER_VPN_TX_PACKETS,
> +	OVPN_A_PEER_LINK_RX_BYTES,
> +	OVPN_A_PEER_LINK_TX_BYTES,
> +	OVPN_A_PEER_LINK_RX_PACKETS,
> +	OVPN_A_PEER_LINK_TX_PACKETS,

How do these differ to standard network statistics? e.g. what is in
/sys/class/net/*/statistics/ ?

	Andrew
kernel test robot March 5, 2024, 10:49 a.m. UTC | #2
Hi Antonio,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Antonio-Quartulli/netlink-add-NLA_POLICY_MAX_LEN-macro/20240304-232835
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240304150914.11444-4-antonio%40openvpn.net
patch subject: [PATCH net-next v2 03/22] ovpn: add basic netlink support
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20240305/202403051838.YHYbFiGD-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240305/202403051838.YHYbFiGD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403051838.YHYbFiGD-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ovpn/netlink.c:82: warning: Function parameter or struct member 'net' not described in 'ovpn_get_dev_from_attrs'
>> drivers/net/ovpn/netlink.c:82: warning: Function parameter or struct member 'attrs' not described in 'ovpn_get_dev_from_attrs'
>> drivers/net/ovpn/netlink.c:181: warning: Function parameter or struct member 'nb' not described in 'ovpn_nl_notify'
>> drivers/net/ovpn/netlink.c:181: warning: Function parameter or struct member 'state' not described in 'ovpn_nl_notify'
>> drivers/net/ovpn/netlink.c:181: warning: Function parameter or struct member '_notify' not described in 'ovpn_nl_notify'
>> drivers/net/ovpn/netlink.c:193: warning: Function parameter or struct member 'ovpn' not described in 'ovpn_nl_init'


vim +82 drivers/net/ovpn/netlink.c

    76	
    77	/**
    78	 * ovpn_get_dev_from_attrs() - retrieve the netdevice a netlink message is targeting
    79	 */
    80	static struct net_device *
    81	ovpn_get_dev_from_attrs(struct net *net, struct nlattr **attrs)
  > 82	{
    83		struct net_device *dev;
    84		int ifindex;
    85	
    86		if (!attrs[OVPN_A_IFINDEX])
    87			return ERR_PTR(-EINVAL);
    88	
    89		ifindex = nla_get_u32(attrs[OVPN_A_IFINDEX]);
    90	
    91		dev = dev_get_by_index(net, ifindex);
    92		if (!dev)
    93			return ERR_PTR(-ENODEV);
    94	
    95		if (!ovpn_dev_is_valid(dev))
    96			goto err_put_dev;
    97	
    98		return dev;
    99	
   100	err_put_dev:
   101		dev_put(dev);
   102	
   103		return ERR_PTR(-EINVAL);
   104	}
   105	
   106	/**
   107	 * ovpn_pre_doit() - Prepare ovpn genl doit request
   108	 * @ops: requested netlink operation
   109	 * @skb: Netlink message with request data
   110	 * @info: receiver information
   111	 *
   112	 * Return: 0 on success or negative error number in case of failure
   113	 */
   114	static int ovpn_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
   115				 struct genl_info *info)
   116	{
   117		struct net *net = genl_info_net(info);
   118		struct net_device *dev;
   119	
   120		/* the OVPN_CMD_NEW_IFACE command is different from the rest as it
   121		 * just expects an IFNAME, while all the others expect an IFINDEX
   122		 */
   123		if (info->genlhdr->cmd == OVPN_CMD_NEW_IFACE) {
   124			if (!info->attrs[OVPN_A_IFNAME]) {
   125				GENL_SET_ERR_MSG(info, "no interface name specified");
   126				return -EINVAL;
   127			}
   128			return 0;
   129		}
   130	
   131		dev = ovpn_get_dev_from_attrs(net, info->attrs);
   132		if (IS_ERR(dev))
   133			return PTR_ERR(dev);
   134	
   135		info->user_ptr[0] = netdev_priv(dev);
   136	
   137		return 0;
   138	}
   139	
   140	/**
   141	 * ovpn_post_doit() - complete ovpn genl doit request
   142	 * @ops: requested netlink operation
   143	 * @skb: Netlink message with request data
   144	 * @info: receiver information
   145	 */
   146	static void ovpn_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
   147				   struct genl_info *info)
   148	{
   149		struct ovpn_struct *ovpn;
   150	
   151		ovpn = info->user_ptr[0];
   152		/* in case of OVPN_CMD_NEW_IFACE, there is no pre-stored device */
   153		if (ovpn)
   154			dev_put(ovpn->dev);
   155	}
   156	
   157	static const struct genl_small_ops ovpn_nl_ops[] = {
   158	};
   159	
   160	static struct genl_family ovpn_nl_family __ro_after_init = {
   161		.hdrsize = 0,
   162		.name = OVPN_NL_NAME,
   163		.version = 1,
   164		.maxattr = NUM_OVPN_A + 1,
   165		.policy = ovpn_nl_policy,
   166		.netnsok = true,
   167		.pre_doit = ovpn_pre_doit,
   168		.post_doit = ovpn_post_doit,
   169		.module = THIS_MODULE,
   170		.small_ops = ovpn_nl_ops,
   171		.n_small_ops = ARRAY_SIZE(ovpn_nl_ops),
   172		.mcgrps = ovpn_nl_mcgrps,
   173		.n_mcgrps = ARRAY_SIZE(ovpn_nl_mcgrps),
   174	};
   175	
   176	/**
   177	 * ovpn_nl_notify() - react to openvpn userspace process exit
   178	 */
   179	static int ovpn_nl_notify(struct notifier_block *nb, unsigned long state,
   180				  void *_notify)
 > 181	{
   182		return NOTIFY_DONE;
   183	}
   184	
   185	static struct notifier_block ovpn_nl_notifier = {
   186		.notifier_call = ovpn_nl_notify,
   187	};
   188	
   189	/**
   190	 * ovpn_nl_init() - perform any ovpn specific netlink initialization
   191	 */
   192	int ovpn_nl_init(struct ovpn_struct *ovpn)
 > 193	{
   194		return 0;
   195	}
   196
Antonio Quartulli March 5, 2024, 3:47 p.m. UTC | #3
On 04/03/2024 22:20, Andrew Lunn wrote:
> On Mon, Mar 04, 2024 at 04:08:54PM +0100, Antonio Quartulli wrote:
>> This commit introduces basic netlink support with
>> registration/unregistration functionalities and stub pre/post-doit.
>>
>> More importantly it introduces the UAPI header file that contains
>> the attributes that are inteded to be used by the netlink API
> 
> intended.
> 
>> implementation.
>>
>> For convience, packet.h is also added containing some macros about
>> the OpenVPN packet format.
>>
>> +/** KEYDIR policy. Can be used for configuring an encryption and a decryption key */
>> +static const struct nla_policy ovpn_nl_policy_keydir[NUM_OVPN_A_KEYDIR] = {
>> +	[OVPN_A_KEYDIR_CIPHER_KEY] = NLA_POLICY_MAX_LEN(U8_MAX),
> 
> I don't know netlink that well. Is this saying keys are limited to 256
> bytes? How future proof is that? I'm not a crypto person, but
> symmetric algorithms, e.g. AES, seem to have reasonably short keys, 32
> bytes, so this seems O.K, to me.

256 bytes should be reasonably large. I don't see anything beyond this 
size appearing anytime soon or at all.

> 
>> +	[OVPN_A_KEYDIR_NONCE_TAIL] = NLA_POLICY_EXACT_LEN(NONCE_TAIL_SIZE),
>> +};
>> +
>> +/** KEYCONF policy */
>> +static const struct nla_policy ovpn_nl_policy_keyconf[NUM_OVPN_A_KEYCONF] = {
>> +	[OVPN_A_KEYCONF_SLOT] = NLA_POLICY_RANGE(NLA_U8, __OVPN_KEY_SLOT_FIRST,
>> +						 NUM_OVPN_KEY_SLOT - 1),
>> +	[OVPN_A_KEYCONF_KEY_ID] = { .type = NLA_U8 },
> 
> Is that 256 keys globally, or just associated to one session?

This is specific to one peer, however, the OpenVPN protocol uses IDs up 
7, therefore U8 is just the smallest unit I could use to fit those few 
values.

> 
>> +	[OVPN_A_KEYCONF_CIPHER_ALG] = { .type = NLA_U16 },
>> +	[OVPN_A_KEYCONF_ENCRYPT_DIR] = NLA_POLICY_NESTED(ovpn_nl_policy_keydir),
>> +	[OVPN_A_KEYCONF_DECRYPT_DIR] = NLA_POLICY_NESTED(ovpn_nl_policy_keydir),
>> +};
>> +
> 
>> +/** Generic message container policy */
>> +static const struct nla_policy ovpn_nl_policy[NUM_OVPN_A] = {
>> +	[OVPN_A_IFINDEX] = { .type = NLA_U32 },
>> +	[OVPN_A_IFNAME] = NLA_POLICY_MAX_LEN(IFNAMSIZ),
> 
> Generally, ifnames are not passed around, only ifindex. An interface
> can have multiple names:
> 
> 12: enlr0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq master br0 state DOWN group default qlen 1000
>      link/ether 3c:ec:ef:7e:0a:90 brd ff:ff:ff:ff:ff:ff
>      altname enp183s0f2
>      altname eno7
> 
> It is better to let userspace figure out the name from the index,
> since the name is mostly a user space concept.

This is strictly related to your next question.
Please see my answer below.

> 
>> +	[OVPN_A_MODE] = NLA_POLICY_RANGE(NLA_U8, __OVPN_MODE_FIRST,
>> +					 NUM_OVPN_MODE - 1),
>> +	[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_nl_policy_peer),
>> +};
> 
>> +static int ovpn_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
>> +			 struct genl_info *info)
>> +{
>> +	struct net *net = genl_info_net(info);
>> +	struct net_device *dev;
>> +
>> +	/* the OVPN_CMD_NEW_IFACE command is different from the rest as it
>> +	 * just expects an IFNAME, while all the others expect an IFINDEX
>> +	 */
> 
> Could you explain that some more. In general, the name should not
> matter to the kernel, udev/systemd might rename it soon after creation
> etc. If it gets moved into a network namespace it might need renaming
> etc.

In a previous discussion it was agreed that we should create ovpn 
interfaces via GENL and not via RTNL.

For this reason ovpn needs userspace to send the name to give the 
interface upon creation. This name is just passed to the networking 
stack upon creation/registration, but it is not stored anywhere else.

Subsequent netlink calls are then all performed by passing an ifindex.

Hence, OVPN_CMD_NEW_IFACE is the only GENL command that required the 
IFNAME to be specified.

Does it make sense?


> 
>> +enum ovpn_nl_peer_attrs {
>> +	OVPN_A_PEER_UNSPEC = 0,
>> +	OVPN_A_PEER_ID,
>> +	OVPN_A_PEER_RX_STATS,
>> +	OVPN_A_PEER_TX_STATS,
> 
> Probably answered later in the patch series: What sort of statistics
> do you expect here. Don't overlap any of the normal network statistics
> with this here, please use the existing kAPIs for those. Anything you
> return here need to be very specific to ovpn.

Actually you found a leftover from an old approach.
OVPN_A_PEER_RX_STATS and OVPN_A_PEER_TX_STATS shall be removed.

The actual stats we store are those below:

> 
>> +	OVPN_A_PEER_VPN_RX_BYTES,
>> +	OVPN_A_PEER_VPN_TX_BYTES,
>> +	OVPN_A_PEER_VPN_RX_PACKETS,
>> +	OVPN_A_PEER_VPN_TX_PACKETS,
>> +	OVPN_A_PEER_LINK_RX_BYTES,
>> +	OVPN_A_PEER_LINK_TX_BYTES,
>> +	OVPN_A_PEER_LINK_RX_PACKETS,
>> +	OVPN_A_PEER_LINK_TX_PACKETS,
> 
> How do these differ to standard network statistics? e.g. what is in
> /sys/class/net/*/statistics/ ?

The first difference is that these stats are per-peer and not 
per-device. Behind each device there might be multiple peers connected.

This way ovpn is able to tell how much data was sent/received by every 
single connected peer.

LINK and VPN store different values.
LINK stats are recorded at the transport layer (before decapsulation or 
after encapsulation), while VPN stats are recorded at the tunnel layer 
(after decapsulation or before encapsulation).

I didn't see how to convey the same information using the standard 
statistics.

Regards,
Andrew Lunn March 5, 2024, 4:23 p.m. UTC | #4
> > 12: enlr0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq master br0 state DOWN group default qlen 1000
> >      link/ether 3c:ec:ef:7e:0a:90 brd ff:ff:ff:ff:ff:ff
> >      altname enp183s0f2
> >      altname eno7
> > 
> > It is better to let userspace figure out the name from the index,
> > since the name is mostly a user space concept.
> 
> This is strictly related to your next question.
> Please see my answer below.
> 
> > 
> > > +	[OVPN_A_MODE] = NLA_POLICY_RANGE(NLA_U8, __OVPN_MODE_FIRST,
> > > +					 NUM_OVPN_MODE - 1),
> > > +	[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_nl_policy_peer),
> > > +};
> > 
> > > +static int ovpn_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
> > > +			 struct genl_info *info)
> > > +{
> > > +	struct net *net = genl_info_net(info);
> > > +	struct net_device *dev;
> > > +
> > > +	/* the OVPN_CMD_NEW_IFACE command is different from the rest as it
> > > +	 * just expects an IFNAME, while all the others expect an IFINDEX
> > > +	 */
> > 
> > Could you explain that some more. In general, the name should not
> > matter to the kernel, udev/systemd might rename it soon after creation
> > etc. If it gets moved into a network namespace it might need renaming
> > etc.
> 
> In a previous discussion it was agreed that we should create ovpn interfaces
> via GENL and not via RTNL.
> 
> For this reason ovpn needs userspace to send the name to give the interface
> upon creation. This name is just passed to the networking stack upon
> creation/registration, but it is not stored anywhere else.
> 
> Subsequent netlink calls are then all performed by passing an ifindex.
> 
> Hence, OVPN_CMD_NEW_IFACE is the only GENL command that required the IFNAME
> to be specified.

I don't really see why GENL vs RTNL makes a difference. The reply to
the request can contain the ifindex of the newly created interface. If
you set the name to "ovpn%d" before calling register_netdevice() the
kernel will find the next free unique ovpn interface name, race
free. So you could have multiple openvpn daemon running, and not have
to worry about races when creating interfaces.

Jakub has been raising questions about this recently for another
patchset. He might comment on this.

> > > +	OVPN_A_PEER_VPN_RX_BYTES,
> > > +	OVPN_A_PEER_VPN_TX_BYTES,
> > > +	OVPN_A_PEER_VPN_RX_PACKETS,
> > > +	OVPN_A_PEER_VPN_TX_PACKETS,
> > > +	OVPN_A_PEER_LINK_RX_BYTES,
> > > +	OVPN_A_PEER_LINK_TX_BYTES,
> > > +	OVPN_A_PEER_LINK_RX_PACKETS,
> > > +	OVPN_A_PEER_LINK_TX_PACKETS,
> > 
> > How do these differ to standard network statistics? e.g. what is in
> > /sys/class/net/*/statistics/ ?
> 
> The first difference is that these stats are per-peer and not per-device.
> Behind each device there might be multiple peers connected.
> 
> This way ovpn is able to tell how much data was sent/received by every
> single connected peer.
> 
> LINK and VPN store different values.
> LINK stats are recorded at the transport layer (before decapsulation or
> after encapsulation), while VPN stats are recorded at the tunnel layer
> (after decapsulation or before encapsulation).
> 
> I didn't see how to convey the same information using the standard
> statistics.

Right, so this in general makes sense. The only question i have now
is, should you be using rtnl_link_stats64. That is the standard
structure for interface statistics.

Again, Jakub likes to comment about statistics...

And in general, maybe add more comments. This is the UAPI, it needs to
be clear and unambiguous. Documentation/networking/ethtool-netlink.rst.

       Andrew
Jakub Kicinski March 5, 2024, 7:39 p.m. UTC | #5
On Tue, 5 Mar 2024 17:23:25 +0100 Andrew Lunn wrote:
> > > > +static int ovpn_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
> > > > +			 struct genl_info *info)
> > > > +{
> > > > +	struct net *net = genl_info_net(info);
> > > > +	struct net_device *dev;
> > > > +
> > > > +	/* the OVPN_CMD_NEW_IFACE command is different from the rest as it
> > > > +	 * just expects an IFNAME, while all the others expect an IFINDEX
> > > > +	 */  
> > > 
> > > Could you explain that some more. In general, the name should not
> > > matter to the kernel, udev/systemd might rename it soon after creation
> > > etc. If it gets moved into a network namespace it might need renaming
> > > etc.  
> > 
> > In a previous discussion it was agreed that we should create ovpn interfaces
> > via GENL and not via RTNL.
> > 
> > For this reason ovpn needs userspace to send the name to give the interface
> > upon creation. This name is just passed to the networking stack upon
> > creation/registration, but it is not stored anywhere else.
> > 
> > Subsequent netlink calls are then all performed by passing an ifindex.
> > 
> > Hence, OVPN_CMD_NEW_IFACE is the only GENL command that required the IFNAME
> > to be specified.  
> 
> I don't really see why GENL vs RTNL makes a difference. The reply to
> the request can contain the ifindex of the newly created interface. If
> you set the name to "ovpn%d" before calling register_netdevice() the
> kernel will find the next free unique ovpn interface name, race
> free. So you could have multiple openvpn daemon running, and not have
> to worry about races when creating interfaces.
> 
> Jakub has been raising questions about this recently for another
> patchset. He might comment on this.

FWIW using ifindex for most ops sounds like the right way to go.
Passing the name to create sounds fine, but as Andrew said, we
should default to "ovpn%d" instead of forcing the user to specify 
the name (and you can echo back the allocated name in the reply
to OVPN_CMD_NEW_IFACE).

Somewhat related - if you require an attr - GENL_REQ_ATTR_CHECK(),
it does the extact setting for you.

> > > > +	OVPN_A_PEER_VPN_RX_BYTES,
> > > > +	OVPN_A_PEER_VPN_TX_BYTES,
> > > > +	OVPN_A_PEER_VPN_RX_PACKETS,
> > > > +	OVPN_A_PEER_VPN_TX_PACKETS,
> > > > +	OVPN_A_PEER_LINK_RX_BYTES,
> > > > +	OVPN_A_PEER_LINK_TX_BYTES,
> > > > +	OVPN_A_PEER_LINK_RX_PACKETS,
> > > > +	OVPN_A_PEER_LINK_TX_PACKETS,  
> > > 
> > > How do these differ to standard network statistics? e.g. what is in
> > > /sys/class/net/*/statistics/ ?  
> > 
> > The first difference is that these stats are per-peer and not per-device.
> > Behind each device there might be multiple peers connected.
> > 
> > This way ovpn is able to tell how much data was sent/received by every
> > single connected peer.
> > 
> > LINK and VPN store different values.
> > LINK stats are recorded at the transport layer (before decapsulation or
> > after encapsulation), while VPN stats are recorded at the tunnel layer
> > (after decapsulation or before encapsulation).
> > 
> > I didn't see how to convey the same information using the standard
> > statistics.  
> 
> Right, so this in general makes sense. The only question i have now
> is, should you be using rtnl_link_stats64. That is the standard
> structure for interface statistics.
> 
> Again, Jakub likes to comment about statistics...
> 
> And in general, maybe add more comments. This is the UAPI, it needs to
> be clear and unambiguous. Documentation/networking/ethtool-netlink.rst.

Or put enough docs in the YAML spec as those comments get rendered in
the uAPI header and in HTML docs :)
Antonio Quartulli March 6, 2024, 2:46 p.m. UTC | #6
On 05/03/2024 20:39, Jakub Kicinski wrote:
> On Tue, 5 Mar 2024 17:23:25 +0100 Andrew Lunn wrote:
>>>>> +static int ovpn_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
>>>>> +			 struct genl_info *info)
>>>>> +{
>>>>> +	struct net *net = genl_info_net(info);
>>>>> +	struct net_device *dev;
>>>>> +
>>>>> +	/* the OVPN_CMD_NEW_IFACE command is different from the rest as it
>>>>> +	 * just expects an IFNAME, while all the others expect an IFINDEX
>>>>> +	 */
>>>>
>>>> Could you explain that some more. In general, the name should not
>>>> matter to the kernel, udev/systemd might rename it soon after creation
>>>> etc. If it gets moved into a network namespace it might need renaming
>>>> etc.
>>>
>>> In a previous discussion it was agreed that we should create ovpn interfaces
>>> via GENL and not via RTNL.
>>>
>>> For this reason ovpn needs userspace to send the name to give the interface
>>> upon creation. This name is just passed to the networking stack upon
>>> creation/registration, but it is not stored anywhere else.
>>>
>>> Subsequent netlink calls are then all performed by passing an ifindex.
>>>
>>> Hence, OVPN_CMD_NEW_IFACE is the only GENL command that required the IFNAME
>>> to be specified.
>>
>> I don't really see why GENL vs RTNL makes a difference. The reply to
>> the request can contain the ifindex of the newly created interface. If
>> you set the name to "ovpn%d" before calling register_netdevice() the
>> kernel will find the next free unique ovpn interface name, race
>> free. So you could have multiple openvpn daemon running, and not have
>> to worry about races when creating interfaces.
>>
>> Jakub has been raising questions about this recently for another
>> patchset. He might comment on this.
> 
> FWIW using ifindex for most ops sounds like the right way to go.
> Passing the name to create sounds fine, but as Andrew said, we
> should default to "ovpn%d" instead of forcing the user to specify
> the name (and you can echo back the allocated name in the reply
> to OVPN_CMD_NEW_IFACE).
> 

Ok, it definitely makes sense and actually makes userspace code simpler.


> Somewhat related - if you require an attr - GENL_REQ_ATTR_CHECK(),
> it does the extact setting for you.

ok, will check it out!

> 
>>>>> +	OVPN_A_PEER_VPN_RX_BYTES,
>>>>> +	OVPN_A_PEER_VPN_TX_BYTES,
>>>>> +	OVPN_A_PEER_VPN_RX_PACKETS,
>>>>> +	OVPN_A_PEER_VPN_TX_PACKETS,
>>>>> +	OVPN_A_PEER_LINK_RX_BYTES,
>>>>> +	OVPN_A_PEER_LINK_TX_BYTES,
>>>>> +	OVPN_A_PEER_LINK_RX_PACKETS,
>>>>> +	OVPN_A_PEER_LINK_TX_PACKETS,
>>>>
>>>> How do these differ to standard network statistics? e.g. what is in
>>>> /sys/class/net/*/statistics/ ?
>>>
>>> The first difference is that these stats are per-peer and not per-device.
>>> Behind each device there might be multiple peers connected.
>>>
>>> This way ovpn is able to tell how much data was sent/received by every
>>> single connected peer.
>>>
>>> LINK and VPN store different values.
>>> LINK stats are recorded at the transport layer (before decapsulation or
>>> after encapsulation), while VPN stats are recorded at the tunnel layer
>>> (after decapsulation or before encapsulation).
>>>
>>> I didn't see how to convey the same information using the standard
>>> statistics.
>>
>> Right, so this in general makes sense. The only question i have now
>> is, should you be using rtnl_link_stats64. That is the standard
>> structure for interface statistics.

@Andrew: do you see how I could return/send this object per-peer to 
userspace?
I think the whole interface stats logic is based on the 
one-stats-per-device concept. Hence, I thought it was meaningful to just 
send my own stats via netlink.

>>
>> Again, Jakub likes to comment about statistics...
>>
>> And in general, maybe add more comments. This is the UAPI, it needs to
>> be clear and unambiguous. Documentation/networking/ethtool-netlink.rst.
> 
> Or put enough docs in the YAML spec as those comments get rendered in
> the uAPI header and in HTML docs :)

Or I'll do both. In any case, I will add more documentation!

Regards,
Andrew Lunn March 6, 2024, 7:10 p.m. UTC | #7
> > > Right, so this in general makes sense. The only question i have now
> > > is, should you be using rtnl_link_stats64. That is the standard
> > > structure for interface statistics.
> 
> @Andrew: do you see how I could return/send this object per-peer to
> userspace?
> I think the whole interface stats logic is based on the one-stats-per-device
> concept. Hence, I thought it was meaningful to just send my own stats via
> netlink.

Ah, interesting. I never looked at the details for
rtnl_link_stats64. It is buried in the middle of ifinfo. So not very
reusable :-(

Idea #2:

A peer is not that different to an interface queue. Jakub recently
posted some code for that:

https://lwn.net/ml/netdev/20240229010221.2408413-2-kuba@kernel.org/

Maybe there are ideas you can borrow from there.

But lets look at this from another direction. What are the use cases
for these statistics? Does the userspace daemon need them, e.g. to
detect a peer which is idle and should be disconnected? Are they
exported via an SNMP MIB?

	Andrew
Antonio Quartulli March 8, 2024, 12:01 a.m. UTC | #8
On 06/03/2024 20:10, Andrew Lunn wrote:
>>>> Right, so this in general makes sense. The only question i have now
>>>> is, should you be using rtnl_link_stats64. That is the standard
>>>> structure for interface statistics.
>>
>> @Andrew: do you see how I could return/send this object per-peer to
>> userspace?
>> I think the whole interface stats logic is based on the one-stats-per-device
>> concept. Hence, I thought it was meaningful to just send my own stats via
>> netlink.
> 
> Ah, interesting. I never looked at the details for
> rtnl_link_stats64. It is buried in the middle of ifinfo. So not very
> reusable :-(
> 
> Idea #2:
> 
> A peer is not that different to an interface queue. Jakub recently
> posted some code for that:
> 
> https://lwn.net/ml/netdev/20240229010221.2408413-2-kuba@kernel.org/
> 
> Maybe there are ideas you can borrow from there.
> 
> But lets look at this from another direction. What are the use cases
> for these statistics? Does the userspace daemon need them, e.g. to
> detect a peer which is idle and should be disconnected? Are they
> exported via an SNMP MIB?

Yes, they are used by userspace. Main usecases are:
* inactivity timeout (if configured, when less than N bytes are 
transferred in a given time the client will be disconnected)
* stats reporting during connection (during the life of a peer, stats 
can be periodically pulled by the user for reporting purposes)
* stats reporting at disconnection (stats are pulled upon disconnection 
in order to report the total traffic related to a peer)

These stats can be reported via a status file or via the "management 
interface" (an interactive interface that can be used to issue live 
commands to a running openvpn daemon).

How this data is actually used is up to the user.

Regards,

> 
> 	Andrew
>
Esben Haabendal March 26, 2024, 11:43 a.m. UTC | #9
Antonio Quartulli <antonio@openvpn.net> writes:

> diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
> index 25964eb89aac..3769f99cfe6f 100644
> --- a/drivers/net/ovpn/main.c
> +++ b/drivers/net/ovpn/main.c
> @@ -101,12 +104,23 @@ static int __init ovpn_init(void)
>  		return err;
>  	}
>  
> +	err = ovpn_nl_register();
> +	if (err) {
> +		pr_err("ovpn: can't register netlink family: %d\n", err);
> +		goto unreg_netdev;
> +	}
> +
>  	return 0;
> +
> +unreg_netdev:
> +	unregister_netdevice_notifier(&ovpn_netdev_notifier);
> +	return err;
>  }
>  
>  static __exit void ovpn_cleanup(void)
>  {
>  	unregister_netdevice_notifier(&ovpn_netdev_notifier);
> +	ovpn_nl_unregister();

Any good reason for not using reverse order from ovpn_init() here?

>  }

/Esben
Antonio Quartulli March 26, 2024, 9:39 p.m. UTC | #10
On 26/03/2024 12:43, Esben Haabendal wrote:
> Antonio Quartulli <antonio@openvpn.net> writes:
> 
>> diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
>> index 25964eb89aac..3769f99cfe6f 100644
>> --- a/drivers/net/ovpn/main.c
>> +++ b/drivers/net/ovpn/main.c
>> @@ -101,12 +104,23 @@ static int __init ovpn_init(void)
>>   		return err;
>>   	}
>>   
>> +	err = ovpn_nl_register();
>> +	if (err) {
>> +		pr_err("ovpn: can't register netlink family: %d\n", err);
>> +		goto unreg_netdev;
>> +	}
>> +
>>   	return 0;
>> +
>> +unreg_netdev:
>> +	unregister_netdevice_notifier(&ovpn_netdev_notifier);
>> +	return err;
>>   }
>>   
>>   static __exit void ovpn_cleanup(void)
>>   {
>>   	unregister_netdevice_notifier(&ovpn_netdev_notifier);
>> +	ovpn_nl_unregister();
> 
> Any good reason for not using reverse order from ovpn_init() here?

not really.
Actually I also prefer uninit in reverse order compared to init.

Will fix this, thanks!

> 
>>   }
> 
> /Esben
diff mbox series

Patch

diff --git a/drivers/net/ovpn/Makefile b/drivers/net/ovpn/Makefile
index 7b8b1d0ff9b4..7e406a69892a 100644
--- a/drivers/net/ovpn/Makefile
+++ b/drivers/net/ovpn/Makefile
@@ -9,3 +9,4 @@ 
 obj-$(CONFIG_OVPN) += ovpn.o
 ovpn-y += main.o
 ovpn-y += io.o
+ovpn-y += netlink.o
diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
index 25964eb89aac..3769f99cfe6f 100644
--- a/drivers/net/ovpn/main.c
+++ b/drivers/net/ovpn/main.c
@@ -8,8 +8,10 @@ 
  */
 
 #include "main.h"
+#include "netlink.h"
 #include "io.h"
 
+#include <linux/genetlink.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/types.h>
@@ -17,6 +19,7 @@ 
 #include <linux/inetdevice.h>
 #include <linux/netdevice.h>
 #include <linux/version.h>
+#include <uapi/linux/ovpn.h>
 
 
 /* Driver info */
@@ -101,12 +104,23 @@  static int __init ovpn_init(void)
 		return err;
 	}
 
+	err = ovpn_nl_register();
+	if (err) {
+		pr_err("ovpn: can't register netlink family: %d\n", err);
+		goto unreg_netdev;
+	}
+
 	return 0;
+
+unreg_netdev:
+	unregister_netdevice_notifier(&ovpn_netdev_notifier);
+	return err;
 }
 
 static __exit void ovpn_cleanup(void)
 {
 	unregister_netdevice_notifier(&ovpn_netdev_notifier);
+	ovpn_nl_unregister();
 }
 
 module_init(ovpn_init);
@@ -116,3 +130,4 @@  MODULE_DESCRIPTION(DRV_DESCRIPTION);
 MODULE_AUTHOR(DRV_COPYRIGHT);
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
+MODULE_ALIAS_GENL_FAMILY(OVPN_NL_NAME);
diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c
new file mode 100644
index 000000000000..2e855ce145e7
--- /dev/null
+++ b/drivers/net/ovpn/netlink.c
@@ -0,0 +1,229 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2020-2024 OpenVPN, Inc.
+ *
+ *  Author:	Antonio Quartulli <antonio@openvpn.net>
+ */
+
+#include "main.h"
+#include "io.h"
+#include "netlink.h"
+#include "ovpnstruct.h"
+#include "packet.h"
+
+#include <uapi/linux/ovpn.h>
+
+#include <linux/netdevice.h>
+#include <linux/netlink.h>
+#include <linux/socket.h>
+#include <linux/types.h>
+#include <net/genetlink.h>
+#include <uapi/linux/in.h>
+#include <uapi/linux/in6.h>
+
+/** The ovpn netlink family */
+static struct genl_family ovpn_nl_family;
+
+enum ovpn_nl_multicast_groups {
+	OVPN_MCGRP_PEERS,
+};
+
+static const struct genl_multicast_group ovpn_nl_mcgrps[] = {
+	[OVPN_MCGRP_PEERS] = { .name = OVPN_NL_MULTICAST_GROUP_PEERS },
+};
+
+/** KEYDIR policy. Can be used for configuring an encryption and a decryption key */
+static const struct nla_policy ovpn_nl_policy_keydir[NUM_OVPN_A_KEYDIR] = {
+	[OVPN_A_KEYDIR_CIPHER_KEY] = NLA_POLICY_MAX_LEN(U8_MAX),
+	[OVPN_A_KEYDIR_NONCE_TAIL] = NLA_POLICY_EXACT_LEN(NONCE_TAIL_SIZE),
+};
+
+/** KEYCONF policy */
+static const struct nla_policy ovpn_nl_policy_keyconf[NUM_OVPN_A_KEYCONF] = {
+	[OVPN_A_KEYCONF_SLOT] = NLA_POLICY_RANGE(NLA_U8, __OVPN_KEY_SLOT_FIRST,
+						 NUM_OVPN_KEY_SLOT - 1),
+	[OVPN_A_KEYCONF_KEY_ID] = { .type = NLA_U8 },
+	[OVPN_A_KEYCONF_CIPHER_ALG] = { .type = NLA_U16 },
+	[OVPN_A_KEYCONF_ENCRYPT_DIR] = NLA_POLICY_NESTED(ovpn_nl_policy_keydir),
+	[OVPN_A_KEYCONF_DECRYPT_DIR] = NLA_POLICY_NESTED(ovpn_nl_policy_keydir),
+};
+
+/** PEER policy */
+static const struct nla_policy ovpn_nl_policy_peer[NUM_OVPN_A_PEER] = {
+	[OVPN_A_PEER_ID] = { .type = NLA_U32 },
+	[OVPN_A_PEER_SOCKADDR_REMOTE] = NLA_POLICY_MIN_LEN(sizeof(struct sockaddr)),
+	[OVPN_A_PEER_SOCKET] = { .type = NLA_U32 },
+	[OVPN_A_PEER_VPN_IPV4] = { .type = NLA_U32 },
+	[OVPN_A_PEER_VPN_IPV6] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)),
+	[OVPN_A_PEER_LOCAL_IP] = NLA_POLICY_MAX_LEN(sizeof(struct in6_addr)),
+	[OVPN_A_PEER_LOCAL_PORT] = NLA_POLICY_MAX_LEN(sizeof(u16)),
+	[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32 },
+	[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32 },
+	[OVPN_A_PEER_DEL_REASON] = NLA_POLICY_RANGE(NLA_U8, __OVPN_DEL_PEER_REASON_FIRST,
+						    NUM_OVPN_DEL_PEER_REASON - 1),
+	[OVPN_A_PEER_KEYCONF] = NLA_POLICY_NESTED(ovpn_nl_policy_keyconf),
+};
+
+/** Generic message container policy */
+static const struct nla_policy ovpn_nl_policy[NUM_OVPN_A] = {
+	[OVPN_A_IFINDEX] = { .type = NLA_U32 },
+	[OVPN_A_IFNAME] = NLA_POLICY_MAX_LEN(IFNAMSIZ),
+	[OVPN_A_MODE] = NLA_POLICY_RANGE(NLA_U8, __OVPN_MODE_FIRST,
+					 NUM_OVPN_MODE - 1),
+	[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_nl_policy_peer),
+};
+
+/**
+ * ovpn_get_dev_from_attrs() - retrieve the netdevice a netlink message is targeting
+ */
+static struct net_device *
+ovpn_get_dev_from_attrs(struct net *net, struct nlattr **attrs)
+{
+	struct net_device *dev;
+	int ifindex;
+
+	if (!attrs[OVPN_A_IFINDEX])
+		return ERR_PTR(-EINVAL);
+
+	ifindex = nla_get_u32(attrs[OVPN_A_IFINDEX]);
+
+	dev = dev_get_by_index(net, ifindex);
+	if (!dev)
+		return ERR_PTR(-ENODEV);
+
+	if (!ovpn_dev_is_valid(dev))
+		goto err_put_dev;
+
+	return dev;
+
+err_put_dev:
+	dev_put(dev);
+
+	return ERR_PTR(-EINVAL);
+}
+
+/**
+ * ovpn_pre_doit() - Prepare ovpn genl doit request
+ * @ops: requested netlink operation
+ * @skb: Netlink message with request data
+ * @info: receiver information
+ *
+ * Return: 0 on success or negative error number in case of failure
+ */
+static int ovpn_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
+			 struct genl_info *info)
+{
+	struct net *net = genl_info_net(info);
+	struct net_device *dev;
+
+	/* the OVPN_CMD_NEW_IFACE command is different from the rest as it
+	 * just expects an IFNAME, while all the others expect an IFINDEX
+	 */
+	if (info->genlhdr->cmd == OVPN_CMD_NEW_IFACE) {
+		if (!info->attrs[OVPN_A_IFNAME]) {
+			GENL_SET_ERR_MSG(info, "no interface name specified");
+			return -EINVAL;
+		}
+		return 0;
+	}
+
+	dev = ovpn_get_dev_from_attrs(net, info->attrs);
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+
+	info->user_ptr[0] = netdev_priv(dev);
+
+	return 0;
+}
+
+/**
+ * ovpn_post_doit() - complete ovpn genl doit request
+ * @ops: requested netlink operation
+ * @skb: Netlink message with request data
+ * @info: receiver information
+ */
+static void ovpn_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
+			   struct genl_info *info)
+{
+	struct ovpn_struct *ovpn;
+
+	ovpn = info->user_ptr[0];
+	/* in case of OVPN_CMD_NEW_IFACE, there is no pre-stored device */
+	if (ovpn)
+		dev_put(ovpn->dev);
+}
+
+static const struct genl_small_ops ovpn_nl_ops[] = {
+};
+
+static struct genl_family ovpn_nl_family __ro_after_init = {
+	.hdrsize = 0,
+	.name = OVPN_NL_NAME,
+	.version = 1,
+	.maxattr = NUM_OVPN_A + 1,
+	.policy = ovpn_nl_policy,
+	.netnsok = true,
+	.pre_doit = ovpn_pre_doit,
+	.post_doit = ovpn_post_doit,
+	.module = THIS_MODULE,
+	.small_ops = ovpn_nl_ops,
+	.n_small_ops = ARRAY_SIZE(ovpn_nl_ops),
+	.mcgrps = ovpn_nl_mcgrps,
+	.n_mcgrps = ARRAY_SIZE(ovpn_nl_mcgrps),
+};
+
+/**
+ * ovpn_nl_notify() - react to openvpn userspace process exit
+ */
+static int ovpn_nl_notify(struct notifier_block *nb, unsigned long state,
+			  void *_notify)
+{
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block ovpn_nl_notifier = {
+	.notifier_call = ovpn_nl_notify,
+};
+
+/**
+ * ovpn_nl_init() - perform any ovpn specific netlink initialization
+ */
+int ovpn_nl_init(struct ovpn_struct *ovpn)
+{
+	return 0;
+}
+
+/**
+ * ovpn_nl_register() - register the ovpn genl nl family
+ */
+int __init ovpn_nl_register(void)
+{
+	int ret;
+
+	ret = genl_register_family(&ovpn_nl_family);
+	if (ret) {
+		pr_err("ovpn: genl_register_family() failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = netlink_register_notifier(&ovpn_nl_notifier);
+	if (ret) {
+		pr_err("ovpn: netlink_register_notifier() failed: %d\n", ret);
+		goto err;
+	}
+
+	return 0;
+err:
+	genl_unregister_family(&ovpn_nl_family);
+	return ret;
+}
+
+/**
+ * ovpn_nl_unregister() - unregister the ovpn genl netlink family
+ */
+void ovpn_nl_unregister(void)
+{
+	netlink_unregister_notifier(&ovpn_nl_notifier);
+	genl_unregister_family(&ovpn_nl_family);
+}
diff --git a/drivers/net/ovpn/netlink.h b/drivers/net/ovpn/netlink.h
new file mode 100644
index 000000000000..eb7f234842ef
--- /dev/null
+++ b/drivers/net/ovpn/netlink.h
@@ -0,0 +1,18 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2020-2024 OpenVPN, Inc.
+ *
+ *  Author:	Antonio Quartulli <antonio@openvpn.net>
+ */
+
+#ifndef _NET_OVPN_NETLINK_H_
+#define _NET_OVPN_NETLINK_H_
+
+struct ovpn_struct;
+
+int ovpn_nl_init(struct ovpn_struct *ovpn);
+int ovpn_nl_register(void);
+void ovpn_nl_unregister(void);
+
+#endif /* _NET_OVPN_NETLINK_H_ */
diff --git a/drivers/net/ovpn/ovpnstruct.h b/drivers/net/ovpn/ovpnstruct.h
new file mode 100644
index 000000000000..932eb90953e0
--- /dev/null
+++ b/drivers/net/ovpn/ovpnstruct.h
@@ -0,0 +1,20 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2019-2024 OpenVPN, Inc.
+ *
+ *  Author:	James Yonan <james@openvpn.net>
+ *		Antonio Quartulli <antonio@openvpn.net>
+ */
+
+#ifndef _NET_OVPN_OVPNSTRUCT_H_
+#define _NET_OVPN_OVPNSTRUCT_H_
+
+#include <linux/netdevice.h>
+
+/* Our state per ovpn interface */
+struct ovpn_struct {
+	struct net_device *dev;
+};
+
+#endif /* _NET_OVPN_OVPNSTRUCT_H_ */
diff --git a/drivers/net/ovpn/packet.h b/drivers/net/ovpn/packet.h
new file mode 100644
index 000000000000..0c6c6165c63c
--- /dev/null
+++ b/drivers/net/ovpn/packet.h
@@ -0,0 +1,42 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2020-2024 OpenVPN, Inc.
+ *
+ *  Author:	Antonio Quartulli <antonio@openvpn.net>
+ *		James Yonan <james@openvpn.net>
+ */
+
+#ifndef _NET_OVPN_PACKET_H_
+#define _NET_OVPN_PACKET_H_
+
+/* When the OpenVPN protocol is run in AEAD mode, use
+ * the OpenVPN packet ID as the AEAD nonce:
+ *
+ *    00000005 521c3b01 4308c041
+ *    [seq # ] [  nonce_tail   ]
+ *    [     12-byte full IV    ] -> NONCE_SIZE
+ *    [4-bytes                   -> NONCE_WIRE_SIZE
+ *    on wire]
+ */
+
+/* OpenVPN nonce size */
+#define NONCE_SIZE 12
+/* amount of bytes of the nonce received from user space */
+#define 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 NONCE_WIRE_SIZE (NONCE_SIZE - sizeof(struct ovpn_nonce_tail))
+
+/* Last 8 bytes of AEAD nonce
+ * Provided by userspace and usually derived from
+ * key material generated during TLS handshake
+ */
+struct ovpn_nonce_tail {
+	u8 u8[NONCE_TAIL_SIZE];
+};
+
+#endif /* _NET_OVPN_PACKET_H_ */
diff --git a/include/uapi/linux/ovpn.h b/include/uapi/linux/ovpn.h
new file mode 100644
index 000000000000..56ba5bae6522
--- /dev/null
+++ b/include/uapi/linux/ovpn.h
@@ -0,0 +1,174 @@ 
+/* SPDX-License-Identifier: (GPL-2.0-only WITH Linux-syscall-note) OR MIT */
+/*
+ *  OpenVPN data channel accelerator
+ *
+ *  Copyright (C) 2019-2023 OpenVPN, Inc.
+ *
+ *  Author:	James Yonan <james@openvpn.net>
+ *		Antonio Quartulli <antonio@openvpn.net>
+ */
+
+#ifndef _UAPI_LINUX_OVPN_H_
+#define _UAPI_LINUX_OVPN_H_
+
+#define OVPN_NL_NAME "ovpn"
+
+#define OVPN_NL_MULTICAST_GROUP_PEERS "peers"
+
+/**
+ * enum ovpn_nl_commands - supported netlink commands
+ */
+enum ovpn_nl_commands {
+	/**
+	 * @OVPN_CMD_UNSPEC: unspecified command to catch errors
+	 */
+	OVPN_CMD_UNSPEC = 0,
+	/**
+	 * @OVPN_CMD_NEW_IFACE: create a new OpenVPN interface
+	 */
+	OVPN_CMD_NEW_IFACE,
+	/**
+	 * @OVPN_CMD_DEL_IFACE: delete an existing OpenVPN interface
+	 */
+	OVPN_CMD_DEL_IFACE,
+	/**
+	 * @OVPN_CMD_SET_PEER: create or update a peer
+	 */
+	OVPN_CMD_SET_PEER,
+	/**
+	 * @OVPN_CMD_GET_PEER: Retrieve the status of a peer or all peers
+	 */
+	OVPN_CMD_GET_PEER,
+	/**
+	 * @OVPN_CMD_DEL_PEER: remove a peer
+	 */
+	OVPN_CMD_DEL_PEER,
+	/**
+	 * @OVPN_CMD_SET_KEY: create or update an existing key in the specified key slot
+	 */
+	OVPN_CMD_SET_KEY,
+	/**
+	 * @OVPN_CMD_SWAP_KEYS: swap keys stored in primary and secondary slots
+	 */
+	OVPN_CMD_SWAP_KEYS,
+	/**
+	 * @OVPN_CMD_DEL_KEY: delete the key stored in the specified key slot
+	 */
+	OVPN_CMD_DEL_KEY,
+};
+
+enum ovpn_cipher_alg {
+	/**
+	 * @OVPN_CIPHER_ALG_NONE: No encryption - reserved for debugging only
+	 */
+	OVPN_CIPHER_ALG_NONE = 0,
+	/**
+	 * @OVPN_CIPHER_ALG_AES_GCM: AES-GCM AEAD cipher with any allowed key size
+	 */
+	OVPN_CIPHER_ALG_AES_GCM,
+	/**
+	 * @OVPN_CIPHER_ALG_CHACHA20_POLY1305: ChaCha20Poly1305 AEAD cipher
+	 */
+	OVPN_CIPHER_ALG_CHACHA20_POLY1305,
+};
+
+enum ovpn_del_peer_reason {
+	__OVPN_DEL_PEER_REASON_FIRST,
+	OVPN_DEL_PEER_REASON_TEARDOWN = __OVPN_DEL_PEER_REASON_FIRST,
+	OVPN_DEL_PEER_REASON_USERSPACE,
+	OVPN_DEL_PEER_REASON_EXPIRED,
+	OVPN_DEL_PEER_REASON_TRANSPORT_ERROR,
+	OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT,
+
+	/* new attrs above this line */
+	NUM_OVPN_DEL_PEER_REASON
+};
+
+enum ovpn_key_slot {
+	__OVPN_KEY_SLOT_FIRST = 0,
+	OVPN_KEY_SLOT_PRIMARY = __OVPN_KEY_SLOT_FIRST,
+	OVPN_KEY_SLOT_SECONDARY,
+
+	/* new attrs above this line */
+	NUM_OVPN_KEY_SLOT
+};
+
+enum ovpn_mode {
+	__OVPN_MODE_FIRST = 0,
+	OVPN_MODE_P2P = __OVPN_MODE_FIRST,
+	OVPN_MODE_MP,
+
+	/* new attrs above this line */
+	NUM_OVPN_MODE
+};
+
+enum ovpn_nl_stats_attrs {
+	OVPN_A_STATS_UNSPEC = 0,
+	OVPN_A_STATS_BYTES,
+	OVPN_A_STATS_PACKETS,
+
+	/* new attrs above this line */
+	NUM_OVPN_A_STATS
+};
+
+enum ovpn_nl_attrs {
+	OVPN_A_UNSPEC = 0,
+	OVPN_A_IFINDEX,
+	OVPN_A_IFNAME,
+	OVPN_A_MODE,
+	OVPN_A_PEER,
+
+	/* new attrs above this line */
+	NUM_OVPN_A
+};
+
+enum ovpn_nl_peer_attrs {
+	OVPN_A_PEER_UNSPEC = 0,
+	OVPN_A_PEER_ID,
+	OVPN_A_PEER_RX_STATS,
+	OVPN_A_PEER_TX_STATS,
+	OVPN_A_PEER_SOCKADDR_REMOTE,
+	OVPN_A_PEER_SOCKET,
+	OVPN_A_PEER_VPN_IPV4,
+	OVPN_A_PEER_VPN_IPV6,
+	OVPN_A_PEER_LOCAL_IP,
+	OVPN_A_PEER_LOCAL_PORT,
+	OVPN_A_PEER_KEEPALIVE_INTERVAL,
+	OVPN_A_PEER_KEEPALIVE_TIMEOUT,
+	OVPN_A_PEER_DEL_REASON,
+	OVPN_A_PEER_KEYCONF,
+	OVPN_A_PEER_VPN_RX_BYTES,
+	OVPN_A_PEER_VPN_TX_BYTES,
+	OVPN_A_PEER_VPN_RX_PACKETS,
+	OVPN_A_PEER_VPN_TX_PACKETS,
+	OVPN_A_PEER_LINK_RX_BYTES,
+	OVPN_A_PEER_LINK_TX_BYTES,
+	OVPN_A_PEER_LINK_RX_PACKETS,
+	OVPN_A_PEER_LINK_TX_PACKETS,
+
+	/* new attrs above this line */
+	NUM_OVPN_A_PEER
+};
+
+enum ovpn_nl_keyconf_attrs {
+	OVPN_A_KEYCONF_UNSPEC = 0,
+	OVPN_A_KEYCONF_SLOT,
+	OVPN_A_KEYCONF_KEY_ID,
+	OVPN_A_KEYCONF_CIPHER_ALG,
+	OVPN_A_KEYCONF_ENCRYPT_DIR,
+	OVPN_A_KEYCONF_DECRYPT_DIR,
+
+	/* new attrs above this line */
+	NUM_OVPN_A_KEYCONF
+};
+
+enum ovpn_nl_keydir_attrs {
+	OVPN_A_KEYDIR_UNSPEC = 0,
+	OVPN_A_KEYDIR_CIPHER_KEY,
+	OVPN_A_KEYDIR_NONCE_TAIL,
+
+	/* new attrs above this line */
+	NUM_OVPN_A_KEYDIR
+};
+
+#endif /* _UAPI_LINUX_OVPN_H_ */