DPDK patches and discussions
 help / color / mirror / Atom feed
From: Somnath Kotur <somnath.kotur@broadcom.com>
To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Cc: Ori Kam <orika@nvidia.com>, Xiaoyun Li <xiaoyun.li@intel.com>,
	Ray Kinsella <mdr@ashroe.eu>,
	Ajit Khaparde <ajit.khaparde@broadcom.com>,
	Nithin Dabilpuram <ndabilpuram@marvell.com>,
	 Kiran Kumar K <kirankumark@marvell.com>,
	Sunil Kumar Kori <skori@marvell.com>,
	 Satha Rao <skoteshwar@marvell.com>,
	"Min Hu (Connor)" <humin29@huawei.com>,
	 Yisen Zhuang <yisen.zhuang@huawei.com>,
	Lijun Ou <oulijun@huawei.com>,
	 Qiming Yang <qiming.yang@intel.com>,
	Qi Zhang <qi.z.zhang@intel.com>,  Matan Azrad <matan@nvidia.com>,
	Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
	 Jerin Jacob <jerinj@marvell.com>,
	Jasvinder Singh <jasvinder.singh@intel.com>,
	 Cristian Dumitrescu <cristian.dumitrescu@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	 Ferruh Yigit <ferruh.yigit@intel.com>, dev <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] ethdev: remove deprecated shared counter attribute
Date: Wed, 6 Oct 2021 15:22:08 +0530	[thread overview]
Message-ID: <CAOBf=muz12M0MPPgy7yO-Ae4u_wNH8Xjd8dy+g-GtMdo3NgAow@mail.gmail.com> (raw)
In-Reply-To: <20210928152300.989961-1-andrew.rybchenko@oktetlabs.ru>

[-- Attachment #1: Type: text/plain, Size: 30679 bytes --]

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>

  parent reply	other threads:[~2021-10-06  9:52 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28 15:23 Andrew Rybchenko
2021-09-28 15:58 ` Stephen Hemminger
2021-09-28 16:25   ` Andrew Rybchenko
2021-10-05 19:11 ` Thomas Monjalon
2021-10-05 19:48   ` Ajit Khaparde
2021-10-06  9:45 ` Matan Azrad
2021-10-08 10:23   ` Andrew Rybchenko
2021-10-06  9:52 ` Somnath Kotur [this message]
2021-10-06 12:04 ` Ori Kam
2021-10-08 10:26 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
2021-10-11 10:02   ` Ori Kam
2021-10-11 10:41     ` Andrew Rybchenko
2021-10-11 11:25       ` Ori Kam
2021-10-11 10:02   ` Matan Azrad
2021-10-12 17:24     ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOBf=muz12M0MPPgy7yO-Ae4u_wNH8Xjd8dy+g-GtMdo3NgAow@mail.gmail.com' \
    --to=somnath.kotur@broadcom.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=cristian.dumitrescu@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=humin29@huawei.com \
    --cc=jasvinder.singh@intel.com \
    --cc=jerinj@marvell.com \
    --cc=kirankumark@marvell.com \
    --cc=matan@nvidia.com \
    --cc=mdr@ashroe.eu \
    --cc=ndabilpuram@marvell.com \
    --cc=orika@nvidia.com \
    --cc=oulijun@huawei.com \
    --cc=qi.z.zhang@intel.com \
    --cc=qiming.yang@intel.com \
    --cc=skori@marvell.com \
    --cc=skoteshwar@marvell.com \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@nvidia.com \
    --cc=xiaoyun.li@intel.com \
    --cc=yisen.zhuang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).