public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Ido Schimmel <idosch@idosch.org>,
	Ido Schimmel <idosch@nvidia.com>,
	netdev@vger.kernel.org, davem@davemloft.net, pabeni@redhat.com,
	jiri@nvidia.com, petrm@nvidia.com, dsahern@gmail.com,
	andrew@lunn.ch, mlxsw@nvidia.com
Subject: Re: [PATCH net-next 00/11] mlxsw: extend line card model by devices and info
Date: Tue, 31 May 2022 09:11:27 +0200	[thread overview]
Message-ID: <YpW/n3Nh8fIYOEe+@nanopsycho> (raw)
In-Reply-To: <20220530125408.3a9cb8ed@kernel.org>

Mon, May 30, 2022 at 09:54:08PM CEST, kuba@kernel.org wrote:
>On Sun, 29 May 2022 11:23:01 +0200 Jiri Pirko wrote:
>> >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?
>
>Nevermind, I think we can iterate over all the groupings.
>Since I hope you agreed that component has an established

Yeah, component=version. I will send a RFC soon that tights it together.

>meaning can we use group instead?

Group of what? Could you provide me example what you mean?


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

Hmm, in that case, I'm not sure how to do this. As cmd options and       
outputs should match, we would have:                                     
                                                                         
devlink dev info                                                         
lc2.fw 19.2010.1310                                                      
                                                                         
here lc2 and fw are concatenated from DEVLINK_ATTR_LINECARD_INDEX and DEVLINK_ATTR_INFO_VERSION_NAME
                                                                         
Now on devlink dev flash side, when I pass "component lc2.fw", how could 
the "devlink dev flash" know to divide it to DEVLINK_ATTR_LINECARD_INDEX 
and FLASH_COMPONENT? Should I parse the cmd line option and figure the
"lcX." prefix into an attribute?
                                                                         
Or, we would have to have something like:                                    
devlink dev flash pci/0000:01:00.0 lc 2 component fw file mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2
                                                                         
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                                             
                                                                         
But that would break the existing JSON output, because "running" is an array:
                "running": {                                             
                    "fw": "1.2.3",                                       
                    "fw.mgmt": "10.20.30"                                
                },                                                       

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?

  reply	other threads:[~2022-05-31  7:11 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-25  3:44 [PATCH net-next 00/11] mlxsw: extend line card model by devices and info Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 01/11] devlink: introduce line card devices support Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 02/11] devlink: introduce line card info get message Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 03/11] devlink: introduce line card device info infrastructure Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 04/11] mlxsw: reg: Extend MDDQ by device_info Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 05/11] mlxsw: core_linecards: Probe provisioned line cards for devices and attach them Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 06/11] selftests: mlxsw: Check devices on provisioned line card Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 07/11] mlxsw: core_linecards: Expose HW revision and INI version Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 08/11] selftests: mlxsw: Check line card info on provisioned line card Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 09/11] mlxsw: reg: Extend MDDQ device_info by FW version fields Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 10/11] mlxsw: core_linecards: Expose device FW version over device info Ido Schimmel
2022-04-25  3:44 ` [PATCH net-next 11/11] selftests: mlxsw: Check device info on activated line card Ido Schimmel
2022-04-25  9:50 ` [PATCH net-next 00/11] mlxsw: extend line card model by devices and info patchwork-bot+netdevbpf
2022-04-25 16:00 ` Jakub Kicinski
2022-04-25 19:39   ` Ido Schimmel
2022-04-25 19:52     ` Jakub Kicinski
2022-04-26  6:57       ` Jiri Pirko
2022-04-26 12:11         ` Andrew Lunn
2022-04-26 12:36           ` Jiri Pirko
2022-04-26 12:41         ` Jakub Kicinski
2022-04-26 14:00           ` Jiri Pirko
2022-04-26 14:51             ` Jakub Kicinski
2022-04-27  7:35               ` Jiri Pirko
2022-04-27 14:14                 ` Jakub Kicinski
2022-04-29 11:51                   ` Jiri Pirko
2022-04-29 18:45                     ` Jakub Kicinski
2022-04-29 19:29                       ` Jiri Pirko
2022-04-29 22:38                         ` Jakub Kicinski
2022-04-30  6:27                           ` Jiri Pirko
2022-05-02 14:39                             ` Jakub Kicinski
2022-05-23  9:42                               ` Jiri Pirko
2022-05-23 17:56                                 ` Jakub Kicinski
2022-05-24  6:46                                   ` Jiri Pirko
2022-05-24 14:31                                     ` Jiri Pirko
2022-05-24 18:00                                       ` Jakub Kicinski
2022-05-25  6:20                                         ` Jiri Pirko
2022-05-25 15:50                                           ` Jakub Kicinski
2022-05-26  9:05                                             ` Jiri Pirko
2022-05-26 10:47                                               ` Jiri Pirko
2022-05-26 11:45                                             ` Jiri Pirko
2022-05-26 17:35                                               ` Jakub Kicinski
2022-05-27  7:27                                                 ` Jiri Pirko
2022-05-28  0:10                                                   ` Jakub Kicinski
2022-05-28  9:09                                                     ` Jiri Pirko
2022-05-28 19:02                                                       ` Jakub Kicinski
2022-05-29  9:23                                                         ` Jiri Pirko
2022-05-30 19:54                                                           ` Jakub Kicinski
2022-05-31  7:11                                                             ` Jiri Pirko [this message]
2022-05-31 15:05                                                               ` Jakub Kicinski
2022-05-31 15:51                                                                 ` Jiri Pirko
2022-05-31 16:08                                                                   ` Jakub Kicinski
2022-05-31 19:34                                                                     ` Jiri Pirko
2022-05-31 22:41                                                                       ` Jakub Kicinski
2022-06-01  7:35                                                                         ` Jiri Pirko
2022-05-28 15:58                                       ` David Ahern
2022-05-29  9:24                                         ` Jiri Pirko
2022-05-31  2:11                                           ` David Ahern
2022-05-31  7:05                                             ` Jiri Pirko
2022-04-26 15:24             ` Andrew Lunn
2022-04-27  7:37               ` Jiri Pirko
2022-04-26  6:47     ` Jiri Pirko
2022-04-26 12:27       ` Andrew Lunn
2022-04-26 12:41         ` Jiri Pirko
2022-04-26 13:45           ` Andrew Lunn
2022-04-26 14:05             ` Jiri Pirko
2022-04-26 15:36               ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YpW/n3Nh8fIYOEe+@nanopsycho \
    --to=jiri@resnulli.us \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=idosch@idosch.org \
    --cc=idosch@nvidia.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=mlxsw@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=petrm@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox