From: Jacob Keller <jacob.e.keller@intel.com>
To: Shannon Nelson <snelson@pensando.io>,
netdev@vger.kernel.org, davem@davemloft.net
Subject: Re: [PATCH v4 net-next 2/5] devlink: collect flash notify params into a struct
Date: Thu, 17 Sep 2020 12:52:31 -0700 [thread overview]
Message-ID: <b04ad9ea-b016-e4f0-3f53-8811d1f32aa3@intel.com> (raw)
In-Reply-To: <20200917030204.50098-3-snelson@pensando.io>
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 <snelson@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.keller@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);
>
>
next prev parent reply other threads:[~2020-09-17 20:13 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-17 3:01 [PATCH v4 net-next 0/5] ionic: add devlink dev flash support Shannon Nelson
2020-09-17 3:02 ` [PATCH v4 net-next 1/5] devlink: add timeout information to status_notify Shannon Nelson
2020-09-17 19:46 ` Jakub Kicinski
2020-09-17 20:45 ` Shannon Nelson
2020-09-17 19:50 ` Jacob Keller
2020-09-17 20:48 ` Shannon Nelson
2020-09-17 3:02 ` [PATCH v4 net-next 2/5] devlink: collect flash notify params into a struct Shannon Nelson
2020-09-17 19:47 ` Jakub Kicinski
2020-09-17 20:46 ` Shannon Nelson
2020-09-17 19:52 ` Jacob Keller [this message]
2020-09-17 3:02 ` [PATCH v4 net-next 3/5] netdevsim: devlink flash timeout message Shannon Nelson
2020-09-17 19:50 ` Jakub Kicinski
2020-09-17 20:47 ` Shannon Nelson
2020-09-17 3:02 ` [PATCH v4 net-next 4/5] ionic: update the fw update api Shannon Nelson
2020-09-17 3:02 ` [PATCH v4 net-next 5/5] ionic: add devlink firmware update Shannon Nelson
2020-09-17 19:52 ` Jakub Kicinski
2020-09-17 20:47 ` Shannon Nelson
2020-09-17 19:55 ` Jacob Keller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b04ad9ea-b016-e4f0-3f53-8811d1f32aa3@intel.com \
--to=jacob.e.keller@intel.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=snelson@pensando.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.