netdev - Re: [PATCH net-next 2/2] mlx4/en_netdev: call notifiers when hw_enc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e007bac4c951486294d4e69d20f7c9ed7040172d.camel@mellanox.com>
Date:   Wed, 24 Jul 2019 20:47:00 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "dcaratti@...hat.com" <dcaratti@...hat.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Tariq Toukan <tariqt@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/2] mlx4/en_netdev: call notifiers when
 hw_enc_features change

On Wed, 2019-07-24 at 16:02 +0200, Davide Caratti wrote:
> ensure to call netdev_features_change() when the driver flips its
> hw_enc_features bits.
> 
> Signed-off-by: Davide Caratti <dcaratti@...hat.com>

The patch is correct, but can you explain how did you come to this ? 
did you encounter any issue with the current code ?

I am asking just because i think the whole dynamic changing of dev-
>hw_enc_features is redundant since mlx4 has the featutres_check
callback.

> ---
>  .../net/ethernet/mellanox/mlx4/en_netdev.c    | 39 ++++++++++++-----
> --
>  1 file changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 52500f744a0e..1b484dc6e1c2 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -2628,6 +2628,30 @@ static int mlx4_en_get_phys_port_id(struct
> net_device *dev,
>  	return 0;
>  }
>  
> +#define MLX4_GSO_PARTIAL_FEATURES (NETIF_F_IP_CSUM |
> NETIF_F_IPV6_CSUM | \
> +				   NETIF_F_RXCSUM | \
> +				   NETIF_F_TSO | NETIF_F_TSO6 | \
> +				   NETIF_F_GSO_UDP_TUNNEL | \
> +				   NETIF_F_GSO_UDP_TUNNEL_CSUM | \
> +				   NETIF_F_GSO_PARTIAL)
> +
> +static void mlx4_set_vxlan_offloads(struct net_device *dev, bool
> enable)
> +{
> +	netdev_features_t hw_enc_features;
> +
> +	rtnl_lock();
> +	hw_enc_features = dev->hw_enc_features;
> +	if (enable)
> +		dev->hw_enc_features |= MLX4_GSO_PARTIAL_FEATURES;
> +	else
> +		dev->hw_enc_features &= ~MLX4_GSO_PARTIAL_FEATURES;
> +
> +	if (hw_enc_features ^ dev->hw_enc_features)
> +		netdev_features_change(dev);
> +
> +	rtnl_unlock();
> +}
> +
>  static void mlx4_en_add_vxlan_offloads(struct work_struct *work)
>  {
>  	int ret;
> @@ -2647,12 +2671,7 @@ static void mlx4_en_add_vxlan_offloads(struct
> work_struct *work)
>  	}
>  
>  	/* set offloads */
> -	priv->dev->hw_enc_features |= NETIF_F_IP_CSUM |
> NETIF_F_IPV6_CSUM |
> -				      NETIF_F_RXCSUM |
> -				      NETIF_F_TSO | NETIF_F_TSO6 |
> -				      NETIF_F_GSO_UDP_TUNNEL |
> -				      NETIF_F_GSO_UDP_TUNNEL_CSUM |
> -				      NETIF_F_GSO_PARTIAL;
> +	mlx4_set_vxlan_offloads(priv->dev, true);
>  }
>  
>  static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
> @@ -2661,13 +2680,7 @@ static void mlx4_en_del_vxlan_offloads(struct
> work_struct *work)
>  	struct mlx4_en_priv *priv = container_of(work, struct
> mlx4_en_priv,
>  						 vxlan_del_task);
>  	/* unset offloads */
> -	priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM |
> NETIF_F_IPV6_CSUM |
> -					NETIF_F_RXCSUM |
> -					NETIF_F_TSO | NETIF_F_TSO6 |
> -					NETIF_F_GSO_UDP_TUNNEL |
> -					NETIF_F_GSO_UDP_TUNNEL_CSUM |
> -					NETIF_F_GSO_PARTIAL);
> -
> +	mlx4_set_vxlan_offloads(priv->dev, false);
>  	ret = mlx4_SET_PORT_VXLAN(priv->mdev->dev, priv->port,
>  				  VXLAN_STEER_BY_OUTER_MAC, 0);
>  	if (ret)