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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220531090852.2b10c344@kernel.org>
Date:   Tue, 31 May 2022 09:08:52 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Ido Schimmel <idosch@...sch.org>, Ido Schimmel <idosch@...dia.com>,
        netdev@...r.kernel.org, davem@...emloft.net, pabeni@...hat.com,
        jiri@...dia.com, petrm@...dia.com, dsahern@...il.com,
        andrew@...n.ch, mlxsw@...dia.com
Subject: Re: [PATCH net-next 00/11] mlxsw: extend line card model by devices
 and info

On Tue, 31 May 2022 17:51:36 +0200 Jiri Pirko wrote:
> Tue, May 31, 2022 at 05:05:55PM CEST, kuba@...nel.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.

😑