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

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <22d63b13-2c20-4ee7-9783-7b061bd6d942@openvpn.net>
Date: Tue, 19 Nov 2024 09:16:11 +0100
From: Antonio Quartulli <antonio@...nvpn.net>
To: Sergey Ryazanov <ryazanov.s.a@...il.com>
Cc: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Donald Hunter <donald.hunter@...il.com>,
 Shuah Khan <shuah@...nel.org>, sd@...asysnail.net,
 Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next v11 03/23] ovpn: add basic netlink support



On 19/11/2024 03:23, Sergey Ryazanov wrote:
> On 15.11.2024 12:19, Antonio Quartulli wrote:
>> On 09/11/2024 00:31, Sergey Ryazanov wrote:
>>> On 29.10.2024 12:47, Antonio Quartulli wrote:
>>>> +/**
>>>> + * struct ovpn_struct - per ovpn interface state
>>>> + * @dev: the actual netdev representing the tunnel
>>>> + * @dev_tracker: reference tracker for associated dev
>>>> + */
>>>> +struct ovpn_struct {
>>>
>>> There is no standard convention how to entitle such structures, so 
>>> the question is basically of out-of-curiosity class. For me, having a 
>>> sturcuture with name 'struct' is like having no name. Did you 
>>> consider to use such names as ovpn_dev or ovpn_iface? Meaning, using 
>>> a name that gives a clue regarding the scope of the content.
>>
>> Yes, I wanted to switch to ovpn_priv, but  did not care much for the 
>> time being :)
>>
>> I can still do it now in v12.
> 
> This topic caused me the biggest doubts. I don't want to ask to rename 
> everything on the final lap. Just want to share an outside perspective 
> on the structure name. And let you decide is it worth or not.
> 
> And if you ask me, ovpn_priv does not give a clue either. The module is 
> too complex for a vague structure name, even after your great work on 
> clearing its design.

Well, the word "priv" to me resembles the "netdev_priv()" call, so it's 
kinda easier to grasp what this is about.
In batman-adv we used the same suffix and it was well received.
Also, if you grep for "_priv " in drivers/net you will see that this is 
a common pattern.

Since I already had in mind to change this struct name, I moved on and 
renamed it to ovpn_priv throughput the patchset (git rebase --exec is my 
friend ;)).

Thanks

Regards,

> 
> -- 
> Sergey

-- 
Antonio Quartulli
OpenVPN Inc.