netdev - Re: [PATCH iproute2-next] devlink: add info subcommand

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 2 Feb 2019 14:06:08 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     dsahern@...il.com, stephen@...workplumber.org,
        netdev@...r.kernel.org, oss-drivers@...ronome.com
Subject: Re: [PATCH iproute2-next] devlink: add info subcommand

On Sat, 2 Feb 2019 16:59:38 +0100, Jiri Pirko wrote:
> Sat, Feb 02, 2019 at 01:03:38AM CET, jakub.kicinski@...ronome.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.

Sorry, yes, will do.

> >Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.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.

Right, I think this dates back to day 1 when the whole thing was called
versions not info.  How about DL_OPT_INFO_VERSION_TYPE?

> > 
> > struct dl_opts {
> > 	uint32_t present; /* flags of present items */

> >@@ -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?

if devlink dev info bus/train versions stored != devlink ... running
then reboot is needed.  That one of main uses for reporting running vs
stored in sections, it's nice to be able to just compare the outputs.