From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2A39DC169C4 for ; Sun, 3 Feb 2019 11:14:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D8CC92083B for ; Sun, 3 Feb 2019 11:14:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=resnulli-us.20150623.gappssmtp.com header.i=@resnulli-us.20150623.gappssmtp.com header.b="gD3D6Bgz" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727610AbfBCLOC (ORCPT ); Sun, 3 Feb 2019 06:14:02 -0500 Received: from mail-wm1-f65.google.com ([209.85.128.65]:51883 "EHLO mail-wm1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726765AbfBCLOC (ORCPT ); Sun, 3 Feb 2019 06:14:02 -0500 Received: by mail-wm1-f65.google.com with SMTP id b11so10331444wmj.1 for ; Sun, 03 Feb 2019 03:14:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=8HlbZ3WnlDyvrIuNy6fN0DJaouH3ahltkXSWv/jm/WE=; b=gD3D6BgzJnb+3RFgFCNp61nmknwZ06oLX+d6uITd1eEMuCEIzZMTFvnxvix8EoIQP0 gy/uxwVQ3Q81gdtTs8rPzSYOqO92VHNmaP6N45OYextURJV9dDuq2PPgjn4ND7nprKto nMduOE8xfaXx1qMs7LR/5PnOdpZO9iMvyp+lCjTTsN0b4HPs56GqMRbcGQN3UJ2bC70k aBB9oaujAD8g5OlvvjNjtJ5UhL+e5tT+ts9QxmeHWmTQod0ZZZxOnUNJahpHSUZYFe9Z V8bB2/ctF6AqVyNuCNik6E4KoXUIE0muGmYGQUsZesw26C96r4rdQR9nQvdjteT5KaYJ l7Sg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=8HlbZ3WnlDyvrIuNy6fN0DJaouH3ahltkXSWv/jm/WE=; b=Un9qsvWSZd7bmt6a22csvFKoECSaehq3uur2m+JTAfLqTbN1WLzhdtLrJ53Aqdt514 Fqkr1jWv0xF9/k1NnpZvGwTIH60l3/HKHR9mpcS7vhUvjFrPhz/GnD2PYXGDlpgtEjnc sbb1zcBkOLo2cvxICnvKN3sDw2qxHkCgJWDAUZmGUElZZ4JnLI3UnZ1xdoluyXV6UTTN 366nM0uAu+QzdauBlS7OCkeyzvIvVGPTnNQD2ZyOXwtebCtwlnRix9T0Q5PsjnrFIIPg KRTctAL4i7Ya5ZOD5nVKVis+ZFyWEPz8bIpR4zI/SoP5Hk7lHXpcs3guOKD+lQbZ3gkn RPag== X-Gm-Message-State: AHQUAuZH17ru+yHTp6NAqxxI46xdpnGMi9FLVTpcu5E5aGUBRmRqVF1V 4qnMNSm1MmzGgNTUF3GZhu1UdQ== X-Google-Smtp-Source: AHgI3Ib+A69Rbb47Fn2VrchXtXkdFq4HBwsioAXmSbbiO2JBtU3uaTFDdTFMJKf/D5D0uYmj8rjlcA== X-Received: by 2002:a1c:650a:: with SMTP id z10mr9301600wmb.148.1549192439213; Sun, 03 Feb 2019 03:13:59 -0800 (PST) Received: from localhost (jirka.pirko.cz. [84.16.102.26]) by smtp.gmail.com with ESMTPSA id w125sm18440559wmb.45.2019.02.03.03.13.58 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 03 Feb 2019 03:13:58 -0800 (PST) Date: Sun, 3 Feb 2019 12:04:52 +0100 From: Jiri Pirko To: Jakub Kicinski Cc: dsahern@gmail.com, stephen@networkplumber.org, netdev@vger.kernel.org, oss-drivers@netronome.com Subject: Re: [PATCH iproute2-next] devlink: add info subcommand Message-ID: <20190203110452.GA2249@nanopsycho> References: <20190202000338.30820-1-jakub.kicinski@netronome.com> <20190202155938.GE2778@nanopsycho> <20190202140608.27691bbc@cakuba.hsd1.ca.comcast.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190202140608.27691bbc@cakuba.hsd1.ca.comcast.net> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Sat, Feb 02, 2019 at 11:06:08PM CET, jakub.kicinski@netronome.com wrote: >On Sat, 2 Feb 2019 16:59:38 +0100, Jiri Pirko wrote: >> 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. > >Sorry, yes, will do. > >> >Signed-off-by: Jakub Kicinski >> >--- >> > 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. I don't know. In the "info", there are more things than versions. Also, I believe in near future there are going to be even more things there. Providing a filtering option for one item and not for other seems incorrect. I think that devlink should not filter in this case. Let it dump the whole output and let the user take care of it by other tools. Each tool should do one thing.