Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 27 Mar 2019 21:12:37 +0100
From:   Jiri Pirko <jiri@resnulli.us>
To:     Michal Kubecek <mkubecek@suse.cz>
Cc:     David Miller <davem@davemloft.net>, netdev@vger.kernel.org,
        Jakub Kicinski <jakub.kicinski@netronome.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        John Linville <linville@tuxdriver.com>,
        Stephen Hemminger <stephen@networkplumber.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v5 12/22] ethtool: provide string sets with
 GET_STRSET request
Message-ID: <20190327201237.GG14297@nanopsycho>
References: <cover.1553532199.git.mkubecek@suse.cz>
 <f5af581e33b4976b84eae7c7788e3fde9adfa83c.1553532199.git.mkubecek@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f5af581e33b4976b84eae7c7788e3fde9adfa83c.1553532199.git.mkubecek@suse.cz>
User-Agent: Mutt/1.11.3 (2019-02-01)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Mon, Mar 25, 2019 at 06:08:30PM CET, mkubecek@suse.cz wrote:
>Requests a contents of one or more string sets, i.e. indexed arrays of
>strings; this information is provided by ETHTOOL_GSSET_INFO and
>ETHTOOL_GSTRINGS commands of ioctl interface. There are three types of
>requests:
>
>  - no NLM_F_DUMP, no device: get "global" stringsets
>  - no NLM_F_DUMP, with device: get string sets related to the device
>  - NLM_F_DUMP, no device: get device related string sets for all devices
>
>It's possible to request all string sets of given type or only specific
>sets. With ETHA_STRSET_COUNTS flag, only set sizes (number of strings) are
>returned.
>
>Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
>---
> Documentation/networking/ethtool-netlink.txt |  46 +-
> include/uapi/linux/ethtool.h                 |   2 +
> include/uapi/linux/ethtool_netlink.h         |  43 ++
> net/ethtool/Makefile                         |   2 +-
> net/ethtool/netlink.c                        |   8 +
> net/ethtool/netlink.h                        |   4 +
> net/ethtool/strset.c                         | 447 +++++++++++++++++++
> 7 files changed, 549 insertions(+), 3 deletions(-)
> create mode 100644 net/ethtool/strset.c

First of all, the code is hard to follow. For reasons I mentioned in
other replies (lack of prefixes, wrappers, etc).

More importantly, why do we need this? This concept of having strings in
kernel for various things and features and sending them to userspace is
weird. Certainly not common for Netlink interface. I believe these
strings should be avoided and all should be communicated to userspace
and back in form of well-defined Netlink attributes. We are introducing
new Netlink API, lets do it properly and don't bring baggage from past.