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

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 28 May 2022 11:09:01 +0200 Jiri Pirko wrote:
> Sat, May 28, 2022 at 02:10:38AM CEST, kuba@kernel.org wrote:
> >
> >Is the "lc1" free-form or generated by the core based on subobjects?
> >Is it carried as a string or object type + id?  
> 
> It could be both:
> 1) for line cards I plan to have a helper to have this generated by core
> 2) for other FW objects, it is up to the driver.

Did you mean "either" or "both"?

> >I guess my suggestion of a CLI mockup has proven its weakness :)  
> 
> I'm not sure I understand what you mean by this sentence. Could you
> please be more blunt? You know, my english is not so good to understand
> some hidden meanings :)

The question of what kind of attribute "lc1" is carried in would had
been answered in posting of a code, while CLI mockup doesn't provide
such detail.

> >
> >I sort of assumed that the DEVLINK_ATTR_INFO_VERSION_NAME is the
> >component, the docs also use the word "component" for it.   
> 
> Okay, that I didn't see.
> 
> >
> >For the nfp for instance we had "fw.app" for the datapath microcode and
> >"fw.mgmt" for the control processor. These are separate partitions on
> >the flash. I don't think we ever implemented writing them separately
> >but it's certainly was our internal plan at some point.  
> 
> Okay, so what you say it, we already have components in "devlink dev
> info". Like you pointed out as an example:
>   fw.app
>   fw.mgmt
> so the flash comment would be:
>   devlink dev flash pci/0000:01:00.0 component fw.app file foo.bin
>   devlink dev flash pci/0000:01:00.0 component fw.mgmt file bar.bin
> ?

Correct.

> If yes, what should be the default in case component is not defined? Do
> we need to expose it in "devlink dev info"? How?

Not defined as in someone tries to flash component X but there is no
version for X in info?

> So to extend this existing facility with my line card example, we would
> have:
> 
> $ devlink dev info
> pci/0000:01:00.0:
>    driver mlxsw_spectrum2
>    versions:
>        fixed:
>          hw.revision A0
>          fw.psid MT_0000000199
> 	 lc1.hw.revision 0
> 	 lc1.fw.psid MT_0000000111
> 	 lc2.hw.revision 0
> 	 lc2.fw.psid MT_0000000111
>        running:
>          fw.version 29.2010.2302
>          fw 29.2010.2302
> 	 lc1.fw 19.2010.1310
> 	 lc1.ini.version 4
> 	 lc2.fw 19.2010.1310
> 	 lc2.ini.version 4
> 
> And then:
> devlink dev flash pci/0000:01:00.0 component lc1.fw file mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2
> 
> Does this sound correct?

I think I suggested something like that in the past, but back then 
I was assuming that lc FW would come from the same large FW bundle
file as the control plan FW, and we would not have to use the component.

Let's step back and look from the automation perspective again.
Assuming we don't want to hardcode matching "lc$i" there how can 
a generic FW update service scan the dev info and decide on what
dev flash command to fire off?

> Also, to avoid free-form, I can imagine to have per-linecard info_get() op
> which would be called for each line card from devlink_nl_info_fill() and
> prefix the "lcX" automatically without driver being involved.
> 
> Sounds good?

Hm. That's moving the matryoshka-ing of the objects from the uAPI level
to the internals. 

If we don't do the string prefix but instead pass the subobject info to
the user space as an attribute per version we can at least avoid
per-subobject commands (DEVLINK_CMD_LINECARD_INFO_GET). Much closer to
how health reporters are implemented than how params are done, so I
think it is a good direction.

We still need to iron out how the automation can go over the main FW
and sub-objects in a generic way.

I still think full devlink sub-instance is better because we will end
up needing params or health. Fake devices can be made with auxbus or
otherwise. But if you really don't want sub-instances we can explore 
the above.