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

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]

Date:   Mon, 11 Dec 2017 19:02:19 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     David Miller <davem@...emloft.net>
Cc:     mkubecek@...e.cz, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface

Mon, Dec 11, 2017 at 05:56:51PM CET, davem@...emloft.net wrote:
>From: Jiri Pirko <jiri@...nulli.us>
>Date: Mon, 11 Dec 2017 17:02:21 +0100
>
>> Mon, Dec 11, 2017 at 02:53:31PM CET, mkubecek@...e.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@...e.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.