Re: [RFCv6 PATCH net-next 02/19] net: replace general features macroes with global netdev_features variables

["shenjian (K)" <shenjian15@huawei.com>]

在 2022/7/21 22:57, Alexander Lobakin 写道:
> From: shenjian (K) <shenjian15@huawei.com>
> Date: Thu, 21 Jul 2022 09:15:59 +0800
>
>> 在 2022/7/20 23:09, Alexander Lobakin 写道:
>>> From: shenjian (K) <shenjian15@huawei.com>
>>> Date: Wed, 20 Apr 2022 17:54:13 +0800
>>>
>>>> 在 2022/4/19 22:49, Alexander Lobakin 写道:
>>>>> From: Jian Shen <shenjian15@huawei.com>
>>>>> Date: Tue, 19 Apr 2022 10:21:49 +0800
>>>>>
>>>>>> There are many netdev_features bits group used in kernel. The
>>>> definition
>>>>>> will be illegal when using feature bit more than 64. Replace these
>>>> macroes
>>>>>> with global netdev_features variables, initialize them when netdev
>>>> module
>>>>>> init.
>>>>>>
>>>>>> Signed-off-by: Jian Shen <shenjian15@huawei.com>
>>>>>> ---
>>>>>>     drivers/net/wireguard/device.c  |  10 +-
>>>>>>     include/linux/netdev_features.h | 102 +++++++++-----
>>>>>>     net/core/Makefile               |   2 +-
>>>>>>     net/core/dev.c                  |  87 ++++++++++++
>>>>>>     net/core/netdev_features.c      | 241
>>>> ++++++++++++++++++++++++++++++++
>>>>>>     5 files changed, 400 insertions(+), 42 deletions(-)
>>>>>>     create mode 100644 net/core/netdev_features.c
>>>>>>
>>>>> --- 8< ---
>>>>>
>>>>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>>>>> index 4d6b57752eee..85bb418e8ef1 100644
>>>>>> --- a/net/core/dev.c
>>>>>> +++ b/net/core/dev.c
>>>>>> @@ -146,6 +146,7 @@
>>>>>>     #include <linux/sctp.h>
>>>>>>     #include <net/udp_tunnel.h>
>>>>>>     #include <linux/net_namespace.h>
>>>>>> +#include <linux/netdev_features_helper.h>
>>>>>>     #include <linux/indirect_call_wrapper.h>
>>>>>>     #include <net/devlink.h>
>>>>>>     #include <linux/pm_runtime.h>
>>>>>> @@ -11255,6 +11256,90 @@ static struct pernet_operations
>>>> __net_initdata default_device_ops = {
>>>>>>         .exit_batch = default_device_exit_batch,
>>>>>>     };
>>>>>>     >> +static void netdev_features_init(void)
>>>>> It is an initialization function, so it must be marked as __init.
>>>> right, I will add it, thanks!
>>>>
>>>>>> +{
>>>>>> +    netdev_features_t features;
>>>>>> +
>>>>>> + netdev_features_set_array(&netif_f_never_change_feature_set,
>>>>>> +                  &netdev_never_change_features);
>>>>>> +
>>>>>> + netdev_features_set_array(&netif_f_gso_feature_set_mask,
>>>>> I'm not sure it does make sense to have an empty newline between
>>>>> each call. I'd leave newlines only between the "regular" blocks
>>>>> and the "multi-call" blocks, I mean, stuff like VLAN, GSO and
>>>>> @netdev_ethtool_features.
>>>> At first, I added empty newline per call for the it used three lines.
>>>> Now the new call just use two lines, I will remove some unnecessary
>>>> blank lines.
>>>>
>>>> Thanks!
>>> I see no news regarding the conversion since the end of April, maybe
>>> I could pick it and finish if nobody objects? I'll preserve the
>>> original authorship for sure.
>>>
>> Hi， Alexander
>>
>> Sorry for late to finish the whole patchset with treewide changes, but
>> I'm still working on it.
>> And most of the convertsions have been completed. I will send to new
>> patchset in two weeks.
> Oh okay, I was only worried that it could be abandoned for some
> reason. Great to hear it's almost done, 120+ drivers is not
> something quick or exciting :)
Thanks, I will speed up efforts.

> I'll start reviewing the series, at least its "core" part, as soon
> as it hits netdev ML. Thanks!
Yes, It's need sufficient review, I want to get as many as possible 
advices for it.
Thanks!

>> Jian
>>
>>>>>> +                  &netdev_gso_features_mask);
> [...]
>
>>> Thanks,
>>> Olek
>>>
>>> .
>>>
> Olek
>
> .
>