netdev - Re: [PATCH net-next v1 1/6] net/mlx5e: Support devlink reload of IPsec core

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: <20221025105814.foarlkx6nmxazeie@sx1>
Date:   Tue, 25 Oct 2022 11:58:14 +0100
From:   Saeed Mahameed <saeed@...nel.org>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Leon Romanovsky <leonro@...dia.com>,
        Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
        Paolo Abeni <pabeni@...hat.com>,
        Saeed Mahameed <saeedm@...dia.com>
Subject: Re: [PATCH net-next v1 1/6] net/mlx5e: Support devlink reload of
 IPsec core

On 24 Oct 19:59, Leon Romanovsky wrote:
>From: Leon Romanovsky <leonro@...dia.com>
>
>Change IPsec initialization flow to allow future creation of hardware
>resources that should be released and allocated during devlink reload
>operation. As part of that change, update function signature to be
>void as no callers are actually interested in it.
>
>Signed-off-by: Leon Romanovsky <leonro@...dia.com>
>---

...

>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>index 794cd8dfe9c9..324e5759b049 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>@@ -761,7 +761,6 @@ static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev,
> 			     struct net_device *netdev)
> {
> 	struct mlx5e_priv *priv = netdev_priv(netdev);
>-	int err;
>
> 	priv->fs = mlx5e_fs_init(priv->profile, mdev,
> 				 !test_bit(MLX5E_STATE_DESTROYING, &priv->state));
>@@ -770,10 +769,6 @@ static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev,
> 		return -ENOMEM;
> 	}
>
>-	err = mlx5e_ipsec_init(priv);
>-	if (err)
>-		mlx5_core_err(mdev, "Uplink rep IPsec initialization failed, %d\n", err);
>-

Original code had ipsec enabled for uplink rep and nic profile only, but not
other vport reps. your below code will enable ipsec and alloc resources for
other vport reps, which will be a huge waste of memory and hw resources on
switchdev system with lots of vports.

Please make sure not to enable ipsec on ALL reps.

> 	mlx5e_vxlan_set_netdev_info(priv);
> 	mlx5e_build_rep_params(netdev);
> 	mlx5e_timestamp_init(priv);
>@@ -783,7 +778,6 @@ static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev,
> static void mlx5e_cleanup_rep(struct mlx5e_priv *priv)
> {
> 	mlx5e_fs_cleanup(priv->fs);
>-	mlx5e_ipsec_cleanup(priv);
> }
>
> static int mlx5e_create_rep_ttc_table(struct mlx5e_priv *priv)
>@@ -1074,6 +1068,8 @@ static void mlx5e_rep_enable(struct mlx5e_priv *priv)

this function isn't invoked from uplink rep, so you broke functionality.

> {
> 	struct mlx5e_rep_priv *rpriv = priv->ppriv;
>
>+	mlx5e_ipsec_init(priv);
>+
> 	mlx5e_set_netdev_mtu_boundaries(priv);
> 	mlx5e_rep_neigh_init(rpriv);
> }
>@@ -1083,6 +1079,7 @@ static void mlx5e_rep_disable(struct mlx5e_priv *priv)
> 	struct mlx5e_rep_priv *rpriv = priv->ppriv;
>
> 	mlx5e_rep_neigh_cleanup(rpriv);
>+	mlx5e_ipsec_cleanup(priv);
> }
>
> static int mlx5e_update_rep_rx(struct mlx5e_priv *priv)
>-- 
>2.37.3
>