Skip to contentSkip to conversation

net/mlx5: fix flow descriptor allocation in Direct Verbs mode.

PATCH Superseded Gregory Etelson 3 participants 5 messages Dec 7, 2020 – Dec 15, 2020 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 net/mlx5: fix flow descriptor allocation in Direct Verbs mode.

Initialize flow descriptor tunnel member during flow creation. Prevent access to stale data and pointers when flow descriptor is reallocated after relase.

Fixes 8bb81f2649b1 ("net/mlx5: use thread specific flow workspace")
---
 drivers/net/mlx5/mlx5_flow_dv.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/net/mlx5/mlx5_flow_dv.c +2 -7
index aa21ff9613..c72f97e05f 100644
@@ -6232,8 +6232,9 @@ flow_dv_prepare(struct rte_eth_dev *dev,
 				   "not enough memory to create flow handle");
 		return NULL;
 	}
-	MLX5_ASSERT(wks->flow_idx + 1 < RTE_DIM(wks->flows));
+	MLX5_ASSERT(wks->flow_idx - 1 < RTE_DIM(wks->flows));
 	dev_flow = &wks->flows[wks->flow_idx++];
+	memset(dev_flow, 0, sizeof(*dev_flow));
 	dev_flow->handle = dev_handle;
 	dev_flow->handle_idx = handle_idx;
 	/*
@@ -6245,12 +6246,6 @@ flow_dv_prepare(struct rte_eth_dev *dev,
 	 */
 	dev_flow->dv.value.size = MLX5_ST_SZ_BYTES(fte_match_param) -
 				  MLX5_ST_SZ_BYTES(fte_match_set_misc4);
-	/*
-	 * The matching value needs to be cleared to 0 before using. In the
-	 * past, it will be automatically cleared when using rte_*alloc
-	 * API. The time consumption will be almost the same as before.
-	 */
-	memset(dev_flow->dv.value.buf, 0, MLX5_ST_SZ_BYTES(fte_match_param));
 	dev_flow->ingress = attr->ingress;
 	dev_flow->dv.transfer = attr->transfer;
 	return dev_flow;
Signature · 1 line
2.29.2
#1 lore raw

Hi Gregory,

Quoted 10 lines

-----Original Message-----
From: Gregory Etelson <[email protected]>
Sent: Monday, December 7, 2020 10:28 PM
To: [email protected]
Cc: Gregory Etelson <[email protected]>; Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Slava Ovsiienko <[email protected]>; Shahaf Shuler <[email protected]>; Xueming(Steven) Li <[email protected]>
Subject: [PATCH] net/mlx5: fix flow descriptor allocation in Direct Verbs mode.

Quoted 5 lines

Initialize flow descriptor tunnel member during flow creation. Prevent access to stale data and pointers when flow descriptor is reallocated after relase.

Fixes 8bb81f2649b1 ("net/mlx5: use thread specific flow workspace")

The initialization issue is introduced in earlier patch.

Quoted 12 lines

---
drivers/net/mlx5/mlx5_flow_dv.c | 9 ++-------

1 file changed, 2 insertions(+), 7 deletions(-)
(quoted diff) +0 -0

b/drivers/net/mlx5/mlx5_flow_dv.c index aa21ff9613..c72f97e05f 100644

drivers/net/mlx5/mlx5_flow_dv.c flow_dv_prepare(struct rte_eth_dev *dev, · line 6235

Quoted 5 lines
drivers/net/mlx5/mlx5_flow_dv.c +1 -1
@@ -6232,8 +6232,9 @@ flow_dv_prepare(struct rte_eth_dev *dev,
 				   "not enough memory to create flow handle");
 		return NULL;
 	}
-	MLX5_ASSERT(wks->flow_idx + 1 < RTE_DIM(wks->flows));
+	MLX5_ASSERT(wks->flow_idx - 1 < RTE_DIM(wks->flows));

wks->flow_idx is used as "next available index", should it be "wks->flow_idx < RTE_DIM(...)"?

Quoted 21 lines
drivers/net/mlx5/mlx5_flow_dv.c +1 -6
 	dev_flow = &wks->flows[wks->flow_idx++];
+	memset(dev_flow, 0, sizeof(*dev_flow));
 	dev_flow->handle = dev_handle;
 	dev_flow->handle_idx = handle_idx;
 	/*
@@ -6245,12 +6246,6 @@ flow_dv_prepare(struct rte_eth_dev *dev,
 	 */
 	dev_flow->dv.value.size = MLX5_ST_SZ_BYTES(fte_match_param) -
 				  MLX5_ST_SZ_BYTES(fte_match_set_misc4);
-	/*
-	 * The matching value needs to be cleared to 0 before using. In the
-	 * past, it will be automatically cleared when using rte_*alloc
-	 * API. The time consumption will be almost the same as before.
-	 */
-	memset(dev_flow->dv.value.buf, 0,
 MLX5_ST_SZ_BYTES(fte_match_param));
 	dev_flow->ingress = attr->ingress;
 	dev_flow->dv.transfer = attr->transfer;
	return dev_flow;

--
2.29.2

#2 lore raw

PATCH v2 1/1 net/mlx5: fix flow descriptor allocation in Direct Verbs mode.

In reply to Gregory Etelson

Initialize flow descriptor tunnel member during flow creation. Prevent access to stale data and pointers when flow descriptor is reallocated after release.
Fix flow index validation.

Fixes e7bfa3596a0a ("net/mlx5: separate the flow handle resource")
Fixes 8bb81f2649b1 ("net/mlx5: use thread specific flow workspace")
---
 drivers/net/mlx5/mlx5_flow_dv.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/net/mlx5/mlx5_flow_dv.c +2 -7
index aa21ff9613..8f7085c951 100644
@@ -6232,8 +6232,9 @@ flow_dv_prepare(struct rte_eth_dev *dev,
 				   "not enough memory to create flow handle");
 		return NULL;
 	}
-	MLX5_ASSERT(wks->flow_idx + 1 < RTE_DIM(wks->flows));
+	MLX5_ASSERT(wks->flow_idx < RTE_DIM(wks->flows));
 	dev_flow = &wks->flows[wks->flow_idx++];
+	memset(dev_flow, 0, sizeof(*dev_flow));
 	dev_flow->handle = dev_handle;
 	dev_flow->handle_idx = handle_idx;
 	/*
@@ -6245,12 +6246,6 @@ flow_dv_prepare(struct rte_eth_dev *dev,
 	 */
 	dev_flow->dv.value.size = MLX5_ST_SZ_BYTES(fte_match_param) -
 				  MLX5_ST_SZ_BYTES(fte_match_set_misc4);
-	/*
-	 * The matching value needs to be cleared to 0 before using. In the
-	 * past, it will be automatically cleared when using rte_*alloc
-	 * API. The time consumption will be almost the same as before.
-	 */
-	memset(dev_flow->dv.value.buf, 0, MLX5_ST_SZ_BYTES(fte_match_param));
 	dev_flow->ingress = attr->ingress;
 	dev_flow->dv.transfer = attr->transfer;
 	return dev_flow;
Signature · 1 line
2.29.2
#3 lore raw
Quoted 57 lines

-----Original Message-----
From: Gregory Etelson <[email protected]>
Sent: Tuesday, December 8, 2020 4:17 PM
To: [email protected]
Cc: Gregory Etelson <[email protected]>; Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Slava Ovsiienko <[email protected]>; Shahaf Shuler <[email protected]>; Bing Zhao <[email protected]>; Xueming(Steven) Li <[email protected]>
Subject: [PATCH v2] net/mlx5: fix flow descriptor allocation in Direct Verbs mode.

Initialize flow descriptor tunnel member during flow creation. Prevent access to stale data and pointers when flow descriptor is reallocated after release.
Fix flow index validation.

Fixes e7bfa3596a0a ("net/mlx5: separate the flow handle resource")
Fixes 8bb81f2649b1 ("net/mlx5: use thread specific flow workspace")

---
drivers/net/mlx5/mlx5_flow_dv.c | 9 ++-------

1 file changed, 2 insertions(+), 7 deletions(-)
(quoted diff) +0 -0

b/drivers/net/mlx5/mlx5_flow_dv.c index aa21ff9613..8f7085c951 100644

drivers/net/mlx5/mlx5_flow_dv.c +2 -7
@@ -6232,8 +6232,9 @@ flow_dv_prepare(struct rte_eth_dev *dev,
 				   "not enough memory to create flow handle");
 		return NULL;
 	}
-	MLX5_ASSERT(wks->flow_idx + 1 < RTE_DIM(wks->flows));
+	MLX5_ASSERT(wks->flow_idx < RTE_DIM(wks->flows));
 	dev_flow = &wks->flows[wks->flow_idx++];
+	memset(dev_flow, 0, sizeof(*dev_flow));
 	dev_flow->handle = dev_handle;
 	dev_flow->handle_idx = handle_idx;
 	/*
@@ -6245,12 +6246,6 @@ flow_dv_prepare(struct rte_eth_dev *dev,
 	 */
 	dev_flow->dv.value.size = MLX5_ST_SZ_BYTES(fte_match_param) -
 				  MLX5_ST_SZ_BYTES(fte_match_set_misc4);
-	/*
-	 * The matching value needs to be cleared to 0 before using. In the
-	 * past, it will be automatically cleared when using rte_*alloc
-	 * API. The time consumption will be almost the same as before.
-	 */
-	memset(dev_flow->dv.value.buf, 0,
 MLX5_ST_SZ_BYTES(fte_match_param));
 	dev_flow->ingress = attr->ingress;
 	dev_flow->dv.transfer = attr->transfer;
	return dev_flow;
Quoted 2 lines

--
2.29.2

Reviwed by: Xueming(Steven) Li <[email protected]>

#4 lore raw
In reply to Gregory Etelson

Hi,

Quoted 11 lines

-----Original Message-----
From: Gregory Etelson <[email protected]>
Sent: Tuesday, December 8, 2020 10:17 AM
To: [email protected]
Cc: Gregory Etelson <[email protected]>; Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Slava Ovsiienko <[email protected]>; Shahaf Shuler <[email protected]>; Bing Zhao <[email protected]>; Xueming(Steven) Li <[email protected]>
Subject: [PATCH v2] net/mlx5: fix flow descriptor allocation in Direct Verbs mode.

Fix wrong headline in commit title.
Used DV instead of Direct verbs for fixing too long line commit title.

Quoted 9 lines

Initialize flow descriptor tunnel member during flow creation. Prevent access to stale data and pointers when flow descriptor is reallocated after release.
Fix flow index validation.

Fixes e7bfa3596a0a ("net/mlx5: separate the flow handle resource")
Fixes 8bb81f2649b1 ("net/mlx5: use thread specific flow workspace")
Quoted 3 lines

---

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh