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

[Jiri Pirko <jiri@resnulli.us>]

Tue, May 31, 2022 at 06:08:52PM CEST, kuba@kernel.org wrote:
>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.

Awesome! I think now it is clearer. To be in sync with devlink dev
flash cmd option, we have to have "lc 1" here, have to think how that
can be done.


>
>> >> 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 :/

Well, you are basically leading my arm when I'm drawing the thing. Looks
like you exactly know what you are looking for. That is why.
If you let me to the stuff my way, we would be already done.
You have to decide which way you want this.

And again, for the record, I strongly believe that a separate dl
instance for this does not make any sense at all :/ I wonder why you
still think it does.


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