Re: [PATCH net-next v25 01/23] net: introduce OpenVPN Data Channel Offload (ovpn)

[Antonio Quartulli]

Hi Jakub,

thanks for taking the time to go through my patchset :)

On 11/04/2025 04:54, Jakub Kicinski wrote:
> On Mon, 07 Apr 2025 21:46:09 +0200 Antonio Quartulli wrote:
> > +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);
> > +
> > +	if (!ovpn_dev_is_valid(dev))
> > +		return NOTIFY_DONE;
> > +
> > +	switch (state) {
> > +	case NETDEV_REGISTER:
> > +		/* add device to internal list for later destruction upon
> > +		 * unregistration
> > +		 */
> > +		break;
> > +	case NETDEV_UNREGISTER:
> > +		/* can be delivered multiple times, so check registered flag,
> > +		 * then destroy the interface
> > +		 */
> > +		break;
> > +	case NETDEV_POST_INIT:
> > +	case NETDEV_GOING_DOWN:
> > +	case NETDEV_DOWN:
> > +	case NETDEV_UP:
> > +	case NETDEV_PRE_UP:
> > +	default:
> > +		return NOTIFY_DONE;
> > +	}
>
> Why are you using a notifier to get events for your own device?

My understanding is that this is the standard approach to:
1) hook in the middle of registration/deregistration;
2) handle events generated by other components/routines.

I see in /drivers/net/ almost every driver registers a notifier for 
their own device.

Isn't this expected?

> > +	return NOTIFY_OK;
> > +}
>
> > +MODULE_DESCRIPTION("OpenVPN data channel offload (ovpn)");
> > +MODULE_AUTHOR("(C) 2020-2025 OpenVPN, Inc.");
>
> Companies can't author code, only people. Note that MODULE_AUTHOR()
> is optional.

Ouch, thanks. Will get this addressed.

Regards,

--
Antonio Quartulli
OpenVPN Inc.