netdev - Re: [PATCH net-next v5 12/22] ethtool: provide string sets with GET

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]

Message-ID: <2c29310b-a2a0-3867-a09f-51f2dc47ecd3@gmail.com>
Date:   Wed, 27 Mar 2019 19:25:47 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Michal Kubecek <mkubecek@...e.cz>,
        David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Cc:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Jiri Pirko <jiri@...nulli.us>, Andrew Lunn <andrew@...n.ch>,
        John Linville <linville@...driver.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v5 12/22] ethtool: provide string sets with
 GET_STRSET request



On 3/25/2019 10:08 AM, Michal Kubecek wrote:
> Requests a contents of one or more string sets, i.e. indexed arrays of
> strings; this information is provided by ETHTOOL_GSSET_INFO and
> ETHTOOL_GSTRINGS commands of ioctl interface. There are three types of
> requests:
> 
>   - no NLM_F_DUMP, no device: get "global" stringsets
>   - no NLM_F_DUMP, with device: get string sets related to the device
>   - NLM_F_DUMP, no device: get device related string sets for all devices
> 
> It's possible to request all string sets of given type or only specific
> sets. With ETHA_STRSET_COUNTS flag, only set sizes (number of strings) are
> returned.
> 
> Signed-off-by: Michal Kubecek <mkubecek@...e.cz>
> ---

[snip]

> diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
> index 5e5d785fe215..1508c16a236e 100644
> --- a/Documentation/networking/ethtool-netlink.txt
> +++ b/Documentation/networking/ethtool-netlink.txt
> @@ -127,6 +127,8 @@ List of message types
>  ---------------------
>  
>      ETHNL_CMD_EVENT			notification only
> +    ETHNL_CMD_GET_STRSET
> +    ETHNL_CMD_SET_STRSET		response only
>  
>  All constants use ETHNL_CMD_ prefix, usually followed by "GET", "SET" or "ACT"
>  to indicate the type.
> @@ -167,6 +169,46 @@ and also multiple events of the same type (e.g. two or more newly registered
>  devices).
>  
>  
> +GET_STRSET
> +----------
> +
> +Requests contents of a string set as provided by ioctl commands
> +ETHTOOL_GSSET_INFO and ETHTOOL_GSTRINGS. String sets are not user writeable so
> +that the corresponding SET_STRSET message is only used in kernel replies.
> +There are two types of string sets: global (independent of a device, e.g.
> +device feature names) and device specific (e.g. device private flags).
> +
> +Request contents:
> +
> +    ETHA_STRSET_DEV		(nested)	device identification
> +    ETHA_STRSET_COUNTS		(flag)		request only string counts

Should not that be part of the nested attribute under
ETHA_STRSET_STRINGSET. We should probably think about adding another
flag which indicates that we want to get the stringset associated data,
see below why.

> +    ETHA_STRSET_STRINGSET	(nested)	string set to request
> +        ETHA_STRINGSET_ID		(u32)		set id
> +
> +Kernel response contents:
> +
> +    ETHA_STRSET_DEV		(nested)	device identification
> +    ETHA_STRSET_STRINGSET	(nested)	string set to request

string set requested?

> +        ETHA_STRINGSET_ID		(u32)		set id
> +        ETHA_STRINGSET_COUNT		(u32)		number of strings
> +        ETHA_STRINGSET_STRINGS		(nested)	array of strings
> +            ETHA_STRING_INDEX			(u32)		string index
> +            ETHA_STRING_VALUE			(string)	string value

This is one of the areas where the legacy ethtool ioctl() is painful
because we need to request a string set to know how many of those exist
to allocate space for those in both kernel and user space.

If we could find a way to have a single command that allows us to dump
stringset (count, values) and associated data, then we save ourselves a
context switch and having to pre-allocate memory accordingly.
-- 
Florian