Each time a flow is allocated in mlx5 PMD the whole buffer,
both rte_flow_hw and mlx5dr_rule parts, are zeroed.
This introduces some wasted work because:
- mlx5dr layer does not assume that mlx5dr_rule must be initialized,
- flow action translation in mlx5 PMD does not need most of the fields
of rte_flow_hw to be zeroed.
To reduce this wasted work, this patch introduces flags field to
flow definition. Each flow field which is not always initialized
during flow creation, will have a correspondent flag set if value is
valid (in other words - it was set during flow creation).
Utilizing this mechanism allows PMD to:
- remove zeroing from flow allocation,
- access some fields (especially from rte_flow_hw_aux) if and only if
corresponding flag is set.
Signed-off-by: Dariusz Sosnowski <dsosnow...@nvidia.com>
Acked-by: Ori Kam <or...@nvidia.com>
---
drivers/net/mlx5/mlx5_flow.h | 24 ++++++++-
drivers/net/mlx5/mlx5_flow_hw.c | 93 +++++++++++++++++++++------------
2 files changed, 83 insertions(+), 34 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index e8f4d2cb16..db65825eab 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1279,6 +1279,26 @@ enum {
MLX5_FLOW_HW_FLOW_OP_TYPE_RSZ_TBL_MOVE,
};
+enum {
+ MLX5_FLOW_HW_FLOW_FLAG_CNT_ID = RTE_BIT32(0),
+ MLX5_FLOW_HW_FLOW_FLAG_FATE_JUMP = RTE_BIT32(1),
+ MLX5_FLOW_HW_FLOW_FLAG_FATE_HRXQ = RTE_BIT32(2),
+ MLX5_FLOW_HW_FLOW_FLAG_AGE_IDX = RTE_BIT32(3),
+ MLX5_FLOW_HW_FLOW_FLAG_MTR_ID = RTE_BIT32(4),
+ MLX5_FLOW_HW_FLOW_FLAG_MATCHER_SELECTOR = RTE_BIT32(5),
+ MLX5_FLOW_HW_FLOW_FLAG_UPD_FLOW = RTE_BIT32(6),
+};
+
+#define MLX5_FLOW_HW_FLOW_FLAGS_ALL ( \
+ MLX5_FLOW_HW_FLOW_FLAG_CNT_ID | \
+ MLX5_FLOW_HW_FLOW_FLAG_FATE_JUMP | \
+ MLX5_FLOW_HW_FLOW_FLAG_FATE_HRXQ | \
+ MLX5_FLOW_HW_FLOW_FLAG_AGE_IDX | \
+ MLX5_FLOW_HW_FLOW_FLAG_MTR_ID | \
+ MLX5_FLOW_HW_FLOW_FLAG_MATCHER_SELECTOR | \
+ MLX5_FLOW_HW_FLOW_FLAG_UPD_FLOW \
+ )
+
#ifdef PEDANTIC
#pragma GCC diagnostic ignored "-Wpedantic"
#endif
@@ -1295,8 +1315,8 @@ struct rte_flow_hw {
uint32_t res_idx;
/** HWS flow rule index passed to mlx5dr. */
uint32_t rule_idx;
- /** Fate action type. */
- uint32_t fate_type;
+ /** Which flow fields (inline or in auxiliary struct) are used. */
+ uint32_t flags;
/** Ongoing flow operation type. */
uint8_t operation_type;
/** Index of pattern template this flow is based on. */
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 025f04ddde..979be4764a 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -2845,6 +2845,7 @@ flow_hw_shared_action_construct(struct rte_eth_dev *dev,
uint32_t queue,
&rule_act->action,
&rule_act->counter.offset))
return -1;
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_CNT_ID;
flow->cnt_id = act_idx;
break;
case MLX5_INDIRECT_ACTION_TYPE_AGE:
@@ -2854,6 +2855,7 @@ flow_hw_shared_action_construct(struct rte_eth_dev *dev,
uint32_t queue,
* it in flow destroy.
*/
mlx5_flow_hw_aux_set_age_idx(flow, aux, act_idx);
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_AGE_IDX;
if (action_flags & MLX5_FLOW_ACTION_INDIRECT_COUNT)
/*
* The mutual update for idirect AGE & COUNT will be
@@ -2869,6 +2871,7 @@ flow_hw_shared_action_construct(struct rte_eth_dev *dev,
uint32_t queue,
¶m->queue_id, &age_cnt,
idx) < 0)
return -1;
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_CNT_ID;
flow->cnt_id = age_cnt;
param->nb_cnts++;
} else {
@@ -3174,7 +3177,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
rule_acts[act_data->action_dst].action =
(!!attr.group) ? jump->hws_action : jump->root_action;
flow->jump = jump;
- flow->fate_type = MLX5_FLOW_FATE_JUMP;
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_FATE_JUMP;
break;
case RTE_FLOW_ACTION_TYPE_RSS:
case RTE_FLOW_ACTION_TYPE_QUEUE:
@@ -3185,7 +3188,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
return -1;
rule_acts[act_data->action_dst].action = hrxq->action;
flow->hrxq = hrxq;
- flow->fate_type = MLX5_FLOW_FATE_QUEUE;
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_FATE_HRXQ;
break;
case MLX5_RTE_FLOW_ACTION_TYPE_RSS:
item_flags = table->its[it_idx]->item_flags;
@@ -3264,7 +3267,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
(!!attr.group) ? jump->hws_action :
jump->root_action;
flow->jump = jump;
- flow->fate_type = MLX5_FLOW_FATE_JUMP;
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_FATE_JUMP;
if (mlx5_aso_mtr_wait(priv->sh, MLX5_HW_INV_QUEUE,
aso_mtr))
return -1;
break;
@@ -3284,6 +3287,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
if (age_idx == 0)
return -rte_errno;
mlx5_flow_hw_aux_set_age_idx(flow, aux, age_idx);
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_AGE_IDX;
if (at->action_flags & MLX5_FLOW_ACTION_INDIRECT_COUNT)
/*
* When AGE uses indirect counter, no need to
@@ -3306,6 +3310,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
);
if (ret != 0)
return ret;
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_CNT_ID;
flow->cnt_id = cnt_id;
break;
case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
@@ -3317,6 +3322,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
);
if (ret != 0)
return ret;
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_CNT_ID;
flow->cnt_id = act_data->shared_counter.id;
break;
case RTE_FLOW_ACTION_TYPE_CONNTRACK:
@@ -3349,6 +3355,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
return ret;
aux = mlx5_flow_hw_aux(dev->data->port_id, flow);
mlx5_flow_hw_aux_set_mtr_id(flow, aux, mtr_idx);
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_MTR_ID;
break;
case RTE_FLOW_ACTION_TYPE_NAT64:
nat64_c = action->conf;
@@ -3360,7 +3367,11 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
}
}
if (at->action_flags & MLX5_FLOW_ACTION_INDIRECT_COUNT) {
+ /* If indirect count is used, then CNT_ID flag should be set. */
+ MLX5_ASSERT(flow->flags & MLX5_FLOW_HW_FLOW_FLAG_CNT_ID);
if (at->action_flags & MLX5_FLOW_ACTION_INDIRECT_AGE) {
+ /* If indirect AGE is used, then AGE_IDX flag should be
set. */
+ MLX5_ASSERT(flow->flags &
MLX5_FLOW_HW_FLOW_FLAG_AGE_IDX);
aux = mlx5_flow_hw_aux(dev->data->port_id, flow);
age_idx = mlx5_flow_hw_aux_get_age_idx(flow, aux) &
MLX5_HWS_AGE_IDX_MASK;
@@ -3398,8 +3409,10 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
flow->res_idx - 1;
rule_acts[hw_acts->push_remove_pos].ipv6_ext.header =
ap->ipv6_push_data;
}
- if (mlx5_hws_cnt_id_valid(hw_acts->cnt_id))
+ if (mlx5_hws_cnt_id_valid(hw_acts->cnt_id)) {
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_CNT_ID;
flow->cnt_id = hw_acts->cnt_id;
+ }
return 0;
}
@@ -3512,7 +3525,7 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
"Port must be started before enqueueing flow
operations");
return NULL;
}
- flow = mlx5_ipool_zmalloc(table->flow, &flow_idx);
+ flow = mlx5_ipool_malloc(table->flow, &flow_idx);
if (!flow)
goto error;
rule_acts = flow_hw_get_dr_action_buffer(priv, table,
action_template_index, queue);
@@ -3531,6 +3544,7 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
} else {
flow->res_idx = flow_idx;
}
+ flow->flags = 0;
/*
* Set the flow operation type here in order to know if the flow memory
* should be freed or not when get the result from dequeue.
@@ -3582,6 +3596,7 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
(struct mlx5dr_rule *)flow->rule);
rte_rwlock_read_unlock(&table->matcher_replace_rwlk);
aux->matcher_selector = selector;
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_MATCHER_SELECTOR;
}
if (likely(!ret)) {
flow_hw_q_inc_flow_ops(priv, queue);
@@ -3655,7 +3670,7 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev
*dev,
"Flow rule index exceeds table size");
return NULL;
}
- flow = mlx5_ipool_zmalloc(table->flow, &flow_idx);
+ flow = mlx5_ipool_malloc(table->flow, &flow_idx);
if (!flow)
goto error;
rule_acts = flow_hw_get_dr_action_buffer(priv, table,
action_template_index, queue);
@@ -3674,6 +3689,7 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev
*dev,
} else {
flow->res_idx = flow_idx;
}
+ flow->flags = 0;
/*
* Set the flow operation type here in order to know if the flow memory
* should be freed or not when get the result from dequeue.
@@ -3715,6 +3731,7 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev
*dev,
(struct mlx5dr_rule *)flow->rule);
rte_rwlock_read_unlock(&table->matcher_replace_rwlk);
aux->matcher_selector = selector;
+ flow->flags |= MLX5_FLOW_HW_FLOW_FLAG_MATCHER_SELECTOR;
}
if (likely(!ret)) {
flow_hw_q_inc_flow_ops(priv, queue);
@@ -3802,6 +3819,7 @@ flow_hw_async_flow_update(struct rte_eth_dev *dev,
} else {
nf->res_idx = of->res_idx;
}
+ nf->flags = 0;
/* Indicate the construction function to set the proper fields. */
nf->operation_type = MLX5_FLOW_HW_FLOW_OP_TYPE_UPDATE;
/*
@@ -3831,6 +3849,7 @@ flow_hw_async_flow_update(struct rte_eth_dev *dev,
*/
of->operation_type = MLX5_FLOW_HW_FLOW_OP_TYPE_UPDATE;
of->user_data = user_data;
+ of->flags |= MLX5_FLOW_HW_FLOW_FLAG_UPD_FLOW;
rule_attr.user_data = of;
ret = mlx5dr_rule_action_update((struct mlx5dr_rule *)of->rule,
action_template_index, rule_acts,
&rule_attr);
@@ -3925,13 +3944,14 @@ flow_hw_age_count_release(struct mlx5_priv *priv,
uint32_t queue,
uint32_t *cnt_queue;
uint32_t age_idx = aux->orig.age_idx;
+ MLX5_ASSERT(flow->flags & MLX5_FLOW_HW_FLOW_FLAG_CNT_ID);
if (mlx5_hws_cnt_is_shared(priv->hws_cpool, flow->cnt_id)) {
- if (age_idx && !mlx5_hws_age_is_indirect(age_idx)) {
+ if ((flow->flags & MLX5_FLOW_HW_FLOW_FLAG_AGE_IDX) &&
+ !mlx5_hws_age_is_indirect(age_idx)) {
/* Remove this AGE parameter from indirect counter. */
mlx5_hws_cnt_age_set(priv->hws_cpool, flow->cnt_id, 0);
/* Release the AGE parameter. */
mlx5_hws_age_action_destroy(priv, age_idx, error);
- mlx5_flow_hw_aux_set_age_idx(flow, aux, 0);
}
return;
}
@@ -3939,8 +3959,7 @@ flow_hw_age_count_release(struct mlx5_priv *priv,
uint32_t queue,
cnt_queue = mlx5_hws_cnt_is_pool_shared(priv) ? NULL : &queue;
/* Put the counter first to reduce the race risk in BG thread. */
mlx5_hws_cnt_pool_put(priv->hws_cpool, cnt_queue, &flow->cnt_id);
- flow->cnt_id = 0;
- if (age_idx) {
+ if (flow->flags & MLX5_FLOW_HW_FLOW_FLAG_AGE_IDX) {
if (mlx5_hws_age_is_indirect(age_idx)) {
uint32_t idx = age_idx & MLX5_HWS_AGE_IDX_MASK;
@@ -3949,7 +3968,6 @@ flow_hw_age_count_release(struct mlx5_priv *priv,
uint32_t queue,
/* Release the AGE parameter. */
mlx5_hws_age_action_destroy(priv, age_idx, error);
}
- mlx5_flow_hw_aux_set_age_idx(flow, aux, age_idx);
}
}
@@ -4079,34 +4097,35 @@ hw_cmpl_flow_update_or_destroy(struct rte_eth_dev *dev,
struct mlx5_priv *priv = dev->data->dev_private;
struct mlx5_aso_mtr_pool *pool = priv->hws_mpool;
struct rte_flow_template_table *table = flow->table;
- struct rte_flow_hw_aux *aux = mlx5_flow_hw_aux(dev->data->port_id,
flow);
/* Release the original resource index in case of update. */
uint32_t res_idx = flow->res_idx;
- if (flow->fate_type == MLX5_FLOW_FATE_JUMP)
- flow_hw_jump_release(dev, flow->jump);
- else if (flow->fate_type == MLX5_FLOW_FATE_QUEUE)
- mlx5_hrxq_obj_release(dev, flow->hrxq);
- if (mlx5_hws_cnt_id_valid(flow->cnt_id))
- flow_hw_age_count_release(priv, queue,
- flow, error);
- if (aux->orig.mtr_id) {
- mlx5_ipool_free(pool->idx_pool, aux->orig.mtr_id);
- aux->orig.mtr_id = 0;
- }
- if (flow->operation_type != MLX5_FLOW_HW_FLOW_OP_TYPE_UPDATE) {
- if (table->resource)
- mlx5_ipool_free(table->resource, res_idx);
- mlx5_ipool_free(table->flow, flow->idx);
- } else {
+ if (flow->flags & MLX5_FLOW_HW_FLOW_FLAGS_ALL) {
struct rte_flow_hw_aux *aux =
mlx5_flow_hw_aux(dev->data->port_id, flow);
- struct rte_flow_hw *upd_flow = &aux->upd_flow;
- rte_memcpy(flow, upd_flow, offsetof(struct rte_flow_hw, rule));
- aux->orig = aux->upd;
- flow->operation_type = MLX5_FLOW_HW_FLOW_OP_TYPE_CREATE;
+ if (flow->flags & MLX5_FLOW_HW_FLOW_FLAG_FATE_JUMP)
+ flow_hw_jump_release(dev, flow->jump);
+ else if (flow->flags & MLX5_FLOW_HW_FLOW_FLAG_FATE_HRXQ)
+ mlx5_hrxq_obj_release(dev, flow->hrxq);
+ if (flow->flags & MLX5_FLOW_HW_FLOW_FLAG_CNT_ID)
+ flow_hw_age_count_release(priv, queue, flow, error);
+ if (flow->flags & MLX5_FLOW_HW_FLOW_FLAG_MTR_ID)
+ mlx5_ipool_free(pool->idx_pool, aux->orig.mtr_id);
+ if (flow->flags & MLX5_FLOW_HW_FLOW_FLAG_UPD_FLOW) {
+ struct rte_flow_hw *upd_flow = &aux->upd_flow;
+
+ rte_memcpy(flow, upd_flow, offsetof(struct rte_flow_hw,
rule));
+ aux->orig = aux->upd;
+ flow->operation_type = MLX5_FLOW_HW_FLOW_OP_TYPE_CREATE;
+ if (table->resource)
+ mlx5_ipool_free(table->resource, res_idx);
+ }
+ }
+ if (flow->operation_type == MLX5_FLOW_HW_FLOW_OP_TYPE_DESTROY ||
+ flow->operation_type == MLX5_FLOW_HW_FLOW_OP_TYPE_RSZ_TBL_DESTROY) {
if (table->resource)
mlx5_ipool_free(table->resource, res_idx);
+ mlx5_ipool_free(table->flow, flow->idx);
}
}
@@ -4121,6 +4140,7 @@ hw_cmpl_resizable_tbl(struct rte_eth_dev *dev,
uint32_t selector = aux->matcher_selector;
uint32_t other_selector = (selector + 1) & 1;
+ MLX5_ASSERT(flow->flags & MLX5_FLOW_HW_FLOW_FLAG_MATCHER_SELECTOR);
switch (flow->operation_type) {
case MLX5_FLOW_HW_FLOW_OP_TYPE_RSZ_TBL_CREATE:
rte_atomic_fetch_add_explicit
@@ -11411,10 +11431,18 @@ flow_hw_query(struct rte_eth_dev *dev, struct
rte_flow *flow,
case RTE_FLOW_ACTION_TYPE_VOID:
break;
case RTE_FLOW_ACTION_TYPE_COUNT:
+ if (!(hw_flow->flags & MLX5_FLOW_HW_FLOW_FLAG_CNT_ID))
+ return rte_flow_error_set(error, EINVAL,
+
RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+ "counter not defined
in the rule");
ret = flow_hw_query_counter(dev, hw_flow->cnt_id, data,
error);
break;
case RTE_FLOW_ACTION_TYPE_AGE:
+ if (!(hw_flow->flags & MLX5_FLOW_HW_FLOW_FLAG_AGE_IDX))
+ return rte_flow_error_set(error, EINVAL,
+
RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+ "age data not
available");
aux = mlx5_flow_hw_aux(dev->data->port_id, hw_flow);
ret = flow_hw_query_age(dev,
mlx5_flow_hw_aux_get_age_idx(hw_flow, aux),
data, error);
@@ -12707,6 +12735,7 @@ flow_hw_update_resized(struct rte_eth_dev *dev,
uint32_t queue,
.burst = attr->postpone,
};
+ MLX5_ASSERT(hw_flow->flags & MLX5_FLOW_HW_FLOW_FLAG_MATCHER_SELECTOR);
/**
* mlx5dr_matcher_resize_rule_move() accepts original table matcher -
* the one that was used BEFORE table resize.
--
2.39.2
