netdev - Re: [RFC PATCH net-next 0/4] ethtool: Add ability to flash and query transceiver modules' firmware

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]

Date:   Tue, 30 Nov 2021 09:54:26 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        davem@...emloft.net, andrew@...n.ch, pali@...nel.org,
        jacob.e.keller@...el.com, vadimp@...dia.com, mlxsw@...dia.com,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [RFC PATCH net-next 0/4] ethtool: Add ability to flash and query
 transceiver modules' firmware

On Tue, Nov 30, 2021 at 10:36:02AM +0200, Ido Schimmel wrote:
> On Mon, Nov 29, 2021 at 09:37:24AM -0800, Jakub Kicinski wrote:
> > 
> > Immediate question I have is why not devlink. We purposefully moved 
> > FW flashing to devlink because I may take long, so doing it under
> > rtnl_lock is really bad. Other advantages exist (like flashing
> > non-Ethernet ports). Ethtool netlink already existed at the time.
> 
> Device firmware flashing doesn't belong in devlink because of locking
> semantics. It belongs in devlink because you are updating the firmware
> of the device, which can instantiate multiple netdevs. For multi-port
> devices, it always seemed weird to tell users "choose some random port
> and use it for 'ethtool -f'". I remember being asked if the command
> needs to be run for all swp* netdevs (no).
> 
> On the other hand, each netdev corresponds to a single transceiver
> module and each transceiver module corresponds to a single netdev
> (modulo split which is a user configuration).

Devlink also has abstraction for ports so it can be used so it is not
necessarily a problem.

> In addition, users are already dumping the EEPROM contents of
> transceiver modules via ethtool and also toggling their settings.
> 
> Given the above, it's beyond me why we should tell users to use anything
> other than ethtool to update transceiver modules' firmware.

As I already mentioned, we should distinguish between ethtool API and
ethtool utility. It is possible to implement the flashing in devlink API
and let both devlink and ethtool utilities use that API.

I'm not saying ethtool API is a wrong choice, IMHO either option has its
pros and cons. I'm just trying to point out that implementation in
devlink API does not necessarily mean one cannot use the ethtool to use
the feature.

Michal