From: Jiri Pirko <jiri@resnulli.us>
To: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: dsahern@gmail.com, stephen@networkplumber.org,
netdev@vger.kernel.org, oss-drivers@netronome.com
Subject: Re: [PATCH iproute2-next] devlink: add info subcommand
Date: Sat, 2 Feb 2019 16:59:38 +0100 [thread overview]
Message-ID: <20190202155938.GE2778@nanopsycho> (raw)
In-Reply-To: <20190202000338.30820-1-jakub.kicinski@netronome.com>
Sat, Feb 02, 2019 at 01:03:38AM CET, jakub.kicinski@netronome.com wrote:
>Add support for reading the device serial number and versions
>from the kernel.
>
>RFCv2:
> - make info subcommand of dev.
Please add some examples of inputs and outputs.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> devlink/devlink.c | 207 +++++++++++++++++++++++++++++++++++++++++
> man/man8/devlink-dev.8 | 36 +++++++
> 2 files changed, 243 insertions(+)
>
>diff --git a/devlink/devlink.c b/devlink/devlink.c
>index 3651e90c1159..3ab046ace8f8 100644
>--- a/devlink/devlink.c
>+++ b/devlink/devlink.c
>@@ -199,6 +199,7 @@ static void ifname_map_free(struct ifname_map *ifname_map)
> #define DL_OPT_REGION_SNAPSHOT_ID BIT(22)
> #define DL_OPT_REGION_ADDRESS BIT(23)
> #define DL_OPT_REGION_LENGTH BIT(24)
>+#define DL_OPT_VERSIONS_TYPE BIT(25)
Why "type"? Confusing.
>
> struct dl_opts {
> uint32_t present; /* flags of present items */
>@@ -230,6 +231,7 @@ struct dl_opts {
> uint32_t region_snapshot_id;
> uint64_t region_address;
> uint64_t region_length;
>+ int versions_attr;
> };
>
> struct dl {
>@@ -383,6 +385,13 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
> [DEVLINK_ATTR_REGION_CHUNK_DATA] = MNL_TYPE_BINARY,
> [DEVLINK_ATTR_REGION_CHUNK_ADDR] = MNL_TYPE_U64,
> [DEVLINK_ATTR_REGION_CHUNK_LEN] = MNL_TYPE_U64,
>+ [DEVLINK_ATTR_INFO_DRIVER_NAME] = MNL_TYPE_STRING,
>+ [DEVLINK_ATTR_INFO_SERIAL_NUMBER] = MNL_TYPE_STRING,
>+ [DEVLINK_ATTR_INFO_VERSION_FIXED] = MNL_TYPE_NESTED,
>+ [DEVLINK_ATTR_INFO_VERSION_RUNNING] = MNL_TYPE_NESTED,
>+ [DEVLINK_ATTR_INFO_VERSION_STORED] = MNL_TYPE_NESTED,
>+ [DEVLINK_ATTR_INFO_VERSION_NAME] = MNL_TYPE_STRING,
>+ [DEVLINK_ATTR_INFO_VERSION_VALUE] = MNL_TYPE_STRING,
> };
>
> static int attr_cb(const struct nlattr *attr, void *data)
>@@ -943,6 +952,21 @@ static int param_cmode_get(const char *cmodestr,
> return 0;
> }
>
>+static int versions_type_get(const char *typestr, int *p_attr)
>+{
>+ if (strcmp(typestr, "fixed") == 0) {
>+ *p_attr = DEVLINK_ATTR_INFO_VERSION_FIXED;
>+ } else if (strcmp(typestr, "stored") == 0) {
>+ *p_attr = DEVLINK_ATTR_INFO_VERSION_STORED;
>+ } else if (strcmp(typestr, "running") == 0) {
>+ *p_attr = DEVLINK_ATTR_INFO_VERSION_RUNNING;
>+ } else {
>+ pr_err("Unknown versions type \"%s\"\n", typestr);
>+ return -EINVAL;
>+ }
>+ return 0;
>+}
>+
> static int dl_argv_parse(struct dl *dl, uint32_t o_required,
> uint32_t o_optional)
> {
>@@ -1178,6 +1202,19 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
> if (err)
> return err;
> o_found |= DL_OPT_REGION_LENGTH;
>+ } else if (dl_argv_match(dl, "versions") &&
>+ (o_all & DL_OPT_VERSIONS_TYPE)) {
>+ const char *versionstr;
>+
>+ dl_arg_inc(dl);
>+ err = dl_argv_str(dl, &versionstr);
>+ if (err)
>+ return err;
>+ err = versions_type_get(versionstr,
>+ &opts->versions_attr);
>+ if (err)
>+ return err;
>+ o_found |= DL_OPT_VERSIONS_TYPE;
> } else {
> pr_err("Unknown option \"%s\"\n", dl_argv(dl));
> return -EINVAL;
>@@ -1443,6 +1480,9 @@ static void cmd_dev_help(void)
> pr_err(" devlink dev param set DEV name PARAMETER value VALUE cmode { permanent | driverinit | runtime }\n");
> pr_err(" devlink dev param show [DEV name PARAMETER]\n");
> pr_err(" devlink dev reload DEV\n");
>+ pr_err(" devlink dev info [ DEV [ { versions [ VTYPE ] } ] ]\n");
>+ pr_err("\n");
>+ pr_err(" VTYPE := { fixed | running | stored }\n");
So you would like to filter according to the version type? Why?
> }
>
> static bool cmp_arr_last_handle(struct dl *dl, const char *bus_name,
>@@ -1775,6 +1815,30 @@ static void pr_out_array_end(struct dl *dl)
> }
> }
>
>+static void pr_out_object_start(struct dl *dl, const char *name)
>+{
>+ if (dl->json_output) {
>+ jsonw_name(dl->jw, name);
>+ jsonw_start_object(dl->jw);
>+ } else {
>+ __pr_out_indent_inc();
>+ __pr_out_newline();
>+ pr_out("%s:", name);
>+ __pr_out_indent_inc();
>+ __pr_out_newline();
>+ }
>+}
>+
>+static void pr_out_object_end(struct dl *dl)
>+{
>+ if (dl->json_output) {
>+ jsonw_end_object(dl->jw);
>+ } else {
>+ __pr_out_indent_dec();
>+ __pr_out_indent_dec();
>+ }
>+}
>+
> static void pr_out_entry_start(struct dl *dl)
> {
> if (dl->json_output)
>@@ -2415,6 +2479,146 @@ static int cmd_dev_reload(struct dl *dl)
> return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
> }
>
>+static void pr_out_versions_single(struct dl *dl, const struct nlmsghdr *nlh,
>+ const char *name, int type)
>+{
>+ struct nlattr *version;
>+
>+ if (dl->opts.versions_attr && dl->opts.versions_attr != type)
>+ return;
>+
>+ mnl_attr_for_each(version, nlh, sizeof(struct genlmsghdr)) {
>+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
>+ const char *ver_value;
>+ const char *ver_name;
>+ int err;
>+
>+ if (mnl_attr_get_type(version) != type)
>+ continue;
>+
>+ err = mnl_attr_parse_nested(version, attr_cb, tb);
>+ if (err != MNL_CB_OK)
>+ continue;
>+
>+ if (!tb[DEVLINK_ATTR_INFO_VERSION_NAME] ||
>+ !tb[DEVLINK_ATTR_INFO_VERSION_VALUE])
>+ continue;
>+
>+ if (name) {
>+ pr_out_object_start(dl, name);
>+ name = NULL;
>+ }
>+
>+ ver_name = mnl_attr_get_str(tb[DEVLINK_ATTR_INFO_VERSION_NAME]);
>+ ver_value = mnl_attr_get_str(tb[DEVLINK_ATTR_INFO_VERSION_VALUE]);
>+
>+ pr_out_str(dl, ver_name, ver_value);
>+ if (!dl->json_output)
>+ __pr_out_newline();
>+ }
>+
>+ if (!name)
>+ pr_out_object_end(dl);
>+}
>+
>+static void pr_out_info(struct dl *dl, const struct nlmsghdr *nlh,
>+ struct nlattr **tb, bool has_versions)
>+{
>+ __pr_out_handle_start(dl, tb, true, false);
>+
>+ __pr_out_indent_inc();
>+ if (!dl->opts.versions_attr && tb[DEVLINK_ATTR_INFO_DRIVER_NAME]) {
>+ struct nlattr *nla_drv = tb[DEVLINK_ATTR_INFO_DRIVER_NAME];
>+
>+ __pr_out_newline();
>+ pr_out_str(dl, "driver", mnl_attr_get_str(nla_drv));
>+ }
>+
>+ if (!dl->opts.versions_attr && tb[DEVLINK_ATTR_INFO_SERIAL_NUMBER]) {
>+ struct nlattr *nla_sn = tb[DEVLINK_ATTR_INFO_SERIAL_NUMBER];
>+
>+ __pr_out_newline();
>+ pr_out_str(dl, "serial_number", mnl_attr_get_str(nla_sn));
>+ }
>+ __pr_out_indent_dec();
>+
>+ if (has_versions) {
>+ pr_out_object_start(dl, "versions");
>+
>+ pr_out_versions_single(dl, nlh, "fixed",
>+ DEVLINK_ATTR_INFO_VERSION_FIXED);
>+ pr_out_versions_single(dl, nlh, "running",
>+ DEVLINK_ATTR_INFO_VERSION_RUNNING);
>+ pr_out_versions_single(dl, nlh, "stored",
>+ DEVLINK_ATTR_INFO_VERSION_STORED);
>+
>+ pr_out_object_end(dl);
>+ }
>+
>+ pr_out_handle_end(dl);
>+}
>+
>+static int cmd_versions_show_cb(const struct nlmsghdr *nlh, void *data)
>+{
>+ struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
>+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
>+ bool has_versions, has_info;
>+ struct dl *dl = data;
>+
>+ mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
>+
>+ if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME])
>+ return MNL_CB_ERROR;
>+
>+ has_versions = tb[DEVLINK_ATTR_INFO_VERSION_FIXED] ||
>+ tb[DEVLINK_ATTR_INFO_VERSION_RUNNING] ||
>+ tb[DEVLINK_ATTR_INFO_VERSION_STORED];
>+ has_info = tb[DEVLINK_ATTR_INFO_DRIVER_NAME] ||
>+ tb[DEVLINK_ATTR_INFO_SERIAL_NUMBER] ||
>+ has_versions;
>+
>+ if (has_info)
>+ pr_out_info(dl, nlh, tb, has_versions);
>+
>+ return MNL_CB_OK;
>+}
>+
>+static void cmd_dev_info_help(void)
>+{
>+ pr_err("Usage: devlink dev info [ DEV [ { versions [ VTYPE ] } ] ]\n");
>+ pr_err("\n");
>+ pr_err(" VTYPE := { fixed | running | stored }\n");
>+}
>+
>+static int cmd_dev_info(struct dl *dl)
>+{
>+ struct nlmsghdr *nlh;
>+ uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
>+ int err;
>+
>+ if (dl_argv_match(dl, "help")) {
>+ cmd_dev_info_help();
>+ return 0;
>+ }
>+
>+ if (dl_argc(dl) == 0)
>+ flags |= NLM_F_DUMP;
>+
>+ nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_INFO_GET, flags);
>+
>+ if (dl_argc(dl) > 0) {
>+ err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE,
>+ DL_OPT_VERSIONS_TYPE);
>+ if (err)
>+ return err;
>+ }
>+
>+ pr_out_section_start(dl, "info");
>+ err = _mnlg_socket_sndrcv(dl->nlg, nlh, cmd_versions_show_cb, dl);
>+ pr_out_section_end(dl);
>+ return err;
>+}
>+
> static int cmd_dev(struct dl *dl)
> {
> if (dl_argv_match(dl, "help")) {
>@@ -2433,6 +2637,9 @@ static int cmd_dev(struct dl *dl)
> } else if (dl_argv_match(dl, "param")) {
> dl_arg_inc(dl);
> return cmd_dev_param(dl);
>+ } else if (dl_argv_match(dl, "info")) {
>+ dl_arg_inc(dl);
>+ return cmd_dev_info(dl);
> }
> pr_err("Command \"%s\" not found\n", dl_argv(dl));
> return -ENOENT;
>diff --git a/man/man8/devlink-dev.8 b/man/man8/devlink-dev.8
>index d985da172aa0..5b05298f88bf 100644
>--- a/man/man8/devlink-dev.8
>+++ b/man/man8/devlink-dev.8
>@@ -63,6 +63,17 @@ devlink-dev \- devlink device configuration
> .BR "devlink dev reload"
> .IR DEV
>
>+.ti -8
>+.BR "devlink dev info"
>+.RI "[ " DEV
>+.RI "["
>+.BR versions
>+.RI "{ "
>+.BR fixed " | " running " | " stored
>+.RI "} "
>+.RI "]"
>+.RI "]"
>+
> .SH "DESCRIPTION"
> .SS devlink dev show - display devlink device attributes
>
>@@ -151,6 +162,31 @@ If this argument is omitted all parameters supported by devlink devices are list
> .I "DEV"
> - Specifies the devlink device to reload.
>
>+.SS devlink dev info - display device information.
>+Display device information provided by the driver. This command can be used
>+to query versions of the hardware components or device components which
>+can't be updated (
>+.I fixed
>+) as well as device firmware which can be updated. For firmware components
>+.I running
>+displays the versions of firmware currently loaded into the device, while
>+.I stored
>+reports the versions in device's flash.
>+.I Running
>+and
>+.I stored
>+versions may differ after flash has been updated, but before reboot.
>+
>+.PP
>+.I "DEV"
>+- specifies the devlink device to show.
>+If this argument is omitted all devices are listed.
>+
>+.PP
>+.BR versions " { " fixed " | " running " | " stored " } "
>+- specifies the versions category to show.
>+If this argument is omitted all categories are listed.
>+
> .SH "EXAMPLES"
> .PP
> devlink dev show
>--
>2.19.2
>
next prev parent reply other threads:[~2019-02-02 16:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-02 0:03 [PATCH iproute2-next] devlink: add info subcommand Jakub Kicinski
2019-02-02 15:59 ` Jiri Pirko [this message]
2019-02-02 22:06 ` Jakub Kicinski
2019-02-03 11:04 ` Jiri Pirko
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=20190202155938.GE2778@nanopsycho \
--to=jiri@resnulli.us \
--cc=dsahern@gmail.com \
--cc=jakub.kicinski@netronome.com \
--cc=netdev@vger.kernel.org \
--cc=oss-drivers@netronome.com \
--cc=stephen@networkplumber.org \
/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.