netdev - Re: extending feature word.

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:	Sat, 2 Apr 2011 20:09:14 -0700
From:	Mahesh Bandewar <maheshb@...gle.com>
To:	Michał Mirosław <mirq-linux@...e.qmqm.pl>
Cc:	linux-netdev <netdev@...r.kernel.org>,
	Ben Hutchings <bhutchings@...arflare.com>,
	David Miller <davem@...emloft.net>
Subject: Re: extending feature word.

On Sat, Apr 2, 2011 at 5:42 AM, Michał Mirosław <mirq-linux@...e.qmqm.pl> wrote:
> On Fri, Apr 01, 2011 at 07:07:05PM -0700, Mahesh Bandewar wrote:
>> Thanks for your comments on my loop-back patch. I was looking at the
>> code today from the perspective of extending various "features" for
>> word to an array of words and as Michael has pointed out, it's a huge
>> change. So I'm thinking on the following lines
>> (include/linux/netdevice.h)
>>
>> +#define DEV_FEATURE_WORDS      2
>> +#define LEGACY_FEATURE_WORD    0
>>        /* currently active device features */
>> -       u32                     features;
>> +       u32                     features[DEV_FEATURE_WORDS];
>>        /* user-changeable features */
>> -       u32                     hw_features;
>> +       u32                     hw_features[DEV_FEATURE_WORDS];
>>        /* user-requested features */
>> -       u32                     wanted_features;
>> +       u32                     wanted_features[DEV_FEATURE_WORDS];
>>        /* VLAN feature mask */
>> -       u32                     vlan_features;
>> +       u32                     vlan_features[DEV_FEATURE_WORDS];
>> +#define legacy_features                features[LEGACY_FEATURE_WORD]
>> +#define legacy_hw_features     hw_features[LEGACY_FEATURE_WORD]
>> +#define legacy_wanted_features wanted_features[LEGACY_FEATURE_WORD]
>> +#define legacy_vlan_features    vlan_features[LEGACY_FEATURE_WORD]
>>
>>
>> So that it will be a matter of -
>>
>> s/features/legacy_features/
>> s/hw_features/legacy_hw_features/
>> s/wanted_features/legacy_wanted_features/
>> s/vlan_features/legacy_vlan_features/
>>
>> to start with as a first step. Once this is done, legacy_* can be
>> changed with respective arrays and modified as per the need. Comments?
>
> You could just s/features/features[0]/ as well. Why the extra hiding?
>
I was thinking that features[0] is really a short term solution sine
there are only couple of bits left in this word and any new will have
to be added into the next word. So eventually it should be managed
properly at all these places where we'll replace it with features[0].
So it's a nice indication that this was the legacy way of dealing and
need to be altered for additional bits (especially where we see
legacy_* stuff) where ever required.

On the other hand replacing with features[0] breaks the NETDEVICE_SHOW
macro in net/core/net-sysfs.c and need to be expanded manually.

> If you want to split the work, it would be clearer to first convert
> hw_features and wanted_features (with all the core code touching it -
> this is the easy part), then vlan_features (this includes drivers'
> and VLAN code) and then features (it's all over).
>
I like the idea of splitting but it will be only useful when all of it
is done and not partially, isn't it? Or am I missing something?

> Best Regards,
> Michał Mirosław
>
--
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