Mon, Feb 11, 2019 at 07:59:20AM CET, jakub.kicin...@netronome.com wrote:
>Add devlink flash update command. Advanced NICs have firmware
>stored in flash and often cryptographically secured. Updating
>that flash is handled by management firmware. Ethtool has a
>flash update command which served us well, however, it has two
>shortcomings:
> - it takes rtnl_lock unnecessarily - really flash update has
> nothing to do with networking, so using a networking device
> as a handle is suboptimal, which leads us to the second one:
> - it requires a functioning netdev - in case device enters an
> error state and can't spawn a netdev (e.g. communication
> with the device fails) there is no netdev to use as a handle
> for flashing.
>
>Devlink already has the ability to report the firmware versions,
>now with the ability to update the firmware/flash we will be
>able to recover devices in bad state.
>
>To enable easy interoperability with ethtool add the target
>partition ID. We may or may not add a different method of
>identification, but there is no such immediate need.
>
>Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
>---
> include/net/devlink.h | 2 ++
> include/uapi/linux/devlink.h | 6 ++++++
> net/core/devlink.c | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 38 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 07660fe4c0e3..55b3478b1291 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -529,6 +529,8 @@ struct devlink_ops {
> struct netlink_ext_ack *extack);
> int (*info_get)(struct devlink *devlink, struct devlink_info_req *req,
> struct netlink_ext_ack *extack);
>+ int (*flash_update)(struct devlink *devlink, const char *path,
>+ u32 target, struct netlink_ext_ack *extack);
> };
>
> static inline void *devlink_priv(struct devlink *devlink)
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index 72d9f7c89190..f4417283fd1b 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -103,6 +103,8 @@ enum devlink_command {
> DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET,
> DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
>
>+ DEVLINK_CMD_FLASH_UPDATE,
>+
> /* add new commands above here */
> __DEVLINK_CMD_MAX,
> DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
>@@ -326,6 +328,10 @@ enum devlink_attr {
> DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS, /* u64 */
> DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD, /* u64 */
> DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER, /* u8 */
>+
>+ DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME, /* string */
>+ DEVLINK_ATTR_FLASH_UPDATE_TARGET_ID, /* u32 */
Do we need to carry this on? I mean, the ef->region is only checked in 4
drivers against ETHTOOL_FLASH_ALL_REGIONS, which is default.
There is this bnxt driver which is the only one working with this value.
I think that for compat, there should be some id-region mapping
provided by driver which the compat layer would use to translate.
I also think that this should be in sync with what is returned in
DEVLINK_ATTR_INFO_VERSION_NAME.
For example:
$ devlink dev info pci/0000:82:00.0
pci/0000:82:00.0:
driver nfp
serial_number 16240145
versions:
fixed:
board.id AMDA0081-0001
board.rev 15
board.vendor SMA
board.model hydrogen
running:
fw.mgmt 010181.010181.0101d4
fw.cpld 0x1030000
fw.app abm-d372b6
fw.undi 0.0.2
chip.init AMDA-0081-0001 20160318164536
stored:
fw.mgmt 010181.010181.0101d4
fw.app abm-d372b6
fw.undi 0.0.2
chip.init AMDA-0081-0001 20160318164536
Now user should be able to use one of the identifiers to flash relevant
fw, like:
devlink dev flash pci/0000:82:00.0 XXX fw.mgmt file flash-boot.bin
I'm not sure about the name of "xxx" attribute. Maybe "id":
devlink dev flash pci/0000:82:00.0 id fw.mgmt file flash-boot.bin
devlink dev flash pci/0000:82:00.0 id fw.cpld file some-other.bin
[...]
