Re: [PATCH 2/9] net-ethtool: Convert (hw_/vlan_/wanted_)features fields from u32 type to u64.

[Ben Hutchings <bhutchings@solarflare.com>]

On Fri, 2011-04-22 at 16:36 -0700, Mahesh Bandewar wrote:
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
>  include/linux/ethtool.h |   26 +++++++------
>  net/core/ethtool.c      |   89 ++++++++++++++++------------------------------
>  2 files changed, 45 insertions(+), 70 deletions(-)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 9de3127..71e8a02 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -605,10 +605,10 @@ struct ethtool_flash {
>   * @never_changed: mask of features not changeable for any device
>   */
>  struct ethtool_get_features_block {
> -	__u32	available;
> -	__u32	requested;
> -	__u32	active;
> -	__u32	never_changed;
> +	__u64	available;
> +	__u64	requested;
> +	__u64	active;
> +	__u64	never_changed;
>  };
>  
>  /**
> @@ -618,10 +618,11 @@ struct ethtool_get_features_block {
>   *       out: number of elements in features[] needed to hold all features
>   * @features: state of features
>   */
> +/* TODO Why is this needed XXX */

Precisely to allow for expansion to more than 32 bits.

>  struct ethtool_gfeatures {
>  	__u32	cmd;
>  	__u32	size;
> -	struct ethtool_get_features_block features[0];
> +	struct ethtool_get_features_block features;
>  };
>  
>  /**
> @@ -630,8 +631,8 @@ struct ethtool_gfeatures {
>   * @requested: values of features to be changed
>   */
>  struct ethtool_set_features_block {
> -	__u32	valid;
> -	__u32	requested;
> +	__u64	valid;
> +	__u64	requested;
>  };
>  
>  /**
> @@ -640,10 +641,11 @@ struct ethtool_set_features_block {
>   * @size: array size of the features[] array
>   * @features: feature change masks
>   */
> +/* TODO Why is this needed XXX */
>  struct ethtool_sfeatures {
>  	__u32	cmd;
>  	__u32	size;
> -	struct ethtool_set_features_block features[0];
> +	struct ethtool_set_features_block features;
>  };
[...]

These structures are part of the userland API, but they are new in
2.6.39.  So they can still be changed up until 2.6.39 is released, but
not afterwards.

If we think 64 bits will be enough for the next 10 years, then let's
just go with a single 64-bit feature word.  If we're not so sure then
then the ethtool API should continue to allow for multiple words
(whether 32-bit or 64-bit).

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.