> -----Original Message-----
> From: Jiri Pirko <jiri@xxxxxxxxxxx>
> Sent: Thursday, July 21, 2022 11:26 PM
> To: Keller, Jacob E <jacob.e.keller@xxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; Jonathan Corbet <corbet@xxxxxxx>; Jiri Pirko
> <jiri@xxxxxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>;
> David Ahern <dsahern@xxxxxxxxxx>; Stephen Hemminger
> <stephen@xxxxxxxxxxxxxxxxxx>; linux-doc@xxxxxxxxxxxxxxx; intel-wired-
> lan@xxxxxxxxxxxxxxxx
> Subject: Re: [net-next v2 1/2] devlink: add dry run attribute to flash update
>
> Thu, Jul 21, 2022 at 11:14:46PM CEST, jacob.e.keller@xxxxxxxxx wrote:
> >Users use the devlink flash interface to request a device driver program or
> >update the device flash chip. In some cases, a user (or script) may want to
> >verify that a given flash update command is supported without actually
> >committing to immediately updating the device. For example, a system
> >administrator may want to validate that a particular flash binary image
> >will be accepted by the device, or simply validate a command before finally
> >committing to it.
> >
> >The current flash update interface lacks a method to support such a dry
> >run. Add a new DEVLINK_ATTR_DRY_RUN attribute which shall be used by a
> >devlink command to indicate that a request is a dry run which should not
> >perform device configuration. Instead, the command should report whether
> >the command or configuration request is valid.
> >
> >While we can validate the initial arguments of the devlink command, a
> >proper dry run must be processed by the device driver. This is required
> >because only the driver can perform validation of the flash binary file.
> >
> >Add a new dry_run parameter to the devlink_flash_update_params struct,
> >along with the associated bit to indicate if a driver supports verifying a
> >dry run.
> >
> >We always check the dry run attribute last in order to allow as much
> >verification of other parameters as possible. For example, even if a driver
> >does not support the dry_run option, we can still validate the other
> >optional parameters such as the overwrite_mask and per-component update
> >name.
> >
> >Document that userspace should take care when issuing a dry run to older
> >kernels, as the flash update command is not strictly verified. Thus,
> >unknown attributes will be ignored and this could cause a request for a dry
> >run to perform an actual update. We can't fix old kernels to verify unknown
> >attributes, but userspace can check the maximum attribute and reject the
> >dry run request if it is not supported by the kernel.
> >
> >Signed-off-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>
> >---
> >
> >Changes since v1:
> >* Add kernel doc comments to devlink_flash_update_params
> >* Reduce indentation by using nla_get_flag
> >
> > .../networking/devlink/devlink-flash.rst | 23 +++++++++++++++++++
> > include/net/devlink.h | 4 ++++
> > include/uapi/linux/devlink.h | 8 +++++++
> > net/core/devlink.c | 17 +++++++++++++-
> > 4 files changed, 51 insertions(+), 1 deletion(-)
> >
> >diff --git a/Documentation/networking/devlink/devlink-flash.rst
> b/Documentation/networking/devlink/devlink-flash.rst
> >index 603e732f00cc..1dc373229a54 100644
> >--- a/Documentation/networking/devlink/devlink-flash.rst
> >+++ b/Documentation/networking/devlink/devlink-flash.rst
> >@@ -44,6 +44,29 @@ preserved across the update. A device may not support
> every combination and
> > the driver for such a device must reject any combination which cannot be
> > faithfully implemented.
> >
> >+Dry run
> >+=======
> >+
> >+Users can request a "dry run" of a flash update by adding the
> >+``DEVLINK_ATTR_DRY_RUN`` attribute to the ``DEVLINK_CMD_FLASH_UPDATE``
> >+command. If the attribute is present, the kernel will only verify that the
> >+provided command is valid. During a dry run, an update is not performed.
> >+
> >+If supported by the driver, the flash image contents are also validated and
> >+the driver may indicate whether the file is a valid flash image for the
> >+device.
> >+
> >+.. code:: shell
> >+
> >+ $ devlink dev flash pci/0000:af:00.0 file image.bin dry-run
> >+ Validating flash binary
> >+
> >+Note that user space should take care when adding this attribute. Older
> >+kernels which do not recognize the attribute may accept the command with an
> >+unknown attribute. This could lead to a request for a dry run which performs
> >+an unexpected update. To avoid this, user space should check the policy dump
> >+and verify that the attribute is recognized before adding it to the command.
> >+
> > Firmware Loading
> > ================
> >
> >diff --git a/include/net/devlink.h b/include/net/devlink.h
> >index 780744b550b8..47b86ccb85b0 100644
> >--- a/include/net/devlink.h
> >+++ b/include/net/devlink.h
> >@@ -613,6 +613,8 @@ enum devlink_param_generic_id {
> > * struct devlink_flash_update_params - Flash Update parameters
> > * @fw: pointer to the firmware data to update from
> > * @component: the flash component to update
> >+ * @overwrite_mask: what sections of flash can be overwritten
>
> Well, strictly speaking, this is not related to this patch and should be
> done in a separate one. But hey, it's a comment, so I guess noone really
> cares.
>
Ah yep. I'll split this out since I need to send a new version to split the iproute stuff into its own thread anyways.
