Re: [PATCH net-next v5 12/22] ethtool: provide string sets with GET_STRSET request
From: Michal Kubecek <hidden>
Date: 2019-03-28 22:28:48
Also in:
lkml
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