netdev - Re: [PATCH net-next] devlink: expand the devlink-info documentation

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] [day] [month] [year] [list]

Message-ID: <20200323211221.018e6541@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 23 Mar 2020 21:12:21 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Randy Dunlap <rdunlap@...radead.org>, jesse.brandeburg@...el.com
Cc:     davem@...emloft.net, netdev@...r.kernel.org, kernel-team@...com,
        eugenem@...com, jacob.e.keller@...el.com, jiri@...nulli.us,
        michael.chan@...adcom.com, snelson@...sando.io
Subject: Re: [PATCH net-next] devlink: expand the devlink-info documentation

On Mon, 23 Mar 2020 18:38:57 -0700 Randy Dunlap wrote:
> Hi-
> Here are a few small changes for you to consider:

Thanks a lot for the corrections!

> On 3/23/20 5:50 PM, Jakub Kicinski wrote:
> > diff --git a/Documentation/networking/devlink/devlink-flash.rst b/Documentation/networking/devlink/devlink-flash.rst
> > new file mode 100644
> > index 000000000000..46cea870117d
> > --- /dev/null
> > +++ b/Documentation/networking/devlink/devlink-flash.rst
> > @@ -0,0 +1,88 @@
> > +.. SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +
> > +.. _devlink_flash:
> > +
> > +=============
> > +Devlink Flash
> > +=============
> > +
> > +The ``devlink-flash`` allows updating device firmware. It replaces the  
> 
>    The ``devlink-flash`` {option, mechanism, command, utility, something} allows

API

> > +devlink parameter can be used to control this behavior
> > +(:ref:`Documentation/networking/devlink/devlink-params.rst <devlink_params_generic>`).
> > +
> > +On-disk firmware files are usually stored in ``/lib/firmware/``.
> > +
> > +Firmware Version Management
> > +===========================
> > +
> > +Drivers are expected to implement ``devlink-flash`` and ``devlink-info``
> > +functionality, which together allow for implementing vendor-independent
> > +automated firmware update facilities.
> > +
> > +``devlink-info`` exposes the ``driver`` name and three version groups
> > +(``fixed``, ``running``, ``stored``).
> > +
> > +``driver`` and ``fixed`` group identify the specific device design,  
> 
>    The ``driver`` and ``fixed`` groups

The ``driver`` attribute and ``fixed`` group

(Only ``fixed`` is a group, driver is a single attr.)

> > +e.g. for looking up applicable firmware updates. This is why ``serial_number``
> > +is not part of the ``fixed`` versions (even though it is fixed) - ``fixed``
> > +versions should identify the design, not a single device.
> > +
> > +``running`` and ``stored`` firmware versions identify the firmware running
> > +on the device, and firmware which will be activated after reboot or device
> > +reset.
> > +
> > +The firmware update agent is supposed to be able to follow this simple
> > +algorithm to update firmware contents, regardless of the device vendor:
> > +
> > +.. code-block:: sh
> > +
> > +  # Get unique HW design identifier
> > +  $hw_id = devlink-dev-info['fixed']
> > +
> > +  # Find out which FW flash we want to use for this NIC
> > +  $want_flash_vers = some-db-backed.lookup($hw_id, 'flash')
> > +
> > +  # Update flash if necessary
> > +  if $want_flash_vers != devlink-dev-info['stored']:
> > +      $file = some-db-backed.download($hw_id, 'flash')
> > +      devlink-dev-flash($file)
> > +
> > +  # Find out the expected overall firmware versions
> > +  $want_fw_vers = some-db-backed.lookup($hw_id, 'all')
> > +
> > +  # Update on-disk file if necessary
> > +  if $want_fw_vers != devlink-dev-info['running']:
> > +      $file = some-db-backed.download($hw_id, 'disk')
> > +      write($file, '/lib/firmware/')
> > +
> > +  # Reboot if necessary
> > +  if $want_fw_vers != devlink-dev-info['running']:
> > +     reboot()  
> 
> or device reset?  or is that expected/given already?

We don't have an API to tell use which one is necessary, let me do:

  # Try device reset, if available
  if $want_fw_vers != devlink-dev-info['running']:
     devlink-reset()
     
  # Reboot, if reset wasn't enough
  if $want_fw_vers != devlink-dev-info['running']:
     reboot()
          
> > +       such as serial numbers. See
> > +       :ref:`Documentation/networking/devlink/devlink-flash.rst <devlink_flash>`
> > +       to understand why.
> > +
> >     * - ``running``
> > -     - Represents the version of the currently running component. For
> > -       example the running version of firmware. These versions generally
> > -       only update after a reboot.  
> 
> not reset?
>
> > +     - Group for information about currently running software/firmware.
> > +       These versions often only update after a reboot, sometimes device reset.  
> 
> oh. ok.

Yup, some of the FW controls PCI interfaces etc. which is pretty hard
to update without upsetting the host.

All other suggestions applied, thanks!