netdev - Re: [PATCH net-next v1] ethtool: ethtool_get_drvinfo: populate drvinfo fields even if callback exits

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: <20221109122641.781b30d9@kernel.org>
Date:   Wed, 9 Nov 2022 12:26:41 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     Vincent Mailhol <mailhol.vincent@...adoo.fr>,
        netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Guangbin Huang <huangguangbin2@...wei.com>,
        Hao Chen <chenhao288@...ilicon.com>,
        Sean Anderson <sean.anderson@...o.com>,
        Tom Rix <trix@...hat.com>,
        Tonghao Zhang <xiangxia.m.yue@...il.com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Marco Bonelli <marco@...eim.net>
Subject: Re: [PATCH net-next v1] ethtool: ethtool_get_drvinfo: populate
 drvinfo fields even if callback exits

On Wed, 9 Nov 2022 19:52:13 +0200 Leon Romanovsky wrote:
> On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote:
> > If ethtool_ops::get_drvinfo() callback isn't set,
> > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and
> > ethtool_drvinfo::bus_info fields.
> > 
> > However, if the driver provides the callback function, those two
> > fields are not touched. This means that the driver has to fill these
> > itself.  
> 
> Can you please point to such drivers?

What you mean by "such drivers" is not clear from the quoted context,
at least to me.

> One can argue that they don't need to touch these fields in a first
> place and ethtool_drvinfo should always overwrite them.

Quite likely most driver prints to .driver and .bus_info can be dropped
with this patch in place. Then again, I'm suspecting it's a bit of a
chicken and an egg problem with people adding new drivers not having 
an incentive to add the print in the core and people who want to add
the print in the core not having any driver that would benefit.
Therefore I'd lean towards accepting Vincent's patch as is even if
the submission can likely be more thorough and strict.

While I'm typing - I've used dev_driver_string() to get the driver
name in the past. Perhaps something to consider?