Messages in this thread | | | Date | Mon, 11 Dec 2017 19:02:19 +0100 | From | Jiri Pirko <> | Subject | Re: [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface |
| |
Mon, Dec 11, 2017 at 05:56:51PM CET, davem@davemloft.net wrote: >From: Jiri Pirko <jiri@resnulli.us> >Date: Mon, 11 Dec 2017 17:02:21 +0100 > >> Mon, Dec 11, 2017 at 02:53:31PM CET, mkubecek@suse.cz wrote: >>>No function implemented yet, only genetlink and module infrastructure. >>>Register/unregister genetlink family "ethtool" and allow the module to be >>>autoloaded by genetlink code (if built as a module, distributions would >>>probably prefer "y"). >>> >>>Signed-off-by: Michal Kubecek <mkubecek@suse.cz> >> >> [...] >> >> >>>+ >>>+/* identifies the device to query/set >>>+ * - use either ifindex or ifname, not both >>>+ * - for dumps and messages not related to a particular devices, fill neither >>>+ * - info_mask is a bitfield, interpretation depends on the command >>>+ */ >>>+struct ethnlmsghdr { >>>+ __u32 ifindex; /* device ifindex */ >>>+ __u16 flags; /* request/response flags */ >>>+ __u16 info_mask; /* request/response info mask */ >>>+ char ifname[IFNAMSIZ]; /* device name */ >> >> Why do you need this header? You can have 2 attrs: >> ETHTOOL_ATTR_IFINDEX and >> ETHTOOL_ATTR_IFNAME >> >> Why do you need per-command flags and info_mask? Could be bitfield >> attr if needed by specific command. > >Jiri, we've had this discussion before :-) > >For elements which are common to most, if not all, requests it makes >sense to define a base netlink message. > >My opinion on this matter has not changed at all since the last time >we discussed this.
The discussion we had before was about flag bitfield that was there *always*. In this case, that is not true. It is either ifindex or ifname. Even rtnetlink has ifname as attribute.
The flags and info_mask is just big mystery. If it is per-command, seems natural to have it as attributes.
> >So unless you have new information to provide to me on this issue >which might change my mind, please accept the result of the previous >discussion which is that a base netlink message is not only >appropriate but desirable. > >Thank you.
| |