| Message ID | 20200801002159.3300425-6-jacob.e.keller@intel.com |
|---|---|
| State | Rejected |
| Delegated to: | David Ahern |
| Headers | show |
| Series | devlink flash update overwrite mask | expand |
On 7/31/20 6:21 PM, Jacob Keller wrote: > 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 | 37 +++++++++++++++++++++++++++++++++++-- > 1 file changed, 35 insertions(+), 2 deletions(-) > 5/5? I only see 2 - 4/5 and 5/5. Please re-send against latest iproute2-next.
Mon, Aug 03, 2020 at 05:53:16PM CEST, dsahern@gmail.com wrote: >On 7/31/20 6:21 PM, Jacob Keller wrote: >> 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 | 37 +++++++++++++++++++++++++++++++++++-- >> 1 file changed, 35 insertions(+), 2 deletions(-) >> > >5/5? I only see 2 - 4/5 and 5/5. Please re-send against latest >iproute2-next. 1-3 are kernel. >
On 8/3/2020 8:53 AM, David Ahern wrote: > On 7/31/20 6:21 PM, Jacob Keller wrote: >> 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 | 37 +++++++++++++++++++++++++++++++++++-- >> 1 file changed, 35 insertions(+), 2 deletions(-) >> > > 5/5? I only see 2 - 4/5 and 5/5. Please re-send against latest > iproute2-next. > Sorry for the confusion here. I sent both the iproute2 and net-next changes to implement it in the kernel.
On 8/3/20 10:56 AM, Jacob Keller wrote: > Sorry for the confusion here. I sent both the iproute2 and net-next > changes to implement it in the kernel. please re-send the iproute2 patch; I already marked it in patchworks. I get sending the patches in 1 go, but kernel and iproute2 patch numbering should be separate.
On 8/3/2020 2:20 PM, David Ahern wrote: > On 8/3/20 10:56 AM, Jacob Keller wrote: >> Sorry for the confusion here. I sent both the iproute2 and net-next >> changes to implement it in the kernel. > > please re-send the iproute2 patch; I already marked it in patchworks. I > get sending the patches in 1 go, but kernel and iproute2 patch numbering > should be separate. > Yep, I'll do that in the future, an will be re-sending a v2 shortly with changes on the kernel side from Jiri's review. Thanks, Jake
On 8/3/2020 8:53 AM, David Ahern wrote: > On 7/31/20 6:21 PM, Jacob Keller wrote: >> 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 | 37 +++++++++++++++++++++++++++++++++++-- >> 1 file changed, 35 insertions(+), 2 deletions(-) >> > > 5/5? I only see 2 - 4/5 and 5/5. Please re-send against latest > iproute2-next. > Slightly unrelated: but the recent change to using a bitfield32 results in a "GENMASK is undefined".. I'm not sure what the proper way to fix this is, since we'd like to still use GENMASK to define the supported bitfields. I guess we need to pull in more headers? Or define something in include/utils.h? Thanks, Jake
On 8/3/20 5:30 PM, Jacob Keller wrote: > > Slightly unrelated: but the recent change to using a bitfield32 results > in a "GENMASK is undefined".. I'm not sure what the proper way to fix > this is, since we'd like to still use GENMASK to define the supported > bitfields. I guess we need to pull in more headers? Or define something > in include/utils.h? > I see that include/linux/bits.h has been pulled into the tools directory for perf and power tools (ie., works fine in userspace). iproute2 is GPL so should be good from a licensing perspective to copy into iproute2. Stephen: any objections?
On 8/3/2020 4:54 PM, David Ahern wrote: > On 8/3/20 5:30 PM, Jacob Keller wrote: >> >> Slightly unrelated: but the recent change to using a bitfield32 results >> in a "GENMASK is undefined".. I'm not sure what the proper way to fix >> this is, since we'd like to still use GENMASK to define the supported >> bitfields. I guess we need to pull in more headers? Or define something >> in include/utils.h? >> > > I see that include/linux/bits.h has been pulled into the tools directory > for perf and power tools (ie., works fine in userspace). > > iproute2 is GPL so should be good from a licensing perspective to copy > into iproute2. Stephen: any objections? > Hmm... Actually, no other uapi header uses GENMASK.. Perhaps its better to just avoid using it in the uapi/linux/devlink.h header...
diff --git a/devlink/devlink.c b/devlink/devlink.c index 7dbe9c7e07a8..a3360a09898b 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 { @@ -1282,6 +1284,19 @@ eswitch_encap_mode_get(const char *typestr, return 0; } +static int flash_overwrite_mask_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) { @@ -1624,6 +1639,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, §ionstr); + if(err) + return err; + err = flash_overwrite_mask_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); @@ -1851,6 +1881,9 @@ 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) + mnl_attr_put_u32(nlh, DEVLINK_ATTR_FLASH_UPDATE_OVERWRITE_MASK, + opts->overwrite_mask); if (opts->present & DL_OPT_HEALTH_REPORTER_NAME) mnl_attr_put_strz(nlh, DEVLINK_ATTR_HEALTH_REPORTER_NAME, opts->reporter_name); @@ -1951,7 +1984,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, @@ -3205,7 +3238,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;
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 | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-)