linux-kernel - Re: [PATCH net-next v5 12/22] ethtool: provide string sets with GET

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190328222841.GI26076@unicorn.suse.cz>
Date:   Thu, 28 Mar 2019 23:28:41 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Andrew Lunn <andrew@...n.ch>,
        John Linville <linville@...driver.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v5 12/22] ethtool: provide string sets with
 GET_STRSET request

On Thu, Mar 28, 2019 at 06:35:24PM +0100, Jiri Pirko wrote:
> 
> Moreover, I think that speed, duplex and type should be sent
> separatelly:
> 
>     ETHTOOL_A_LINK_MODE_LIST_OUR start nest
>       ETHTOOL_A_LINK_MODE start nest
>         ETHTOOL_A_LINK_MODE_SPEED = 100 /* this should be u64 */
> 	ETHTOOL_A_LINK_MODE_DUPLEX = ETHTOOL_LINK_MODE_DUPLEX_FULL
> 	ETHTOOL_A_LINK_MODE_TYPE = ETHTOOL_LINK_MODE_TYPE_BASET
>       ETHTOOL_A_LINK_MODE start end
>       ETHTOOL_A_LINK_MODE start nest
>         ETHTOOL_A_LINK_MODE_SPEED = 10
> 	ETHTOOL_A_LINK_MODE_DUPLEX = ETHTOOL_LINK_MODE_DUPLEX_HALF
> 	ETHTOOL_A_LINK_MODE_TYPE = ETHTOOL_LINK_MODE_TYPE_BASET
>       ETHTOOL_A_LINK_MODE start end
>     ETHTOOL_A_LINK_MODE_LIST_PEER end nest
> 
> Does not really make sense to combine those 3 attributes together.

This helped me to realize the primary source of misunderstanding: as I'm
working on kernel and ethtool implementation simultaneously, I don't
look only at the API itself and don't consider only if its design is
clean and logical. I always think about the actual userspace programs
wanting to use the interface and try to imagine what the design means
for them.

So when you say e.g. "this belongs to rtnetlink", I have to admit that
from strictly logical point of view you are right but at the same time,
chain of thoughts starts: "that means two sockets; we need a table which
command needs which, some might need both, monitor will have to listen
at two sockets, that's fork/pthread/poll..."

>From this point of view, the scheme above which, on its own, makes
perfect sense (there might be a bit of a problem with 10000baseR_FEC
mode but that one is a trouble in any scheme), means that one side will
translate the link mode number to the three parameters and the other
will look the triplet out in its own table to get the link mode number.
On userspace side, there will be another translation between link mode
number and name. What exactly is the gain from such representation?
No idea.

There is one crucial difference between ethtool and devlink. You are
building devlink from scratch so you define the logic, define the API
based on that logic and make both NIC drivers and userspace conform to
it. With ethtool, the situation is exactly the opposite: on one side,
there are ethtool_ops as a constraint, on the other, it's ethtool with
its feature set and users used to it. Changing ethtool_ops is going to
be slow and painful process. Changing the users and their habits...
we already know how that works.

Michal