Offending commits introduced 2 structures which held temporary
parameters for flow rule created. These parameters were passed
to "rule_acts" buffers stored inside template table and
they were used only during a call to mlx5dr_rule_create().
After mlx5dr_rule_create() finished, "rule_acts" buffer is not cleared,
for performance reasons, since on the next flow rule create
it will be overwritten.
GCC 14.2.1, when compiled with -03, issued the following warning:
```
drivers/net/mlx5/mlx5_flow_hw.c:3520:51:
error: storing the address of local variable
'ap' in '*<unknown>.<U2688>.modify_header.data'
[-Werror=dangling-pointer=]
3520 | rule_acts[pos].modify_header.data =
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
3521 | (uint8_t *)ap->mhdr_cmd;
| ~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/mlx5/mlx5_flow_hw.c: In function 'flow_hw_async_flow_create':
drivers/net/mlx5/mlx5_flow_hw.c:3925:43: note: 'ap' declared here
3925 | struct mlx5_flow_hw_action_params ap;
| ^~
drivers/net/mlx5/mlx5_flow_hw.c:3910:59: note: 'table' declared here
3910 | struct rte_flow_template_table *table,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
```
Which is technically correct. This warning could be fixed by
casting "ap->mhdr_cmd" to "void *", but this is just a workaround.
Since what mlx5 PMD cares about is that there is only one instance
of mlx5_flow_hw_action_params per queue, this patch moves this struct
from the stack to mlx5_hw_q struct.
The same is done for mlx5_flow_hw_pattern_params for similar reasons.
Fixes: 1d2744f5028d ("net/mlx5: remove flow action parameters from job")
Fixes: 57fd15fa373d ("net/mlx5: remove flow pattern from job")
Signed-off-by: Dariusz Sosnowski <dsosnow...@nvidia.com>
Acked-by: Ori Kam <or...@nvidia.com>
---
drivers/net/mlx5/mlx5.h | 28 ++++++++++++++++++++++++++++
drivers/net/mlx5/mlx5_flow.h | 25 -------------------------
drivers/net/mlx5/mlx5_flow_hw.c | 9 +++------
3 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 503366580b..bc25d72c0e 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -425,6 +425,30 @@ enum mlx5_hw_indirect_type {
#define MLX5_HW_MAX_ITEMS (16)
+#define MLX5_MHDR_MAX_CMD ((MLX5_MAX_MODIFY_NUM) * 2 + 1)
+#define MLX5_PUSH_MAX_LEN 128
+#define MLX5_ENCAP_MAX_LEN 132
+
+/** Container for flow action data constructed during flow rule creation. */
+struct mlx5_flow_hw_action_params {
+ /** Array of constructed modify header commands. */
+ struct mlx5_modification_cmd mhdr_cmd[MLX5_MHDR_MAX_CMD];
+ /** Constructed encap/decap data buffer. */
+ uint8_t encap_data[MLX5_ENCAP_MAX_LEN];
+ /** Constructed IPv6 routing data buffer. */
+ uint8_t ipv6_push_data[MLX5_PUSH_MAX_LEN];
+};
+
+/** Container for dynamically generated flow items used during flow rule
creation. */
+struct mlx5_flow_hw_pattern_params {
+ /** Array of dynamically generated flow items. */
+ struct rte_flow_item items[MLX5_HW_MAX_ITEMS];
+ /** Temporary REPRESENTED_PORT item generated by PMD. */
+ struct rte_flow_item_ethdev port_spec;
+ /** Temporary TAG item generated by PMD. */
+ struct rte_flow_item_tag tag_spec;
+};
+
/* HW steering flow management job descriptor. */
struct mlx5_hw_q_job {
uint32_t type; /* Job type. */
@@ -449,6 +473,10 @@ struct __rte_cache_aligned mlx5_hw_q {
struct rte_ring *indir_iq; /* Indirect action SW in progress queue. */
struct rte_ring *flow_transfer_pending;
struct rte_ring *flow_transfer_completed;
+ /* Action's ARGUMENT resource buffer for rule creation. */
+ struct mlx5_flow_hw_action_params ap;
+ /* Holds spec value for any implicitly added item. */
+ struct mlx5_flow_hw_pattern_params pp;
};
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 693e07218d..d4d985fa3b 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -646,9 +646,6 @@ struct mlx5_flow_dv_matcher {
struct mlx5_flow_dv_match_params mask; /**< Matcher mask. */
};
-#define MLX5_PUSH_MAX_LEN 128
-#define MLX5_ENCAP_MAX_LEN 132
-
/* Encap/decap resource structure. */
struct mlx5_flow_dv_encap_decap_resource {
struct mlx5_list_entry entry;
@@ -1420,28 +1417,6 @@ typedef int
const struct rte_flow_action *,
struct mlx5dr_rule_action *);
-#define MLX5_MHDR_MAX_CMD ((MLX5_MAX_MODIFY_NUM) * 2 + 1)
-
-/** Container for flow action data constructed during flow rule creation. */
-struct mlx5_flow_hw_action_params {
- /** Array of constructed modify header commands. */
- struct mlx5_modification_cmd mhdr_cmd[MLX5_MHDR_MAX_CMD];
- /** Constructed encap/decap data buffer. */
- uint8_t encap_data[MLX5_ENCAP_MAX_LEN];
- /** Constructed IPv6 routing data buffer. */
- uint8_t ipv6_push_data[MLX5_PUSH_MAX_LEN];
-};
-
-/** Container for dynamically generated flow items used during flow rule
creation. */
-struct mlx5_flow_hw_pattern_params {
- /** Array of dynamically generated flow items. */
- struct rte_flow_item items[MLX5_HW_MAX_ITEMS];
- /** Temporary REPRESENTED_PORT item generated by PMD. */
- struct rte_flow_item_ethdev port_spec;
- /** Temporary TAG item generated by PMD. */
- struct rte_flow_item_tag tag_spec;
-};
-
/* rte flow action translate to DR action struct. */
struct mlx5_action_construct_data {
LIST_ENTRY(mlx5_action_construct_data) next;
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 2a9ef71cd8..88d6859bac 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -3978,8 +3978,6 @@ flow_hw_async_flow_create_generic(struct rte_eth_dev *dev,
.burst = attr->postpone,
};
struct mlx5dr_rule_action *rule_acts;
- struct mlx5_flow_hw_action_params ap;
- struct mlx5_flow_hw_pattern_params pp;
struct rte_flow_hw *flow = NULL;
const struct rte_flow_item *rule_items;
uint32_t flow_idx = 0;
@@ -4035,14 +4033,14 @@ flow_hw_async_flow_create_generic(struct rte_eth_dev
*dev,
* No need to copy and contrust a new "actions" list based on the
* user's input, in order to save the cost.
*/
- if (flow_hw_actions_construct(dev, flow, &ap,
+ if (flow_hw_actions_construct(dev, flow, &priv->hw_q[queue].ap,
&table->ats[action_template_index],
table->its[pattern_template_index]->item_flags,
flow->table, actions,
rule_acts, queue, error))
goto error;
rule_items = flow_hw_get_rule_items(dev, table, items,
- pattern_template_index, &pp);
+ pattern_template_index,
&priv->hw_q[queue].pp);
if (!rule_items)
goto error;
if (likely(!rte_flow_template_table_resizable(dev->data->port_id,
&table->cfg.attr))) {
@@ -4185,7 +4183,6 @@ flow_hw_async_flow_update(struct rte_eth_dev *dev,
.burst = attr->postpone,
};
struct mlx5dr_rule_action *rule_acts;
- struct mlx5_flow_hw_action_params ap;
struct rte_flow_hw *of = (struct rte_flow_hw *)flow;
struct rte_flow_hw *nf;
struct rte_flow_hw_aux *aux;
@@ -4236,7 +4233,7 @@ flow_hw_async_flow_update(struct rte_eth_dev *dev,
* No need to copy and contrust a new "actions" list based on the
* user's input, in order to save the cost.
*/
- if (flow_hw_actions_construct(dev, nf, &ap,
+ if (flow_hw_actions_construct(dev, nf, &priv->hw_q[queue].ap,
&table->ats[action_template_index],
table->its[nf->mt_idx]->item_flags,
table, actions,
--
2.39.5
