netdev - Re: [PATCH v2 net-next 1/2] ethtool - Support for configurable RSS hash key

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:	Sun, 23 Feb 2014 00:51:33 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Venkat Duvvuru <VenkatKumar.Duvvuru@...lex.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next 1/2] ethtool - Support for configurable RSS
 hash key

On Sat, 2014-02-22 at 17:21 +0530, Venkat Duvvuru wrote:
[...]
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -828,22 +828,30 @@ struct ethtool_rxnfc {
>  
> 
>  /**
> - * struct ethtool_rxfh_indir - command to get or set RX flow hash indirection
> - * @cmd: Specific command number - %ETHTOOL_GRXFHINDIR or %ETHTOOL_SRXFHINDIR
> - * @size: On entry, the array size of the user buffer, which may be zero.
> - *	On return from %ETHTOOL_GRXFHINDIR, the array size of the hardware
> - *	indirection table.
> - * @ring_index: RX ring/queue index for each hash value
> + * struct ethtool_rxfh - command to get or set RX flow hash indirection or/and
> + * hash key.
> + * @cmd: Specific command number - %ETHTOOL_GRSSH or %ETHTOOL_SRSSH
> + * @indir_size: On entry, the array size of the user buffer, which may be zero.
> + *		On return from %ETHTOOL_GRSSH, the array size of the hardware
> + *		indirection table.
> + * @key_size:	On entry, the array size of the user buffer in bytes,
> + *		which may be zero.
> + *		On return from %ETHTOOL_GRSSH, the size of the RSS hash key.
> + * @rsvd:	Reserved for future extensions.
> + * @rss_config: RX ring/queue index for each hash value or/and hash key
> + *		respectively.
>   *
> - * For %ETHTOOL_GRXFHINDIR, a @size of zero means that only the size
> - * should be returned.  For %ETHTOOL_SRXFHINDIR, a @size of zero means
> - * the table should be reset to default values.  This last feature
> + * For %ETHTOOL_GRSSH, a @indir_size and key_size of zero means that only the
> + * size should be returned.  For %ETHTOOL_SRSSH, a @indir_size of zero means
> + * the indir table should be reset to default values.  This last feature
>   * is not supported by the original implementations.
>   */
> -struct ethtool_rxfh_indir {
> -	__u32	cmd;
> -	__u32	size;
> -	__u32	ring_index[0];
> +struct ethtool_rxfh {
> +	__u32   cmd;
> +	__u32   indir_size;
> +	__u32   key_size;
> +	__u32	rsvd[3];
> +	__u32   rss_config[0];
>  };
>
>  /**
> @@ -1102,8 +1110,8 @@ enum ethtool_sfeatures_retval_bits {
>  #define ETHTOOL_SRXNTUPLE	0x00000035 /* Add an n-tuple filter to device */
>  #define ETHTOOL_GRXNTUPLE	0x00000036 /* deprecated */
>  #define ETHTOOL_GSSET_INFO	0x00000037 /* Get string set info */
> -#define ETHTOOL_GRXFHINDIR	0x00000038 /* Get RX flow hash indir'n table */
> -#define ETHTOOL_SRXFHINDIR	0x00000039 /* Set RX flow hash indir'n table */
> +#define ETHTOOL_GRSSH		0x00000038 /* Get RX flow hash indir'n table */
> +#define ETHTOOL_SRSSH		0x00000039 /* Set RX flow hash indir'n table */
>  
>  #define ETHTOOL_GFEATURES	0x0000003a /* Get device offload settings */
>  #define ETHTOOL_SFEATURES	0x0000003b /* Change device offload settings */
[...]

You can't just redefine commands like this.  Think what will happen if
someone uses an old version of ethtool with a new kernel.

You have to keep supporting the old ethtool command numbers and
structure.  And you have to make the ethtool core keep working with all
the other drivers that don't support setting the key yet (or update them
so they do).

Ben.

-- 
Ben Hutchings
All the simple programs have been written, and all the good names taken.

Download attachment "signature.asc" of type "application/pgp-signature" (812 bytes)