| 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
| ||
|
Message-ID: <20220422224257.pa7p2uuo4qau5ezi@sx1>
Date: Fri, 22 Apr 2022 15:42:57 -0700
From: Saeed Mahameed <saeedm@...dia.com>
To: Leon Romanovsky <leon@...nel.org>
Cc: Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
"David S . Miller" <davem@...emloft.net>,
Leon Romanovsky <leonro@...dia.com>,
Jason Gunthorpe <jgg@...dia.com>,
linux-netdev <netdev@...r.kernel.org>,
Raed Salem <raeds@...dia.com>
Subject: Re: [PATCH net-next v1 13/17] net/mlx5: Simplify IPsec capabilities
logic
On 19 Apr 13:13, Leon Romanovsky wrote:
>From: Leon Romanovsky <leonro@...dia.com>
>
>Reduce number of hard-coded IPsec capabilities by making sure
>that mlx5_ipsec_device_caps() sets only supported bits.
>
>As part of this change, remove _accel_ notations from the names
>and prepare the code to IPsec full offload mode.
>
Can you explain why remove __accel__ notation ?
__accel__ notation and decoupling from other common netdev features is done
for modularity purpose, en_accel directories are separated so we can
implement complex/stateful accelerations while avoid contaminating/affecting
common data-path performance sensitives flows.
I think keeping __accel__ notations is a must here for the above reasons,
unless you have a more strong reason to remove it..
>Reviewed-by: Raed Salem <raeds@...dia.com>
>Signed-off-by: Leon Romanovsky <leonro@...dia.com>
>---
> .../mellanox/mlx5/core/en_accel/ipsec.c | 16 ++------------
> .../mellanox/mlx5/core/en_accel/ipsec.h | 9 +++-----
> .../mlx5/core/en_accel/ipsec_offload.c | 22 +++++++++----------
> 3 files changed, 16 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 Clean IPsec FS add/delete rules28729b1cc6e6..be7650d2cfd3 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
>@@ -215,7 +215,7 @@ static inline int mlx5e_xfrm_validate_state(struct xfrm_state *x)
> return -EINVAL;
> }
> if (x->props.flags & XFRM_STATE_ESN &&
>- !(mlx5_ipsec_device_caps(priv->mdev) & MLX5_ACCEL_IPSEC_CAP_ESN)) {
>+ !(mlx5_ipsec_device_caps(priv->mdev) & MLX5_IPSEC_CAP_ESN)) {
> netdev_info(netdev, "Cannot offload ESN xfrm states\n");
> return -EINVAL;
> }
>@@ -262,11 +262,6 @@ static inline int mlx5e_xfrm_validate_state(struct xfrm_state *x)
> netdev_info(netdev, "Cannot offload xfrm states with geniv other than seqiv\n");
> return -EINVAL;
> }
>- if (x->props.family == AF_INET6 &&
>- !(mlx5_ipsec_device_caps(priv->mdev) & MLX5_ACCEL_IPSEC_CAP_IPV6)) {
>- netdev_info(netdev, "IPv6 xfrm state offload is not supported by this device\n");
>- return -EINVAL;
>- }
> return 0;
> }
>
>@@ -457,12 +452,6 @@ void mlx5e_ipsec_build_netdev(struct mlx5e_priv *priv)
> if (!mlx5_ipsec_device_caps(mdev))
> return;
>
>- if (!(mlx5_ipsec_device_caps(mdev) & MLX5_ACCEL_IPSEC_CAP_ESP) ||
>- !MLX5_CAP_ETH(mdev, swp)) {
>- mlx5_core_dbg(mdev, "mlx5e: ESP and SWP offload not supported\n");
>- return;
>- }
>-
> mlx5_core_info(mdev, "mlx5e: IPSec ESP acceleration enabled\n");
> netdev->xfrmdev_ops = &mlx5e_ipsec_xfrmdev_ops;
> netdev->features |= NETIF_F_HW_ESP;
>@@ -476,8 +465,7 @@ void mlx5e_ipsec_build_netdev(struct mlx5e_priv *priv)
> netdev->features |= NETIF_F_HW_ESP_TX_CSUM;
> netdev->hw_enc_features |= NETIF_F_HW_ESP_TX_CSUM;
>
>- if (!(mlx5_ipsec_device_caps(mdev) & MLX5_ACCEL_IPSEC_CAP_LSO) ||
>- !MLX5_CAP_ETH(mdev, swp_lso)) {
>+ if (!MLX5_CAP_ETH(mdev, swp_lso)) {
> mlx5_core_dbg(mdev, "mlx5e: ESP LSO not supported\n");
> return;
> }
>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 af1467cbb7c7..97c55620089d 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
>@@ -102,12 +102,9 @@ struct mlx5_accel_esp_xfrm_attrs {
> u8 is_ipv6;
> };
>
>-enum mlx5_accel_ipsec_cap {
>- MLX5_ACCEL_IPSEC_CAP_DEVICE = 1 << 0,
>- MLX5_ACCEL_IPSEC_CAP_ESP = 1 << 1,
>- MLX5_ACCEL_IPSEC_CAP_IPV6 = 1 << 2,
>- MLX5_ACCEL_IPSEC_CAP_LSO = 1 << 3,
>- MLX5_ACCEL_IPSEC_CAP_ESN = 1 << 4,
>+enum mlx5_ipsec_cap {
>+ MLX5_IPSEC_CAP_CRYPTO = 1 << 0,
>+ MLX5_IPSEC_CAP_ESN = 1 << 1,
> };
>
> struct mlx5e_priv;
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c
>index 817747d5229e..b44bce3f4ef1 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c
>@@ -7,7 +7,7 @@
>
> u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev)
> {
>- u32 caps;
>+ u32 caps = 0;
>
> if (!MLX5_CAP_GEN(mdev, ipsec_offload))
> return 0;
>@@ -19,23 +19,23 @@ u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev)
> MLX5_HCA_CAP_GENERAL_OBJECT_TYPES_IPSEC))
> return 0;
>
>- if (!MLX5_CAP_IPSEC(mdev, ipsec_crypto_offload) ||
>- !MLX5_CAP_ETH(mdev, insert_trailer))
>- return 0;
>-
> if (!MLX5_CAP_FLOWTABLE_NIC_TX(mdev, ipsec_encrypt) ||
> !MLX5_CAP_FLOWTABLE_NIC_RX(mdev, ipsec_decrypt))
> return 0;
>
>- caps = MLX5_ACCEL_IPSEC_CAP_DEVICE | MLX5_ACCEL_IPSEC_CAP_IPV6 |
>- MLX5_ACCEL_IPSEC_CAP_LSO;
>+ if (!MLX5_CAP_IPSEC(mdev, ipsec_crypto_esp_aes_gcm_128_encrypt) ||
>+ !MLX5_CAP_IPSEC(mdev, ipsec_crypto_esp_aes_gcm_128_decrypt))
>+ return 0;
>
>- if (MLX5_CAP_IPSEC(mdev, ipsec_crypto_esp_aes_gcm_128_encrypt) &&
>- MLX5_CAP_IPSEC(mdev, ipsec_crypto_esp_aes_gcm_128_decrypt))
>- caps |= MLX5_ACCEL_IPSEC_CAP_ESP;
>+ if (MLX5_CAP_IPSEC(mdev, ipsec_crypto_offload) &&
>+ MLX5_CAP_ETH(mdev, insert_trailer) && MLX5_CAP_ETH(mdev, swp))
>+ caps |= MLX5_IPSEC_CAP_CRYPTO;
>+
>+ if (!caps)
>+ return 0;
>
> if (MLX5_CAP_IPSEC(mdev, ipsec_esn))
>- caps |= MLX5_ACCEL_IPSEC_CAP_ESN;
>+ caps |= MLX5_IPSEC_CAP_ESN;
>
> /* We can accommodate up to 2^24 different IPsec objects
> * because we use up to 24 bit in flow table metadata
>--
>2.35.1
>
Powered by blists - more mailing lists