netdev - Re: [PATCH net-next v1 15/17] net/mlx5: Cleanup XFRM attributes struct

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: <Ym4/SM8XDIwEyiRB@unreal>
Date:   Sun, 1 May 2022 11:05:28 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Saeed Mahameed <saeedm@...dia.com>
Cc:     Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jason Gunthorpe <jgg@...dia.com>,
        linux-netdev <netdev@...r.kernel.org>,
        Raed Salem <raeds@...dia.com>
Subject: Re: [PATCH net-next v1 15/17] net/mlx5: Cleanup XFRM attributes
 struct

On Fri, Apr 22, 2022 at 03:45:02PM -0700, Saeed Mahameed wrote:
> On 19 Apr 13:13, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@...dia.com>
> > 
> > Remove everything that is not used or from mlx5_accel_esp_xfrm_attrs,
> > together with change type of spi to store proper type from the beginning.
> > 
> > Reviewed-by: Raed Salem <raeds@...dia.com>
> > Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> > ---
> > .../mellanox/mlx5/core/en_accel/ipsec.c       | 10 ++-------
> > .../mellanox/mlx5/core/en_accel/ipsec.h       | 21 ++-----------------
> > .../mellanox/mlx5/core/en_accel/ipsec_fs.c    |  4 ++--
> > .../mlx5/core/en_accel/ipsec_offload.c        |  4 ++--
> > 4 files changed, 8 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> > index be7650d2cfd3..35e2bb301c26 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> > @@ -137,7 +137,7 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,
> > 				   struct mlx5_accel_esp_xfrm_attrs *attrs)
> > {
> > 	struct xfrm_state *x = sa_entry->x;
> > -	struct aes_gcm_keymat *aes_gcm = &attrs->keymat.aes_gcm;
> > +	struct aes_gcm_keymat *aes_gcm = &attrs->aes_gcm;
> > 	struct aead_geniv_ctx *geniv_ctx;
> > 	struct crypto_aead *aead;
> > 	unsigned int crypto_data_len, key_len;
> > @@ -171,12 +171,6 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,
> > 			attrs->flags |= MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP;
> > 	}
> > 
> > -	/* rx handle */
> > -	attrs->sa_handle = sa_entry->handle;
> > -
> > -	/* algo type */
> > -	attrs->keymat_type = MLX5_ACCEL_ESP_KEYMAT_AES_GCM;
> > -
> > 	/* action */
> > 	attrs->action = (!(x->xso.flags & XFRM_OFFLOAD_INBOUND)) ?
> > 			MLX5_ACCEL_ESP_ACTION_ENCRYPT :
> > @@ -187,7 +181,7 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,
> > 			MLX5_ACCEL_ESP_FLAGS_TUNNEL;
> > 
> > 	/* spi */
> > -	attrs->spi = x->id.spi;
> > +	attrs->spi = be32_to_cpu(x->id.spi);
> > 
> > 	/* source , destination ips */
> > 	memcpy(&attrs->saddr, x->props.saddr.a6, sizeof(attrs->saddr));
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
> > index 97c55620089d..16bcceec16c4 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
> > @@ -55,11 +55,6 @@ enum mlx5_accel_esp_action {
> > 	MLX5_ACCEL_ESP_ACTION_ENCRYPT,
> > };
> > 
> > -enum mlx5_accel_esp_keymats {
> > -	MLX5_ACCEL_ESP_KEYMAT_AES_NONE,
> > -	MLX5_ACCEL_ESP_KEYMAT_AES_GCM,
> > -};
> > -
> > struct aes_gcm_keymat {
> > 	u64   seq_iv;
> > 
> > @@ -73,21 +68,9 @@ struct aes_gcm_keymat {
> > struct mlx5_accel_esp_xfrm_attrs {
> > 	enum mlx5_accel_esp_action action;
> > 	u32   esn;
> > -	__be32 spi;
> > -	u32   seq;
> > -	u32   tfc_pad;
> > +	u32   spi;
> > 	u32   flags;
> > -	u32   sa_handle;
> > -	union {
> > -		struct {
> > -			u32 size;
> > -
> > -		} bmp;
> > -	} replay;
> > -	enum mlx5_accel_esp_keymats keymat_type;
> > -	union {
> > -		struct aes_gcm_keymat aes_gcm;
> > -	} keymat;
> 
> Why do we have so many unused fields ? are these leftovers from FPGA ipsec ?

It is combination of leftovers and extra layering.

Thanks