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:

quoted

Sat, May 28, 2022 at 02:10:38AM CEST, kuba@kernel.org wrote:

quoted

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.

quoted

quoted

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.

quoted

quoted

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.

quoted

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.

quoted

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?

quoted

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?

quoted

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.