Re: [PATCH net-next 00/11] mlxsw: extend line card model by devices and info

[Jiri Pirko <jiri@resnulli.us>]

Mon, Apr 25, 2022 at 09:52:18PM CEST, kuba@kernel.org wrote:
>On Mon, 25 Apr 2022 22:39:57 +0300 Ido Schimmel wrote:
>> > :/ what is a line card device? You must provide document what you're
>> > doing, this:
>> > 
>> >  .../networking/devlink/devlink-linecard.rst   |   4 +
>> > 
>> > is not enough.
>> > 
>> > How many operations and attributes are you going to copy&paste?
>> > 
>> > Is linking devlink instances into a hierarchy a better approach?  
>> 
>> In this particular case, these devices are gearboxes. They are running
>> their own firmware and we want user space to be able to query and update
>> the running firmware version.
>
>Nothing too special, then, we don't create "devices" for every
>component of the system which can have a separate FW. That's where
>"components" are intended to be used..

*
Sure, that is why I re-used components :)
But you have to somehow link the component to the particular gearbox on
particular line card. Say, you need to flash GB on line card 8. This is
basically providing a way to expose this relationship to user. Also, the
"lc info" shows the FW version for gearboxes. As Ido mentioned, the GB
versions could be listed in "devlink dev info" in theory. But then, you
need to somehow expose the relationship with line card as well.

I don't see any simpler iface than this.


>
>> The idea (implemented in the next patchset) is to let these devices
>> expose their own "component name", which can then be plugged into the
>> existing flash command:
>> 
>>     $ devlink lc show pci/0000:01:00.0 lc 8
>>     pci/0000:01:00.0:
>>       lc 8 state active type 16x100G
>>         supported_types:
>>            16x100G
>>         devices:
>>           device 0 flashable true component lc8_dev0
>>           device 1 flashable false
>>           device 2 flashable false
>>           device 3 flashable false
>>     $ devlink dev flash pci/0000:01:00.0 file some_file.mfa2 component lc8_dev0
>
>IDK if it's just me or this assumes deep knowledge of the system.
>I don't understand why we need to list devices 1-3 at all. And they
>don't even have names. No information is exposed. 

There are 4 gearboxes on the line card. They share the same flash. So if
you flash gearbox 0, the rest will use the same FW.
I'm exposing them for the sake of completeness. Also, the interface
needs to be designed as a list anyway, as different line cards may have
separate flash per gearbox.

What's is the harm in exposing devices 1-3? If you insist, we can hide
them.


>
>There are many components on any networking device, including plenty
>40G-R4 -> 25G-R1 gearboxes out there.
>
>> Registering a separate devlink instance for these devices sounds like an
>> overkill to me. If you are not OK with a separate command (e.g.,
>> DEVLINK_CMD_LINECARD_INFO_GET), then extending DEVLINK_CMD_INFO_GET is
>> also an option. We discussed this during internal review, but felt that
>> the current approach is cleaner.
>
>I don't know what you have queued, so if you don't need a full devlink
>instance (IOW line cards won't need more individual config) that's fine.

Yeah, incoparable, the devlink dev and line card device - gearbox.


>For just FW flashing you can list many devices and update the
>components... no need to introduce new objects or uAPI.

Please see * above.


>
>> > Would you mind if I revert this?  
>> 
>> I can't stop you, but keep in mind that it's already late here and that
>> tomorrow I'm AFK (reserve duty) and won't be able to tag it. Jiri should
>> be available to continue this discussion tomorrow morning, so probably
>> best to wait for his feedback.
>
>Sure, no rush.