Re: [PATCH net-next v5 12/22] ethtool: provide string sets with GET_STRSET request

From: Michal Kubecek
Date: Thu Mar 28 2019 - 16:43:40 EST


On Thu, Mar 28, 2019 at 11:52:56AM -0700, Jakub Kicinski wrote:
> On Thu, 28 Mar 2019 18:35:24 +0100, Jiri Pirko wrote:
> > Thu, Mar 28, 2019 at 03:04:28PM CET, mkubecek@xxxxxxx wrote:
> > >On Thu, Mar 28, 2019 at 02:43:13PM +0100, Jiri Pirko wrote:
> > >>
> > >> I don't like this. This should not be bitfield/set. This should be
> > >> simply nested array of enum values:
> > >>
> > >> enum ethtool_link_mode {
> > >> ETHTOOL_LINK_MODE_10baseT_Half,
> > >> ETHTOOL_LINK_MODE_10baseT_Full,
> > >> ETHTOOL_LINK_MODE_100baseT_Half,
> > >> ETHTOOL_LINK_MODE_100baseT_Full,
> > >> ETHTOOL_LINK_MODE_1000baseT_Full,
> > >> };
> > >
> > >We already have such enum. The problem with your "no string" approach is
> > >that it requires all userspace applications to (1) keep this enum in
> >
> > That is how it is usually done. UAPI defines ATTRS and values, userspace
> > assigns appropriate strings.
>
> +1 FWIW, I'm with Jiri on the string situation.

And I'm still waiting for any of you to tell me how would you handle
private flags, stats, tests etc. without the string sets.

Ditching the verbose form of bit sets would be a nuisance for userspace
using the interface but compared to e.g. having to mix three different
kernel interfaces, it's just a minor problem. Ditching the static string
sets would mean giving up the opportunity to get rid of having to sync
all kinds of tables with every userspace consumer whenever a new flag is
introduced. Pity... but still doable. But how do you want to do without
the string sets which are provided by drivers?

Michal