Date:   Fri, 8 Oct 2021 10:54:07 +0800
From:   "shenjian (K)" <shenjian15@...wei.com>
To:     Saeed Mahameed <saeed@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        "Alexander Lobakin" <alexandr.lobakin@...el.com>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <hkallweit1@...il.com>,
        <netdev@...r.kernel.org>, <linuxarm@...neuler.org>
Subject: Re: [RFCv2 net-next 000/167] net: extend the netdev_features_t

Hi, Saeed

Sorry to reply late.


在 2021/10/5 6:30, Saeed Mahameed 写道:
> On Mon, 2021-10-04 at 16:59 +0200, Andrew Lunn wrote:
>> On Fri, Oct 01, 2021 at 05:17:10PM +0200, Alexander Lobakin wrote:
>>> From: Jian Shen <shenjian15@...wei.com>
>>> Date: Wed, 29 Sep 2021 23:50:47 +0800
>>>
>>> Hi,
>>>
>>>> For the prototype of netdev_features_t is u64, and the number
>>>> of netdevice feature bits is 64 now. So there is no space to
>>>> introduce new feature bit.
>>>>
>>>> This patchset try to solve it by change the prototype of
>>>> netdev_features_t from u64 to bitmap. With this change,
>>>> it's necessary to introduce a set of bitmap operation helpers
>>>> for netdev features. Meanwhile, the functions which use
>>>> netdev_features_t as return value are also need to be changed,
>>>> return the result as an output parameter.
>>>>
>>>> With above changes, it will affect hundreds of files, and all the
>>>> nic drivers. To make it easy to be reviewed, split the changes
>>>> to 167 patches to 5 parts.
>>> If you leave the current feature field set (features, hw_features
>>> etc.) as is and just add new ones as bitmaps -- I mean, to place
>>> only newly added features there -- you won't have to change this in
>>> hundreds of drivers.
>> That makes things messy for the future. Two different ways to express
>> the same thing. And it is a trap waiting for developers to fall
>> into. Is this a new feature or an old feature bit? Should i add it to
>> the old or new bitmap? Will the compiler error out if i get it wrong,
>> or silently accept it?
>>
>>> Another option is to introduce new fields as bitmaps and mirror all
>>> features there, but also keep the current ones. This implies some
>>> code duplication -- to keep both sets in sync -- but it will also
>>> allow to avoid such diffstats. Developers could switch their
>>> drivers
>>> one-by-one then, and once they finish converting,
>> Which will never happen. Most developers will say, why bother, it
>> works as it is, i'm too lazy. And many drivers don't have an active
>> developer, and so won't get converted.
>>
>> Yes it is a big patchset, but at the end, we get a uniform API which
>> is future proof, and no traps waiting for developers to fall into.
>>
> I agree, i had to visit this topic a year ago or so, and the only
> conclusion was is to solve this the hard way, introduce a totally new
> mechanism, the safest way is to remove old netdev_features_t fields
> from netdev and add new ones (both names and types), so compiler will
> catch you if you missed to convert a place.

Yes, I agree that this way is safest, and it's necessary to ensure the 
compiler can
catch any missed convertion or incorrect convertion.

I'm a bit  confused about  use new names.  If so, all most the changes 
will have
be put into a single patch, which may be too big to be reviewed.


>
> maybe hide the implementation details and abstract it away from drivers
> using getters and manipulation APIs, it is not that bad since drivers
> are already not supposed to modify netdev_features directly.
>

For the getters and manipulation APIs， do you mean the helpers for 
netdev_features,
using bitmap ops ?


>>     Andrew
>
> .
>

Powered by blists - more mailing lists