On Tue, Mar 26, 2019 at 02:42:51PM +0100, Jiri Pirko wrote:

Tue, Mar 26, 2019 at 02:24:27PM CET, mkubecek@suse.cz wrote:

quoted

On Tue, Mar 26, 2019 at 01:09:09PM +0100, Jiri Pirko wrote:

quoted

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

quoted

+Device identification
+---------------------
+
+When appropriate, network device is identified by a nested attribute named
+ETHA_*_DEV. This attribute can contain

Isn't it ETHA_DEV_*? I must admit I'm a bit confused.

ETHA_*_DEV is the nesting attribute (e.g. ETHA_SETTINGS_DEV), ETHA_DEV_*
(ETHA_DEV_INDEX and ETHA_DEV_NAME) are in the nest.

Yeah. I wonder why you need to duplicate this. Can this be in top-lever
attr enum that is shared among all commands? It is there anyway and
looks a bit silly to have "DEV" attr separate for every command.
Something like this:

ATTR_IFINDEX
ATTR_IFNAME
ATTR_SOMEOTHER (flags perhaps)
ATTR_CMD_SPECIFIC_NEST_START
  ATTR_CMDX_SOMETHING
  ATTR_CMDX_SOMETHING2
  ATTR_CMDX_SOMETHING3
ATTR_CMD_SPECIFIC_NEST_END

I would rather prefer the opposite:

ATTR_HEADER
    ATTR_IFINDEX
    ATTR_IFNAME
    ATTR_INFO_MASK
    ATTR_PER_QUEUE
ATTR_CMDX_SOMETHING
ATTR_CMDX_SOMETHING2
ATTR_CMDX_SOMETHING3
...

This way the "header" with universal attributes (not specfic to
a particular message type) would be kept together at the beginning even
after we need to add some more later and command specific attributes
would still have fixed numbers (starting from 2). I'll think about it
some more and check what would be pros and cons of the two variants
when parsing and generating the messages.

quoted

quoted

quoted

+Messages of type "get" are used by userspace to request information and
+usually do not contain any attributes (that may be added later for dump
+filtering). Kernel response is in the form of corresponding "set" message;

Okay. Do we want reply to "*_cmd_something_get" command to be
"*_cmd_something_set". That sounds odd. Why reply has to be "cmd"? Why
not something like "reply" or "response"?
This should work for both "doit/dumpit" and notifications.

As stated right below, the aim is to use the same format for replies to
GET requests as userspace uses for related SET requests. We could use
different id (genlmsghdr::cmd) but that seemed like a waste for no actual
gain.

I understand. I just wonder if the replies/notifications could use the
same name, not having "set" in it. I know we have it like this in many
netlink ifaces, it is however confusing to users. So once we are doing
this from scratch, we can do it differently.

How about

  ETHTOOL_MSG_GET_FOO  for get requests
  ETHTOOL_MSG_FOO      for get replies, notifications and set requests
  ETHTOOL_MSG_ACT_FOO  for actions (renegotiation, reset, blinking, ...)

?

Michal