Wed, Mar 27, 2019 at 11:56:08PM CET, mkubecek@suse.cz wrote:

On Wed, Mar 27, 2019 at 09:12:37PM +0100, Jiri Pirko wrote:

quoted

Mon, Mar 25, 2019 at 06:08:30PM CET, mkubecek@suse.cz wrote:

quoted

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 <redacted>
---
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.

For some of the global string sets where the values have fixed meaning
and new values are only appended to, it would be possible. But even for
those, keeping the list in sync between kernel and ethtool is often less
than perfect. And ethtool is only one of the tools using the interface.
So even in this case, I don't see string identifiers (or tags or names
or whatever we call them) as a baggage from past, rather as a solution
to a problem some of the existing interfaces have.

Then e.g. netdev features are not fixed in this sense: the same bit
index represents different feature in different kernels. I guess we

Should not be exposed in bits in first place. See devlink params for
example.

could introduce some fixed numeric identifiers for them and map between
those and actual indices but I don't see an advantage of such approach.

But the really important question is how would you handle what is
currently described by "per device" string sets, i.e. private flags,
(ethtool) statistics, tests, ...? For these, the list depends on the
driver or even device.

Yes, those per-driver things have to be strings. That's what we did for
devlink params.

Tests also could be handled in similar way as devlink params.

For stats, I believe that uint->string per-driver mapping
needs to be exposed.