netdev - Re: [PATCH net] net/mlx5e: Fix ipsec/tls netdev features build

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] [day] [month] [year] [list]

Message-ID: <3f8d7d14f8d88f6e7667a489eee8651fda596462.camel@kernel.org>
Date:   Wed, 24 Mar 2021 15:29:52 -0700
From:   Saeed Mahameed <saeed@...nel.org>
To:     wenxu@...oud.cn, kuba@...nel.org, davem@...emloft.net
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net] net/mlx5e: Fix ipsec/tls netdev features build

On Wed, 2021-03-24 at 12:15 +0800, wenxu@...oud.cn wrote:
> From: wenxu <wenxu@...oud.cn>
> 
> Ipsec and tls netdev features build should be done after the
> mlx5e_init_ipesc/tls which finishs the init for the ipsec/tls
> in the driver.
> 
> Fixes: 3ef14e463f6e ("net/mlx5e: Separate between netdev objects and
> mlx5e profiles initialization")
> Signed-off-by: wenxu <wenxu@...oud.cn>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 158f947..14c3f1f 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -5218,8 +5218,6 @@ static void mlx5e_build_nic_netdev(struct
> net_device *netdev)
>         netdev->priv_flags       |= IFF_UNICAST_FLT;
>  
>         mlx5e_set_netdev_dev_addr(netdev);
> -       mlx5e_ipsec_build_netdev(priv);
> -       mlx5e_tls_build_netdev(priv);
>  }
>  
>  void mlx5e_create_q_counters(struct mlx5e_priv *priv)
> @@ -5274,10 +5272,15 @@ static int mlx5e_nic_init(struct
> mlx5_core_dev *mdev,
>         err = mlx5e_ipsec_init(priv);
>         if (err)
>                 mlx5_core_err(mdev, "IPSec initialization failed,
> %d\n", err);
> +       else
> +               mlx5e_ipsec_build_netdev(priv);
> +
>  

Hi Wenxu, thanks for the patch. 
I know that IPSec got broken and i am working on a fix now,

Regarding your patch it is wrong to call ipsec/tls_build_netdev here
since it is too late, the netdev might be registered already and we
shouldn't be updating netdev->features directly. 

My plan is to setup the netdev features regardless of the
mlx5e_ipsec_init() state, and to remove the dependency.


>         err = mlx5e_tls_init(priv);
>         if (err)
>                 mlx5_core_err(mdev, "TLS initialization failed,
> %d\n", err);
> +       else
> +               mlx5e_tls_build_netdev(priv);
>  
>         err = mlx5e_devlink_port_register(priv);
>         if (err)

Did you encounter any issues with TLS ? because currently i don't see
any dependency between mlx5e_tls_build_netdev() and mlx5e_tls_init()
and the code should work as is.. but i might be missing something,
anyway I will provide a similar fix to ipsec.

Thanks For the report again.

Saeed.