Re: extending feature word.

[Mahesh Bandewar <maheshb@google.com>]

On Fri, Apr 8, 2011 at 3:05 AM, Michał Mirosław <mirq-linux@rere.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];
>
> Hmm. There might be no point in making features field an array.
> This gives us nothing really. Maybe just add features_2 or similar?
> If we ever get to the point there need to be more than two words for
> features we can think of some abstraction layer then.
>
That is right! making it an array doesn't really buy us anything
unless there is a uniform way of managing all the bits spread across
multiple words inside that array. This was the reason why I have
changed that array into a bitmap in the patch that I have posted
earlier. This way the upper limit (currently only 32 bits) will be
removed and we'll have a long term solution. There will be little bit
of work involved but 'doing-things-right' has cost associated.

> Or we might add a new field and put there NETIF_F_LLTX, NETIF_F_HIGHDMA
> and others that are not user changeable ever. Those don't need dynamic
> propagation to slave devices (e.g. VLAN) and wanted/hw_features for them.
>
This will certainly buy us some time but will be a temporary fix until
we runout of bits again. Also adding a second word (separate from the
first word) will create fragmentation and different approaches to
manage these two words and (I think) wont be desirable.

There will be another approach where we change this to u64 and
postpone the problem little longer and probably wait for u128 to make
it even longer. This is again a mid-term fix and not really a
solution.

In the patch that I have posted, I have changed these fiels to bitmaps
and a plan to take it there. This will _solve_ the problem once and
for all.


Thanks,
--mahesh..

> Best Regards,
> Michał Mirosław
>