[Devel] [PATCH vz9] vlan, gre: set NETIF_F_VIRTUAL flag before it is checked

[Kirill Tkhai]

On 21.02.2022 13:50, Nikita Yushchenko wrote:
> Setting NETIF_F_VIRTUAL in net_device .ndo_init method is too late: that
> method is called from register_netdevice() later than ve_is_dev_movable()
> that uses that flag.
> 
> Proper location to set the flag is the setup method called at netdev
> creation time.
> 
> Feature: net: whitelist allowed Container network devices
> 
> Signed-off-by: Nikita Yushchenko <nikita.yushchenko at virtuozzo.com>
> ---
>  net/8021q/vlan_dev.c | 7 ++++---
>  net/ipv4/ip_gre.c    | 6 +++---
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index dde3d3e32092..1a468888b4b9 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -573,9 +573,6 @@ static int vlan_dev_init(struct net_device *dev)
>  			   NETIF_F_ALL_FCOE;
>  
>  	dev->features |= dev->hw_features | NETIF_F_LLTX;
> -#ifdef CONFIG_VE
> -	dev->ve_features = NETIF_F_VIRTUAL;
> -#endif

I don't like this. Instead of adding dev->ve_feature as just an extension of dev->features and
instead of initialization them in the same places as dev->features, this introduces unnatural
complexity.

We should better just move our "if (!ve_is_super(net->owner_ve) && ve_is_dev_movable(dev))" check
below in register_netdevice() to the same place we check for "dev->hw_features | dev->features".

NACK

>  	dev->gso_max_size = real_dev->gso_max_size;
>  	dev->gso_max_segs = real_dev->gso_max_segs;
>  	if (dev->features & NETIF_F_VLAN_FEATURES)
> @@ -866,4 +863,8 @@ void vlan_setup(struct net_device *dev)
>  	dev->max_mtu		= ETH_MAX_MTU;
>  
>  	eth_zero_addr(dev->broadcast);
> +
> +#ifdef CONFIG_VE
> +	dev->ve_features = NETIF_F_VIRTUAL;
> +#endif
>  }
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index a5497bfa3dfd..b8995e4c7a76 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -945,6 +945,9 @@ static void ipgre_tunnel_setup(struct net_device *dev)
>  	dev->netdev_ops		= &ipgre_netdev_ops;
>  	dev->type		= ARPHRD_IPGRE;
>  	ip_tunnel_setup(dev, ipgre_net_id);
> +#ifdef CONFIG_VE
> +	dev->ve_features = NETIF_F_VIRTUAL;
> +#endif
>  }
>  
>  static void __gre_tunnel_init(struct net_device *dev)
> @@ -977,9 +980,6 @@ static void __gre_tunnel_init(struct net_device *dev)
>  		 */
>  		dev->features |= NETIF_F_LLTX;
>  	}
> -#ifdef CONFIG_VE
> -	dev->ve_features = NETIF_F_VIRTUAL;
> -#endif
>  }
>  
>  static int ipgre_tunnel_init(struct net_device *dev)
>