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

[Jiri Pirko <jiri@resnulli.us>]

Sat, May 28, 2022 at 09:02:53PM CEST, kuba@kernel.org wrote:
>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"?

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 

Yes, you did.


>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?

Hardcode matching lc$i? I don't follow. It is a part of the
version/component name.
So if devlink dev info outputs:
lc2.fw 19.2010.1310
then you use for devlink dev flash:
devlink dev flash pci/0000:01:00.0 component lc2.fw file mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2
Same name, same string.

What am I missing?



>
>> 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.

Sorry, I'm a bit lost. Could you please provide some example about how
you envision it? For me it is a guessing game :/
My guess is you would like to add to the version nest where
DEVLINK_ATTR_INFO_VERSION_NAME resides for example
DEVLINK_ATTR_LINECARD_INDEX?

Correct?


>
>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.

I really don't.