netdev - Re: [PATCH 2/9] net-ethtool: Convert (hw_/vlan_/wanted

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] [day] [month] [year] [list]

Message-ID: <BANLkTinNORHLJ_YqzMmFZo9v7CjS+ZrhWA@mail.gmail.com>
Date:	Mon, 25 Apr 2011 11:14:16 -0700
From:	Mahesh Bandewar <maheshb@...gle.com>
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Michał Mirosław <mirq-linux@...e.qmqm.pl>
Subject: Re: [PATCH 2/9] net-ethtool: Convert (hw_/vlan_/wanted_)features
 fields from u32 type to u64.

On Sat, Apr 23, 2011 at 10:30 PM, Ben Hutchings
<bhutchings@...arflare.com> wrote:
> On Fri, 2011-04-22 at 16:36 -0700, Mahesh Bandewar wrote:
>> Signed-off-by: Mahesh Bandewar <maheshb@...gle.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.
>
:) That comment was not supposed to be part of the patch and was just
to aid my thinking while we were discussing about the approach to
extend features. (I thought) if we have decided not to use use arrays,
then why to complicate this interface, so was attempting to simplify
the interface.

>>  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.
>
got it!

> 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).
>
I think even 64 bits may not be enough, so in the earlier thread I had
mentioned that, this would (probably) give us next two years! I didn't
know this constraint though.

--mahesh..

> 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.
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html