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:

quoted

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