On 9/16/2020 8:02 PM, Shannon Nelson wrote:
> The dev flash status notify function parameter lists are getting
> rather long, so add a struct to be filled and passed rather than
> continuously changing the function signatures.
>
> Signed-off-by: Shannon Nelson <snel...@pensando.io>
Guess I should have read farther in the series. I would have expected
this patch first before introducing the timeout.
LGTM.
Reviewed-by: Jacob Keller <jacob.e.kel...@intel.com>
> ---
> include/net/devlink.h | 21 ++++++++++++
> net/core/devlink.c | 80 +++++++++++++++++++++++--------------------
> 2 files changed, 63 insertions(+), 38 deletions(-)
>
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index f206accf80ad..9ab2014885cb 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -391,6 +391,27 @@ struct devlink_param_gset_ctx {
> enum devlink_param_cmode cmode;
> };
>
> +/**
> + * struct devlink_flash_notify - devlink dev flash notify data
> + * @cmd: devlink notify command code
> + * @status_msg: current status string
> + * @component: firmware component being updated
> + * @done: amount of work completed of total amount
> + * @total: amount of work expected to be done
> + * @timeout: expected max timeout in seconds
> + *
> + * These are values to be given to userland to be displayed in order
> + * to show current activity in a firmware update process.
> + */
> +struct devlink_flash_notify {
> + enum devlink_command cmd;
> + const char *status_msg;
> + const char *component;
> + unsigned long done;
> + unsigned long total;
> + unsigned long timeout;
> +};
> +
> /**
> * struct devlink_param - devlink configuration parameter data
> * @name: name of the parameter
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 01f855e53e06..816f27a18b16 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -3021,41 +3021,36 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb,
> struct genl_info *info)
>
> static int devlink_nl_flash_update_fill(struct sk_buff *msg,
> struct devlink *devlink,
> - enum devlink_command cmd,
> - const char *status_msg,
> - const char *component,
> - unsigned long done,
> - unsigned long total,
> - unsigned long timeout)
> + struct devlink_flash_notify *params)
> {
> void *hdr;
>
> - hdr = genlmsg_put(msg, 0, 0, &devlink_nl_family, 0, cmd);
> + hdr = genlmsg_put(msg, 0, 0, &devlink_nl_family, 0, params->cmd);
> if (!hdr)
> return -EMSGSIZE;
>
> if (devlink_nl_put_handle(msg, devlink))
> goto nla_put_failure;
>
> - if (cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS)
> + if (params->cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS)
> goto out;
>
> - if (status_msg &&
> + if (params->status_msg &&
> nla_put_string(msg, DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG,
> - status_msg))
> + params->status_msg))
> goto nla_put_failure;
> - if (component &&
> + if (params->component &&
> nla_put_string(msg, DEVLINK_ATTR_FLASH_UPDATE_COMPONENT,
> - component))
> + params->component))
> goto nla_put_failure;
> if (nla_put_u64_64bit(msg, DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE,
> - done, DEVLINK_ATTR_PAD))
> + params->done, DEVLINK_ATTR_PAD))
> goto nla_put_failure;
> if (nla_put_u64_64bit(msg, DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL,
> - total, DEVLINK_ATTR_PAD))
> + params->total, DEVLINK_ATTR_PAD))
> goto nla_put_failure;
> if (nla_put_u64_64bit(msg, DEVLINK_ATTR_FLASH_UPDATE_STATUS_TIMEOUT,
> - timeout, DEVLINK_ATTR_PAD))
> + params->timeout, DEVLINK_ATTR_PAD))
> goto nla_put_failure;
>
> out:
> @@ -3068,26 +3063,20 @@ static int devlink_nl_flash_update_fill(struct
> sk_buff *msg,
> }
>
> static void __devlink_flash_update_notify(struct devlink *devlink,
> - enum devlink_command cmd,
> - const char *status_msg,
> - const char *component,
> - unsigned long done,
> - unsigned long total,
> - unsigned long timeout)
> + struct devlink_flash_notify *params)
> {
> struct sk_buff *msg;
> int err;
>
> - WARN_ON(cmd != DEVLINK_CMD_FLASH_UPDATE &&
> - cmd != DEVLINK_CMD_FLASH_UPDATE_END &&
> - cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS);
> + WARN_ON(params->cmd != DEVLINK_CMD_FLASH_UPDATE &&
> + params->cmd != DEVLINK_CMD_FLASH_UPDATE_END &&
> + params->cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS);
>
> msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> if (!msg)
> return;
>
> - err = devlink_nl_flash_update_fill(msg, devlink, cmd, status_msg,
> - component, done, total, timeout);
> + err = devlink_nl_flash_update_fill(msg, devlink, params);
> if (err)
> goto out_free_msg;
>
> @@ -3101,17 +3090,21 @@ static void __devlink_flash_update_notify(struct
> devlink *devlink,
>
> void devlink_flash_update_begin_notify(struct devlink *devlink)
> {
> - __devlink_flash_update_notify(devlink,
> - DEVLINK_CMD_FLASH_UPDATE,
> - NULL, NULL, 0, 0, 0);
> + struct devlink_flash_notify params = {
> + .cmd = DEVLINK_CMD_FLASH_UPDATE,
> + };
> +
> + __devlink_flash_update_notify(devlink, ¶ms);
> }
> EXPORT_SYMBOL_GPL(devlink_flash_update_begin_notify);
>
> void devlink_flash_update_end_notify(struct devlink *devlink)
> {
> - __devlink_flash_update_notify(devlink,
> - DEVLINK_CMD_FLASH_UPDATE_END,
> - NULL, NULL, 0, 0, 0);
> + struct devlink_flash_notify params = {
> + .cmd = DEVLINK_CMD_FLASH_UPDATE_END,
> + };
> +
> + __devlink_flash_update_notify(devlink, ¶ms);
> }
> EXPORT_SYMBOL_GPL(devlink_flash_update_end_notify);
>
> @@ -3121,9 +3114,15 @@ void devlink_flash_update_status_notify(struct devlink
> *devlink,
> unsigned long done,
> unsigned long total)
> {
> - __devlink_flash_update_notify(devlink,
> - DEVLINK_CMD_FLASH_UPDATE_STATUS,
> - status_msg, component, done, total, 0);
> + struct devlink_flash_notify params = {
> + .cmd = DEVLINK_CMD_FLASH_UPDATE_STATUS,
> + .status_msg = status_msg,
> + .component = component,
> + .done = done,
> + .total = total,
> + };
> +
> + __devlink_flash_update_notify(devlink, ¶ms);
> }
> EXPORT_SYMBOL_GPL(devlink_flash_update_status_notify);
>
> @@ -3132,9 +3131,14 @@ void devlink_flash_update_timeout_notify(struct
> devlink *devlink,
> const char *component,
> unsigned long timeout)
> {
> - __devlink_flash_update_notify(devlink,
> - DEVLINK_CMD_FLASH_UPDATE_STATUS,
> - status_msg, component, 0, 0, timeout);
> + struct devlink_flash_notify params = {
> + .cmd = DEVLINK_CMD_FLASH_UPDATE_STATUS,
> + .status_msg = status_msg,
> + .component = component,
> + .timeout = timeout,
> + };
> +
> + __devlink_flash_update_notify(devlink, ¶ms);
> }
> EXPORT_SYMBOL_GPL(devlink_flash_update_timeout_notify);
>
>
