Thu, Jan 17, 2019 at 10:59:16PM CET, era...@mellanox.com wrote:
>Add devlink health diagnose command, in order to run a diagnose
>operation over a specific reporter.
>
>It is expected from driver's callback for diagnose command to fill it
>via the buffer descriptors API. Devlink will parse it and convert it to
>netlink nla API in order to pass it to the user.
>
>Signed-off-by: Eran Ben Elisha <era...@mellanox.com>
>Reviewed-by: Moshe Shemesh <mo...@mellanox.com>
>---
> include/uapi/linux/devlink.h | 1 +
> net/core/devlink.c | 51 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+)
>
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index 1c186fd77343..51b4d7612cf8 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -92,6 +92,7 @@ enum devlink_command {
> DEVLINK_CMD_HEALTH_REPORTER_GET,
> DEVLINK_CMD_HEALTH_REPORTER_SET,
> DEVLINK_CMD_HEALTH_REPORTER_RECOVER,
>+ DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE,
>
> /* add new commands above here */
> __DEVLINK_CMD_MAX,
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index b224d0d31c0c..57252ca31e1e 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -4500,6 +4500,50 @@ static int
>devlink_nl_cmd_health_reporter_recover_doit(struct sk_buff *skb,
> return devlink_health_reporter_recover(reporter, NULL);
> }
>
>+static int devlink_nl_cmd_health_reporter_diagnose_doit(struct sk_buff *skb,
>+ struct genl_info *info)
>+{
>+ struct devlink *devlink = info->user_ptr[0];
>+ struct devlink_health_reporter *reporter;
>+ u64 num_of_buffers;
>+ int err;
>+
>+ reporter = devlink_health_reporter_get_from_info(devlink, info);
>+ if (!reporter)
>+ return -EINVAL;
>+
>+ if (!reporter->ops->diagnose)
>+ return -EOPNOTSUPP;
>+
>+ num_of_buffers =
>+ DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->diagnose_size);
This is odd. You need to convert to number of "buffers" the magic value
the driver provided. And then couple lines below....
>+
>+ mutex_lock(&reporter->diagnose_lock);
>+ devlink_health_buffers_reset(reporter->diagnose_buffers_array,
>+ num_of_buffers);
>+
>+ err = reporter->ops->diagnose(reporter,
>+ reporter->diagnose_buffers_array,
>+ DEVLINK_HEALTH_BUFFER_SIZE,
>+ num_of_buffers);
....you call the driver with "buffers array", buffer size and number of
buffers. It should be just:
struct devlink_msg_ctx msg_ctx;
err = reporter->ops->diagnose(reporter, &msg_ctx);
and then;
err = devlink_health_buffer_snd(info,
devlink_cmd_health_reporter_diagnose,
0, &msg_ctx);
>+ if (err)
>+ goto out;
>+
>+ err = devlink_health_buffer_snd(info,
>+ devlink_cmd_health_reporter_diagnose,
>+ 0, reporter->diagnose_buffers_array,
>+ num_of_buffers);
>+ if (err)
>+ goto out;
>+
>+ mutex_unlock(&reporter->diagnose_lock);
>+ return 0;
>+
>+out:
>+ mutex_unlock(&reporter->diagnose_lock);
>+ return err;
>+}
>+
> static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
> [DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING },
> [DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING },
>@@ -4770,6 +4814,13 @@ static const struct genl_ops devlink_nl_ops[] = {
> .flags = GENL_ADMIN_PERM,
> .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
> },
>+ {
>+ .cmd = DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE,
>+ .doit = devlink_nl_cmd_health_reporter_diagnose_doit,
>+ .policy = devlink_nl_policy,
>+ .flags = GENL_ADMIN_PERM,
>+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
>+ },
> };
>
> static struct genl_family devlink_nl_family __ro_after_init = {
>--
>2.17.1
>
