lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f11e8a14-deb0-456f-bb4a-b5e4e16a79d7@openvpn.net>
Date: Fri, 11 Apr 2025 10:04:10 +0200
From: Antonio Quartulli <antonio@...nvpn.net>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>, Donald Hunter <donald.hunter@...il.com>,
 Shuah Khan <shuah@...nel.org>, sd@...asysnail.net, ryazanov.s.a@...il.com,
 Andrew Lunn <andrew+netdev@...n.ch>, Simon Horman <horms@...nel.org>,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
 Xiao Liang <shaw.leon@...il.com>, steffen.klassert@...unet.com,
 antony.antony@...unet.com
Subject: Re: [PATCH net-next v25 01/23] net: introduce OpenVPN Data Channel
 Offload (ovpn)

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.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ