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

Jakub Kicinski
2022-05-31T16:08:58Z

On Tue, 31 May 2022 17:51:36 +0200 Jiri Pirko wrote:
> Tue, May 31, 2022 at 05:05:55PM CEST, kuba@kernel.org wrote:
> >> Group of what? Could you provide me example what you mean?
> >
> >Group of components. As explained component has an existing meaning,
> >we can't reuse the term with a different one now.
>
> I still don't follow. I don't want to reuse it.
> Really, could you be more specific and show examples, please?

What you had in your previous examples, just don't call it components
but come up with a new term:

$ devlink dev info
pci/0000:01:00.0:
driver mlxsw_spectrum2
versions:
fixed:
hw.revision A0
fw.psid MT_0000000199
running:
fw.version 29.2010.2302
fw 29.2010.2302
groups? sections? subordinates? : <= here
lc1:
versions:
fixed:
hw.revision 0
fw.psid MT_0000000111
running:
fw 19.2010.1310
ini.version 4

Note that lc1 is not a nest at netlink level but user space can group
the attrs pretty trivially.

> >> But to be consistent with the output, we would have to change "devlink
> >> dev info" to something like:
> >> pci/0000:01:00.0:
> >> versions:
> >> running:
> >> fw 1.2.3
> >> fw.mgmt 10.20.30
> >> lc 2 fw 19.2010.1310
> >
> >Yup.
>
> Set, you say "yup" but below you say it should be in a separate nest.
> That is confusing me.

Ah, sorry. I hope the above is clear.

> >> But that would break the existing JSON output, because "running" is an array:
> >> "running": {
> >> "fw": "1.2.3",
> >> "fw.mgmt": "10.20.30"
> >> },
> >
> >No, the lc versions should be in separate nests. Since they are not
> >updated when flashing main FW mixing them into existing versions would
> >break uAPI.
>
> Could you please draw it out for me exacly as you envision it? We are
> dancing around it, I can't really understand what exactly do you mean.

Why would I prototype your feature for you? I prefer a separate dl
instance. If you want to explore other options the "drawing out" is
on you :/

> >> So probably better to stick to "lcx.y" notation in both devlink dev info
> >> and flash and split/squash to attributes internally. What do you think?
> >
> >BTW how do you intend to activate the new FW? Extend the reload command?
>
> Not sure now. Extending reload is an option. Have to think about it.

😑