Re: [PATCH net-next v11 03/23] ovpn: add basic netlink support

[Sergey Ryazanov <ryazanov.s.a@gmail.com>]

On 29.10.2024 12:47, Antonio Quartulli wrote:
> This commit introduces basic netlink support with family
> registration/unregistration functionalities and stub pre/post-doit.
> 
> More importantly it introduces the YAML uAPI description along
> with its auto-generated files:
> - include/uapi/linux/ovpn.h
> - drivers/net/ovpn/netlink-gen.c
> - drivers/net/ovpn/netlink-gen.h
> 
> Cc: donald.hunter@gmail.com
> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

[skipped]

> diff --git a/Documentation/netlink/specs/ovpn.yaml b/Documentation/netlink/specs/ovpn.yaml

[skipped]

> +attribute-sets:
> +  -
> +    name: peer
> +    attributes:
> +      -
> +        name: id
> +        type: u32
> +        doc: |
> +          The unique ID of the peer. To be used to identify peers during
> +          operations

nit: could you specify the scope of uniqueness? I believe it is not 
globally uniq, it is just interface uniq, right?

> +        checks:
> +          max: 0xFFFFFF

[skipped]

> diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
> index 369a5a2b2fc1a497c8444e59f9b058eb40e49524..d5bdb0055f4dd3a6e32dc6e792bed1e7fd59e101 100644
> --- a/drivers/net/ovpn/main.c
> +++ b/drivers/net/ovpn/main.c
> @@ -7,11 +7,15 @@
>    *		James Yonan <james@openvpn.net>
>    */
>   
> +#include <linux/genetlink.h>
>   #include <linux/module.h>
>   #include <linux/netdevice.h>
>   #include <net/rtnetlink.h>
> +#include <uapi/linux/ovpn.h>
>   
> +#include "ovpnstruct.h"
>   #include "main.h"
> +#include "netlink.h"
>   #include "io.h"
>   
>   /* Driver info */
> @@ -37,7 +41,7 @@ static int ovpn_newlink(struct net *src_net, struct net_device *dev,
>   }
>   
>   static struct rtnl_link_ops ovpn_link_ops = {
> -	.kind = "ovpn",
> +	.kind = OVPN_FAMILY_NAME,

nit: are you sure that the link kind is the same as the GENL family? I 
mean, they are both deriviated from the protocol name that is common for 
both entities, but is it making RTNL kind a derivative of GENL family?

>   	.netns_refund = false,
>   	.newlink = ovpn_newlink,
>   	.dellink = unregister_netdevice_queue,
> @@ -93,8 +97,16 @@ static int __init ovpn_init(void)
>   		goto unreg_netdev;
>   	}

--
Sergey