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

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 27 Apr 2022 09:35:34 +0200 Jiri Pirko wrote:
> >> The relationship-by-name sounds a bit fragile to me. The names of
> >> components are up to the individual drivers.  
> >
> >I asked you how the automation will operate. You must answer questions
> >if you want to have a discussion. Automation is the relevant part.  
> 
> Automation, not sure. It would probably just see type of gearbox and
> flash it. Not sure I understand the question, perhaps you could explain?
> Plus, the possibility is to auto-flash the GB from driver directly.
> 
> 
> >You're not designing an interface for SDK users but for end users.  
> 
> Sure, that is the aim of this API. Human end user. That is why I wanted
> the user to see the relationships between devlink dev, line cards and
> the gearboxes on them. If you want to limit the visibility, sure, just
> tell me how.

Okay, we have completely different views on what the goals should be.
Perhaps that explains the differences in the design.

Of the three API levels (SDK, automation, human) I think automation
is the only one that's interesting to us in Linux. SDK interfaces are
necessarily too low level as they expose too much of internal details
to standardize. Humans are good with dealing with uncertainty and
diverse so there's no a good benchmark.

The benchmark for automation is - can a machine use this API across
different vendors to reliably achieve its goals. For FW info/flashing
the goal is keeping the FW versions up to date. This is documented:

https://www.kernel.org/doc/html/latest/networking/devlink/devlink-flash.html#firmware-version-management

What would the pseudo code look like with "line cards" in the picture?
Apply RFC1925 truth 12.

> >> There is no new command for that, only one nested attribute which
> >> carries the device list added to the existing command. They are no new
> >> objects, they are just few nested values.  
> >
> >DEVLINK_CMD_LINECARD_INFO_GET  
> 
> Okay, that is not only to expose devices. That is also to expose info
> about linecards, like HW revision, INI version etc. Where else to put
> it? I can perhaps embed it into devlink dev info, but I thought separate
> command would be more suitable. object cmd, object info cmd. It is
> more clear I believe.

> >> If so, how does the user know if/when to flash it?
> >> If not, where would you list it if devices nest is not the correct place?  
> >
> >Let me mock up what I had in mind for you since it did not come thru 
> >in the explanation:
> >
> >$ devlink dev info show pci/0000:01:00.0
> >    versions:
> >        fixed:
> >          hw.revision 0
> >          lc2.hw.revision a
> >          lc8.hw.revision b
> >        running:
> >          ini.version 4
> >          lc2.gearbox 1.1.3
> >          lc8.gearbox 1.2.3  
> 
> Would be rather:
> 
>           lc2.gearbox0 1.1.3
>           lc2.gearbox1 1.2.4

I thought you said your gearboxes all the the same FW? 
Theoretically, yes. Theoretically, I can also have nested "line cards".

>           lc8.gearbox0 1.2.3
> 
> Okay, I see. So instead of having clear api with relationships and
> clear human+machine readability we have squahed indexes into strings.
> I fail to see the benefit, other than no-api-extension :/ On contrary.

Show me the real life use for all the "clear api with relationships"
and I'll shut up.

I would not take falling back to physical (HW) hierarchy for the API
design as a point of pride. Seems lazy if I'm completely honest.
Someone else's HW may have a different hierarchy, and you're just
forcing the automation engineer iterate over irrelevant structures
("devices").

My hunch is that automation will not want to deal with line cards
separately, and flash the entire devices in one go to a tested and
verified bundle blob provided by the vendor. If they do want to poke 
at line cards - the information is still there in what I described.

> >$ devlink lc show pci/0000:01:00.0 lc 8
> >pci/0000:01:00.0:
> >  lc 8 state active type 16x100G
> >    supported_types:
> >      16x100G
> >    versions: 
> >      lc8.hw.revision (a) 
> >      lc8.gearbox (1.2.3)
> >
> >Where the data in the brackets is optionally fetched thru the existing
> >"dev info" API, but rendered together by the user space.  
> 
> Quite odd. I find it questionable to say at least to mix multiple
> command netlink outputs into one output.

Really? So we're going to be designing kernel APIs so that each message
contains complete information and can't contain references now?

> The processing of it would be a small nightmare considering the way
> how the netlink message processing works in iproute2 :/