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

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 29 Apr 2022 13:51:33 +0200 Jiri Pirko wrote:
> >Of the three API levels (SDK, automation, human) I think automation
> >is the only one that's interesting to us in Linux. SDK interfaces are
> >necessarily too low level as they expose too much of internal details
> >to standardize. Humans are good with dealing with uncertainty and
> >diverse so there's no a good benchmark.
> >
> >The benchmark for automation is - can a machine use this API across
> >different vendors to reliably achieve its goals. For FW info/flashing
> >the goal is keeping the FW versions up to date. This is documented:
> >
> >https://www.kernel.org/doc/html/latest/networking/devlink/devlink-flash.html#firmware-version-management
> >
> >What would the pseudo code look like with "line cards" in the picture?
> >Apply RFC1925 truth 12.  
> 
> Something like this:
> 
> $lc_count = array_size(devlink-lc-info[$handle])
> 
> for ($lcnum = 0; $lcnum < $lc_count; lcnum++):
>     $dev_count = array_size(devlink-lc-info[$handle][$lcnum])
> 
>     for ($devnum = 0; $devnum < $dev_count; $devnum++):

Here goes the iteration I complained about in my previous message.
Tracking FW versions makes most sense at the level of a product (as 
in the unit of HW one can purchase from the system vendor). That
integrates well with system tracking HW in the fleet. Product in your
case will be a line card or populated chassis, I believe.

>         # Get unique HW design identifier (gearbox id)
>         $hw_id = devlink-lc-info[$handle][$lcnum][$devnum]['fw.psid']

1) you can't use 'fw.psid' in generic logic, it's a Melvidia's invention
2) looking at your cover letter there's no fw.psid for the device
   reported, the automation will not work, Q.E.D.

>         # Find out which FW flash we want to use for this device
>         $want_flash_vers = some-db-backed.lookup($hw_id, 'flash')
> 
>         # Update flash if necessary
>         if $want_flash_vers != devlink-lc-info[$handle][$lcnum][$devnum]['fw']:
>             $file = some-db-backed.download($hw_id, 'flash')
>             $component = devlink-lc[$handle][$lcnum][$devnum]['component']
>             devlink-dev-flash($handle, $component, $file)
> 
> devlink-reload()
> 
> Clear indexes, not squashed somewhere in middle of string.
> 
> >I thought you said your gearboxes all the the same FW? 
> >Theoretically, yes. Theoretically, I can also have nested "line cards".  
> 
> Well, yeah. I was under impresion that possibility of having multiple
> devices on the same LC is not close to 0. But I get your point.
> 
> Let's try to figure out he iface as you want it:
> We will have devlink dev info extended to be able to provide info
> about the LC/gearbox. Let's work with same prefix "lcX." for all
> info related to line card X.
> 
> First problem is, who is going to enforce this prefix. It is driver's
> responsibility to provide the string which is put out. The solution
> would be to have a helper similar to devlink_info_version_*_put()
> called devlink_info_lc_version_*_put() that would accept lc pointer and
> add the prefix. Does it make sense to you?
> 
> We need 3 things:
> 1) current version of gearbox FW 
>    That is easy, we have it - "versions"
>    (DEVLINK_ATTR_INFO_VERSION_* nested attrs). We can have multiple
>    nests that would carry the versions for individiual line cards.
>    Example:
>        versions:
>            fixed:
>              hw.revision 0
>              lc2.hw.revision a
>              lc8.hw.revision b
>            running:
>              ini.version 4
>              lc2.gearbox.fw.version 1.1.3
>              lc8.gearbox.fw.version 1.2.3
> 2) HW id (as you have it in your pseudocode), it is PSID in case of
>    mlxsw. We already have PSID for ASIC:
>    ....
>    This should be also easy, as this is exposed as "fixed version" in a
>    same way as previous example.
>    Example:
>        versions:
>            fixed:
>              lc2.gearbox.fw.psid XXX
>              lc8.gearbox.fw.psid YYY
> 3) Component name
>    This one is a bit tricky. It is not a version, so put it under
>    "versions" does not make much sense.
>    Also, there are more than one. Looking at how versions are done,
>    multiple nests of attr type DEVLINK_ATTR_INFO_VERSION_* are put to
>    the message. So we can introduce new attr DEVLINK_ATTR_INFO_FLASH_COMPONENT
>    and put one per linecard/gearbox.
>    Here arain we need some kind of helper to driver to call to put this
>    into msg: devlink_info_lc_flash_component_put()
>    Example:
>      pci/0000:01:00.0:
>        driver mlxsw_spectrum3
>        flash_components:
>          lc2.gearbox.fw
>          lc8.gearbox.fw
> 
>     Then the flashing of the gearbox will be done by:
>     # devlink dev flash pci/0000:01:00.0 file mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2 component lc2.gearbox.fw

The main question to me is whether users will want to flash the entire
device, or update line cards individually.

What's inside mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2? Doesn't
sound like it's FW just for a single gearbox?

> >Really? So we're going to be designing kernel APIs so that each message
> >contains complete information and can't contain references now?  
> 
> Can you give me an exapmple of devlink or any other iproute2 app cmd
> that actually does something similar to this?

Off the top of my head - doesn't ip has some caches for name resolution
etc.? Either way current code in iproute2.git is hardly written on the
stone tablets.