[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6e7b19fd-c8da-439d-9b9d-4dd3b6a5567d@openvpn.net>
Date: Wed, 8 May 2024 09:53:40 +0200
From: Antonio Quartulli <antonio@...nvpn.net>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, Sergey Ryazanov <ryazanov.s.a@...il.com>,
Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
Andrew Lunn <andrew@...n.ch>, Esben Haabendal <esben@...nix.com>
Subject: Re: [PATCH net-next v3 04/24] ovpn: add basic interface
creation/destruction/management routines
On 08/05/2024 02:18, Jakub Kicinski wrote:
> On Mon, 6 May 2024 03:16:17 +0200 Antonio Quartulli wrote:
>
>> diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
>> index ad3813419c33..338e99dfe886 100644
>> --- a/drivers/net/ovpn/io.c
>> +++ b/drivers/net/ovpn/io.c
>> @@ -11,6 +11,26 @@
>> #include <linux/skbuff.h>
>>
>> #include "io.h"
>> +#include "ovpnstruct.h"
>> +#include "netlink.h"
>> +
>> +int ovpn_struct_init(struct net_device *dev)
>> +{
>> + struct ovpn_struct *ovpn = netdev_priv(dev);
>> + int err;
>> +
>> + ovpn->dev = dev;
>> +
>> + err = ovpn_nl_init(ovpn);
>> + if (err < 0)
>> + return err;
>> +
>> + dev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
>
> Set pcpu_stat_type, core will allocate for you
ok
>
>> + if (!dev->tstats)
>> + return -ENOMEM;
>> +
>> + return 0;
>> +}
>
>> +/**
>> + * ovpn_struct_init - Initialize the netdevice private area
>> + * @dev: the device to initialize
>> + *
>> + * Return: 0 on success or a negative error code otherwise
>> + */
>> +int ovpn_struct_init(struct net_device *dev);
>
> Weak preference for kdoc to go with the implementation, not declaration.
oh ok - this wasn't clear.
Will move the kdoc next to the implementation.
>
>> +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,
>> + .ndo_get_stats64 = dev_get_tstats64,
>
> Core should count pcpu stats automatically
Thanks for pointing this out.
I see dev_get_stats() takes care of all this for us.
>
>> +};
>> +
>> bool ovpn_dev_is_valid(const struct net_device *dev)
>> {
>> return dev->netdev_ops->ndo_start_xmit == ovpn_net_xmit;
>> }
>
>> + list_add(&ovpn->dev_list, &dev_list);
>> + rtnl_unlock();
>> +
>> + /* turn carrier explicitly off after registration, this way state is
>> + * clearly defined
>> + */
>> + netif_carrier_off(dev);
>
> carrier off inside the locked section, user can call open
> immediately after unlock
ok, will move it up.
--
Antonio Quartulli
OpenVPN Inc.
Powered by blists - more mailing lists