On Sat, 2 Feb 2019 16:59:38 +0100, Jiri Pirko wrote:
> Sat, Feb 02, 2019 at 01:03:38AM CET, jakub.kicin...@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.
Sorry, yes, will do.
> >Signed-off-by: Jakub Kicinski <jakub.kicin...@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.
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.
