Skip to contentSkip to conversation

Support tunnel mode in mlx5 IPsec packet offload

PATCH In review Leon Romanovsky 2 participants 25 messages Apr 10, 2023 – Apr 13, 2023

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover Support tunnel mode in mlx5 IPsec packet offload

From: Leon Romanovsky <[email protected]>

Hi,

This series extends mlx5 to support tunnel mode in its IPsec packet offload implementation.

Thanks

---------------------------------------------------------------------
I would like to ask to apply it directly to netdev tree as PR is not really needed here.
---------------------------------------------------------------------

Leon Romanovsky (10):
net/mlx5e: Add IPsec packet offload tunnel bits net/mlx5e: Check IPsec packet offload tunnel capabilities net/mlx5e: Configure IPsec SA tables to support tunnel mode net/mlx5e: Prepare IPsec packet reformat code for tunnel mode net/mlx5e: Support IPsec RX packet offload in tunnel mode net/mlx5e: Support IPsec TX packet offload in tunnel mode net/mlx5e: Listen to ARP events to update IPsec L2 headers in tunnel
mode
net/mlx5: Allow blocking encap changes in eswitch net/mlx5e: Create IPsec table with tunnel support only when encap is
disabled
net/mlx5e: Accept tunnel mode for IPsec packet offload

 .../mellanox/mlx5/core/en_accel/ipsec.c       | 201 ++++++++++++++-
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  11 +-
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 243 +++++++++++++++---
 .../mlx5/core/en_accel/ipsec_offload.c        |   6 +
 .../net/ethernet/mellanox/mlx5/core/eswitch.h |  14 +
 .../mellanox/mlx5/core/eswitch_offloads.c     |  48 ++++
 include/linux/mlx5/mlx5_ifc.h                 |   8 +-
 7 files changed, 484 insertions(+), 47 deletions(-)
Signature · 1 line
2.39.2
#1 lore raw

PATCH 1/10 net/mlx5e: Add IPsec packet offload tunnel bits

From: Leon Romanovsky <[email protected]>

Extend packet reformat types and flow table capabilities with IPsec packet offload tunnel bits.

---
 include/linux/mlx5/mlx5_ifc.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
include/linux/mlx5/mlx5_ifc.h +6 -2
index e47d6c58da35..3e899844e84c 100644
@@ -456,9 +456,11 @@ struct mlx5_ifc_flow_table_prop_layout_bits {
 	u8         max_ft_level[0x8];
 
 	u8         reformat_add_esp_trasport[0x1];
-	u8         reserved_at_41[0x2];
+	u8         reformat_l2_to_l3_esp_tunnel[0x1];
+	u8         reserved_at_42[0x1];
 	u8         reformat_del_esp_trasport[0x1];
-	u8         reserved_at_44[0x2];
+	u8         reformat_l3_esp_tunnel_to_l2[0x1];
+	u8         reserved_at_45[0x1];
 	u8         execute_aso[0x1];
 	u8         reserved_at_47[0x19];
 
@@ -6599,7 +6601,9 @@ enum mlx5_reformat_ctx_type {
 	MLX5_REFORMAT_TYPE_L3_TUNNEL_TO_L2 = 0x3,
 	MLX5_REFORMAT_TYPE_L2_TO_L3_TUNNEL = 0x4,
 	MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV4 = 0x5,
+	MLX5_REFORMAT_TYPE_L2_TO_L3_ESP_TUNNEL = 0x6,
 	MLX5_REFORMAT_TYPE_DEL_ESP_TRANSPORT = 0x8,
+	MLX5_REFORMAT_TYPE_L3_ESP_TUNNEL_TO_L2 = 0x9,
 	MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV6 = 0xb,
 	MLX5_REFORMAT_TYPE_INSERT_HDR = 0xf,
 	MLX5_REFORMAT_TYPE_REMOVE_HDR = 0x10,
Signature · 1 line
2.39.2
#3 lore raw

PATCH 2/10 net/mlx5e: Check IPsec packet offload tunnel capabilities

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Validate tunnel mode support for IPsec packet offload.

---
 drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h    | 1 +
 .../ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c    | 6 ++++++
 2 files changed, 7 insertions(+)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +1 -0
index 52890d7dce6b..bb89e18b17b4 100644
@@ -107,6 +107,7 @@ enum mlx5_ipsec_cap {
 	MLX5_IPSEC_CAP_PACKET_OFFLOAD	= 1 << 2,
 	MLX5_IPSEC_CAP_ROCE             = 1 << 3,
 	MLX5_IPSEC_CAP_PRIO             = 1 << 4,
+	MLX5_IPSEC_CAP_TUNNEL           = 1 << 5,
 };
 
 struct mlx5e_priv;
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +6 -0
index 5fddb86bb35e..df90e19066bc 100644
@@ -48,6 +48,12 @@ u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev)
 		if (MLX5_CAP_FLOWTABLE_NIC_TX(mdev, ignore_flow_level) &&
 		    MLX5_CAP_FLOWTABLE_NIC_RX(mdev, ignore_flow_level))
 			caps |= MLX5_IPSEC_CAP_PRIO;
+
+		if (MLX5_CAP_FLOWTABLE_NIC_TX(mdev,
+					      reformat_l2_to_l3_esp_tunnel) &&
+		    MLX5_CAP_FLOWTABLE_NIC_RX(mdev,
+					      reformat_l3_esp_tunnel_to_l2))
+			caps |= MLX5_IPSEC_CAP_TUNNEL;
 	}
 
 	if (mlx5_get_roce_state(mdev) &&
Signature · 1 line
2.39.2
#5 lore raw

PATCH 3/10 net/mlx5e: Configure IPsec SA tables to support tunnel mode

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Create SA flow steering tables both for RX and TX with tunnel reformat property. This allows to add and delete extra headers needed for tunnel mode.

---
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 23 ++++++++++++-------
 1 file changed, 15 insertions(+), 8 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +15 -8
index b47794d4146e..060be020ca64 100644
@@ -118,7 +118,7 @@ static void ipsec_chains_put_table(struct mlx5_fs_chains *chains, u32 prio)
 
 static struct mlx5_flow_table *ipsec_ft_create(struct mlx5_flow_namespace *ns,
 					       int level, int prio,
-					       int max_num_groups)
+					       int max_num_groups, u32 flags)
 {
 	struct mlx5_flow_table_attr ft_attr = {};
 
@@ -127,6 +127,7 @@ static struct mlx5_flow_table *ipsec_ft_create(struct mlx5_flow_namespace *ns,
 	ft_attr.max_fte = NUM_IPSEC_FTE;
 	ft_attr.level = level;
 	ft_attr.prio = prio;
+	ft_attr.flags = flags;
 
 	return mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
 }
@@ -267,6 +268,7 @@ static int rx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 	struct mlx5_flow_destination default_dest;
 	struct mlx5_flow_destination dest[2];
 	struct mlx5_flow_table *ft;
+	u32 flags = 0;
 	int err;
 
 	default_dest = mlx5_ttc_get_default_dest(ttc, family2tt(family));
@@ -277,7 +279,7 @@ static int rx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 		return err;
 
 	ft = ipsec_ft_create(ns, MLX5E_ACCEL_FS_ESP_FT_ERR_LEVEL,
-			     MLX5E_NIC_PRIO, 1);
+			     MLX5E_NIC_PRIO, 1, 0);
 	if (IS_ERR(ft)) {
 		err = PTR_ERR(ft);
 		goto err_fs_ft_status;
@@ -300,8 +302,10 @@ static int rx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 		goto err_add;
 
 	/* Create FT */
-	ft = ipsec_ft_create(ns, MLX5E_ACCEL_FS_ESP_FT_LEVEL, MLX5E_NIC_PRIO,
-			     2);
+	if (mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_TUNNEL)
+		flags = MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT;
+	ft = ipsec_ft_create(ns, MLX5E_ACCEL_FS_ESP_FT_LEVEL, MLX5E_NIC_PRIO, 2,
+			     flags);
 	if (IS_ERR(ft)) {
 		err = PTR_ERR(ft);
 		goto err_fs_ft;
@@ -327,7 +331,7 @@ static int rx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 	}
 
 	ft = ipsec_ft_create(ns, MLX5E_ACCEL_FS_POL_FT_LEVEL, MLX5E_NIC_PRIO,
-			     2);
+			     2, 0);
 	if (IS_ERR(ft)) {
 		err = PTR_ERR(ft);
 		goto err_pol_ft;
@@ -511,9 +515,10 @@ static int tx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec_tx *tx,
 {
 	struct mlx5_flow_destination dest = {};
 	struct mlx5_flow_table *ft;
+	u32 flags = 0;
 	int err;
 
-	ft = ipsec_ft_create(tx->ns, 2, 0, 1);
+	ft = ipsec_ft_create(tx->ns, 2, 0, 1, 0);
 	if (IS_ERR(ft))
 		return PTR_ERR(ft);
 	tx->ft.status = ft;
@@ -522,7 +527,9 @@ static int tx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec_tx *tx,
 	if (err)
 		goto err_status_rule;
 
-	ft = ipsec_ft_create(tx->ns, 1, 0, 4);
+	if (mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_TUNNEL)
+		flags = MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT;
+	ft = ipsec_ft_create(tx->ns, 1, 0, 4, flags);
 	if (IS_ERR(ft)) {
 		err = PTR_ERR(ft);
 		goto err_sa_ft;
@@ -541,7 +548,7 @@ static int tx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec_tx *tx,
 		goto connect_roce;
 	}
 
-	ft = ipsec_ft_create(tx->ns, 0, 0, 2);
+	ft = ipsec_ft_create(tx->ns, 0, 0, 2, 0);
 	if (IS_ERR(ft)) {
 		err = PTR_ERR(ft);
 		goto err_pol_ft;
Signature · 1 line
2.39.2
#7 lore raw

PATCH 4/10 net/mlx5e: Prepare IPsec packet reformat code for tunnel mode

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Refactor setup_pkt_reformat() function to accommodate future extension to support tunnel mode.

---
 .../mellanox/mlx5/core/en_accel/ipsec.c       |  1 +
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  2 +-
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 78 ++++++++++++++-----
 3 files changed, 60 insertions(+), 21 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1 -0
index def01bfde610..359da277c03a 100644
@@ -297,6 +297,7 @@ void mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,
 	attrs->upspec.sport = ntohs(x->sel.sport);
 	attrs->upspec.sport_mask = ntohs(x->sel.sport_mask);
 	attrs->upspec.proto = x->sel.proto;
+	attrs->mode = x->props.mode;
 
 	mlx5e_ipsec_init_limits(sa_entry, attrs);
 }
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +1 -1
index bb89e18b17b4..ae525420a492 100644
@@ -77,7 +77,7 @@ struct mlx5_replay_esn {
 
 struct mlx5_accel_esp_xfrm_attrs {
 	u32   spi;
-	u32   flags;
+	u32   mode;
 	struct aes_gcm_keymat aes_gcm;
 
 	union {
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +58 -20

98 lines not on this page · raw message

Signature · 1 line
2.39.2
#8 lore raw

Re: [PATCH net-next 04/10] net/mlx5e: Prepare IPsec packet reformat code for tunnel mode

Quoted 6 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Refactor setup_pkt_reformat() function to accommodate future extension to support tunnel mode.

...

Quoted 16 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +8 -0
index 060be020ca64..980583fb1e52 100644
@@ -836,40 +836,78 @@ static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir,
 	return 0;
 }
 
+static int
+setup_pkt_transport_reformat(struct mlx5_accel_esp_xfrm_attrs *attrs,
+			     struct mlx5_pkt_reformat_params *reformat_params)
+{
+	u8 *reformatbf;
+	__be32 spi;
+
+	switch (attrs->dir) {

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 856

Quoted 10 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +10 -0
+	case XFRM_DEV_OFFLOAD_IN:
+		reformat_params->type = MLX5_REFORMAT_TYPE_DEL_ESP_TRANSPORT;
+		break;
+	case XFRM_DEV_OFFLOAD_OUT:
+		if (attrs->family == AF_INET)
+			reformat_params->type =
+				MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV4;
+		else
+			reformat_params->type =
+				MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV6;

Maybe this is nicer? Maybe not.

		reformat_params->type = attrs->family == AF_INET ?
			MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV4 :
			MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV6;

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 858

Quoted 2 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +2 -0
+
+		reformatbf = kzalloc(16, GFP_KERNEL);

I know you are just moving code around.
But 16 is doing a lot of work in this function.
Could it be a #define ?

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 864

Quoted 6 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +6 -0
+		if (!reformatbf)
+			return -ENOMEM;
+
+		/* convert to network format */
+		spi = htonl(attrs->spi);
+		memcpy(reformatbf, &spi, 4);

This seems to be a lot of work to copy a word.
But anyway, maybe:

		memcpy(reformatbf, &spi, sizeof(spi));
Quoted 12 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +12 -0
+
+		reformat_params->param_0 = attrs->authsize;
+		reformat_params->size = 16;
+		reformat_params->data = reformatbf;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+

...

#9 lore raw

Re: [PATCH net-next 04/10] net/mlx5e: Prepare IPsec packet reformat code for tunnel mode

Quoted 39 lines · Simon Horman
Quoted 6 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Refactor setup_pkt_reformat() function to accommodate future extension to support tunnel mode.

...

Quoted 16 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +8 -0
index 060be020ca64..980583fb1e52 100644
@@ -836,40 +836,78 @@ static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir,
 	return 0;
 }
 
+static int
+setup_pkt_transport_reformat(struct mlx5_accel_esp_xfrm_attrs *attrs,
+			     struct mlx5_pkt_reformat_params *reformat_params)
+{
+	u8 *reformatbf;
+	__be32 spi;
+
+	switch (attrs->dir) {

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 856

Quoted 10 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +10 -0
+	case XFRM_DEV_OFFLOAD_IN:
+		reformat_params->type = MLX5_REFORMAT_TYPE_DEL_ESP_TRANSPORT;
+		break;
+	case XFRM_DEV_OFFLOAD_OUT:
+		if (attrs->family == AF_INET)
+			reformat_params->type =
+				MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV4;
+		else
+			reformat_params->type =
+				MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV6;

Maybe this is nicer? Maybe not.

Quoted 1 line
		reformat_params->type = attrs->family == AF_INET ?

I didn't like it because of the line above, IMHO it is too long and has too many indirection.

Quoted 9 lines
			MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV4 :
			MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV6;

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 858

Quoted 2 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +2 -0
+
+		reformatbf = kzalloc(16, GFP_KERNEL);

I know you are just moving code around.
But 16 is doing a lot of work in this function.
Could it be a #define ?

I'll change

Quoted 11 lines

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 864

Quoted 7 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +6 -0
+		if (!reformatbf)
+			return -ENOMEM;
+
+		/* convert to network format */
+		spi = htonl(attrs->spi);
+		memcpy(reformatbf, &spi, 4);

This seems to be a lot of work to copy a word.
But anyway, maybe:

Quoted 1 line
		memcpy(reformatbf, &spi, sizeof(spi));

Will do.

Quoted 15 lines
Quoted 13 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +12 -0
+
+		reformat_params->param_0 = attrs->authsize;
+		reformat_params->size = 16;
+		reformat_params->data = reformatbf;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+

...

#10 lore raw

Re: [PATCH net-next 04/10] net/mlx5e: Prepare IPsec packet reformat code for tunnel mode

Quoted 40 lines · Leon Romanovsky
Quoted 39 lines · Simon Horman
Quoted 6 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Refactor setup_pkt_reformat() function to accommodate future extension to support tunnel mode.

...

Quoted 16 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +8 -0
index 060be020ca64..980583fb1e52 100644
@@ -836,40 +836,78 @@ static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir,
 	return 0;
 }
 
+static int
+setup_pkt_transport_reformat(struct mlx5_accel_esp_xfrm_attrs *attrs,
+			     struct mlx5_pkt_reformat_params *reformat_params)
+{
+	u8 *reformatbf;
+	__be32 spi;
+
+	switch (attrs->dir) {

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 856

Quoted 10 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +10 -0
+	case XFRM_DEV_OFFLOAD_IN:
+		reformat_params->type = MLX5_REFORMAT_TYPE_DEL_ESP_TRANSPORT;
+		break;
+	case XFRM_DEV_OFFLOAD_OUT:
+		if (attrs->family == AF_INET)
+			reformat_params->type =
+				MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV4;
+		else
+			reformat_params->type =
+				MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV6;

Maybe this is nicer? Maybe not.

Quoted 4 lines
Quoted 1 line
		reformat_params->type = attrs->family == AF_INET ?

I didn't like it because of the line above, IMHO it is too long and has too many indirection.

Yeah, it's not ideal.

Quoted 2 lines
Quoted 2 lines
			MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV4 :
			MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV6;

...

#11 lore raw

Re: [PATCH net-next 04/10] net/mlx5e: Prepare IPsec packet reformat code for tunnel mode

In reply to Simon Horman
Quoted 43 lines · Simon Horman
Quoted 6 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Refactor setup_pkt_reformat() function to accommodate future extension to support tunnel mode.

...

Quoted 16 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +8 -0
index 060be020ca64..980583fb1e52 100644
@@ -836,40 +836,78 @@ static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir,
 	return 0;
 }
 
+static int
+setup_pkt_transport_reformat(struct mlx5_accel_esp_xfrm_attrs *attrs,
+			     struct mlx5_pkt_reformat_params *reformat_params)
+{
+	u8 *reformatbf;
+	__be32 spi;
+
+	switch (attrs->dir) {

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 856

Quoted 10 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +10 -0
+	case XFRM_DEV_OFFLOAD_IN:
+		reformat_params->type = MLX5_REFORMAT_TYPE_DEL_ESP_TRANSPORT;
+		break;
+	case XFRM_DEV_OFFLOAD_OUT:
+		if (attrs->family == AF_INET)
+			reformat_params->type =
+				MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV4;
+		else
+			reformat_params->type =
+				MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV6;

Maybe this is nicer? Maybe not.

		reformat_params->type = attrs->family == AF_INET ?
			MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV4 :
			MLX5_REFORMAT_TYPE_ADD_ESP_TRANSPORT_OVER_IPV6;
Quoted 6 lines

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 858

Quoted 2 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +2 -0
+
+		reformatbf = kzalloc(16, GFP_KERNEL);

I know you are just moving code around.
But 16 is doing a lot of work in this function.
Could it be a #define ?

Done

Quoted 11 lines

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 864

Quoted 7 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +6 -0
+		if (!reformatbf)
+			return -ENOMEM;
+
+		/* convert to network format */
+		spi = htonl(attrs->spi);
+		memcpy(reformatbf, &spi, 4);

This seems to be a lot of work to copy a word.
But anyway, maybe:

Quoted 1 line
		memcpy(reformatbf, &spi, sizeof(spi));

Done

Quoted 15 lines
Quoted 13 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +12 -0
+
+		reformat_params->param_0 = attrs->authsize;
+		reformat_params->size = 16;
+		reformat_params->data = reformatbf;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+

...

#12 lore raw

PATCH 5/10 net/mlx5e: Support IPsec RX packet offload in tunnel mode

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Extend mlx5 driver with logic to support IPsec RX packet offload in tunnel mode.

---
 .../mellanox/mlx5/core/en_accel/ipsec.c       | 36 ++++++++++++
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  2 +
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 57 +++++++++++++++++++
 3 files changed, 95 insertions(+)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +36 -0
index 359da277c03a..7c55b37c1c01 100644
@@ -242,6 +242,41 @@ static void mlx5e_ipsec_init_limits(struct mlx5e_ipsec_sa_entry *sa_entry,
 	attrs->lft.numb_rounds_soft = (u64)n;
 }
 
+static void mlx5e_ipsec_init_macs(struct mlx5e_ipsec_sa_entry *sa_entry,
+				  struct mlx5_accel_esp_xfrm_attrs *attrs)
+{
+	struct mlx5_core_dev *mdev = mlx5e_ipsec_sa2dev(sa_entry);
+	struct xfrm_state *x = sa_entry->x;
+	struct net_device *netdev;
+	struct neighbour *n;
+	u8 addr[ETH_ALEN];
+
+	if (attrs->mode != XFRM_MODE_TUNNEL &&
+	    attrs->type != XFRM_DEV_OFFLOAD_PACKET)
+		return;
+
+	netdev = x->xso.real_dev;
+
+	mlx5_query_mac_address(mdev, addr);
+	switch (attrs->dir) {
+	case XFRM_DEV_OFFLOAD_IN:
+		ether_addr_copy(attrs->dmac, addr);
+		n = neigh_lookup(&arp_tbl, &attrs->saddr.a4, netdev);
+		if (!n) {
+			n = neigh_create(&arp_tbl, &attrs->saddr.a4, netdev);
+			if (IS_ERR(n))
+				return;
+			neigh_event_send(n, NULL);
+		}
+		neigh_ha_snapshot(addr, n, netdev);
+		ether_addr_copy(attrs->smac, addr);
+		break;
+	default:
+		return;
+	}
+	neigh_release(n);
+}
+
 void mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,
 					struct mlx5_accel_esp_xfrm_attrs *attrs)
 {
@@ -300,6 +335,7 @@ void mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,
 	attrs->mode = x->props.mode;
 
 	mlx5e_ipsec_init_limits(sa_entry, attrs);
+	mlx5e_ipsec_init_macs(sa_entry, attrs);
 }
 
 static int mlx5e_xfrm_validate_state(struct mlx5_core_dev *mdev,
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +2 -0
index ae525420a492..77384ffa4451 100644
@@ -99,6 +99,8 @@ struct mlx5_accel_esp_xfrm_attrs {
 	u32 authsize;
 	u32 reqid;
 	struct mlx5_ipsec_lft lft;
+	u8 smac[ETH_ALEN];
+	u8 dmac[ETH_ALEN];
 };
 
 enum mlx5_ipsec_cap {
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +57 -0

69 lines not on this page · raw message

Signature · 1 line
2.39.2
#13 lore raw

Re: [PATCH net-next 05/10] net/mlx5e: Support IPsec RX packet offload in tunnel mode

Quoted 6 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Extend mlx5 driver with logic to support IPsec RX packet offload in tunnel mode.

...

Quoted 11 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +3 -0
index 980583fb1e52..8ecaf4100b9c 100644
@@ -836,6 +836,60 @@ static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir,
 	return 0;
 }
 
+static int
+setup_pkt_tunnel_reformat(struct mlx5_core_dev *mdev,
+			  struct mlx5_accel_esp_xfrm_attrs *attrs,

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 851

Quoted 10 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +10 -0
+			  struct mlx5_pkt_reformat_params *reformat_params)
+{
+	union {
+		struct {
+			u8 dmac[6];
+			u8 smac[6];
+			__be16 ethertype;
+		} __packed;
+		u8 raw[ETH_HLEN];
+	} __packed *mac_hdr;

Can struct ethhrd be used here?
I think it has the same layout as the fields of the inner structure above. And I don't think the union is giving us much: the raw field seems unused.

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 857

Quoted 6 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +6 -0
+	char *reformatbf;
+	size_t bfflen;
+
+	bfflen = sizeof(*mac_hdr);
+
+	reformatbf = kzalloc(bfflen, GFP_KERNEL);

I'm not sure that reformatbf is providing much value. Perhaps:

	mac_hdr = kzalloc(bfflen, GFP_KERNEL);

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 861

Quoted 4 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +4 -0
+	if (!reformatbf)
+		return -ENOMEM;
+
+	mac_hdr = (void *)reformatbf;

If you must cast, perhaps to the type of mac_hdr, which is not void *.

Quoted 30 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +30 -0
+	switch (attrs->family) {
+	case AF_INET:
+		mac_hdr->ethertype = htons(ETH_P_IP);
+		break;
+	case AF_INET6:
+		mac_hdr->ethertype = htons(ETH_P_IPV6);
+		break;
+	default:
+		goto free_reformatbf;
+	}
+
+	ether_addr_copy(mac_hdr->dmac, attrs->dmac);
+	ether_addr_copy(mac_hdr->smac, attrs->smac);
+
+	switch (attrs->dir) {
+	case XFRM_DEV_OFFLOAD_IN:
+		reformat_params->type = MLX5_REFORMAT_TYPE_L3_ESP_TUNNEL_TO_L2;
+		break;
+	default:
+		goto free_reformatbf;
+	}
+
+	reformat_params->size = bfflen;
+	reformat_params->data = reformatbf;
+	return 0;
+
+free_reformatbf:
+	kfree(reformatbf);
+	return -EINVAL;
+}

...

#14 lore raw

Re: [PATCH net-next 05/10] net/mlx5e: Support IPsec RX packet offload in tunnel mode

Quoted 18 lines · Simon Horman
Quoted 6 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Extend mlx5 driver with logic to support IPsec RX packet offload in tunnel mode.

...

Quoted 8 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +0 -0
index 980583fb1e52..8ecaf4100b9c 100644
@@ -836,6 +836,60 @@ static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir,
 	return 0;
 }
 
Quoted 16 lines

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 851

Quoted 13 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +13 -0
+static int
+setup_pkt_tunnel_reformat(struct mlx5_core_dev *mdev,
+			  struct mlx5_accel_esp_xfrm_attrs *attrs,
+			  struct mlx5_pkt_reformat_params *reformat_params)
+{
+	union {
+		struct {
+			u8 dmac[6];
+			u8 smac[6];
+			__be16 ethertype;
+		} __packed;
+		u8 raw[ETH_HLEN];
+	} __packed *mac_hdr;

Can struct ethhrd be used here?
I think it has the same layout as the fields of the inner structure above.

Yes, it can, will change.

Quoted 1 line

And I don't think the union is giving us much: the raw field seems unused.

Thanks, it isn't needed here.

Quoted 9 lines

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 857

Quoted 7 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +6 -0
+	char *reformatbf;
+	size_t bfflen;
+
+	bfflen = sizeof(*mac_hdr);
+
+	reformatbf = kzalloc(bfflen, GFP_KERNEL);

I'm not sure that reformatbf is providing much value.

It is more useful in next patch, where reformatbf will hold ESP, ipv4/6 and ETH headers.

Quoted 10 lines

Perhaps:

	mac_hdr = kzalloc(bfflen, GFP_KERNEL);

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c static int setup_modify_header(struct mlx5_core_dev *mdev, u32 val, u8 dir, · line 861

Quoted 4 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +4 -0
+	if (!reformatbf)
+		return -ENOMEM;
+
+	mac_hdr = (void *)reformatbf;

If you must cast, perhaps to the type of mac_hdr, which is not void *.

I'll change.

Quoted 33 lines
Quoted 31 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +30 -0
+	switch (attrs->family) {
+	case AF_INET:
+		mac_hdr->ethertype = htons(ETH_P_IP);
+		break;
+	case AF_INET6:
+		mac_hdr->ethertype = htons(ETH_P_IPV6);
+		break;
+	default:
+		goto free_reformatbf;
+	}
+
+	ether_addr_copy(mac_hdr->dmac, attrs->dmac);
+	ether_addr_copy(mac_hdr->smac, attrs->smac);
+
+	switch (attrs->dir) {
+	case XFRM_DEV_OFFLOAD_IN:
+		reformat_params->type = MLX5_REFORMAT_TYPE_L3_ESP_TUNNEL_TO_L2;
+		break;
+	default:
+		goto free_reformatbf;
+	}
+
+	reformat_params->size = bfflen;
+	reformat_params->data = reformatbf;
+	return 0;
+
+free_reformatbf:
+	kfree(reformatbf);
+	return -EINVAL;
+}

...

#15 lore raw

PATCH 6/10 net/mlx5e: Support IPsec TX packet offload in tunnel mode

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Extend mlx5 driver with logic to support IPsec TX packet offload in tunnel mode.

---
 .../mellanox/mlx5/core/en_accel/ipsec.c       | 12 +++++
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 52 +++++++++++++++++++
 2 files changed, 64 insertions(+)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +12 -0
index 7c55b37c1c01..36f3ffd54355 100644
@@ -271,6 +271,18 @@ static void mlx5e_ipsec_init_macs(struct mlx5e_ipsec_sa_entry *sa_entry,
 		neigh_ha_snapshot(addr, n, netdev);
 		ether_addr_copy(attrs->smac, addr);
 		break;
+	case XFRM_DEV_OFFLOAD_OUT:
+		ether_addr_copy(attrs->smac, addr);
+		n = neigh_lookup(&arp_tbl, &attrs->daddr.a4, netdev);
+		if (!n) {
+			n = neigh_create(&arp_tbl, &attrs->daddr.a4, netdev);
+			if (IS_ERR(n))
+				return;
+			neigh_event_send(n, NULL);
+		}
+		neigh_ha_snapshot(addr, n, netdev);
+		ether_addr_copy(attrs->dmac, addr);
+		break;
 	default:
 		return;
 	}
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +52 -0

75 lines not on this page · raw message

Signature · 1 line
2.39.2
#16 lore raw

PATCH 7/10 net/mlx5e: Listen to ARP events to update IPsec L2 headers in tunnel mode

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

In IPsec packet offload mode all header manipulations are performed by hardware, which is responsible to add/remove L2 header with source and destinations MACs.

CX-7 devices don't support offload of in-kernel routing functionality, as such HW needs external help to fill other side MAC as it isn't available for HW.

As a solution, let's listen to neigh ARP updates and reconfigure IPsec rules on the fly once new MAC data information arrives.

---
 .../mellanox/mlx5/core/en_accel/ipsec.c       | 132 +++++++++++++++++-
 .../mellanox/mlx5/core/en_accel/ipsec.h       |   5 +
 2 files changed, 130 insertions(+), 7 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +125 -7
index 36f3ffd54355..b64281fd4142 100644
@@ -35,12 +35,14 @@
 #include <crypto/aead.h>
 #include <linux/inetdevice.h>
 #include <linux/netdevice.h>
+#include <net/netevent.h>
 
 #include "en.h"
 #include "ipsec.h"
 #include "ipsec_rxtx.h"
 
 #define MLX5_IPSEC_RESCHED msecs_to_jiffies(1000)
+#define MLX5E_IPSEC_TUNNEL_SA XA_MARK_1
 
 static struct mlx5e_ipsec_sa_entry *to_ipsec_sa_entry(struct xfrm_state *x)
 {
@@ -251,7 +253,7 @@ static void mlx5e_ipsec_init_macs(struct mlx5e_ipsec_sa_entry *sa_entry,
 	struct neighbour *n;
 	u8 addr[ETH_ALEN];
 
-	if (attrs->mode != XFRM_MODE_TUNNEL &&
+	if (attrs->mode != XFRM_MODE_TUNNEL ||
 	    attrs->type != XFRM_DEV_OFFLOAD_PACKET)
 		return;
 
@@ -267,6 +269,8 @@ static void mlx5e_ipsec_init_macs(struct mlx5e_ipsec_sa_entry *sa_entry,
 			if (IS_ERR(n))
 				return;
 			neigh_event_send(n, NULL);
+			attrs->drop = true;
+			break;
 		}
 		neigh_ha_snapshot(addr, n, netdev);
 		ether_addr_copy(attrs->smac, addr);
@@ -279,6 +283,8 @@ static void mlx5e_ipsec_init_macs(struct mlx5e_ipsec_sa_entry *sa_entry,
 			if (IS_ERR(n))
 				return;
 			neigh_event_send(n, NULL);
+			attrs->drop = true;
+			break;
 		}
 		neigh_ha_snapshot(addr, n, netdev);
 		ether_addr_copy(attrs->dmac, addr);
@@ -507,34 +513,81 @@ static void mlx5e_ipsec_set_esn_ops(struct mlx5e_ipsec_sa_entry *sa_entry)
 	sa_entry->set_iv_op = mlx5e_ipsec_set_iv;
 }
 
+static void mlx5e_ipsec_handle_netdev_event(struct work_struct *_work)
+{
+	struct mlx5e_ipsec_work *work =
+		container_of(_work, struct mlx5e_ipsec_work, work);
+	struct mlx5e_ipsec_sa_entry *sa_entry = work->sa_entry;
+	struct mlx5e_ipsec_netevent_data *data = work->data;
+	struct mlx5_accel_esp_xfrm_attrs *attrs;
+
+	attrs = &sa_entry->attrs;
+
+	switch (attrs->dir) {
+	case XFRM_DEV_OFFLOAD_IN:
+		ether_addr_copy(attrs->smac, data->addr);
+		break;
+	case XFRM_DEV_OFFLOAD_OUT:
+		ether_addr_copy(attrs->dmac, data->addr);
+		break;
+	default:
+		WARN_ON_ONCE(true);
+	}
+	attrs->drop = false;
+	mlx5e_accel_ipsec_fs_modify(sa_entry);
+}
+
 static int mlx5_ipsec_create_work(struct mlx5e_ipsec_sa_entry *sa_entry)
 {
 	struct xfrm_state *x = sa_entry->x;
 	struct mlx5e_ipsec_work *work;
+	void *data = NULL;
 
 	switch (x->xso.type) {
 	case XFRM_DEV_OFFLOAD_CRYPTO:
 		if (!(x->props.flags & XFRM_STATE_ESN))
 			return 0;
 		break;
+	case XFRM_DEV_OFFLOAD_PACKET:
+		if (x->props.mode != XFRM_MODE_TUNNEL)
+			return 0;
+		break;
 	default:
-		return 0;
+		break;
 	}
 
 	work = kzalloc(sizeof(*work), GFP_KERNEL);
 	if (!work)
 		return -ENOMEM;
 
-	work->data = kzalloc(sizeof(*sa_entry), GFP_KERNEL);
-	if (!work->data) {
-		kfree(work);
-		return -ENOMEM;
+	switch (x->xso.type) {
+	case XFRM_DEV_OFFLOAD_CRYPTO:
+		data = kzalloc(sizeof(*sa_entry), GFP_KERNEL);
+		if (!data)
+			goto free_work;
+
+		INIT_WORK(&work->work, mlx5e_ipsec_modify_state);
+		break;
+	case XFRM_DEV_OFFLOAD_PACKET:
+		data = kzalloc(sizeof(struct mlx5e_ipsec_netevent_data),
+			       GFP_KERNEL);
+		if (!data)
+			goto free_work;
+
+		INIT_WORK(&work->work, mlx5e_ipsec_handle_netdev_event);
+		break;
+	default:
+		break;
 	}
 
-	INIT_WORK(&work->work, mlx5e_ipsec_modify_state);
+	work->data = data;
 	work->sa_entry = sa_entry;
 	sa_entry->work = work;
 	return 0;
+
+free_work:
+	kfree(work);
+	return -ENOMEM;
 }
 
 static int mlx5e_ipsec_create_dwork(struct mlx5e_ipsec_sa_entry *sa_entry)
@@ -629,6 +682,12 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x,
 	if (sa_entry->dwork)
 		queue_delayed_work(ipsec->wq, &sa_entry->dwork->dwork,
 				   MLX5_IPSEC_RESCHED);
+
+	if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
+	    x->props.mode == XFRM_MODE_TUNNEL)
+		xa_set_mark(&ipsec->sadb, sa_entry->ipsec_obj_id,
+			    MLX5E_IPSEC_TUNNEL_SA);
+
 out:
 	x->xso.offload_handle = (unsigned long)sa_entry;
 	return 0;
@@ -651,6 +710,7 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x,
 static void mlx5e_xfrm_del_state(struct xfrm_state *x)
 {
 	struct mlx5e_ipsec_sa_entry *sa_entry = to_ipsec_sa_entry(x);
+	struct mlx5_accel_esp_xfrm_attrs *attrs = &sa_entry->attrs;
 	struct mlx5e_ipsec *ipsec = sa_entry->ipsec;
 	struct mlx5e_ipsec_sa_entry *old;
 
@@ -659,6 +719,12 @@ static void mlx5e_xfrm_del_state(struct xfrm_state *x)
 
 	old = xa_erase_bh(&ipsec->sadb, sa_entry->ipsec_obj_id);
 	WARN_ON(old != sa_entry);
+
+	if (attrs->mode == XFRM_MODE_TUNNEL &&
+	    attrs->type == XFRM_DEV_OFFLOAD_PACKET)
+		/* Make sure that no ARP requests are running in parallel */
+		flush_workqueue(ipsec->wq);
+
 }
 
 static void mlx5e_xfrm_free_state(struct xfrm_state *x)
@@ -683,6 +749,46 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)
 	kfree(sa_entry);
 }
 
+static int mlx5e_ipsec_netevent_event(struct notifier_block *nb,
+				      unsigned long event, void *ptr)
+{
+	struct mlx5_accel_esp_xfrm_attrs *attrs;
+	struct mlx5e_ipsec_netevent_data *data;
+	struct mlx5e_ipsec_sa_entry *sa_entry;
+	struct mlx5e_ipsec *ipsec;
+	struct neighbour *n = ptr;
+	struct net_device *netdev;
+	struct xfrm_state *x;
+	unsigned long idx;
+
+	if (event != NETEVENT_NEIGH_UPDATE || !(n->nud_state & NUD_VALID))
+		return NOTIFY_DONE;
+
+	ipsec = container_of(nb, struct mlx5e_ipsec, netevent_nb);
+	xa_for_each_marked(&ipsec->sadb, idx, sa_entry, MLX5E_IPSEC_TUNNEL_SA) {
+		attrs = &sa_entry->attrs;
+
+		if (attrs->family == AF_INET) {
+			if (!neigh_key_eq32(n, &attrs->saddr.a4) &&
+			    !neigh_key_eq32(n, &attrs->daddr.a4))
+				continue;
+		} else {
+			if (!neigh_key_eq128(n, &attrs->saddr.a4) &&
+			    !neigh_key_eq128(n, &attrs->daddr.a4))
+				continue;
+		}
+
+		x = sa_entry->x;
+		netdev = x->xso.real_dev;
+		data = sa_entry->work->data;
+
+		neigh_ha_snapshot(data->addr, n, netdev);
+		queue_work(ipsec->wq, &sa_entry->work->work);
+	}
+
+	return NOTIFY_DONE;
+}
+
 void mlx5e_ipsec_init(struct mlx5e_priv *priv)
 {
 	struct mlx5e_ipsec *ipsec;
@@ -711,6 +817,13 @@ void mlx5e_ipsec_init(struct mlx5e_priv *priv)
 			goto err_aso;
 	}
 
+	if (mlx5_ipsec_device_caps(priv->mdev) & MLX5_IPSEC_CAP_TUNNEL) {
+		ipsec->netevent_nb.notifier_call = mlx5e_ipsec_netevent_event;
+		ret = register_netevent_notifier(&ipsec->netevent_nb);
+		if (ret)
+			goto clear_aso;
+	}
+
 	ret = mlx5e_accel_ipsec_fs_init(ipsec);
 	if (ret)
 		goto err_fs_init;
@@ -721,6 +834,9 @@ void mlx5e_ipsec_init(struct mlx5e_priv *priv)
 	return;
 
 err_fs_init:
+	if (mlx5_ipsec_device_caps(priv->mdev) & MLX5_IPSEC_CAP_TUNNEL)
+		unregister_netevent_notifier(&ipsec->netevent_nb);
+clear_aso:
 	if (mlx5_ipsec_device_caps(priv->mdev) & MLX5_IPSEC_CAP_PACKET_OFFLOAD)
 		mlx5e_ipsec_aso_cleanup(ipsec);
 err_aso:
@@ -739,6 +855,8 @@ void mlx5e_ipsec_cleanup(struct mlx5e_priv *priv)
 		return;
 
 	mlx5e_accel_ipsec_fs_cleanup(ipsec);
+	if (mlx5_ipsec_device_caps(priv->mdev) & MLX5_IPSEC_CAP_TUNNEL)
+		unregister_netevent_notifier(&ipsec->netevent_nb);
 	if (mlx5_ipsec_device_caps(priv->mdev) & MLX5_IPSEC_CAP_PACKET_OFFLOAD)
 		mlx5e_ipsec_aso_cleanup(ipsec);
 	destroy_workqueue(ipsec->wq);
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +5 -0
index 77384ffa4451..d06c896eadb6 100644
@@ -144,6 +144,10 @@ struct mlx5e_ipsec_work {
 	void *data;
 };
 
+struct mlx5e_ipsec_netevent_data {
+	u8 addr[ETH_ALEN];
+};
+
 struct mlx5e_ipsec_dwork {
 	struct delayed_work dwork;
 	struct mlx5e_ipsec_sa_entry *sa_entry;
@@ -169,6 +173,7 @@ struct mlx5e_ipsec {
 	struct mlx5e_ipsec_tx *tx;
 	struct mlx5e_ipsec_aso *aso;
 	struct notifier_block nb;
+	struct notifier_block netevent_nb;
 	struct mlx5_ipsec_fs *roce;
 };
 
Signature · 1 line
2.39.2
#17 lore raw

PATCH 8/10 net/mlx5: Allow blocking encap changes in eswitch

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Existing eswitch encap option enables header encapsulation. Unfortunately currently available hardware isn't able to perform double encapsulation, which can happen once IPsec packet offload tunnel mode is used together with encap mode set to BASIC.

So as a solution for misconfiguration, provide an option to block encap changes, which will be used for IPsec packet offload.

---
 .../net/ethernet/mellanox/mlx5/core/eswitch.h | 14 ++++++
 .../mellanox/mlx5/core/eswitch_offloads.c     | 48 +++++++++++++++++++
 2 files changed, 62 insertions(+)
drivers/net/ethernet/mellanox/mlx5/core/eswitch.h +14 -0
index 19e9a77c4633..e9d68fdf68f5 100644
@@ -263,6 +263,7 @@ struct mlx5_esw_offload {
 	const struct mlx5_eswitch_rep_ops *rep_ops[NUM_REP_TYPES];
 	u8 inline_mode;
 	atomic64_t num_flows;
+	u64 num_block_encap;
 	enum devlink_eswitch_encap_mode encap;
 	struct ida vport_metadata_ida;
 	unsigned int host_number; /* ECPF supports one external host */
@@ -748,6 +749,9 @@ void mlx5_eswitch_offloads_destroy_single_fdb(struct mlx5_eswitch *master_esw,
 					      struct mlx5_eswitch *slave_esw);
 int mlx5_eswitch_reload_reps(struct mlx5_eswitch *esw);
 
+bool mlx5_eswitch_block_encap(struct mlx5_core_dev *dev);
+void mlx5_eswitch_unblock_encap(struct mlx5_core_dev *dev);
+
 static inline int mlx5_eswitch_num_vfs(struct mlx5_eswitch *esw)
 {
 	if (mlx5_esw_allowed(esw))
@@ -761,6 +765,7 @@ mlx5_eswitch_get_slow_fdb(struct mlx5_eswitch *esw)
 {
 	return esw->fdb_table.offloads.slow_fdb;
 }
+
 #else  /* CONFIG_MLX5_ESWITCH */
 /* eswitch API stubs */
 static inline int  mlx5_eswitch_init(struct mlx5_core_dev *dev) { return 0; }
@@ -805,6 +810,15 @@ mlx5_eswitch_reload_reps(struct mlx5_eswitch *esw)
 {
 	return 0;
 }
+
+static inline bool mlx5_eswitch_block_encap(struct mlx5_core_dev *dev)
+{
+	return true;
+}
+
+static inline void mlx5_eswitch_unblock_encap(struct mlx5_core_dev *dev)
+{
+}
 #endif /* CONFIG_MLX5_ESWITCH */
 
 #endif /* __MLX5_ESWITCH_H__ */
drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c +48 -0

60 lines not on this page · raw message

Signature · 1 line
2.39.2
#18 lore raw

PATCH 9/10 net/mlx5e: Create IPsec table with tunnel support only when encap is disabled

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Current hardware doesn't support double encapsulation which is happening when IPsec packet offload tunnel mode is configured together with eswitch encap option.

Any user attempt to add new SA/policy after he/she sets encap mode, will generate the following FW syndrome:

mlx5_core 0000:08:00.0: mlx5_cmd_out_err:803:(pid 1904): CREATE_FLOW_TABLE(0x930) op_mod(0x0) failed, status bad parameter(0x3), syndrome (0xa43321), err(-22)

Make sure that we block encap changes before creating flow steering tables. This is applicable only for packet offload in tunnel mode, while packet offload in transport mode and crypto offload, don't have such limitation as they don't perform encapsulation.

---
 .../mellanox/mlx5/core/en_accel/ipsec.c       |  7 ++++
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  1 +
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 33 +++++++++++++++++--
 3 files changed, 38 insertions(+), 3 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +7 -0
index b64281fd4142..e95004ac7a20 100644
@@ -668,6 +668,13 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x,
 	if (err)
 		goto err_hw_ctx;
 
+	if (x->props.mode == XFRM_MODE_TUNNEL &&
+	    x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
+	    !mlx5e_ipsec_fs_tunnel_enabled(sa_entry)) {
+		NL_SET_ERR_MSG_MOD(extack, "Packet offload tunnel mode is disabled due to encap settings");
+		goto err_add_rule;
+	}
+
 	/* We use *_bh() variant because xfrm_timer_handler(), which runs
 	 * in softirq context, can reach our state delete logic and we need
 	 * xa_erase_bh() there.
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +1 -0
index d06c896eadb6..f7f7c09d2b32 100644
@@ -251,6 +251,7 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_ipsec_sa_entry *sa_entry);
 int mlx5e_accel_ipsec_fs_add_pol(struct mlx5e_ipsec_pol_entry *pol_entry);
 void mlx5e_accel_ipsec_fs_del_pol(struct mlx5e_ipsec_pol_entry *pol_entry);
 void mlx5e_accel_ipsec_fs_modify(struct mlx5e_ipsec_sa_entry *sa_entry);
+bool mlx5e_ipsec_fs_tunnel_enabled(struct mlx5e_ipsec_sa_entry *sa_entry);
 
 int mlx5_ipsec_create_sa_ctx(struct mlx5e_ipsec_sa_entry *sa_entry);
 void mlx5_ipsec_free_sa_ctx(struct mlx5e_ipsec_sa_entry *sa_entry);
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +30 -3
index b1f759c378d9..6a8f1c0f4912 100644
@@ -4,6 +4,7 @@
 #include <linux/netdevice.h>
 #include "en.h"
 #include "en/fs.h"
+#include "eswitch.h"
 #include "ipsec.h"
 #include "fs_core.h"
 #include "lib/ipsec_fs_roce.h"
@@ -37,6 +38,7 @@ struct mlx5e_ipsec_rx {
 	struct mlx5e_ipsec_rule status;
 	struct mlx5e_ipsec_fc *fc;
 	struct mlx5_fs_chains *chains;
+	u8 allow_tunnel_mode : 1;
 };
 
 struct mlx5e_ipsec_tx {
@@ -46,6 +48,7 @@ struct mlx5e_ipsec_tx {
 	struct mlx5_flow_namespace *ns;
 	struct mlx5e_ipsec_fc *fc;
 	struct mlx5_fs_chains *chains;
+	u8 allow_tunnel_mode : 1;
 };
 
 /* IPsec RX flow steering */
@@ -253,7 +256,8 @@ static void rx_destroy(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 	mlx5_del_flow_rules(rx->sa.rule);
 	mlx5_destroy_flow_group(rx->sa.group);
 	mlx5_destroy_flow_table(rx->ft.sa);
-
+	if (rx->allow_tunnel_mode)
+		mlx5_eswitch_unblock_encap(mdev);
 	mlx5_del_flow_rules(rx->status.rule);
 	mlx5_modify_header_dealloc(mdev, rx->status.modify_hdr);
 	mlx5_destroy_flow_table(rx->ft.status);
@@ -304,6 +308,8 @@ static int rx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 
 	/* Create FT */
 	if (mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_TUNNEL)
+		rx->allow_tunnel_mode = mlx5_eswitch_block_encap(mdev);
+	if (rx->allow_tunnel_mode)
 		flags = MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT;
 	ft = ipsec_ft_create(ns, MLX5E_ACCEL_FS_ESP_FT_LEVEL, MLX5E_NIC_PRIO, 2,
 			     flags);
@@ -361,6 +367,8 @@ static int rx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 err_fs:
 	mlx5_destroy_flow_table(rx->ft.sa);
 err_fs_ft:
+	if (rx->allow_tunnel_mode)
+		mlx5_eswitch_unblock_encap(mdev);
 	mlx5_del_flow_rules(rx->status.rule);
 	mlx5_modify_header_dealloc(mdev, rx->status.modify_hdr);
 err_add:
@@ -495,7 +503,8 @@ static int ipsec_counter_rule_tx(struct mlx5_core_dev *mdev, struct mlx5e_ipsec_
 }
 
 /* IPsec TX flow steering */
-static void tx_destroy(struct mlx5e_ipsec_tx *tx, struct mlx5_ipsec_fs *roce)
+static void tx_destroy(struct mlx5_core_dev *mdev, struct mlx5e_ipsec_tx *tx,
+		       struct mlx5_ipsec_fs *roce)
 {
 	mlx5_ipsec_fs_roce_tx_destroy(roce);
 	if (tx->chains) {
@@ -507,6 +516,8 @@ static void tx_destroy(struct mlx5e_ipsec_tx *tx, struct mlx5_ipsec_fs *roce)
 	}
 
 	mlx5_destroy_flow_table(tx->ft.sa);
+	if (tx->allow_tunnel_mode)
+		mlx5_eswitch_unblock_encap(mdev);
 	mlx5_del_flow_rules(tx->status.rule);
 	mlx5_destroy_flow_table(tx->ft.status);
 }
@@ -529,6 +540,8 @@ static int tx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec_tx *tx,
 		goto err_status_rule;
 
 	if (mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_TUNNEL)
+		tx->allow_tunnel_mode = mlx5_eswitch_block_encap(mdev);
+	if (tx->allow_tunnel_mode)
 		flags = MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT;
 	ft = ipsec_ft_create(tx->ns, 1, 0, 4, flags);
 	if (IS_ERR(ft)) {
@@ -580,6 +593,8 @@ static int tx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec_tx *tx,
 err_pol_ft:
 	mlx5_destroy_flow_table(tx->ft.sa);
 err_sa_ft:
+	if (tx->allow_tunnel_mode)
+		mlx5_eswitch_unblock_encap(mdev);
 	mlx5_del_flow_rules(tx->status.rule);
 err_status_rule:
 	mlx5_destroy_flow_table(tx->ft.status);
@@ -608,7 +623,7 @@ static void tx_put(struct mlx5e_ipsec *ipsec, struct mlx5e_ipsec_tx *tx)
 	if (--tx->ft.refcnt)
 		return;
 
-	tx_destroy(tx, ipsec->roce);
+	tx_destroy(ipsec->mdev, tx, ipsec->roce);
 }
 
 static struct mlx5_flow_table *tx_ft_get_policy(struct mlx5_core_dev *mdev,
@@ -1607,3 +1622,15 @@ void mlx5e_accel_ipsec_fs_modify(struct mlx5e_ipsec_sa_entry *sa_entry)
 	mlx5e_accel_ipsec_fs_del_rule(sa_entry);
 	memcpy(sa_entry, &sa_entry_shadow, sizeof(*sa_entry));
 }
+
+bool mlx5e_ipsec_fs_tunnel_enabled(struct mlx5e_ipsec_sa_entry *sa_entry)
+{
+	struct mlx5e_ipsec_rx *rx =
+		ipsec_rx(sa_entry->ipsec, sa_entry->attrs.family);
+	struct mlx5e_ipsec_tx *tx = sa_entry->ipsec->tx;
+
+	if (sa_entry->attrs.dir == XFRM_DEV_OFFLOAD_OUT)
+		return tx->allow_tunnel_mode;
+
+	return rx->allow_tunnel_mode;
+}
Signature · 1 line
2.39.2
#19 lore raw

Re: [PATCH net-next 09/10] net/mlx5e: Create IPsec table with tunnel support only when encap is disabled

Quoted 12 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Current hardware doesn't support double encapsulation which is happening when IPsec packet offload tunnel mode is configured together with eswitch encap option.

Any user attempt to add new SA/policy after he/she sets encap mode, will generate the following FW syndrome:

mlx5_core 0000:08:00.0: mlx5_cmd_out_err:803:(pid 1904): CREATE_FLOW_TABLE(0x930) op_mod(0x0) failed, status bad parameter(0x3), syndrome (0xa43321), err(-22)

Quoted 7 lines

Make sure that we block encap changes before creating flow steering tables. This is applicable only for packet offload in tunnel mode, while packet offload in transport mode and crypto offload, don't have such limitation as they don't perform encapsulation.

Hi Raed and Leon,

some minor feedback from me below.

Quoted 11 lines
---
 .../mellanox/mlx5/core/en_accel/ipsec.c       |  7 ++++
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  1 +
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 33 +++++++++++++++++--
 3 files changed, 38 insertions(+), 3 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0 -0
index b64281fd4142..e95004ac7a20 100644

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c static int mlx5e_xfrm_add_state(struct xfrm_state *x, · line 675

Quoted 8 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +5 -0
@@ -668,6 +668,13 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x,
 	if (err)
 		goto err_hw_ctx;
 
+	if (x->props.mode == XFRM_MODE_TUNNEL &&
+	    x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
+	    !mlx5e_ipsec_fs_tunnel_enabled(sa_entry)) {
+		NL_SET_ERR_MSG_MOD(extack, "Packet offload tunnel mode is disabled due to encap settings");
+		goto err_add_rule;

The err_add_rule will return err.
But err is zero here.
Perhaps it should be set to an negative error code?

Flagged by Smatch as:

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:753 mlx5e_xfrm_free_state() error: we previously assumed 'sa_entry->work' could be null (see line 744)

Quoted 5 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2 -0
+	}
+
 	/* We use *_bh() variant because xfrm_timer_handler(), which runs
 	 * in softirq context, can reach our state delete logic and we need
 	 * xa_erase_bh() there.

...

#20 lore raw

Re: [PATCH net-next 09/10] net/mlx5e: Create IPsec table with tunnel support only when encap is disabled

Quoted 39 lines · Simon Horman
Quoted 12 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Current hardware doesn't support double encapsulation which is happening when IPsec packet offload tunnel mode is configured together with eswitch encap option.

Any user attempt to add new SA/policy after he/she sets encap mode, will generate the following FW syndrome:

mlx5_core 0000:08:00.0: mlx5_cmd_out_err:803:(pid 1904): CREATE_FLOW_TABLE(0x930) op_mod(0x0) failed, status bad parameter(0x3), syndrome (0xa43321), err(-22)

Quoted 7 lines

Make sure that we block encap changes before creating flow steering tables. This is applicable only for packet offload in tunnel mode, while packet offload in transport mode and crypto offload, don't have such limitation as they don't perform encapsulation.

Hi Raed and Leon,

some minor feedback from me below.

Quoted 14 lines
---
 .../mellanox/mlx5/core/en_accel/ipsec.c       |  7 ++++
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  1 +
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 33 +++++++++++++++++--
 3 files changed, 38 insertions(+), 3 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0 -0
index b64281fd4142..e95004ac7a20 100644
@@ -668,6 +668,13 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x,
 	if (err)
 		goto err_hw_ctx;
 
Quoted 9 lines

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c static int mlx5e_xfrm_add_state(struct xfrm_state *x, · line 675

Quoted 5 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +5 -0
+	if (x->props.mode == XFRM_MODE_TUNNEL &&
+	    x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
+	    !mlx5e_ipsec_fs_tunnel_enabled(sa_entry)) {
+		NL_SET_ERR_MSG_MOD(extack, "Packet offload tunnel mode is disabled due to encap settings");
+		goto err_add_rule;

The err_add_rule will return err.
But err is zero here.
Perhaps it should be set to an negative error code?

Thanks, I overlooked it.

Quoted 4 lines

Flagged by Smatch as:

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:753 mlx5e_xfrm_free_state() error: we previously assumed 'sa_entry->work' could be null (see line 744)

I don't get such warnings from my CI, will try to understand why.

What are the command line arguments you use to run smatch? What is the version of smatch?

Thanks

Quoted 8 lines
Quoted 6 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2 -0
+	}
+
 	/* We use *_bh() variant because xfrm_timer_handler(), which runs
 	 * in softirq context, can reach our state delete logic and we need
 	 * xa_erase_bh() there.

...

#21 lore raw

Re: [PATCH net-next 09/10] net/mlx5e: Create IPsec table with tunnel support only when encap is disabled

Quoted 53 lines · Leon Romanovsky
Quoted 39 lines · Simon Horman
Quoted 12 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Current hardware doesn't support double encapsulation which is happening when IPsec packet offload tunnel mode is configured together with eswitch encap option.

Any user attempt to add new SA/policy after he/she sets encap mode, will generate the following FW syndrome:

mlx5_core 0000:08:00.0: mlx5_cmd_out_err:803:(pid 1904): CREATE_FLOW_TABLE(0x930) op_mod(0x0) failed, status bad parameter(0x3), syndrome (0xa43321), err(-22)

Quoted 7 lines

Make sure that we block encap changes before creating flow steering tables. This is applicable only for packet offload in tunnel mode, while packet offload in transport mode and crypto offload, don't have such limitation as they don't perform encapsulation.

Hi Raed and Leon,

some minor feedback from me below.

Quoted 14 lines
---
 .../mellanox/mlx5/core/en_accel/ipsec.c       |  7 ++++
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  1 +
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 33 +++++++++++++++++--
 3 files changed, 38 insertions(+), 3 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0 -0
index b64281fd4142..e95004ac7a20 100644
@@ -668,6 +668,13 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x,
 	if (err)
 		goto err_hw_ctx;
 
Quoted 9 lines

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c static int mlx5e_xfrm_add_state(struct xfrm_state *x, · line 675

Quoted 5 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +5 -0
+	if (x->props.mode == XFRM_MODE_TUNNEL &&
+	    x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
+	    !mlx5e_ipsec_fs_tunnel_enabled(sa_entry)) {
+		NL_SET_ERR_MSG_MOD(extack, "Packet offload tunnel mode is disabled due to encap settings");
+		goto err_add_rule;

The err_add_rule will return err.
But err is zero here.
Perhaps it should be set to an negative error code?

Thanks, I overlooked it.

Quoted 1 line
Quoted 7 lines
Quoted 3 lines

Flagged by Smatch as:

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:753 mlx5e_xfrm_free_state() error: we previously assumed 'sa_entry->work' could be null (see line 744)

I don't get such warnings from my CI, will try to understand why.

What are the command line arguments you use to run smatch?

Hi Leon,

I run Smatch like this:

.../smatch/smatch_scripts/kchecker \
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.o

Quoted 1 line

What is the version of smatch?

I see this with Smatch 1.73.

In writing this email, I noticed that Smatch seems to flag a problem in net-next. Which seems to be a valid concern.

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:753 mlx5e_xfrm_free_state() error: we previously assumed 'sa_entry->work' could be null (see line 744)

#22 lore raw

Re: [PATCH net-next 09/10] net/mlx5e: Create IPsec table with tunnel support only when encap is disabled

Quoted 71 lines · Simon Horman
Quoted 53 lines · Leon Romanovsky
Quoted 39 lines · Simon Horman
Quoted 12 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Current hardware doesn't support double encapsulation which is happening when IPsec packet offload tunnel mode is configured together with eswitch encap option.

Any user attempt to add new SA/policy after he/she sets encap mode, will generate the following FW syndrome:

mlx5_core 0000:08:00.0: mlx5_cmd_out_err:803:(pid 1904): CREATE_FLOW_TABLE(0x930) op_mod(0x0) failed, status bad parameter(0x3), syndrome (0xa43321), err(-22)

Quoted 7 lines

Make sure that we block encap changes before creating flow steering tables. This is applicable only for packet offload in tunnel mode, while packet offload in transport mode and crypto offload, don't have such limitation as they don't perform encapsulation.

Hi Raed and Leon,

some minor feedback from me below.

Quoted 14 lines
---
 .../mellanox/mlx5/core/en_accel/ipsec.c       |  7 ++++
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  1 +
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 33 +++++++++++++++++--
 3 files changed, 38 insertions(+), 3 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0 -0
index b64281fd4142..e95004ac7a20 100644
@@ -668,6 +668,13 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x,
 	if (err)
 		goto err_hw_ctx;
 
Quoted 9 lines

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c static int mlx5e_xfrm_add_state(struct xfrm_state *x, · line 675

Quoted 5 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +5 -0
+	if (x->props.mode == XFRM_MODE_TUNNEL &&
+	    x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
+	    !mlx5e_ipsec_fs_tunnel_enabled(sa_entry)) {
+		NL_SET_ERR_MSG_MOD(extack, "Packet offload tunnel mode is disabled due to encap settings");
+		goto err_add_rule;

The err_add_rule will return err.
But err is zero here.
Perhaps it should be set to an negative error code?

Thanks, I overlooked it.

Quoted 1 line
Quoted 7 lines
Quoted 3 lines

Flagged by Smatch as:

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:753 mlx5e_xfrm_free_state() error: we previously assumed 'sa_entry->work' could be null (see line 744)

I don't get such warnings from my CI, will try to understand why.

What are the command line arguments you use to run smatch?

Hi Leon,

I run Smatch like this:

.../smatch/smatch_scripts/kchecker \
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.o

Quoted 1 line

What is the version of smatch?

Quoted 7 lines

I see this with Smatch 1.73.

In writing this email, I noticed that Smatch seems to flag a problem in net-next. Which seems to be a valid concern.

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:753 mlx5e_xfrm_free_state() error: we previously assumed 'sa_entry->work' could be null (see line 744)

Thanks, I'll take a look when will return to the office.

#23 lore raw

Re: [PATCH net-next 09/10] net/mlx5e: Create IPsec table with tunnel support only when encap is disabled

Quoted 75 lines · Leon Romanovsky
Quoted 74 lines · Simon Horman
Quoted 53 lines · Leon Romanovsky
Quoted 39 lines · Simon Horman
Quoted 12 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Current hardware doesn't support double encapsulation which is happening when IPsec packet offload tunnel mode is configured together with eswitch encap option.

Any user attempt to add new SA/policy after he/she sets encap mode, will generate the following FW syndrome:

mlx5_core 0000:08:00.0: mlx5_cmd_out_err:803:(pid 1904): CREATE_FLOW_TABLE(0x930) op_mod(0x0) failed, status bad parameter(0x3), syndrome (0xa43321), err(-22)

Quoted 7 lines

Make sure that we block encap changes before creating flow steering tables. This is applicable only for packet offload in tunnel mode, while packet offload in transport mode and crypto offload, don't have such limitation as they don't perform encapsulation.

Hi Raed and Leon,

some minor feedback from me below.

Quoted 14 lines
---
 .../mellanox/mlx5/core/en_accel/ipsec.c       |  7 ++++
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  1 +
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 33 +++++++++++++++++--
 3 files changed, 38 insertions(+), 3 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0 -0
index b64281fd4142..e95004ac7a20 100644
@@ -668,6 +668,13 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x,
 	if (err)
 		goto err_hw_ctx;
 
Quoted 9 lines

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c static int mlx5e_xfrm_add_state(struct xfrm_state *x, · line 675

Quoted 5 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +5 -0
+	if (x->props.mode == XFRM_MODE_TUNNEL &&
+	    x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
+	    !mlx5e_ipsec_fs_tunnel_enabled(sa_entry)) {
+		NL_SET_ERR_MSG_MOD(extack, "Packet offload tunnel mode is disabled due to encap settings");
+		goto err_add_rule;

The err_add_rule will return err.
But err is zero here.
Perhaps it should be set to an negative error code?

Thanks, I overlooked it.

Quoted 1 line
Quoted 7 lines
Quoted 3 lines

Flagged by Smatch as:

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:753 mlx5e_xfrm_free_state() error: we previously assumed 'sa_entry->work' could be null (see line 744)

I don't get such warnings from my CI, will try to understand why.

What are the command line arguments you use to run smatch?

Hi Leon,

I run Smatch like this:

.../smatch/smatch_scripts/kchecker \
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.o

Quoted 1 line

What is the version of smatch?

I see this with Smatch 1.73.

Quoted 6 lines
Quoted 4 lines

In writing this email, I noticed that Smatch seems to flag a problem in net-next. Which seems to be a valid concern.

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:753 mlx5e_xfrm_free_state() error: we previously assumed 'sa_entry->work' could be null (see line 744)

Thanks, I'll take a look when will return to the office.

I tried it now and still don't get this warning.

Thanks

#24 lore raw

PATCH 10/10 net/mlx5e: Accept tunnel mode for IPsec packet offload

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Open mlx5 driver to accept IPsec tunnel mode.

---
 .../ethernet/mellanox/mlx5/core/en_accel/ipsec.c  | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +8 -7
index e95004ac7a20..03fbbf84a1ae 100644
@@ -422,6 +422,11 @@ static int mlx5e_xfrm_validate_state(struct mlx5_core_dev *mdev,
 		return -EINVAL;
 	}
 
+	if (x->props.mode != XFRM_MODE_TRANSPORT && x->props.mode != XFRM_MODE_TUNNEL) {
+		NL_SET_ERR_MSG_MOD(extack, "Only transport and tunnel xfrm states may be offloaded");
+		return -EINVAL;
+	}
+
 	switch (x->xso.type) {
 	case XFRM_DEV_OFFLOAD_CRYPTO:
 		if (!(mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_CRYPTO)) {
@@ -429,11 +434,6 @@ static int mlx5e_xfrm_validate_state(struct mlx5_core_dev *mdev,
 			return -EINVAL;
 		}
 
-		if (x->props.mode != XFRM_MODE_TRANSPORT &&
-		    x->props.mode != XFRM_MODE_TUNNEL) {
-			NL_SET_ERR_MSG_MOD(extack, "Only transport and tunnel xfrm states may be offloaded");
-			return -EINVAL;
-		}
 		break;
 	case XFRM_DEV_OFFLOAD_PACKET:
 		if (!(mlx5_ipsec_device_caps(mdev) &
@@ -442,8 +442,9 @@ static int mlx5e_xfrm_validate_state(struct mlx5_core_dev *mdev,
 			return -EINVAL;
 		}
 
-		if (x->props.mode != XFRM_MODE_TRANSPORT) {
-			NL_SET_ERR_MSG_MOD(extack, "Only transport xfrm states may be offloaded in packet mode");
+		if (x->props.mode == XFRM_MODE_TUNNEL &&
+		    !(mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_TUNNEL)) {
+			NL_SET_ERR_MSG_MOD(extack, "Packet offload is not supported for tunnel mode");
 			return -EINVAL;
 		}
 
Signature · 1 line
2.39.2