[iproute2-next v4 0/2] devlink: add flash update overwrite mask

[iproute2-next v4 0/2] devlink: add flash update overwrite mask

[Jacob Keller]

This series implements the iproute2 side of the new
DEVLINK_ATTR_FLASH_UPDATE_OVERWRITE_MASK.

This attribute is used to allow userspace to indicate what a device should
do with various subsections of a flash component when updating. For example,
a flash component might contain vital data such as the PCIe serial number or
configuration fields such as settings that control device bootup.

The overwrite mask allows the user to specify what behavior they want when
performing an update. If nothing is specified, then the update should
preserve all vital fields and configuration.

By specifying "overwrite identifiers" the user requests that the flash
update should overwrite any identifiers in the updated flash component with
identifier values from the provided flash image.

  $devlink dev flash pci/0000:af:00.0 file flash_image.bin overwrite identifiers

By specifying "overwrite settings" the user requests that the flash update
should overwrite any settings in the updated flash component with setting
values from the provided flash image.

  $devlink dev flash pci/0000:af:00.0 file flash_image.bin overwrite settings

These options may be combined, in which case both subsections will be sent
in the overwrite mask, resulting in a request to overwrite all settings and
identifiers stored in the updated flash components.

  $devlink dev flash pci/0000:af:00.0 file flash_image.bin overwrite settings overwrite identifiers

Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Jakub Kicinski <kuba@kernel.org>

Jacob Keller (2):
  Update devlink header for overwrite mask attribute
  devlink: support setting the overwrite mask

 devlink/devlink.c            | 48 ++++++++++++++++++++++++++++++++++--
 include/uapi/linux/devlink.h | 27 ++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)


base-commit: ad34d5fadb0b4699b0fe136fc408685e26bb1b43
-- 
2.28.0.218.ge27853923b9d

[iproute2-next v4 1/2] Update devlink header for overwrite mask attribute

[Jacob Keller]

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 include/uapi/linux/devlink.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index b7f23faae901..dc267058600d 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -228,6 +228,28 @@ enum {
 	DEVLINK_ATTR_STATS_MAX = __DEVLINK_ATTR_STATS_MAX - 1
 };
 
+/* Specify what sections of a flash component can be overwritten when
+ * performing an update. Overwriting of firmware binary sections is always
+ * implicitly assumed to be allowed.
+ *
+ * Each section must be documented in
+ * Documentation/networking/devlink/devlink-flash.rst
+ *
+ */
+enum {
+	DEVLINK_FLASH_OVERWRITE_SETTINGS_BIT,
+	DEVLINK_FLASH_OVERWRITE_IDENTIFIERS_BIT,
+
+	__DEVLINK_FLASH_OVERWRITE_MAX_BIT,
+	DEVLINK_FLASH_OVERWRITE_MAX_BIT = __DEVLINK_FLASH_OVERWRITE_MAX_BIT - 1
+};
+
+#define DEVLINK_FLASH_OVERWRITE_SETTINGS BIT(DEVLINK_FLASH_OVERWRITE_SETTINGS_BIT)
+#define DEVLINK_FLASH_OVERWRITE_IDENTIFIERS BIT(DEVLINK_FLASH_OVERWRITE_IDENTIFIERS_BIT)
+
+#define DEVLINK_SUPPORTED_FLASH_OVERWRITE_SECTIONS \
+	(BIT(__DEVLINK_FLASH_OVERWRITE_MAX_BIT) - 1)
+
 /**
  * enum devlink_trap_action - Packet trap action.
  * @DEVLINK_TRAP_ACTION_DROP: Packet is dropped by the device and a copy is not
@@ -458,6 +480,11 @@ enum devlink_attr {
 	DEVLINK_ATTR_PORT_LANES,			/* u32 */
 	DEVLINK_ATTR_PORT_SPLITTABLE,			/* u8 */
 
+	DEVLINK_ATTR_PORT_EXTERNAL,		/* u8 */
+	DEVLINK_ATTR_PORT_CONTROLLER_NUMBER,	/* u32 */
+
+	DEVLINK_ATTR_FLASH_UPDATE_OVERWRITE_MASK,	/* bitfield32 */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
-- 
2.28.0.218.ge27853923b9d.dirty

[iproute2-next v4 2/2] devlink: support setting the overwrite mask

[Jacob Keller]

Add support for specifying the overwrite sections to allow in the flash
update command. This is done by adding a new "overwrite" option which
can take either "settings" or "identifiers" passing the overwrite mode
multiple times will combine the fields using bitwise-OR.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 devlink/devlink.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 007677a5c564..cafac9ca7acd 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -302,6 +302,7 @@ static void ifname_map_free(struct ifname_map *ifname_map)
 #define DL_OPT_TRAP_POLICER_BURST	BIT(36)
 #define DL_OPT_HEALTH_REPORTER_AUTO_DUMP     BIT(37)
 #define DL_OPT_PORT_FUNCTION_HW_ADDR BIT(38)
+#define DL_OPT_FLASH_OVERWRITE		BIT(39)
 
 struct dl_opts {
 	uint64_t present; /* flags of present items */
@@ -349,6 +350,7 @@ struct dl_opts {
 	uint64_t trap_policer_burst;
 	char port_function_hw_addr[MAX_ADDR_LEN];
 	uint32_t port_function_hw_addr_len;
+	uint32_t overwrite_mask;
 };
 
 struct dl {
@@ -1285,6 +1287,19 @@ eswitch_encap_mode_get(const char *typestr,
 	return 0;
 }
 
+static int flash_overwrite_section_get(const char *sectionstr, uint32_t *mask)
+{
+	if (strcmp(sectionstr, "settings") == 0) {
+		*mask |= DEVLINK_FLASH_OVERWRITE_SETTINGS;
+	} else if (strcmp(sectionstr, "identifiers") == 0) {
+		*mask |= DEVLINK_FLASH_OVERWRITE_IDENTIFIERS;
+	} else {
+		pr_err("Unknown overwrite section \"%s\"\n", sectionstr);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int param_cmode_get(const char *cmodestr,
 			   enum devlink_param_cmode *cmode)
 {
@@ -1627,6 +1642,21 @@ static int dl_argv_parse(struct dl *dl, uint64_t o_required,
 			if (err)
 				return err;
 			o_found |= DL_OPT_FLASH_COMPONENT;
+
+		} else if (dl_argv_match(dl, "overwrite") &&
+				(o_all & DL_OPT_FLASH_OVERWRITE)) {
+			const char *sectionstr;
+
+			dl_arg_inc(dl);
+			err = dl_argv_str(dl, &sectionstr);
+			if(err)
+				return err;
+			err = flash_overwrite_section_get(sectionstr,
+							  &opts->overwrite_mask);
+			if (err)
+				return err;
+			o_found |= DL_OPT_FLASH_OVERWRITE;
+
 		} else if (dl_argv_match(dl, "reporter") &&
 			   (o_all & DL_OPT_HEALTH_REPORTER_NAME)) {
 			dl_arg_inc(dl);
@@ -1767,6 +1797,18 @@ dl_function_attr_put(struct nlmsghdr *nlh, const struct dl_opts *opts)
 	mnl_attr_nest_end(nlh, nest);
 }
 
+static void
+dl_flash_update_overwrite_put(struct nlmsghdr *nlh, const struct dl_opts *opts)
+{
+	struct nla_bitfield32 overwrite_mask;
+
+	overwrite_mask.selector = DEVLINK_SUPPORTED_FLASH_OVERWRITE_SECTIONS;
+	overwrite_mask.value = opts->overwrite_mask;
+
+	mnl_attr_put(nlh, DEVLINK_ATTR_FLASH_UPDATE_OVERWRITE_MASK,
+		     sizeof(overwrite_mask), &overwrite_mask);
+}
+
 static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
 {
 	struct dl_opts *opts = &dl->opts;
@@ -1854,6 +1896,8 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
 	if (opts->present & DL_OPT_FLASH_COMPONENT)
 		mnl_attr_put_strz(nlh, DEVLINK_ATTR_FLASH_UPDATE_COMPONENT,
 				  opts->flash_component);
+	if (opts->present & DL_OPT_FLASH_OVERWRITE)
+		dl_flash_update_overwrite_put(nlh, opts);
 	if (opts->present & DL_OPT_HEALTH_REPORTER_NAME)
 		mnl_attr_put_strz(nlh, DEVLINK_ATTR_HEALTH_REPORTER_NAME,
 				  opts->reporter_name);
@@ -1954,7 +1998,7 @@ static void cmd_dev_help(void)
 	pr_err("       devlink dev param show [DEV name PARAMETER]\n");
 	pr_err("       devlink dev reload DEV [ netns { PID | NAME | ID } ]\n");
 	pr_err("       devlink dev info [ DEV ]\n");
-	pr_err("       devlink dev flash DEV file PATH [ component NAME ]\n");
+	pr_err("       devlink dev flash DEV file PATH [ component NAME ] [ overwrite SECTION ]\n");
 }
 
 static bool cmp_arr_last_handle(struct dl *dl, const char *bus_name,
@@ -3219,7 +3263,7 @@ static int cmd_dev_flash(struct dl *dl)
 			       NLM_F_REQUEST | NLM_F_ACK);
 
 	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE | DL_OPT_FLASH_FILE_NAME,
-				DL_OPT_FLASH_COMPONENT);
+				DL_OPT_FLASH_COMPONENT | DL_OPT_FLASH_OVERWRITE);
 	if (err)
 		return err;
 
-- 
2.28.0.218.ge27853923b9d.dirty

Re: [iproute2-next v4 0/2] devlink: add flash update overwrite mask

[David Ahern]

On 9/9/20 3:28 PM, Jacob Keller wrote:
> This series implements the iproute2 side of the new
> DEVLINK_ATTR_FLASH_UPDATE_OVERWRITE_MASK.
> 
> This attribute is used to allow userspace to indicate what a device should
> do with various subsections of a flash component when updating. For example,
> a flash component might contain vital data such as the PCIe serial number or
> configuration fields such as settings that control device bootup.
> 
> The overwrite mask allows the user to specify what behavior they want when
> performing an update. If nothing is specified, then the update should
> preserve all vital fields and configuration.
> 
> By specifying "overwrite identifiers" the user requests that the flash
> update should overwrite any identifiers in the updated flash component with
> identifier values from the provided flash image.
> 
>   $devlink dev flash pci/0000:af:00.0 file flash_image.bin overwrite identifiers
> 
> By specifying "overwrite settings" the user requests that the flash update
> should overwrite any settings in the updated flash component with setting
> values from the provided flash image.
> 
>   $devlink dev flash pci/0000:af:00.0 file flash_image.bin overwrite settings
> 
> These options may be combined, in which case both subsections will be sent
> in the overwrite mask, resulting in a request to overwrite all settings and
> identifiers stored in the updated flash components.
> 
>   $devlink dev flash pci/0000:af:00.0 file flash_image.bin overwrite settings overwrite identifiers
> 
> Cc: Jiri Pirko <jiri@mellanox.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> 
> Jacob Keller (2):
>   Update devlink header for overwrite mask attribute
>   devlink: support setting the overwrite mask
> 
>  devlink/devlink.c            | 48 ++++++++++++++++++++++++++++++++++--
>  include/uapi/linux/devlink.h | 27 ++++++++++++++++++++
>  2 files changed, 73 insertions(+), 2 deletions(-)
> 
> 
> base-commit: ad34d5fadb0b4699b0fe136fc408685e26bb1b43
> 

Jacob:

Compile fails on Ubuntu 20.04:

devlink
    CC       devlink.o
In file included from devlink.c:29:
devlink.c: In function ‘flash_overwrite_section_get’:
../include/uapi/linux/devlink.h:249:42: warning: implicit declaration of
function ‘_BITUL’ [-Wimplicit-function-declaration]
  249 | #define DEVLINK_FLASH_OVERWRITE_SETTINGS
_BITUL(DEVLINK_FLASH_OVERWRITE_SETTINGS_BIT)
      |                                          ^~~~~~
devlink.c:1293:12: note: in expansion of macro
‘DEVLINK_FLASH_OVERWRITE_SETTINGS’
 1293 |   *mask |= DEVLINK_FLASH_OVERWRITE_SETTINGS;
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    CC       mnlg.o
    LINK     devlink

I updated headers in -next; please redo the patch set and roll the cover
letter details in patch 2.

Re: [iproute2-next v4 0/2] devlink: add flash update overwrite mask

[Jacob Keller]

On 9/29/2020 8:33 AM, David Ahern wrote:
> On 9/9/20 3:28 PM, Jacob Keller wrote:
>> This series implements the iproute2 side of the new
>> DEVLINK_ATTR_FLASH_UPDATE_OVERWRITE_MASK.
>>
>> This attribute is used to allow userspace to indicate what a device should
>> do with various subsections of a flash component when updating. For example,
>> a flash component might contain vital data such as the PCIe serial number or
>> configuration fields such as settings that control device bootup.
>>
>> The overwrite mask allows the user to specify what behavior they want when
>> performing an update. If nothing is specified, then the update should
>> preserve all vital fields and configuration.
>>
>> By specifying "overwrite identifiers" the user requests that the flash
>> update should overwrite any identifiers in the updated flash component with
>> identifier values from the provided flash image.
>>
>>   $devlink dev flash pci/0000:af:00.0 file flash_image.bin overwrite identifiers
>>
>> By specifying "overwrite settings" the user requests that the flash update
>> should overwrite any settings in the updated flash component with setting
>> values from the provided flash image.
>>
>>   $devlink dev flash pci/0000:af:00.0 file flash_image.bin overwrite settings
>>
>> These options may be combined, in which case both subsections will be sent
>> in the overwrite mask, resulting in a request to overwrite all settings and
>> identifiers stored in the updated flash components.
>>
>>   $devlink dev flash pci/0000:af:00.0 file flash_image.bin overwrite settings overwrite identifiers
>>
>> Cc: Jiri Pirko <jiri@mellanox.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>>
>> Jacob Keller (2):
>>   Update devlink header for overwrite mask attribute
>>   devlink: support setting the overwrite mask
>>
>>  devlink/devlink.c            | 48 ++++++++++++++++++++++++++++++++++--
>>  include/uapi/linux/devlink.h | 27 ++++++++++++++++++++
>>  2 files changed, 73 insertions(+), 2 deletions(-)
>>
>>
>> base-commit: ad34d5fadb0b4699b0fe136fc408685e26bb1b43
>>
> 
> Jacob:
> 
> Compile fails on Ubuntu 20.04:
> 
> devlink
>     CC       devlink.o
> In file included from devlink.c:29:
> devlink.c: In function ‘flash_overwrite_section_get’:
> ../include/uapi/linux/devlink.h:249:42: warning: implicit declaration of
> function ‘_BITUL’ [-Wimplicit-function-declaration]
>   249 | #define DEVLINK_FLASH_OVERWRITE_SETTINGS
> _BITUL(DEVLINK_FLASH_OVERWRITE_SETTINGS_BIT)
>       |                                          ^~~~~~
> devlink.c:1293:12: note: in expansion of macro
> ‘DEVLINK_FLASH_OVERWRITE_SETTINGS’
>  1293 |   *mask |= DEVLINK_FLASH_OVERWRITE_SETTINGS;
>       |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     CC       mnlg.o
>     LINK     devlink
> 
> I updated headers in -next; please redo the patch set and roll the cover
> letter details in patch 2.
> 

This appears to be because uapi/linux/const.h isn't included... I am not
sure what the correct fix here is.. should this be part of the include
chain for uapi/linux/devlink.h? I could add this to the devlink.c file
but that feels incorrect since the definition/usage is in
uapi/linux/devlink.h...

Thanks,
Jake