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

[Alexander Lobakin <alexandr.lobakin@intel.com>]

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.

> +{
> +	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.

> +				  &netdev_gso_features_mask);
> +
> +	netdev_features_set_array(&netif_f_ip_csum_feature_set,
> +				  &netdev_ip_csum_features);
> +
> +	netdev_features_set_array(&netif_f_csum_feature_set_mask,
> +				  &netdev_csum_features_mask);
> +
> +	netdev_features_set_array(&netif_f_general_tso_feature_set,
> +				  &netdev_general_tso_features);
> +
> +	netdev_features_set_array(&netif_f_all_tso_feature_set,
> +				  &netdev_all_tso_features);
> +
> +	netdev_features_set_array(&netif_f_tso_ecn_feature_set,
> +				  &netdev_tso_ecn_features);
> +
> +	netdev_features_set_array(&netif_f_all_fcoe_feature_set,
> +				  &netdev_all_fcoe_features);
> +
> +	netdev_features_set_array(&netif_f_gso_soft_feature_set,
> +				  &netdev_gso_software_features);
> +
> +	netdev_features_set_array(&netif_f_one_for_all_feature_set,
> +				  &netdev_one_for_all_features);
> +
> +	netdev_features_set_array(&netif_f_all_for_all_feature_set,
> +				  &netdev_all_for_all_features);
> +
> +	netdev_features_set_array(&netif_f_upper_disables_feature_set,
> +				  &netdev_upper_disable_features);
> +
> +	netdev_features_set_array(&netif_f_soft_feature_set,
> +				  &netdev_soft_features);
> +
> +	netdev_features_set_array(&netif_f_soft_off_feature_set,
> +				  &netdev_soft_off_features);
> +
> +	netdev_features_set_array(&netif_f_rx_vlan_feature_set,
> +				  &netdev_rx_vlan_features);
> +
> +	netdev_features_set_array(&netif_f_tx_vlan_feature_set,
> +				  &netdev_tx_vlan_features);
> +
> +	netdev_features_set_array(&netif_f_vlan_filter_feature_set,
> +				  &netdev_vlan_filter_features);
> +
> +	netdev_all_vlan_features = netdev_rx_vlan_features;
> +	netdev_features_set(&netdev_all_vlan_features, netdev_tx_vlan_features);
> +	netdev_features_set(&netdev_all_vlan_features,
> +			    netdev_vlan_filter_features);
> +
> +	netdev_features_set_array(&netif_f_ctag_vlan_feature_set,
> +				  &netdev_ctag_vlan_features);
> +
> +	netdev_features_set_array(&netif_f_stag_vlan_feature_set,
> +				  &netdev_stag_vlan_features);
> +
> +	netdev_features_set_array(&netif_f_gso_encap_feature_set,
> +				  &netdev_gso_encap_all_features);
> +
> +	netdev_features_set_array(&netif_f_xfrm_feature_set,
> +				  &netdev_xfrm_features);
> +
> +	netdev_features_set_array(&netif_f_tls_feature_set,
> +				  &netdev_tls_features);
> +
> +	netdev_csum_gso_features_mask =
> +		netdev_features_or(netdev_gso_software_features,
> +				   netdev_csum_features_mask);
> +
> +	netdev_features_fill(&features);
> +	netdev_ethtool_features =
> +		netdev_features_andnot(features, netdev_never_change_features);
> +}
> +
>  /*
>   *	Initialize the DEV module. At boot time this walks the device list and
>   *	unhooks any devices that fail to initialise (normally hardware not

--- 8< ---

> -- 
> 2.33.0

Thanks,
Al