ethdev: remove deprecated shared counter attribute
Checks
Commit Message
Indirect actions should be used to do shared counters.
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
app/test-pmd/cmdline_flow.c | 10 --
doc/guides/prog_guide/rte_flow.rst | 19 +--
doc/guides/rel_notes/deprecation.rst | 4 -
doc/guides/rel_notes/release_21_11.rst | 4 +
drivers/net/bnxt/tf_ulp/ulp_rte_parser.c | 5 -
drivers/net/cnxk/cnxk_rte_flow.c | 8 --
drivers/net/hns3/hns3_flow.c | 3 +-
drivers/net/ice/ice_fdir_filter.c | 4 +-
drivers/net/mlx5/mlx5.h | 7 --
drivers/net/mlx5/mlx5_flow_dv.c | 135 ++-------------------
drivers/net/mlx5/mlx5_flow_verbs.c | 22 +---
drivers/net/octeontx2/otx2_flow_parse.c | 10 --
drivers/net/sfc/sfc_mae.c | 9 +-
drivers/net/softnic/rte_eth_softnic_flow.c | 7 --
lib/ethdev/rte_flow.h | 17 +--
15 files changed, 23 insertions(+), 241 deletions(-)
Comments
On Tue, 28 Sep 2021 18:23:00 +0300
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote:
> @@ -2498,24 +2498,11 @@ struct rte_flow_query_age {
> * Counters can be retrieved and reset through ``rte_flow_query()``, see
> * ``struct rte_flow_query_count``.
> *
> - * @deprecated Shared attribute is deprecated, use generic
> - * RTE_FLOW_ACTION_TYPE_INDIRECT action.
> - *
> - * The shared flag indicates whether the counter is unique to the flow rule the
> - * action is specified with, or whether it is a shared counter.
> - *
> - * For a count action with the shared flag set, then then a global device
> - * namespace is assumed for the counter id, so that any matched flow rules using
> - * a count action with the same counter id on the same port will contribute to
> - * that counter.
> - *
> * For ports within the same switch domain then the counter id namespace extends
> * to all ports within that switch domain.
> */
> struct rte_flow_action_count {
> - /** @deprecated Share counter ID with other flow rules. */
> - uint32_t shared:1;
> - uint32_t reserved:31; /**< Reserved, must be zero. */
> + uint32_t reserved; /**< Reserved, must be zero. */
> uint32_t id; /**< Counter ID. */
Reserved fields are often source of future problems.
You should change each driver to check that reserved field
return -ENOTSUP if non-zero. That way if reserved field is ever
used in future it won't break API/ABI.
The other option is to just remove the reserved field and take the API/ABI
hit now.
On 9/28/21 6:58 PM, Stephen Hemminger wrote:
> On Tue, 28 Sep 2021 18:23:00 +0300
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote:
>
>> @@ -2498,24 +2498,11 @@ struct rte_flow_query_age {
>> * Counters can be retrieved and reset through ``rte_flow_query()``, see
>> * ``struct rte_flow_query_count``.
>> *
>> - * @deprecated Shared attribute is deprecated, use generic
>> - * RTE_FLOW_ACTION_TYPE_INDIRECT action.
>> - *
>> - * The shared flag indicates whether the counter is unique to the flow rule the
>> - * action is specified with, or whether it is a shared counter.
>> - *
>> - * For a count action with the shared flag set, then then a global device
>> - * namespace is assumed for the counter id, so that any matched flow rules using
>> - * a count action with the same counter id on the same port will contribute to
>> - * that counter.
>> - *
>> * For ports within the same switch domain then the counter id namespace extends
>> * to all ports within that switch domain.
>> */
>> struct rte_flow_action_count {
>> - /** @deprecated Share counter ID with other flow rules. */
>> - uint32_t shared:1;
>> - uint32_t reserved:31; /**< Reserved, must be zero. */
>> + uint32_t reserved; /**< Reserved, must be zero. */
>> uint32_t id; /**< Counter ID. */
>
> Reserved fields are often source of future problems.
> You should change each driver to check that reserved field
> return -ENOTSUP if non-zero. That way if reserved field is ever
> used in future it won't break API/ABI.
>
> The other option is to just remove the reserved field and take the API/ABI
> hit now.
>
I agree. If there is no objections, I'll remove it in v2.
28/09/2021 17:23, Andrew Rybchenko:
> Indirect actions should be used to do shared counters.
>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
Thanks Andrew, we need more maintainers like you :)
On Tue, Oct 5, 2021 at 12:11 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> 28/09/2021 17:23, Andrew Rybchenko:
> > Indirect actions should be used to do shared counters.
> >
> > Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>
> Thanks Andrew, we need more maintainers like you :)
>
>
>
Hi Andrew
Thank you for the big effort to adjust mlx5.
I left some comments inside.
If you feel it is too much, we can do a later patch to improve.
Matan
> From: Andrew Rybchenko
> Indirect actions should be used to do shared counters.
>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
> app/test-pmd/cmdline_flow.c | 10 --
> doc/guides/prog_guide/rte_flow.rst | 19 +--
> doc/guides/rel_notes/deprecation.rst | 4 -
> doc/guides/rel_notes/release_21_11.rst | 4 +
> drivers/net/bnxt/tf_ulp/ulp_rte_parser.c | 5 -
> drivers/net/cnxk/cnxk_rte_flow.c | 8 --
> drivers/net/hns3/hns3_flow.c | 3 +-
> drivers/net/ice/ice_fdir_filter.c | 4 +-
> drivers/net/mlx5/mlx5.h | 7 --
> drivers/net/mlx5/mlx5_flow_dv.c | 135 ++-------------------
> drivers/net/mlx5/mlx5_flow_verbs.c | 22 +---
> drivers/net/octeontx2/otx2_flow_parse.c | 10 --
> drivers/net/sfc/sfc_mae.c | 9 +-
> drivers/net/softnic/rte_eth_softnic_flow.c | 7 --
> lib/ethdev/rte_flow.h | 17 +--
> 15 files changed, 23 insertions(+), 241 deletions(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index bb22294dd3..0b5856c7d5 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -322,7 +322,6 @@ enum index {
> ACTION_QUEUE_INDEX,
> ACTION_DROP,
> ACTION_COUNT,
> - ACTION_COUNT_SHARED,
> ACTION_COUNT_ID,
> ACTION_RSS,
> ACTION_RSS_FUNC,
> @@ -1451,7 +1450,6 @@ static const enum index action_queue[] = {
>
> static const enum index action_count[] = {
> ACTION_COUNT_ID,
> - ACTION_COUNT_SHARED,
> ACTION_NEXT,
> ZERO,
> };
> @@ -3712,14 +3710,6 @@ static const struct token token_list[] = {
> .args = ARGS(ARGS_ENTRY(struct rte_flow_action_count, id)),
> .call = parse_vc_conf,
> },
> - [ACTION_COUNT_SHARED] = {
> - .name = "shared",
> - .help = "shared counter",
> - .next = NEXT(action_count, NEXT_ENTRY(COMMON_BOOLEAN)),
> - .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_action_count,
> - shared, 1)),
> - .call = parse_vc_conf,
> - },
> [ACTION_RSS] = {
> .name = "rss",
> .help = "spread packets among several queues", diff --git
> a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 2b42d5ec8c..3cb014c1fa 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -158,7 +158,7 @@ Several pattern items and actions are valid and can be
> used in both directions. At least one direction must be specified.
>
> Specifying both directions at once for a given rule is not recommended but -
> may be valid in a few cases (e.g. shared counters).
> +may be valid in a few cases.
>
> Attribute: Transfer
> ^^^^^^^^^^^^^^^^^^^
> @@ -1491,9 +1491,7 @@ Actions are performed in list order:
> +=======+========+============+=======+
> | 0 | MARK | ``mark`` | 0x2a |
> +-------+--------+------------+-------+
> - | 1 | COUNT | ``shared`` | 0 |
> - | | +------------+-------+
> - | | | ``id`` | 0 |
> + | 1 | COUNT | ``id`` | 0 |
> +-------+--------+------------+-------+
> | 2 | QUEUE | ``queue`` | 10 |
> +-------+--------+------------+-------+
> @@ -1734,20 +1732,9 @@ action must specify a unique id.
> Counters can be retrieved and reset through ``rte_flow_query()``, see
> ``struct rte_flow_query_count``.
>
> -The shared flag indicates whether the counter is unique to the flow rule the
> -action is specified with, or whether it is a shared counter.
> -
> -For a count action with the shared flag set, then a global device -namespace
> is assumed for the counter id, so that any matched flow rules using -a count
> action with the same counter id on the same port will contribute to -that
> counter.
> -
> For ports within the same switch domain then the counter id namespace
> extends to all ports within that switch domain.
>
> -The shared flag is DEPRECATED and ``INDIRECT`` ``COUNT`` action should be
> used -to make shared counters.
> -
> .. _table_rte_flow_action_count:
>
> .. table:: COUNT
> @@ -1755,8 +1742,6 @@ to make shared counters.
> +------------+---------------------------------+
> | Field | Value |
> +============+=================================+
> - | ``shared`` | DEPRECATED, shared counter flag |
> - +------------+---------------------------------+
> | ``id`` | counter id |
> +------------+---------------------------------+
>
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 59445a6f42..21ef39841f 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -124,10 +124,6 @@ Deprecation Notices
> to support modifying fields larger than 64 bits.
> In addition, documentation will be updated to clarify byte order.
>
> -* ethdev: Attribute ``shared`` of the ``struct rte_flow_action_count``
> - is deprecated and will be removed in DPDK 21.11. Shared counters should
> - be managed using shared actions API (``rte_flow_shared_action_create``
> etc).
> -
> * ethdev: Definition of the flow API action
> ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> is ambiguous and needs clarification.
> Structure ``rte_flow_action_port_id`` will be extended to specify diff --git
> a/doc/guides/rel_notes/release_21_11.rst
> b/doc/guides/rel_notes/release_21_11.rst
> index a84c912f20..0d91ad5d7b 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -126,6 +126,10 @@ Removed Items
> blacklist/whitelist are removed. Users must use the new
> block/allow list arguments.
>
> +* ethdev: Removed deprecated ``shared`` attribute of the
> + ``struct rte_flow_action_count``. Shared counters should be managed
> + using indirect actions API (``rte_flow_action_handle_create`` etc).
> +
>
> API Changes
> -----------
> diff --git a/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
> b/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
> index 3a9c9bba27..f1e270af8b 100644
> --- a/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
> +++ b/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
> @@ -2111,11 +2111,6 @@ ulp_rte_count_act_handler(const struct
> rte_flow_action *action_item,
>
> act_count = action_item->conf;
> if (act_count) {
> - if (act_count->shared) {
> - BNXT_TF_DBG(ERR,
> - "Parse Error:Shared count not supported\n");
> - return BNXT_TF_RC_PARSE_ERR;
> - }
> memcpy(&act_prop-
> >act_details[BNXT_ULP_ACT_PROP_IDX_COUNT],
> &act_count->id,
> BNXT_ULP_ACT_PROP_SZ_COUNT); diff --git
> a/drivers/net/cnxk/cnxk_rte_flow.c b/drivers/net/cnxk/cnxk_rte_flow.c
> index 32c1b5dee5..27defd2fa9 100644
> --- a/drivers/net/cnxk/cnxk_rte_flow.c
> +++ b/drivers/net/cnxk/cnxk_rte_flow.c
> @@ -110,7 +110,6 @@ cnxk_map_actions(struct rte_eth_dev *eth_dev,
> const struct rte_flow_attr *attr,
> struct roc_npc_action in_actions[], uint32_t *flowkey_cfg) {
> struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
> - const struct rte_flow_action_count *act_count;
> const struct rte_flow_action_queue *act_q;
> int i = 0, rc = 0;
> int rq;
> @@ -131,13 +130,6 @@ cnxk_map_actions(struct rte_eth_dev *eth_dev,
> const struct rte_flow_attr *attr,
> break;
>
> case RTE_FLOW_ACTION_TYPE_COUNT:
> - act_count = (const struct rte_flow_action_count *)
> - actions->conf;
> -
> - if (act_count->shared == 1) {
> - plt_npc_dbg("Shared counter is not supported");
> - goto err_exit;
> - }
> in_actions[i].type = ROC_NPC_ACTION_TYPE_COUNT;
> break;
>
> diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
> index 841e0b9da3..fe1a387526 100644
> --- a/drivers/net/hns3/hns3_flow.c
> +++ b/drivers/net/hns3/hns3_flow.c
> @@ -1782,8 +1782,7 @@ hns3_flow_create(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
> goto out;
>
> if (fdir_rule.flags & HNS3_RULE_FLAG_COUNTER) {
> - ret = hns3_counter_new(dev, fdir_rule.act_cnt.shared,
> - fdir_rule.act_cnt.id, error);
> + ret = hns3_counter_new(dev, 0, fdir_rule.act_cnt.id,
> + error);
> if (ret)
> goto out;
>
> diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
> index e0cca7cb3c..14e4e069c4 100644
> --- a/drivers/net/ice/ice_fdir_filter.c
> +++ b/drivers/net/ice/ice_fdir_filter.c
> @@ -1339,9 +1339,7 @@ ice_fdir_create_filter(struct ice_adapter *ad,
> if (filter->input.cnt_ena) {
> struct rte_flow_action_count *act_count = &filter->act_count;
>
> - filter->counter = ice_fdir_counter_alloc(pf,
> - act_count->shared,
> - act_count->id);
> + filter->counter = ice_fdir_counter_alloc(pf, 0,
> + act_count->id);
> if (!filter->counter) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ACTION, NULL, diff --git
> a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> 3581414b78..ba48a0fee2 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -324,7 +324,6 @@ struct mlx5_lb_ctx { #define
> MLX5_MAX_PENDING_QUERIES 4 #define MLX5_CNT_CONTAINER_RESIZE
> 64 #define MLX5_CNT_SHARED_OFFSET 0x80000000 -#define
> IS_LEGACY_SHARED_CNT(cnt) (!!((cnt) & MLX5_CNT_SHARED_OFFSET))
> #define IS_BATCH_CNT(cnt) (((cnt) & (MLX5_CNT_SHARED_OFFSET - 1)) >= \
> MLX5_CNT_BATCH_OFFSET) #define MLX5_CNT_SIZE
> (sizeof(struct mlx5_flow_counter)) @@ -392,12 +391,6 @@ struct
> mlx5_flow_counter_shared {
> };
> };
>
> -/* Shared counter configuration. */
> -struct mlx5_shared_counter_conf {
> - struct rte_eth_dev *dev; /* The device shared counter belongs to. */
> - uint32_t id; /* The shared counter ID. */
> -};
> -
> struct mlx5_flow_counter_pool;
> /* Generic counters information. */
> struct mlx5_flow_counter {
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index b610ad3ef4..91314d5ea5 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3308,33 +3308,11 @@ flow_dv_validate_action_set_tag(struct
> rte_eth_dev *dev,
> return 0;
> }
>
> -/**
> - * Check if action counter is shared by either old or new mechanism.
> - *
> - * @param[in] action
> - * Pointer to the action structure.
> - *
> - * @return
> - * True when counter is shared, false otherwise.
> - */
> -static inline bool
> -is_shared_action_count(const struct rte_flow_action *action) -{
> - const struct rte_flow_action_count *count =
> - (const struct rte_flow_action_count *)action->conf;
> -
> - if ((int)action->type == MLX5_RTE_FLOW_ACTION_TYPE_COUNT)
> - return true;
> - return !!(count && count->shared);
> -}
> -
> /**
> * Validate count action.
> *
> * @param[in] dev
> * Pointer to rte_eth_dev structure.
> - * @param[in] shared
> - * Indicator if action is shared.
> * @param[in] action_flags
> * Holds the actions detected until now.
> * @param[out] error
> @@ -3344,7 +3322,7 @@ is_shared_action_count(const struct
> rte_flow_action *action)
> * 0 on success, a negative errno value otherwise and rte_errno is set.
> */
> static int
> -flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
> +flow_dv_validate_action_count(struct rte_eth_dev *dev,
> uint64_t action_flags,
> struct rte_flow_error *error) { @@ -3356,11 +3334,6 @@
> flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
> return rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> "duplicate count actions set");
> - if (shared && (action_flags & MLX5_FLOW_ACTION_AGE) &&
This check is relevant to the indirect action case in some places, see below.
> - !priv->sh->flow_hit_aso_en)
> - return rte_flow_error_set(error, EINVAL,
> - RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> - "old age and shared count combination is not
> supported");
> #ifdef HAVE_IBV_FLOW_DEVX_COUNTERS
> return 0;
> #endif
> @@ -5658,9 +5631,7 @@ flow_dv_validate_action_sample(uint64_t
> *action_flags,
> break;
> case RTE_FLOW_ACTION_TYPE_COUNT:
> ret = flow_dv_validate_action_count
> - (dev, is_shared_action_count(act),
> - *action_flags | sub_action_flags,
> - error);
> + (dev, *action_flags | sub_action_flags,
> + error);
> if (ret < 0)
> return ret;
> *count = act->conf; @@ -6230,60 +6201,6 @@
> flow_dv_counter_alloc(struct rte_eth_dev *dev, uint32_t age)
> return 0;
> }
>
> -/**
> - * Allocate a shared flow counter.
> - *
> - * @param[in] ctx
> - * Pointer to the shared counter configuration.
> - * @param[in] data
> - * Pointer to save the allocated counter index.
> - *
> - * @return
> - * Index to flow counter on success, 0 otherwise and rte_errno is set.
> - */
> -
> -static int32_t
> -flow_dv_counter_alloc_shared_cb(void *ctx, union mlx5_l3t_data *data) -{
> - struct mlx5_shared_counter_conf *conf = ctx;
> - struct rte_eth_dev *dev = conf->dev;
> - struct mlx5_flow_counter *cnt;
> -
> - data->dword = flow_dv_counter_alloc(dev, 0);
> - data->dword |= MLX5_CNT_SHARED_OFFSET;
> - cnt = flow_dv_counter_get_by_idx(dev, data->dword, NULL);
> - cnt->shared_info.id = conf->id;
> - return 0;
> -}
> -
> -/**
> - * Get a shared flow counter.
> - *
> - * @param[in] dev
> - * Pointer to the Ethernet device structure.
> - * @param[in] id
> - * Counter identifier.
> - *
> - * @return
> - * Index to flow counter on success, 0 otherwise and rte_errno is set.
> - */
> -static uint32_t
> -flow_dv_counter_get_shared(struct rte_eth_dev *dev, uint32_t id) -{
> - struct mlx5_priv *priv = dev->data->dev_private;
> - struct mlx5_shared_counter_conf conf = {
> - .dev = dev,
> - .id = id,
> - };
> - union mlx5_l3t_data data = {
> - .dword = 0,
> - };
> -
> - mlx5_l3t_prepare_entry(priv->sh->cnt_id_tbl, id, &data,
> - flow_dv_counter_alloc_shared_cb, &conf);
> - return data.dword;
> -}
> -
Need to remove cnt_id_tbl and from sh and all of its management, no?
> /**
> * Get age param from counter index.
> *
> @@ -6366,16 +6283,6 @@ flow_dv_counter_free(struct rte_eth_dev *dev,
> uint32_t counter)
> if (pool->is_aged) {
> flow_dv_counter_remove_from_age(dev, counter, cnt);
> } else {
> - /*
> - * If the counter action is shared by ID, the l3t_clear_entry
> - * function reduces its references counter. If after the
> - * reduction the action is still referenced, the function
> - * returns here and does not release it.
> - */
> - if (IS_LEGACY_SHARED_CNT(counter) &&
> - mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl,
> - cnt->shared_info.id))
> - return;
> /*
> * If the counter action is shared by indirect action API,
> * the atomic function reduces its references counter.
> @@ -6385,8 +6292,7 @@ flow_dv_counter_free(struct rte_eth_dev *dev,
> uint32_t counter)
You can remove the ID sharing notice in this comment.
> * indirect action API, shared info is 1 before the reduction,
> * so this condition is failed and function doesn't return here.
> */
> - if (!IS_LEGACY_SHARED_CNT(counter) &&
> - __atomic_sub_fetch(&cnt->shared_info.refcnt, 1,
> + if (__atomic_sub_fetch(&cnt->shared_info.refcnt, 1,
> __ATOMIC_RELAXED))
> return;
> }
> @@ -7275,7 +7181,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
> }
> for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
> int type = actions->type;
> - bool shared_count = false;
>
> if (!mlx5_flow_os_action_supported(type))
> return rte_flow_error_set(error, ENOTSUP, @@ -7427,8 +7332,7
> @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr
> *attr,
> break;
> case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
> case RTE_FLOW_ACTION_TYPE_COUNT:
> - shared_count = is_shared_action_count(actions);
We need shared_count for the indirect count case(which can be shared with different flows),
So, if the action is MLX5_RTE_FLOW_ACTION_TYPE_COUNT, shared_count should be true.
It is relevant to the validate function below and for the check at the end of the function.
> - ret = flow_dv_validate_action_count(dev, shared_count,
> + ret = flow_dv_validate_action_count(dev,
> action_flags,
> error);
> if (ret < 0)
> @@ -7747,12 +7651,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
> * mutual exclusion with share counter actions.
> */
> if (!priv->sh->flow_hit_aso_en) {
> - if (shared_count)
> - return rte_flow_error_set
> - (error, EINVAL,
> - RTE_FLOW_ERROR_TYPE_ACTION,
> - NULL,
> - "old age and shared count combination is not
> supported");
> if (sample_count)
> return rte_flow_error_set
> (error, EINVAL, @@ -10837,16 +10735,14 @@
> flow_dv_translate_action_port_id(struct rte_eth_dev *dev, static uint32_t
> flow_dv_translate_create_counter(struct rte_eth_dev *dev,
> struct mlx5_flow *dev_flow,
> - const struct rte_flow_action_count *count,
> + const struct rte_flow_action_count *count
> + __rte_unused,
> const struct rte_flow_action_age *age) {
> uint32_t counter;
> struct mlx5_age_param *age_param;
>
> - if (count && count->shared)
> - counter = flow_dv_counter_get_shared(dev, count->id);
> - else
> - counter = flow_dv_counter_alloc(dev, !!age);
> + counter = flow_dv_counter_alloc(dev, !!age);
> if (!counter || age == NULL)
> return counter;
> age_param = flow_dv_counter_idx_get_age(dev, counter); @@ -
> 13216,8 +13112,7 @@ flow_dv_translate(struct rte_eth_dev *dev,
> * when they are not shared.
> */
> if (action_flags & MLX5_FLOW_ACTION_AGE) {
> - if ((non_shared_age &&
> - count && !count->shared) ||
> + if ((non_shared_age && count) ||
> !(priv->sh->flow_hit_aso_en &&
> (attr->group || attr->transfer))) {
> /* Creates age by counters. */ @@ -17469,19 +17364,7
> @@ flow_dv_action_validate(struct rte_eth_dev *dev,
> "Indirect age action not supported");
> return flow_dv_validate_action_age(0, action, dev, err);
> case RTE_FLOW_ACTION_TYPE_COUNT:
> - /*
> - * There are two mechanisms to share the action count.
> - * The old mechanism uses the shared field to share, while the
> - * new mechanism uses the indirect action API.
> - * This validation comes to make sure that the two mechanisms
> - * are not combined.
> - */
> - if (is_shared_action_count(action))
> - return rte_flow_error_set(err, ENOTSUP,
> - RTE_FLOW_ERROR_TYPE_ACTION,
> - NULL,
> - "Mix shared and indirect counter is not supported");
> - return flow_dv_validate_action_count(dev, true, 0, err);
> + return flow_dv_validate_action_count(dev, 0, err);
Did you also consider improving struct mlx5_flow_counter_shared? Then, maybe we don't need it anymore.
> case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> if (!priv->sh->ct_aso_en)
> return rte_flow_error_set(err, ENOTSUP, diff --git
> a/drivers/net/mlx5/mlx5_flow_verbs.c
> b/drivers/net/mlx5/mlx5_flow_verbs.c
> index b93fd4d2c9..1627c3905f 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -250,8 +250,6 @@ flow_verbs_counter_create(struct rte_eth_dev
> *dev,
> *
> * @param[in] dev
> * Pointer to the Ethernet device structure.
> - * @param[in] shared
> - * Indicate if this counter is shared with other flows.
> * @param[in] id
> * Counter identifier.
> *
> @@ -259,21 +257,17 @@ flow_verbs_counter_create(struct rte_eth_dev
> *dev,
> * Index to the counter, 0 otherwise and rte_errno is set.
> */
> static uint32_t
> -flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared,
> uint32_t id)
> +flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t id
> +__rte_unused)
> {
> struct mlx5_priv *priv = dev->data->dev_private;
> struct mlx5_flow_counter_mng *cmng = &priv->sh->cmng;
> struct mlx5_flow_counter_pool *pool = NULL;
> struct mlx5_flow_counter *cnt = NULL;
> - union mlx5_l3t_data data;
> uint32_t n_valid = cmng->n_valid;
> uint32_t pool_idx, cnt_idx;
> uint32_t i;
> int ret;
>
> - if (shared && !mlx5_l3t_get_entry(priv->sh->cnt_id_tbl, id, &data) &&
> - data.dword)
> - return data.dword;
> for (pool_idx = 0; pool_idx < n_valid; ++pool_idx) {
> pool = cmng->pools[pool_idx];
> if (!pool)
> @@ -320,13 +314,6 @@ flow_verbs_counter_new(struct rte_eth_dev *dev,
> uint32_t shared, uint32_t id)
> TAILQ_REMOVE(&pool->counters[0], cnt, next);
> i = MLX5_CNT_ARRAY_IDX(pool, cnt);
> cnt_idx = MLX5_MAKE_CNT_IDX(pool_idx, i);
> - if (shared) {
> - data.dword = cnt_idx;
> - if (mlx5_l3t_set_entry(priv->sh->cnt_id_tbl, id, &data))
> - return 0;
> - cnt->shared_info.id = id;
> - cnt_idx |= MLX5_CNT_SHARED_OFFSET;
> - }
> /* Create counter with Verbs. */
> ret = flow_verbs_counter_create(dev, cnt);
> if (!ret) {
> @@ -352,14 +339,10 @@ flow_verbs_counter_new(struct rte_eth_dev
> *dev, uint32_t shared, uint32_t id) static void
> flow_verbs_counter_release(struct rte_eth_dev *dev, uint32_t counter) {
> - struct mlx5_priv *priv = dev->data->dev_private;
> struct mlx5_flow_counter_pool *pool;
> struct mlx5_flow_counter *cnt;
>
> cnt = flow_verbs_counter_get_by_idx(dev, counter, &pool);
> - if (IS_LEGACY_SHARED_CNT(counter) &&
> - mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl, cnt->shared_info.id))
> - return;
> #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
> claim_zero(mlx5_glue->destroy_counter_set
> ((struct ibv_counter_set *)cnt->dcs_when_active)); @@ -1198,8
> +1181,7 @@ flow_verbs_translate_action_count(struct mlx5_flow
> *dev_flow, #endif
>
> if (!flow->counter) {
> - flow->counter = flow_verbs_counter_new(dev, count->shared,
> - count->id);
> + flow->counter = flow_verbs_counter_new(dev, count->id);
> if (!flow->counter)
> return rte_flow_error_set(error, rte_errno,
> RTE_FLOW_ERROR_TYPE_ACTION, diff --git
> a/drivers/net/octeontx2/otx2_flow_parse.c
> b/drivers/net/octeontx2/otx2_flow_parse.c
> index 63a33142a5..30a232f033 100644
> --- a/drivers/net/octeontx2/otx2_flow_parse.c
> +++ b/drivers/net/octeontx2/otx2_flow_parse.c
> @@ -901,7 +901,6 @@ otx2_flow_parse_actions(struct rte_eth_dev *dev,
> struct otx2_eth_dev *hw = dev->data->dev_private;
> struct otx2_npc_flow_info *npc = &hw->npc_flow;
> const struct rte_flow_action_port_id *port_act;
> - const struct rte_flow_action_count *act_count;
> const struct rte_flow_action_mark *act_mark;
> const struct rte_flow_action_queue *act_q;
> const struct rte_flow_action_vf *vf_act; @@ -947,15 +946,6 @@
> otx2_flow_parse_actions(struct rte_eth_dev *dev,
> break;
>
> case RTE_FLOW_ACTION_TYPE_COUNT:
> - act_count =
> - (const struct rte_flow_action_count *)
> - actions->conf;
> -
> - if (act_count->shared == 1) {
> - errmsg = "Shared Counters not supported";
> - errcode = ENOTSUP;
> - goto err_exit;
> - }
> /* Indicates, need a counter */
> flow->ctr_id = 1;
> req_act |= OTX2_FLOW_ACT_COUNT; diff --git
> a/drivers/net/sfc/sfc_mae.c b/drivers/net/sfc/sfc_mae.c index
> 4b520bc619..5fcdf9c2f5 100644
> --- a/drivers/net/sfc/sfc_mae.c
> +++ b/drivers/net/sfc/sfc_mae.c
> @@ -2802,16 +2802,12 @@ sfc_mae_rule_parse_action_mark(struct
> sfc_adapter *sa,
>
> static int
> sfc_mae_rule_parse_action_count(struct sfc_adapter *sa,
> - const struct rte_flow_action_count *conf,
> + const struct rte_flow_action_count *conf
> + __rte_unused,
> efx_mae_actions_t *spec) {
> int rc;
>
> - if (conf->shared) {
> - rc = ENOTSUP;
> - goto fail_counter_shared;
> - }
> -
> if ((sa->counter_rxq.state & SFC_COUNTER_RXQ_INITIALIZED) == 0) {
> sfc_err(sa,
> "counter queue is not configured for COUNT action"); @@ -
> 2837,7 +2833,6 @@ sfc_mae_rule_parse_action_count(struct sfc_adapter
> *sa,
> fail_populate_count:
> fail_no_service_core:
> fail_counter_queue_uninit:
> -fail_counter_shared:
>
> return rc;
> }
> diff --git a/drivers/net/softnic/rte_eth_softnic_flow.c
> b/drivers/net/softnic/rte_eth_softnic_flow.c
> index 27eaf380cd..39038d26d8 100644
> --- a/drivers/net/softnic/rte_eth_softnic_flow.c
> +++ b/drivers/net/softnic/rte_eth_softnic_flow.c
> @@ -1448,13 +1448,6 @@ flow_rule_action_get(struct pmd_internals
> *softnic,
> action,
> "COUNT: Null configuration");
>
> - if (conf->shared)
> - return rte_flow_error_set(error,
> - ENOTSUP,
> - RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> - conf,
> - "COUNT: Shared counters not supported");
> -
> if (n_count)
> return rte_flow_error_set(error,
> ENOTSUP, diff --git a/lib/ethdev/rte_flow.h
> b/lib/ethdev/rte_flow.h index 7b1ed7f110..5306e8ca4b 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -75,7 +75,7 @@ extern "C" {
> * At least one direction must be specified.
> *
> * Specifying both directions at once for a given rule is not recommended
> - * but may be valid in a few cases (e.g. shared counter).
> + * but may be valid in a few cases.
> */
> struct rte_flow_attr {
> uint32_t group; /**< Priority group. */ @@ -2498,24 +2498,11 @@ struct
> rte_flow_query_age {
> * Counters can be retrieved and reset through ``rte_flow_query()``, see
> * ``struct rte_flow_query_count``.
> *
> - * @deprecated Shared attribute is deprecated, use generic
> - * RTE_FLOW_ACTION_TYPE_INDIRECT action.
> - *
> - * The shared flag indicates whether the counter is unique to the flow rule
> the
> - * action is specified with, or whether it is a shared counter.
> - *
> - * For a count action with the shared flag set, then then a global device
> - * namespace is assumed for the counter id, so that any matched flow rules
> using
> - * a count action with the same counter id on the same port will contribute
> to
> - * that counter.
> - *
> * For ports within the same switch domain then the counter id namespace
> extends
> * to all ports within that switch domain.
> */
> struct rte_flow_action_count {
> - /** @deprecated Share counter ID with other flow rules. */
> - uint32_t shared:1;
> - uint32_t reserved:31; /**< Reserved, must be zero. */
> + uint32_t reserved; /**< Reserved, must be zero. */
> uint32_t id; /**< Counter ID. */ };
>
> --
> 2.30.2
On Tue, Sep 28, 2021 at 8:53 PM Andrew Rybchenko
<andrew.rybchenko@oktetlabs.ru> wrote:
>
> Indirect actions should be used to do shared counters.
>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
> app/test-pmd/cmdline_flow.c | 10 --
> doc/guides/prog_guide/rte_flow.rst | 19 +--
> doc/guides/rel_notes/deprecation.rst | 4 -
> doc/guides/rel_notes/release_21_11.rst | 4 +
> drivers/net/bnxt/tf_ulp/ulp_rte_parser.c | 5 -
> drivers/net/cnxk/cnxk_rte_flow.c | 8 --
> drivers/net/hns3/hns3_flow.c | 3 +-
> drivers/net/ice/ice_fdir_filter.c | 4 +-
> drivers/net/mlx5/mlx5.h | 7 --
> drivers/net/mlx5/mlx5_flow_dv.c | 135 ++-------------------
> drivers/net/mlx5/mlx5_flow_verbs.c | 22 +---
> drivers/net/octeontx2/otx2_flow_parse.c | 10 --
> drivers/net/sfc/sfc_mae.c | 9 +-
> drivers/net/softnic/rte_eth_softnic_flow.c | 7 --
> lib/ethdev/rte_flow.h | 17 +--
> 15 files changed, 23 insertions(+), 241 deletions(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index bb22294dd3..0b5856c7d5 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -322,7 +322,6 @@ enum index {
> ACTION_QUEUE_INDEX,
> ACTION_DROP,
> ACTION_COUNT,
> - ACTION_COUNT_SHARED,
> ACTION_COUNT_ID,
> ACTION_RSS,
> ACTION_RSS_FUNC,
> @@ -1451,7 +1450,6 @@ static const enum index action_queue[] = {
>
> static const enum index action_count[] = {
> ACTION_COUNT_ID,
> - ACTION_COUNT_SHARED,
> ACTION_NEXT,
> ZERO,
> };
> @@ -3712,14 +3710,6 @@ static const struct token token_list[] = {
> .args = ARGS(ARGS_ENTRY(struct rte_flow_action_count, id)),
> .call = parse_vc_conf,
> },
> - [ACTION_COUNT_SHARED] = {
> - .name = "shared",
> - .help = "shared counter",
> - .next = NEXT(action_count, NEXT_ENTRY(COMMON_BOOLEAN)),
> - .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_action_count,
> - shared, 1)),
> - .call = parse_vc_conf,
> - },
> [ACTION_RSS] = {
> .name = "rss",
> .help = "spread packets among several queues",
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 2b42d5ec8c..3cb014c1fa 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -158,7 +158,7 @@ Several pattern items and actions are valid and can be used in both
> directions. At least one direction must be specified.
>
> Specifying both directions at once for a given rule is not recommended but
> -may be valid in a few cases (e.g. shared counters).
> +may be valid in a few cases.
>
> Attribute: Transfer
> ^^^^^^^^^^^^^^^^^^^
> @@ -1491,9 +1491,7 @@ Actions are performed in list order:
> +=======+========+============+=======+
> | 0 | MARK | ``mark`` | 0x2a |
> +-------+--------+------------+-------+
> - | 1 | COUNT | ``shared`` | 0 |
> - | | +------------+-------+
> - | | | ``id`` | 0 |
> + | 1 | COUNT | ``id`` | 0 |
> +-------+--------+------------+-------+
> | 2 | QUEUE | ``queue`` | 10 |
> +-------+--------+------------+-------+
> @@ -1734,20 +1732,9 @@ action must specify a unique id.
> Counters can be retrieved and reset through ``rte_flow_query()``, see
> ``struct rte_flow_query_count``.
>
> -The shared flag indicates whether the counter is unique to the flow rule the
> -action is specified with, or whether it is a shared counter.
> -
> -For a count action with the shared flag set, then a global device
> -namespace is assumed for the counter id, so that any matched flow rules using
> -a count action with the same counter id on the same port will contribute to
> -that counter.
> -
> For ports within the same switch domain then the counter id namespace extends
> to all ports within that switch domain.
>
> -The shared flag is DEPRECATED and ``INDIRECT`` ``COUNT`` action should be used
> -to make shared counters.
> -
> .. _table_rte_flow_action_count:
>
> .. table:: COUNT
> @@ -1755,8 +1742,6 @@ to make shared counters.
> +------------+---------------------------------+
> | Field | Value |
> +============+=================================+
> - | ``shared`` | DEPRECATED, shared counter flag |
> - +------------+---------------------------------+
> | ``id`` | counter id |
> +------------+---------------------------------+
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 59445a6f42..21ef39841f 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -124,10 +124,6 @@ Deprecation Notices
> to support modifying fields larger than 64 bits.
> In addition, documentation will be updated to clarify byte order.
>
> -* ethdev: Attribute ``shared`` of the ``struct rte_flow_action_count``
> - is deprecated and will be removed in DPDK 21.11. Shared counters should
> - be managed using shared actions API (``rte_flow_shared_action_create`` etc).
> -
> * ethdev: Definition of the flow API action ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> is ambiguous and needs clarification.
> Structure ``rte_flow_action_port_id`` will be extended to specify
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index a84c912f20..0d91ad5d7b 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -126,6 +126,10 @@ Removed Items
> blacklist/whitelist are removed. Users must use the new
> block/allow list arguments.
>
> +* ethdev: Removed deprecated ``shared`` attribute of the
> + ``struct rte_flow_action_count``. Shared counters should be managed
> + using indirect actions API (``rte_flow_action_handle_create`` etc).
> +
>
> API Changes
> -----------
> diff --git a/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c b/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
> index 3a9c9bba27..f1e270af8b 100644
> --- a/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
> +++ b/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
> @@ -2111,11 +2111,6 @@ ulp_rte_count_act_handler(const struct rte_flow_action *action_item,
>
> act_count = action_item->conf;
> if (act_count) {
> - if (act_count->shared) {
> - BNXT_TF_DBG(ERR,
> - "Parse Error:Shared count not supported\n");
> - return BNXT_TF_RC_PARSE_ERR;
> - }
> memcpy(&act_prop->act_details[BNXT_ULP_ACT_PROP_IDX_COUNT],
> &act_count->id,
> BNXT_ULP_ACT_PROP_SZ_COUNT);
> diff --git a/drivers/net/cnxk/cnxk_rte_flow.c b/drivers/net/cnxk/cnxk_rte_flow.c
> index 32c1b5dee5..27defd2fa9 100644
> --- a/drivers/net/cnxk/cnxk_rte_flow.c
> +++ b/drivers/net/cnxk/cnxk_rte_flow.c
> @@ -110,7 +110,6 @@ cnxk_map_actions(struct rte_eth_dev *eth_dev, const struct rte_flow_attr *attr,
> struct roc_npc_action in_actions[], uint32_t *flowkey_cfg)
> {
> struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
> - const struct rte_flow_action_count *act_count;
> const struct rte_flow_action_queue *act_q;
> int i = 0, rc = 0;
> int rq;
> @@ -131,13 +130,6 @@ cnxk_map_actions(struct rte_eth_dev *eth_dev, const struct rte_flow_attr *attr,
> break;
>
> case RTE_FLOW_ACTION_TYPE_COUNT:
> - act_count = (const struct rte_flow_action_count *)
> - actions->conf;
> -
> - if (act_count->shared == 1) {
> - plt_npc_dbg("Shared counter is not supported");
> - goto err_exit;
> - }
> in_actions[i].type = ROC_NPC_ACTION_TYPE_COUNT;
> break;
>
> diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
> index 841e0b9da3..fe1a387526 100644
> --- a/drivers/net/hns3/hns3_flow.c
> +++ b/drivers/net/hns3/hns3_flow.c
> @@ -1782,8 +1782,7 @@ hns3_flow_create(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
> goto out;
>
> if (fdir_rule.flags & HNS3_RULE_FLAG_COUNTER) {
> - ret = hns3_counter_new(dev, fdir_rule.act_cnt.shared,
> - fdir_rule.act_cnt.id, error);
> + ret = hns3_counter_new(dev, 0, fdir_rule.act_cnt.id, error);
> if (ret)
> goto out;
>
> diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
> index e0cca7cb3c..14e4e069c4 100644
> --- a/drivers/net/ice/ice_fdir_filter.c
> +++ b/drivers/net/ice/ice_fdir_filter.c
> @@ -1339,9 +1339,7 @@ ice_fdir_create_filter(struct ice_adapter *ad,
> if (filter->input.cnt_ena) {
> struct rte_flow_action_count *act_count = &filter->act_count;
>
> - filter->counter = ice_fdir_counter_alloc(pf,
> - act_count->shared,
> - act_count->id);
> + filter->counter = ice_fdir_counter_alloc(pf, 0, act_count->id);
> if (!filter->counter) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index 3581414b78..ba48a0fee2 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -324,7 +324,6 @@ struct mlx5_lb_ctx {
> #define MLX5_MAX_PENDING_QUERIES 4
> #define MLX5_CNT_CONTAINER_RESIZE 64
> #define MLX5_CNT_SHARED_OFFSET 0x80000000
> -#define IS_LEGACY_SHARED_CNT(cnt) (!!((cnt) & MLX5_CNT_SHARED_OFFSET))
> #define IS_BATCH_CNT(cnt) (((cnt) & (MLX5_CNT_SHARED_OFFSET - 1)) >= \
> MLX5_CNT_BATCH_OFFSET)
> #define MLX5_CNT_SIZE (sizeof(struct mlx5_flow_counter))
> @@ -392,12 +391,6 @@ struct mlx5_flow_counter_shared {
> };
> };
>
> -/* Shared counter configuration. */
> -struct mlx5_shared_counter_conf {
> - struct rte_eth_dev *dev; /* The device shared counter belongs to. */
> - uint32_t id; /* The shared counter ID. */
> -};
> -
> struct mlx5_flow_counter_pool;
> /* Generic counters information. */
> struct mlx5_flow_counter {
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index b610ad3ef4..91314d5ea5 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3308,33 +3308,11 @@ flow_dv_validate_action_set_tag(struct rte_eth_dev *dev,
> return 0;
> }
>
> -/**
> - * Check if action counter is shared by either old or new mechanism.
> - *
> - * @param[in] action
> - * Pointer to the action structure.
> - *
> - * @return
> - * True when counter is shared, false otherwise.
> - */
> -static inline bool
> -is_shared_action_count(const struct rte_flow_action *action)
> -{
> - const struct rte_flow_action_count *count =
> - (const struct rte_flow_action_count *)action->conf;
> -
> - if ((int)action->type == MLX5_RTE_FLOW_ACTION_TYPE_COUNT)
> - return true;
> - return !!(count && count->shared);
> -}
> -
> /**
> * Validate count action.
> *
> * @param[in] dev
> * Pointer to rte_eth_dev structure.
> - * @param[in] shared
> - * Indicator if action is shared.
> * @param[in] action_flags
> * Holds the actions detected until now.
> * @param[out] error
> @@ -3344,7 +3322,7 @@ is_shared_action_count(const struct rte_flow_action *action)
> * 0 on success, a negative errno value otherwise and rte_errno is set.
> */
> static int
> -flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
> +flow_dv_validate_action_count(struct rte_eth_dev *dev,
> uint64_t action_flags,
> struct rte_flow_error *error)
> {
> @@ -3356,11 +3334,6 @@ flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
> return rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> "duplicate count actions set");
> - if (shared && (action_flags & MLX5_FLOW_ACTION_AGE) &&
> - !priv->sh->flow_hit_aso_en)
> - return rte_flow_error_set(error, EINVAL,
> - RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> - "old age and shared count combination is not supported");
> #ifdef HAVE_IBV_FLOW_DEVX_COUNTERS
> return 0;
> #endif
> @@ -5658,9 +5631,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
> break;
> case RTE_FLOW_ACTION_TYPE_COUNT:
> ret = flow_dv_validate_action_count
> - (dev, is_shared_action_count(act),
> - *action_flags | sub_action_flags,
> - error);
> + (dev, *action_flags | sub_action_flags, error);
> if (ret < 0)
> return ret;
> *count = act->conf;
> @@ -6230,60 +6201,6 @@ flow_dv_counter_alloc(struct rte_eth_dev *dev, uint32_t age)
> return 0;
> }
>
> -/**
> - * Allocate a shared flow counter.
> - *
> - * @param[in] ctx
> - * Pointer to the shared counter configuration.
> - * @param[in] data
> - * Pointer to save the allocated counter index.
> - *
> - * @return
> - * Index to flow counter on success, 0 otherwise and rte_errno is set.
> - */
> -
> -static int32_t
> -flow_dv_counter_alloc_shared_cb(void *ctx, union mlx5_l3t_data *data)
> -{
> - struct mlx5_shared_counter_conf *conf = ctx;
> - struct rte_eth_dev *dev = conf->dev;
> - struct mlx5_flow_counter *cnt;
> -
> - data->dword = flow_dv_counter_alloc(dev, 0);
> - data->dword |= MLX5_CNT_SHARED_OFFSET;
> - cnt = flow_dv_counter_get_by_idx(dev, data->dword, NULL);
> - cnt->shared_info.id = conf->id;
> - return 0;
> -}
> -
> -/**
> - * Get a shared flow counter.
> - *
> - * @param[in] dev
> - * Pointer to the Ethernet device structure.
> - * @param[in] id
> - * Counter identifier.
> - *
> - * @return
> - * Index to flow counter on success, 0 otherwise and rte_errno is set.
> - */
> -static uint32_t
> -flow_dv_counter_get_shared(struct rte_eth_dev *dev, uint32_t id)
> -{
> - struct mlx5_priv *priv = dev->data->dev_private;
> - struct mlx5_shared_counter_conf conf = {
> - .dev = dev,
> - .id = id,
> - };
> - union mlx5_l3t_data data = {
> - .dword = 0,
> - };
> -
> - mlx5_l3t_prepare_entry(priv->sh->cnt_id_tbl, id, &data,
> - flow_dv_counter_alloc_shared_cb, &conf);
> - return data.dword;
> -}
> -
> /**
> * Get age param from counter index.
> *
> @@ -6366,16 +6283,6 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
> if (pool->is_aged) {
> flow_dv_counter_remove_from_age(dev, counter, cnt);
> } else {
> - /*
> - * If the counter action is shared by ID, the l3t_clear_entry
> - * function reduces its references counter. If after the
> - * reduction the action is still referenced, the function
> - * returns here and does not release it.
> - */
> - if (IS_LEGACY_SHARED_CNT(counter) &&
> - mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl,
> - cnt->shared_info.id))
> - return;
> /*
> * If the counter action is shared by indirect action API,
> * the atomic function reduces its references counter.
> @@ -6385,8 +6292,7 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
> * indirect action API, shared info is 1 before the reduction,
> * so this condition is failed and function doesn't return here.
> */
> - if (!IS_LEGACY_SHARED_CNT(counter) &&
> - __atomic_sub_fetch(&cnt->shared_info.refcnt, 1,
> + if (__atomic_sub_fetch(&cnt->shared_info.refcnt, 1,
> __ATOMIC_RELAXED))
> return;
> }
> @@ -7275,7 +7181,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
> }
> for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
> int type = actions->type;
> - bool shared_count = false;
>
> if (!mlx5_flow_os_action_supported(type))
> return rte_flow_error_set(error, ENOTSUP,
> @@ -7427,8 +7332,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
> break;
> case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
> case RTE_FLOW_ACTION_TYPE_COUNT:
> - shared_count = is_shared_action_count(actions);
> - ret = flow_dv_validate_action_count(dev, shared_count,
> + ret = flow_dv_validate_action_count(dev,
> action_flags,
> error);
> if (ret < 0)
> @@ -7747,12 +7651,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
> * mutual exclusion with share counter actions.
> */
> if (!priv->sh->flow_hit_aso_en) {
> - if (shared_count)
> - return rte_flow_error_set
> - (error, EINVAL,
> - RTE_FLOW_ERROR_TYPE_ACTION,
> - NULL,
> - "old age and shared count combination is not supported");
> if (sample_count)
> return rte_flow_error_set
> (error, EINVAL,
> @@ -10837,16 +10735,14 @@ flow_dv_translate_action_port_id(struct rte_eth_dev *dev,
> static uint32_t
> flow_dv_translate_create_counter(struct rte_eth_dev *dev,
> struct mlx5_flow *dev_flow,
> - const struct rte_flow_action_count *count,
> + const struct rte_flow_action_count *count
> + __rte_unused,
> const struct rte_flow_action_age *age)
> {
> uint32_t counter;
> struct mlx5_age_param *age_param;
>
> - if (count && count->shared)
> - counter = flow_dv_counter_get_shared(dev, count->id);
> - else
> - counter = flow_dv_counter_alloc(dev, !!age);
> + counter = flow_dv_counter_alloc(dev, !!age);
> if (!counter || age == NULL)
> return counter;
> age_param = flow_dv_counter_idx_get_age(dev, counter);
> @@ -13216,8 +13112,7 @@ flow_dv_translate(struct rte_eth_dev *dev,
> * when they are not shared.
> */
> if (action_flags & MLX5_FLOW_ACTION_AGE) {
> - if ((non_shared_age &&
> - count && !count->shared) ||
> + if ((non_shared_age && count) ||
> !(priv->sh->flow_hit_aso_en &&
> (attr->group || attr->transfer))) {
> /* Creates age by counters. */
> @@ -17469,19 +17364,7 @@ flow_dv_action_validate(struct rte_eth_dev *dev,
> "Indirect age action not supported");
> return flow_dv_validate_action_age(0, action, dev, err);
> case RTE_FLOW_ACTION_TYPE_COUNT:
> - /*
> - * There are two mechanisms to share the action count.
> - * The old mechanism uses the shared field to share, while the
> - * new mechanism uses the indirect action API.
> - * This validation comes to make sure that the two mechanisms
> - * are not combined.
> - */
> - if (is_shared_action_count(action))
> - return rte_flow_error_set(err, ENOTSUP,
> - RTE_FLOW_ERROR_TYPE_ACTION,
> - NULL,
> - "Mix shared and indirect counter is not supported");
> - return flow_dv_validate_action_count(dev, true, 0, err);
> + return flow_dv_validate_action_count(dev, 0, err);
> case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> if (!priv->sh->ct_aso_en)
> return rte_flow_error_set(err, ENOTSUP,
> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
> index b93fd4d2c9..1627c3905f 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -250,8 +250,6 @@ flow_verbs_counter_create(struct rte_eth_dev *dev,
> *
> * @param[in] dev
> * Pointer to the Ethernet device structure.
> - * @param[in] shared
> - * Indicate if this counter is shared with other flows.
> * @param[in] id
> * Counter identifier.
> *
> @@ -259,21 +257,17 @@ flow_verbs_counter_create(struct rte_eth_dev *dev,
> * Index to the counter, 0 otherwise and rte_errno is set.
> */
> static uint32_t
> -flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared, uint32_t id)
> +flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t id __rte_unused)
> {
> struct mlx5_priv *priv = dev->data->dev_private;
> struct mlx5_flow_counter_mng *cmng = &priv->sh->cmng;
> struct mlx5_flow_counter_pool *pool = NULL;
> struct mlx5_flow_counter *cnt = NULL;
> - union mlx5_l3t_data data;
> uint32_t n_valid = cmng->n_valid;
> uint32_t pool_idx, cnt_idx;
> uint32_t i;
> int ret;
>
> - if (shared && !mlx5_l3t_get_entry(priv->sh->cnt_id_tbl, id, &data) &&
> - data.dword)
> - return data.dword;
> for (pool_idx = 0; pool_idx < n_valid; ++pool_idx) {
> pool = cmng->pools[pool_idx];
> if (!pool)
> @@ -320,13 +314,6 @@ flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared, uint32_t id)
> TAILQ_REMOVE(&pool->counters[0], cnt, next);
> i = MLX5_CNT_ARRAY_IDX(pool, cnt);
> cnt_idx = MLX5_MAKE_CNT_IDX(pool_idx, i);
> - if (shared) {
> - data.dword = cnt_idx;
> - if (mlx5_l3t_set_entry(priv->sh->cnt_id_tbl, id, &data))
> - return 0;
> - cnt->shared_info.id = id;
> - cnt_idx |= MLX5_CNT_SHARED_OFFSET;
> - }
> /* Create counter with Verbs. */
> ret = flow_verbs_counter_create(dev, cnt);
> if (!ret) {
> @@ -352,14 +339,10 @@ flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared, uint32_t id)
> static void
> flow_verbs_counter_release(struct rte_eth_dev *dev, uint32_t counter)
> {
> - struct mlx5_priv *priv = dev->data->dev_private;
> struct mlx5_flow_counter_pool *pool;
> struct mlx5_flow_counter *cnt;
>
> cnt = flow_verbs_counter_get_by_idx(dev, counter, &pool);
> - if (IS_LEGACY_SHARED_CNT(counter) &&
> - mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl, cnt->shared_info.id))
> - return;
> #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
> claim_zero(mlx5_glue->destroy_counter_set
> ((struct ibv_counter_set *)cnt->dcs_when_active));
> @@ -1198,8 +1181,7 @@ flow_verbs_translate_action_count(struct mlx5_flow *dev_flow,
> #endif
>
> if (!flow->counter) {
> - flow->counter = flow_verbs_counter_new(dev, count->shared,
> - count->id);
> + flow->counter = flow_verbs_counter_new(dev, count->id);
> if (!flow->counter)
> return rte_flow_error_set(error, rte_errno,
> RTE_FLOW_ERROR_TYPE_ACTION,
> diff --git a/drivers/net/octeontx2/otx2_flow_parse.c b/drivers/net/octeontx2/otx2_flow_parse.c
> index 63a33142a5..30a232f033 100644
> --- a/drivers/net/octeontx2/otx2_flow_parse.c
> +++ b/drivers/net/octeontx2/otx2_flow_parse.c
> @@ -901,7 +901,6 @@ otx2_flow_parse_actions(struct rte_eth_dev *dev,
> struct otx2_eth_dev *hw = dev->data->dev_private;
> struct otx2_npc_flow_info *npc = &hw->npc_flow;
> const struct rte_flow_action_port_id *port_act;
> - const struct rte_flow_action_count *act_count;
> const struct rte_flow_action_mark *act_mark;
> const struct rte_flow_action_queue *act_q;
> const struct rte_flow_action_vf *vf_act;
> @@ -947,15 +946,6 @@ otx2_flow_parse_actions(struct rte_eth_dev *dev,
> break;
>
> case RTE_FLOW_ACTION_TYPE_COUNT:
> - act_count =
> - (const struct rte_flow_action_count *)
> - actions->conf;
> -
> - if (act_count->shared == 1) {
> - errmsg = "Shared Counters not supported";
> - errcode = ENOTSUP;
> - goto err_exit;
> - }
> /* Indicates, need a counter */
> flow->ctr_id = 1;
> req_act |= OTX2_FLOW_ACT_COUNT;
> diff --git a/drivers/net/sfc/sfc_mae.c b/drivers/net/sfc/sfc_mae.c
> index 4b520bc619..5fcdf9c2f5 100644
> --- a/drivers/net/sfc/sfc_mae.c
> +++ b/drivers/net/sfc/sfc_mae.c
> @@ -2802,16 +2802,12 @@ sfc_mae_rule_parse_action_mark(struct sfc_adapter *sa,
>
> static int
> sfc_mae_rule_parse_action_count(struct sfc_adapter *sa,
> - const struct rte_flow_action_count *conf,
> + const struct rte_flow_action_count *conf
> + __rte_unused,
> efx_mae_actions_t *spec)
> {
> int rc;
>
> - if (conf->shared) {
> - rc = ENOTSUP;
> - goto fail_counter_shared;
> - }
> -
> if ((sa->counter_rxq.state & SFC_COUNTER_RXQ_INITIALIZED) == 0) {
> sfc_err(sa,
> "counter queue is not configured for COUNT action");
> @@ -2837,7 +2833,6 @@ sfc_mae_rule_parse_action_count(struct sfc_adapter *sa,
> fail_populate_count:
> fail_no_service_core:
> fail_counter_queue_uninit:
> -fail_counter_shared:
>
> return rc;
> }
> diff --git a/drivers/net/softnic/rte_eth_softnic_flow.c b/drivers/net/softnic/rte_eth_softnic_flow.c
> index 27eaf380cd..39038d26d8 100644
> --- a/drivers/net/softnic/rte_eth_softnic_flow.c
> +++ b/drivers/net/softnic/rte_eth_softnic_flow.c
> @@ -1448,13 +1448,6 @@ flow_rule_action_get(struct pmd_internals *softnic,
> action,
> "COUNT: Null configuration");
>
> - if (conf->shared)
> - return rte_flow_error_set(error,
> - ENOTSUP,
> - RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> - conf,
> - "COUNT: Shared counters not supported");
> -
> if (n_count)
> return rte_flow_error_set(error,
> ENOTSUP,
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 7b1ed7f110..5306e8ca4b 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -75,7 +75,7 @@ extern "C" {
> * At least one direction must be specified.
> *
> * Specifying both directions at once for a given rule is not recommended
> - * but may be valid in a few cases (e.g. shared counter).
> + * but may be valid in a few cases.
> */
> struct rte_flow_attr {
> uint32_t group; /**< Priority group. */
> @@ -2498,24 +2498,11 @@ struct rte_flow_query_age {
> * Counters can be retrieved and reset through ``rte_flow_query()``, see
> * ``struct rte_flow_query_count``.
> *
> - * @deprecated Shared attribute is deprecated, use generic
> - * RTE_FLOW_ACTION_TYPE_INDIRECT action.
> - *
> - * The shared flag indicates whether the counter is unique to the flow rule the
> - * action is specified with, or whether it is a shared counter.
> - *
> - * For a count action with the shared flag set, then then a global device
> - * namespace is assumed for the counter id, so that any matched flow rules using
> - * a count action with the same counter id on the same port will contribute to
> - * that counter.
> - *
> * For ports within the same switch domain then the counter id namespace extends
> * to all ports within that switch domain.
> */
> struct rte_flow_action_count {
> - /** @deprecated Share counter ID with other flow rules. */
> - uint32_t shared:1;
> - uint32_t reserved:31; /**< Reserved, must be zero. */
> + uint32_t reserved; /**< Reserved, must be zero. */
> uint32_t id; /**< Counter ID. */
> };
>
> --
> 2.30.2
>
Acked-by: Somnath Kotur <somnath.kotur@broadcom.com>
Hi Andrew,
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
> Sent: Tuesday, September 28, 2021 6:23 PM
> Subject: [dpdk-dev] [PATCH] ethdev: remove deprecated shared counter
> attribute
>
> Indirect actions should be used to do shared counters.
>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
> app/test-pmd/cmdline_flow.c | 10 --
> doc/guides/prog_guide/rte_flow.rst | 19 +--
> doc/guides/rel_notes/deprecation.rst | 4 -
> doc/guides/rel_notes/release_21_11.rst | 4 +
> drivers/net/bnxt/tf_ulp/ulp_rte_parser.c | 5 -
> drivers/net/cnxk/cnxk_rte_flow.c | 8 --
> drivers/net/hns3/hns3_flow.c | 3 +-
> drivers/net/ice/ice_fdir_filter.c | 4 +-
> drivers/net/mlx5/mlx5.h | 7 --
> drivers/net/mlx5/mlx5_flow_dv.c | 135 ++-------------------
> drivers/net/mlx5/mlx5_flow_verbs.c | 22 +---
> drivers/net/octeontx2/otx2_flow_parse.c | 10 --
> drivers/net/sfc/sfc_mae.c | 9 +-
> drivers/net/softnic/rte_eth_softnic_flow.c | 7 --
> lib/ethdev/rte_flow.h | 17 +--
> 15 files changed, 23 insertions(+), 241 deletions(-)
>
Acked-by: Ori Kam <orika@nvidia.com>
For the rte_flow level and testpmd part.
Best,
Ori
Hi Matan,
Many thanks for review. See below.
Andrew.
On 10/6/21 12:45 PM, Matan Azrad wrote:
> Hi Andrew
>
> Thank you for the big effort to adjust mlx5.
> I left some comments inside.
> If you feel it is too much, we can do a later patch to improve.
>
> Matan
>
>
>> From: Andrew Rybchenko
>> Indirect actions should be used to do shared counters.
>>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
[snip]
>> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
>> b/drivers/net/mlx5/mlx5_flow_dv.c index b610ad3ef4..91314d5ea5 100644
>> --- a/drivers/net/mlx5/mlx5_flow_dv.c
>> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
>> @@ -3308,33 +3308,11 @@ flow_dv_validate_action_set_tag(struct
>> rte_eth_dev *dev,
>> return 0;
>> }
>>
>> -/**
>> - * Check if action counter is shared by either old or new mechanism.
>> - *
>> - * @param[in] action
>> - * Pointer to the action structure.
>> - *
>> - * @return
>> - * True when counter is shared, false otherwise.
>> - */
>> -static inline bool
>> -is_shared_action_count(const struct rte_flow_action *action) -{
>> - const struct rte_flow_action_count *count =
>> - (const struct rte_flow_action_count *)action->conf;
>> -
>> - if ((int)action->type == MLX5_RTE_FLOW_ACTION_TYPE_COUNT)
>> - return true;
So, it is shared if and only if type is
MLX5_RTE_FLOW_ACTION_TYPE_COUNT.
>> - return !!(count && count->shared);
>> -}
>> -
>> /**
>> * Validate count action.
>> *
>> * @param[in] dev
>> * Pointer to rte_eth_dev structure.
>> - * @param[in] shared
>> - * Indicator if action is shared.
>> * @param[in] action_flags
>> * Holds the actions detected until now.
>> * @param[out] error
>> @@ -3344,7 +3322,7 @@ is_shared_action_count(const struct
>> rte_flow_action *action)
>> * 0 on success, a negative errno value otherwise and rte_errno is set.
>> */
>> static int
>> -flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
>> +flow_dv_validate_action_count(struct rte_eth_dev *dev,
>> uint64_t action_flags,
>> struct rte_flow_error *error) { @@ -3356,11 +3334,6 @@
>> flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
>> return rte_flow_error_set(error, EINVAL,
>> RTE_FLOW_ERROR_TYPE_ACTION, NULL,
>> "duplicate count actions set");
>> - if (shared && (action_flags & MLX5_FLOW_ACTION_AGE) &&
>
> This check is relevant to the indirect action case in some places, see below.
Yes, this if should be preserved. Thanks.
>
>
>> - !priv->sh->flow_hit_aso_en)
>> - return rte_flow_error_set(error, EINVAL,
>> - RTE_FLOW_ERROR_TYPE_ACTION, NULL,
>> - "old age and shared count combination is not
>> supported");
>> #ifdef HAVE_IBV_FLOW_DEVX_COUNTERS
>> return 0;
>> #endif
>> @@ -5658,9 +5631,7 @@ flow_dv_validate_action_sample(uint64_t
>> *action_flags,
>> break;
>> case RTE_FLOW_ACTION_TYPE_COUNT:
>> ret = flow_dv_validate_action_count
>> - (dev, is_shared_action_count(act),
>> - *action_flags | sub_action_flags,
>> - error);
>> + (dev, *action_flags | sub_action_flags,
>> + error);
>> if (ret < 0)
>> return ret;
>> *count = act->conf; @@ -6230,60 +6201,6 @@
>> flow_dv_counter_alloc(struct rte_eth_dev *dev, uint32_t age)
>> return 0;
>> }
>>
>> -/**
>> - * Allocate a shared flow counter.
>> - *
>> - * @param[in] ctx
>> - * Pointer to the shared counter configuration.
>> - * @param[in] data
>> - * Pointer to save the allocated counter index.
>> - *
>> - * @return
>> - * Index to flow counter on success, 0 otherwise and rte_errno is set.
>> - */
>> -
>> -static int32_t
>> -flow_dv_counter_alloc_shared_cb(void *ctx, union mlx5_l3t_data *data) -{
>> - struct mlx5_shared_counter_conf *conf = ctx;
>> - struct rte_eth_dev *dev = conf->dev;
>> - struct mlx5_flow_counter *cnt;
>> -
>> - data->dword = flow_dv_counter_alloc(dev, 0);
>> - data->dword |= MLX5_CNT_SHARED_OFFSET;
>> - cnt = flow_dv_counter_get_by_idx(dev, data->dword, NULL);
>> - cnt->shared_info.id = conf->id;
>> - return 0;
>> -}
>> -
>> -/**
>> - * Get a shared flow counter.
>> - *
>> - * @param[in] dev
>> - * Pointer to the Ethernet device structure.
>> - * @param[in] id
>> - * Counter identifier.
>> - *
>> - * @return
>> - * Index to flow counter on success, 0 otherwise and rte_errno is set.
>> - */
>> -static uint32_t
>> -flow_dv_counter_get_shared(struct rte_eth_dev *dev, uint32_t id) -{
>> - struct mlx5_priv *priv = dev->data->dev_private;
>> - struct mlx5_shared_counter_conf conf = {
>> - .dev = dev,
>> - .id = id,
>> - };
>> - union mlx5_l3t_data data = {
>> - .dword = 0,
>> - };
>> -
>> - mlx5_l3t_prepare_entry(priv->sh->cnt_id_tbl, id, &data,
>> - flow_dv_counter_alloc_shared_cb, &conf);
>> - return data.dword;
>> -}
>> -
>
> Need to remove cnt_id_tbl and from sh and all of its management, no?
Looks. Will do in v2.
>
>> /**
>> * Get age param from counter index.
>> *
>> @@ -6366,16 +6283,6 @@ flow_dv_counter_free(struct rte_eth_dev *dev,
>> uint32_t counter)
>> if (pool->is_aged) {
>> flow_dv_counter_remove_from_age(dev, counter, cnt);
>> } else {
>> - /*
>> - * If the counter action is shared by ID, the l3t_clear_entry
>> - * function reduces its references counter. If after the
>> - * reduction the action is still referenced, the function
>> - * returns here and does not release it.
>> - */
>> - if (IS_LEGACY_SHARED_CNT(counter) &&
>> - mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl,
>> - cnt->shared_info.id))
>> - return;
>> /*
>> * If the counter action is shared by indirect action API,
>> * the atomic function reduces its references counter.
>> @@ -6385,8 +6292,7 @@ flow_dv_counter_free(struct rte_eth_dev *dev,
>> uint32_t counter)
>
> You can remove the ID sharing notice in this comment.
OK, will do.
>
>> * indirect action API, shared info is 1 before the reduction,
>> * so this condition is failed and function doesn't return here.
>> */
>> - if (!IS_LEGACY_SHARED_CNT(counter) &&
>> - __atomic_sub_fetch(&cnt->shared_info.refcnt, 1,
>> + if (__atomic_sub_fetch(&cnt->shared_info.refcnt, 1,
>> __ATOMIC_RELAXED))
>> return;
>> }
>> @@ -7275,7 +7181,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const
>> struct rte_flow_attr *attr,
>> }
>> for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
>> int type = actions->type;
>> - bool shared_count = false;
>>
>> if (!mlx5_flow_os_action_supported(type))
>> return rte_flow_error_set(error, ENOTSUP, @@ -7427,8 +7332,7
>> @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr
>> *attr,
>> break;
>> case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
>> case RTE_FLOW_ACTION_TYPE_COUNT:
>> - shared_count = is_shared_action_count(actions);
>
> We need shared_count for the indirect count case(which can be shared with different flows),
> So, if the action is MLX5_RTE_FLOW_ACTION_TYPE_COUNT, shared_count should be true.
> It is relevant to the validate function below and for the check at the end of the function.
Thanks, will fix in v2.
>
>
>> - ret = flow_dv_validate_action_count(dev, shared_count,
>> + ret = flow_dv_validate_action_count(dev,
>> action_flags,
>> error);
>> if (ret < 0)
>> @@ -17469,19 +17364,7 @@ flow_dv_action_validate(struct rte_eth_dev *dev,
>> "Indirect age action not supported");
>> return flow_dv_validate_action_age(0, action, dev, err);
>> case RTE_FLOW_ACTION_TYPE_COUNT:
>> - /*
>> - * There are two mechanisms to share the action count.
>> - * The old mechanism uses the shared field to share, while the
>> - * new mechanism uses the indirect action API.
>> - * This validation comes to make sure that the two mechanisms
>> - * are not combined.
>> - */
>> - if (is_shared_action_count(action))
>> - return rte_flow_error_set(err, ENOTSUP,
>> - RTE_FLOW_ERROR_TYPE_ACTION,
>> - NULL,
>> - "Mix shared and indirect counter is not supported");
>> - return flow_dv_validate_action_count(dev, true, 0, err);
>> + return flow_dv_validate_action_count(dev, 0, err);
>
>
> Did you also consider improving struct mlx5_flow_counter_shared? Then, maybe we don't need it anymore.
Comments say that reference counters there are used for
indirect actions as well. So, I'll keep it for
follow up cleanups.
[snip]
@@ -322,7 +322,6 @@ enum index {
ACTION_QUEUE_INDEX,
ACTION_DROP,
ACTION_COUNT,
- ACTION_COUNT_SHARED,
ACTION_COUNT_ID,
ACTION_RSS,
ACTION_RSS_FUNC,
@@ -1451,7 +1450,6 @@ static const enum index action_queue[] = {
static const enum index action_count[] = {
ACTION_COUNT_ID,
- ACTION_COUNT_SHARED,
ACTION_NEXT,
ZERO,
};
@@ -3712,14 +3710,6 @@ static const struct token token_list[] = {
.args = ARGS(ARGS_ENTRY(struct rte_flow_action_count, id)),
.call = parse_vc_conf,
},
- [ACTION_COUNT_SHARED] = {
- .name = "shared",
- .help = "shared counter",
- .next = NEXT(action_count, NEXT_ENTRY(COMMON_BOOLEAN)),
- .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_action_count,
- shared, 1)),
- .call = parse_vc_conf,
- },
[ACTION_RSS] = {
.name = "rss",
.help = "spread packets among several queues",
@@ -158,7 +158,7 @@ Several pattern items and actions are valid and can be used in both
directions. At least one direction must be specified.
Specifying both directions at once for a given rule is not recommended but
-may be valid in a few cases (e.g. shared counters).
+may be valid in a few cases.
Attribute: Transfer
^^^^^^^^^^^^^^^^^^^
@@ -1491,9 +1491,7 @@ Actions are performed in list order:
+=======+========+============+=======+
| 0 | MARK | ``mark`` | 0x2a |
+-------+--------+------------+-------+
- | 1 | COUNT | ``shared`` | 0 |
- | | +------------+-------+
- | | | ``id`` | 0 |
+ | 1 | COUNT | ``id`` | 0 |
+-------+--------+------------+-------+
| 2 | QUEUE | ``queue`` | 10 |
+-------+--------+------------+-------+
@@ -1734,20 +1732,9 @@ action must specify a unique id.
Counters can be retrieved and reset through ``rte_flow_query()``, see
``struct rte_flow_query_count``.
-The shared flag indicates whether the counter is unique to the flow rule the
-action is specified with, or whether it is a shared counter.
-
-For a count action with the shared flag set, then a global device
-namespace is assumed for the counter id, so that any matched flow rules using
-a count action with the same counter id on the same port will contribute to
-that counter.
-
For ports within the same switch domain then the counter id namespace extends
to all ports within that switch domain.
-The shared flag is DEPRECATED and ``INDIRECT`` ``COUNT`` action should be used
-to make shared counters.
-
.. _table_rte_flow_action_count:
.. table:: COUNT
@@ -1755,8 +1742,6 @@ to make shared counters.
+------------+---------------------------------+
| Field | Value |
+============+=================================+
- | ``shared`` | DEPRECATED, shared counter flag |
- +------------+---------------------------------+
| ``id`` | counter id |
+------------+---------------------------------+
@@ -124,10 +124,6 @@ Deprecation Notices
to support modifying fields larger than 64 bits.
In addition, documentation will be updated to clarify byte order.
-* ethdev: Attribute ``shared`` of the ``struct rte_flow_action_count``
- is deprecated and will be removed in DPDK 21.11. Shared counters should
- be managed using shared actions API (``rte_flow_shared_action_create`` etc).
-
* ethdev: Definition of the flow API action ``RTE_FLOW_ACTION_TYPE_PORT_ID``
is ambiguous and needs clarification.
Structure ``rte_flow_action_port_id`` will be extended to specify
@@ -126,6 +126,10 @@ Removed Items
blacklist/whitelist are removed. Users must use the new
block/allow list arguments.
+* ethdev: Removed deprecated ``shared`` attribute of the
+ ``struct rte_flow_action_count``. Shared counters should be managed
+ using indirect actions API (``rte_flow_action_handle_create`` etc).
+
API Changes
-----------
@@ -2111,11 +2111,6 @@ ulp_rte_count_act_handler(const struct rte_flow_action *action_item,
act_count = action_item->conf;
if (act_count) {
- if (act_count->shared) {
- BNXT_TF_DBG(ERR,
- "Parse Error:Shared count not supported\n");
- return BNXT_TF_RC_PARSE_ERR;
- }
memcpy(&act_prop->act_details[BNXT_ULP_ACT_PROP_IDX_COUNT],
&act_count->id,
BNXT_ULP_ACT_PROP_SZ_COUNT);
@@ -110,7 +110,6 @@ cnxk_map_actions(struct rte_eth_dev *eth_dev, const struct rte_flow_attr *attr,
struct roc_npc_action in_actions[], uint32_t *flowkey_cfg)
{
struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
- const struct rte_flow_action_count *act_count;
const struct rte_flow_action_queue *act_q;
int i = 0, rc = 0;
int rq;
@@ -131,13 +130,6 @@ cnxk_map_actions(struct rte_eth_dev *eth_dev, const struct rte_flow_attr *attr,
break;
case RTE_FLOW_ACTION_TYPE_COUNT:
- act_count = (const struct rte_flow_action_count *)
- actions->conf;
-
- if (act_count->shared == 1) {
- plt_npc_dbg("Shared counter is not supported");
- goto err_exit;
- }
in_actions[i].type = ROC_NPC_ACTION_TYPE_COUNT;
break;
@@ -1782,8 +1782,7 @@ hns3_flow_create(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
goto out;
if (fdir_rule.flags & HNS3_RULE_FLAG_COUNTER) {
- ret = hns3_counter_new(dev, fdir_rule.act_cnt.shared,
- fdir_rule.act_cnt.id, error);
+ ret = hns3_counter_new(dev, 0, fdir_rule.act_cnt.id, error);
if (ret)
goto out;
@@ -1339,9 +1339,7 @@ ice_fdir_create_filter(struct ice_adapter *ad,
if (filter->input.cnt_ena) {
struct rte_flow_action_count *act_count = &filter->act_count;
- filter->counter = ice_fdir_counter_alloc(pf,
- act_count->shared,
- act_count->id);
+ filter->counter = ice_fdir_counter_alloc(pf, 0, act_count->id);
if (!filter->counter) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -324,7 +324,6 @@ struct mlx5_lb_ctx {
#define MLX5_MAX_PENDING_QUERIES 4
#define MLX5_CNT_CONTAINER_RESIZE 64
#define MLX5_CNT_SHARED_OFFSET 0x80000000
-#define IS_LEGACY_SHARED_CNT(cnt) (!!((cnt) & MLX5_CNT_SHARED_OFFSET))
#define IS_BATCH_CNT(cnt) (((cnt) & (MLX5_CNT_SHARED_OFFSET - 1)) >= \
MLX5_CNT_BATCH_OFFSET)
#define MLX5_CNT_SIZE (sizeof(struct mlx5_flow_counter))
@@ -392,12 +391,6 @@ struct mlx5_flow_counter_shared {
};
};
-/* Shared counter configuration. */
-struct mlx5_shared_counter_conf {
- struct rte_eth_dev *dev; /* The device shared counter belongs to. */
- uint32_t id; /* The shared counter ID. */
-};
-
struct mlx5_flow_counter_pool;
/* Generic counters information. */
struct mlx5_flow_counter {
@@ -3308,33 +3308,11 @@ flow_dv_validate_action_set_tag(struct rte_eth_dev *dev,
return 0;
}
-/**
- * Check if action counter is shared by either old or new mechanism.
- *
- * @param[in] action
- * Pointer to the action structure.
- *
- * @return
- * True when counter is shared, false otherwise.
- */
-static inline bool
-is_shared_action_count(const struct rte_flow_action *action)
-{
- const struct rte_flow_action_count *count =
- (const struct rte_flow_action_count *)action->conf;
-
- if ((int)action->type == MLX5_RTE_FLOW_ACTION_TYPE_COUNT)
- return true;
- return !!(count && count->shared);
-}
-
/**
* Validate count action.
*
* @param[in] dev
* Pointer to rte_eth_dev structure.
- * @param[in] shared
- * Indicator if action is shared.
* @param[in] action_flags
* Holds the actions detected until now.
* @param[out] error
@@ -3344,7 +3322,7 @@ is_shared_action_count(const struct rte_flow_action *action)
* 0 on success, a negative errno value otherwise and rte_errno is set.
*/
static int
-flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
+flow_dv_validate_action_count(struct rte_eth_dev *dev,
uint64_t action_flags,
struct rte_flow_error *error)
{
@@ -3356,11 +3334,6 @@ flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
return rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION, NULL,
"duplicate count actions set");
- if (shared && (action_flags & MLX5_FLOW_ACTION_AGE) &&
- !priv->sh->flow_hit_aso_en)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION, NULL,
- "old age and shared count combination is not supported");
#ifdef HAVE_IBV_FLOW_DEVX_COUNTERS
return 0;
#endif
@@ -5658,9 +5631,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
break;
case RTE_FLOW_ACTION_TYPE_COUNT:
ret = flow_dv_validate_action_count
- (dev, is_shared_action_count(act),
- *action_flags | sub_action_flags,
- error);
+ (dev, *action_flags | sub_action_flags, error);
if (ret < 0)
return ret;
*count = act->conf;
@@ -6230,60 +6201,6 @@ flow_dv_counter_alloc(struct rte_eth_dev *dev, uint32_t age)
return 0;
}
-/**
- * Allocate a shared flow counter.
- *
- * @param[in] ctx
- * Pointer to the shared counter configuration.
- * @param[in] data
- * Pointer to save the allocated counter index.
- *
- * @return
- * Index to flow counter on success, 0 otherwise and rte_errno is set.
- */
-
-static int32_t
-flow_dv_counter_alloc_shared_cb(void *ctx, union mlx5_l3t_data *data)
-{
- struct mlx5_shared_counter_conf *conf = ctx;
- struct rte_eth_dev *dev = conf->dev;
- struct mlx5_flow_counter *cnt;
-
- data->dword = flow_dv_counter_alloc(dev, 0);
- data->dword |= MLX5_CNT_SHARED_OFFSET;
- cnt = flow_dv_counter_get_by_idx(dev, data->dword, NULL);
- cnt->shared_info.id = conf->id;
- return 0;
-}
-
-/**
- * Get a shared flow counter.
- *
- * @param[in] dev
- * Pointer to the Ethernet device structure.
- * @param[in] id
- * Counter identifier.
- *
- * @return
- * Index to flow counter on success, 0 otherwise and rte_errno is set.
- */
-static uint32_t
-flow_dv_counter_get_shared(struct rte_eth_dev *dev, uint32_t id)
-{
- struct mlx5_priv *priv = dev->data->dev_private;
- struct mlx5_shared_counter_conf conf = {
- .dev = dev,
- .id = id,
- };
- union mlx5_l3t_data data = {
- .dword = 0,
- };
-
- mlx5_l3t_prepare_entry(priv->sh->cnt_id_tbl, id, &data,
- flow_dv_counter_alloc_shared_cb, &conf);
- return data.dword;
-}
-
/**
* Get age param from counter index.
*
@@ -6366,16 +6283,6 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
if (pool->is_aged) {
flow_dv_counter_remove_from_age(dev, counter, cnt);
} else {
- /*
- * If the counter action is shared by ID, the l3t_clear_entry
- * function reduces its references counter. If after the
- * reduction the action is still referenced, the function
- * returns here and does not release it.
- */
- if (IS_LEGACY_SHARED_CNT(counter) &&
- mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl,
- cnt->shared_info.id))
- return;
/*
* If the counter action is shared by indirect action API,
* the atomic function reduces its references counter.
@@ -6385,8 +6292,7 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
* indirect action API, shared info is 1 before the reduction,
* so this condition is failed and function doesn't return here.
*/
- if (!IS_LEGACY_SHARED_CNT(counter) &&
- __atomic_sub_fetch(&cnt->shared_info.refcnt, 1,
+ if (__atomic_sub_fetch(&cnt->shared_info.refcnt, 1,
__ATOMIC_RELAXED))
return;
}
@@ -7275,7 +7181,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
}
for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
int type = actions->type;
- bool shared_count = false;
if (!mlx5_flow_os_action_supported(type))
return rte_flow_error_set(error, ENOTSUP,
@@ -7427,8 +7332,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
break;
case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
case RTE_FLOW_ACTION_TYPE_COUNT:
- shared_count = is_shared_action_count(actions);
- ret = flow_dv_validate_action_count(dev, shared_count,
+ ret = flow_dv_validate_action_count(dev,
action_flags,
error);
if (ret < 0)
@@ -7747,12 +7651,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
* mutual exclusion with share counter actions.
*/
if (!priv->sh->flow_hit_aso_en) {
- if (shared_count)
- return rte_flow_error_set
- (error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- NULL,
- "old age and shared count combination is not supported");
if (sample_count)
return rte_flow_error_set
(error, EINVAL,
@@ -10837,16 +10735,14 @@ flow_dv_translate_action_port_id(struct rte_eth_dev *dev,
static uint32_t
flow_dv_translate_create_counter(struct rte_eth_dev *dev,
struct mlx5_flow *dev_flow,
- const struct rte_flow_action_count *count,
+ const struct rte_flow_action_count *count
+ __rte_unused,
const struct rte_flow_action_age *age)
{
uint32_t counter;
struct mlx5_age_param *age_param;
- if (count && count->shared)
- counter = flow_dv_counter_get_shared(dev, count->id);
- else
- counter = flow_dv_counter_alloc(dev, !!age);
+ counter = flow_dv_counter_alloc(dev, !!age);
if (!counter || age == NULL)
return counter;
age_param = flow_dv_counter_idx_get_age(dev, counter);
@@ -13216,8 +13112,7 @@ flow_dv_translate(struct rte_eth_dev *dev,
* when they are not shared.
*/
if (action_flags & MLX5_FLOW_ACTION_AGE) {
- if ((non_shared_age &&
- count && !count->shared) ||
+ if ((non_shared_age && count) ||
!(priv->sh->flow_hit_aso_en &&
(attr->group || attr->transfer))) {
/* Creates age by counters. */
@@ -17469,19 +17364,7 @@ flow_dv_action_validate(struct rte_eth_dev *dev,
"Indirect age action not supported");
return flow_dv_validate_action_age(0, action, dev, err);
case RTE_FLOW_ACTION_TYPE_COUNT:
- /*
- * There are two mechanisms to share the action count.
- * The old mechanism uses the shared field to share, while the
- * new mechanism uses the indirect action API.
- * This validation comes to make sure that the two mechanisms
- * are not combined.
- */
- if (is_shared_action_count(action))
- return rte_flow_error_set(err, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ACTION,
- NULL,
- "Mix shared and indirect counter is not supported");
- return flow_dv_validate_action_count(dev, true, 0, err);
+ return flow_dv_validate_action_count(dev, 0, err);
case RTE_FLOW_ACTION_TYPE_CONNTRACK:
if (!priv->sh->ct_aso_en)
return rte_flow_error_set(err, ENOTSUP,
@@ -250,8 +250,6 @@ flow_verbs_counter_create(struct rte_eth_dev *dev,
*
* @param[in] dev
* Pointer to the Ethernet device structure.
- * @param[in] shared
- * Indicate if this counter is shared with other flows.
* @param[in] id
* Counter identifier.
*
@@ -259,21 +257,17 @@ flow_verbs_counter_create(struct rte_eth_dev *dev,
* Index to the counter, 0 otherwise and rte_errno is set.
*/
static uint32_t
-flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared, uint32_t id)
+flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t id __rte_unused)
{
struct mlx5_priv *priv = dev->data->dev_private;
struct mlx5_flow_counter_mng *cmng = &priv->sh->cmng;
struct mlx5_flow_counter_pool *pool = NULL;
struct mlx5_flow_counter *cnt = NULL;
- union mlx5_l3t_data data;
uint32_t n_valid = cmng->n_valid;
uint32_t pool_idx, cnt_idx;
uint32_t i;
int ret;
- if (shared && !mlx5_l3t_get_entry(priv->sh->cnt_id_tbl, id, &data) &&
- data.dword)
- return data.dword;
for (pool_idx = 0; pool_idx < n_valid; ++pool_idx) {
pool = cmng->pools[pool_idx];
if (!pool)
@@ -320,13 +314,6 @@ flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared, uint32_t id)
TAILQ_REMOVE(&pool->counters[0], cnt, next);
i = MLX5_CNT_ARRAY_IDX(pool, cnt);
cnt_idx = MLX5_MAKE_CNT_IDX(pool_idx, i);
- if (shared) {
- data.dword = cnt_idx;
- if (mlx5_l3t_set_entry(priv->sh->cnt_id_tbl, id, &data))
- return 0;
- cnt->shared_info.id = id;
- cnt_idx |= MLX5_CNT_SHARED_OFFSET;
- }
/* Create counter with Verbs. */
ret = flow_verbs_counter_create(dev, cnt);
if (!ret) {
@@ -352,14 +339,10 @@ flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared, uint32_t id)
static void
flow_verbs_counter_release(struct rte_eth_dev *dev, uint32_t counter)
{
- struct mlx5_priv *priv = dev->data->dev_private;
struct mlx5_flow_counter_pool *pool;
struct mlx5_flow_counter *cnt;
cnt = flow_verbs_counter_get_by_idx(dev, counter, &pool);
- if (IS_LEGACY_SHARED_CNT(counter) &&
- mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl, cnt->shared_info.id))
- return;
#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
claim_zero(mlx5_glue->destroy_counter_set
((struct ibv_counter_set *)cnt->dcs_when_active));
@@ -1198,8 +1181,7 @@ flow_verbs_translate_action_count(struct mlx5_flow *dev_flow,
#endif
if (!flow->counter) {
- flow->counter = flow_verbs_counter_new(dev, count->shared,
- count->id);
+ flow->counter = flow_verbs_counter_new(dev, count->id);
if (!flow->counter)
return rte_flow_error_set(error, rte_errno,
RTE_FLOW_ERROR_TYPE_ACTION,
@@ -901,7 +901,6 @@ otx2_flow_parse_actions(struct rte_eth_dev *dev,
struct otx2_eth_dev *hw = dev->data->dev_private;
struct otx2_npc_flow_info *npc = &hw->npc_flow;
const struct rte_flow_action_port_id *port_act;
- const struct rte_flow_action_count *act_count;
const struct rte_flow_action_mark *act_mark;
const struct rte_flow_action_queue *act_q;
const struct rte_flow_action_vf *vf_act;
@@ -947,15 +946,6 @@ otx2_flow_parse_actions(struct rte_eth_dev *dev,
break;
case RTE_FLOW_ACTION_TYPE_COUNT:
- act_count =
- (const struct rte_flow_action_count *)
- actions->conf;
-
- if (act_count->shared == 1) {
- errmsg = "Shared Counters not supported";
- errcode = ENOTSUP;
- goto err_exit;
- }
/* Indicates, need a counter */
flow->ctr_id = 1;
req_act |= OTX2_FLOW_ACT_COUNT;
@@ -2802,16 +2802,12 @@ sfc_mae_rule_parse_action_mark(struct sfc_adapter *sa,
static int
sfc_mae_rule_parse_action_count(struct sfc_adapter *sa,
- const struct rte_flow_action_count *conf,
+ const struct rte_flow_action_count *conf
+ __rte_unused,
efx_mae_actions_t *spec)
{
int rc;
- if (conf->shared) {
- rc = ENOTSUP;
- goto fail_counter_shared;
- }
-
if ((sa->counter_rxq.state & SFC_COUNTER_RXQ_INITIALIZED) == 0) {
sfc_err(sa,
"counter queue is not configured for COUNT action");
@@ -2837,7 +2833,6 @@ sfc_mae_rule_parse_action_count(struct sfc_adapter *sa,
fail_populate_count:
fail_no_service_core:
fail_counter_queue_uninit:
-fail_counter_shared:
return rc;
}
@@ -1448,13 +1448,6 @@ flow_rule_action_get(struct pmd_internals *softnic,
action,
"COUNT: Null configuration");
- if (conf->shared)
- return rte_flow_error_set(error,
- ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- conf,
- "COUNT: Shared counters not supported");
-
if (n_count)
return rte_flow_error_set(error,
ENOTSUP,
@@ -75,7 +75,7 @@ extern "C" {
* At least one direction must be specified.
*
* Specifying both directions at once for a given rule is not recommended
- * but may be valid in a few cases (e.g. shared counter).
+ * but may be valid in a few cases.
*/
struct rte_flow_attr {
uint32_t group; /**< Priority group. */
@@ -2498,24 +2498,11 @@ struct rte_flow_query_age {
* Counters can be retrieved and reset through ``rte_flow_query()``, see
* ``struct rte_flow_query_count``.
*
- * @deprecated Shared attribute is deprecated, use generic
- * RTE_FLOW_ACTION_TYPE_INDIRECT action.
- *
- * The shared flag indicates whether the counter is unique to the flow rule the
- * action is specified with, or whether it is a shared counter.
- *
- * For a count action with the shared flag set, then then a global device
- * namespace is assumed for the counter id, so that any matched flow rules using
- * a count action with the same counter id on the same port will contribute to
- * that counter.
- *
* For ports within the same switch domain then the counter id namespace extends
* to all ports within that switch domain.
*/
struct rte_flow_action_count {
- /** @deprecated Share counter ID with other flow rules. */
- uint32_t shared:1;
- uint32_t reserved:31; /**< Reserved, must be zero. */
+ uint32_t reserved; /**< Reserved, must be zero. */
uint32_t id; /**< Counter ID. */
};