netdev - Re: [PATCH net-next 02/12] net/mlx5: LAG, Refactor lag logic

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: <80b140ac-e9ff-45b2-ad4e-f18f12be531f@nvidia.com>
Date: Tue, 17 Dec 2024 17:47:03 +0200
From: Mark Bloch <mbloch@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Alexander Lobakin <aleksander.lobakin@...el.com>,
 rongwei liu <rongweil@...dia.com>, Tariq Toukan <tariqt@...dia.com>,
 "David S. Miller" <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>,
 Eric Dumazet <edumazet@...gle.com>, Andrew Lunn <andrew+netdev@...n.ch>,
 Leon Romanovsky <leonro@...dia.com>, netdev@...r.kernel.org,
 Saeed Mahameed <saeedm@...dia.com>, Gal Pressman <gal@...dia.com>
Subject: Re: [PATCH net-next 02/12] net/mlx5: LAG, Refactor lag logic

On 17/12/2024 16:58, Jakub Kicinski wrote:
> On Tue, 17 Dec 2024 14:52:55 +0200 Mark Bloch wrote:
>>> All drivers must have its symbols prefixed, otherwise there might be
>>> name conflicts at anytime and also it's not clear where a definition
>>> comes from if it's not prefixed.
>>
>> However, those aren't exported symbols, they are used exclusively by the mlx5 lag code.
>> I don't see any added value in prefixing internal functions with mlx5 unless it adds
>> context to the logic.
>> Here it's very clear we are going over the members that are stored inside the ldev struct.
> 
> Prefixing the symbols makes it easier to read your code for people 
> who aren't exclusively working on mlx5. Also useful when reading
> mlx5-originated stack traces.

I agree with that, but since every entry point into mlx5 lag
already uses a prefixed function, any stack trace will clearly
show where the issue originates. I’m not strongly opposed to
this change, there’s already an internal version that addresses
these comments.

Just to note for example, a function name like txring_txq()
is fairly common with a generic name, and several drivers
use the exact same name, with fbnic being one such case.

I don't see a clear rule about this stuff.

Mark