Re: [PATCH net-next v3 04/24] ovpn: add basic interface creation/destruction/management routines

Antonio Quartulli
2024-05-08T07:52:27Z

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.