Re: [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface

[David Miller]

From: Jiri Pirko <jiri@xxxxxxxxxxx>
Date: Mon, 11 Dec 2017 17:02:21 +0100

> Mon, Dec 11, 2017 at 02:53:31PM CET, mkubecek@xxxxxxx 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@xxxxxxx>
> 
> [...]
> 
> 
>>+
>>+/* 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.

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.