On Wed, Oct 03, 2018 at 03:29:12PM +0000, Slava Ovsiienko wrote:
> Mellanox mlx5 PMD supports Flow Counters via Verbs library.
> The current implementation is based on the Mellanox proprietary
> Verbs library included in MLNX OFED packages. The Flow Counter
> support is recently added into linux-rdma release (v19),
> so the mlx5 PMD update is needed to provide Counter feature
> on the base of linux-rdma.
>
> mlx5 PMD can be compiled with MLNX OFED or linux-rdma v19+
> and provide flow counters for both.
>
> Signed-off-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
> ---
> drivers/net/mlx5/Makefile | 10 ++++++++
> drivers/net/mlx5/mlx5.c | 6 +++++
> drivers/net/mlx5/mlx5_flow.c | 9 ++++++-
> drivers/net/mlx5/mlx5_flow.h | 4 +++
> drivers/net/mlx5/mlx5_flow_verbs.c | 52
> ++++++++++++++++++++++++++++++--------
> drivers/net/mlx5/mlx5_glue.c | 41 ++++++++++++++++++++++++++++++
> drivers/net/mlx5/mlx5_glue.h | 16 ++++++++++++
> 7 files changed, 127 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
> index ca1de9f..e3d2156 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -162,6 +162,16 @@ mlx5_autoconf.h.new:
> $(RTE_SDK)/buildtools/auto-config-h.sh
> type 'struct ibv_counter_set_init_attr' \
> $(AUTOCONF_OUTPUT)
> $Q sh -- '$<' '$@' \
> + HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT \
> + infiniband/verbs.h \
> + type 'struct ibv_counters_init_attr' \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45 \
> + infiniband/verbs.h \
> + type 'struct ibv_counters_init_attr' \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
I still don't understand what is different between the two. These are exactly
same checking, then why do you need to have two different macros? From this
script, HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT is same as
HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45, isn't it?
And if you make some changes in Makefile, you should also make corresponding
changes for the meson build.
> HAVE_RDMA_NL_NLDEV \
> rdma/rdma_netlink.h \
> enum RDMA_NL_NLDEV \
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 4be6a1c..81f6ba1 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -739,8 +739,10 @@
> unsigned int mprq_min_stride_num_n = 0;
> unsigned int mprq_max_stride_num_n = 0;
> #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> +#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> struct ibv_counter_set_description cs_desc = { .counter_type = 0 };
> #endif
> +#endif
> struct ether_addr mac;
> char name[RTE_ETH_NAME_MAX_LEN];
> int own_domain_id = 0;
> @@ -1009,11 +1011,15 @@
> DRV_LOG(DEBUG, "checksum offloading is %ssupported",
> (config.hw_csum ? "" : "not "));
> #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> +#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> config.flow_counter_en = !!attr.max_counter_sets;
> mlx5_glue->describe_counter_set(ctx, 0, &cs_desc);
> DRV_LOG(DEBUG, "counter type = %d, num of cs = %ld, attributes = %d",
> cs_desc.counter_type, cs_desc.num_of_cs,
> cs_desc.attributes);
> +#else
> + config.flow_counter_en = 1;
> +#endif
> #endif
> config.ind_table_max_size =
> attr.rss_caps.max_rwq_indirection_table_size;
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 8007bf1..652580c 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -2306,6 +2306,13 @@ struct rte_flow *
> if (flow->actions & MLX5_ACTION_COUNT) {
> struct rte_flow_query_count *qc = data;
> uint64_t counters[2] = {0, 0};
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> + int err = mlx5_glue->query_counter_set(
> + flow->counter->cs,
> + counters,
> + RTE_DIM(counters),
> + IBV_READ_COUNTERS_ATTR_PREFER_CACHED);
> +#else
> struct ibv_query_counter_set_attr query_cs_attr = {
> .cs = flow->counter->cs,
> .query_flags = IBV_COUNTER_SET_FORCE_UPDATE,
> @@ -2316,7 +2323,7 @@ struct rte_flow *
> };
> int err = mlx5_glue->query_counter_set(&query_cs_attr,
> &query_out);
> -
> +#endif
> if (err)
> return rte_flow_error_set
> (error, err,
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 10d700a..a3b82dd 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -222,7 +222,11 @@ struct mlx5_flow_counter {
> uint32_t shared:1; /**< Share counter ID with other flow rules. */
> uint32_t ref_cnt:31; /**< Reference counter. */
> uint32_t id; /**< Counter ID. */
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> + struct ibv_counters *cs; /**< Holds the counters for the rule. */
> +#else
> struct ibv_counter_set *cs; /**< Holds the counters for the rule. */
> +#endif
> uint64_t hits; /**< Number of packets matched by the rule. */
> uint64_t bytes; /**< Number of bytes matched by the rule. */
> };
> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
> b/drivers/net/mlx5/mlx5_flow_verbs.c
> index 05ab5fd..1c8bdba 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -48,27 +48,32 @@
> static struct mlx5_flow_counter *
> flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared, uint32_t id)
> {
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> struct priv *priv = dev->data->dev_private;
> struct mlx5_flow_counter *cnt;
>
> - LIST_FOREACH(cnt, &priv->flow_counters, next) {
> - if (!cnt->shared || cnt->shared != shared)
> - continue;
> - if (cnt->id != id)
> - continue;
> - cnt->ref_cnt++;
> - return cnt;
> + if (shared) {
> + LIST_FOREACH(cnt, &priv->flow_counters, next)
> + if (cnt->shared && cnt->id == id) {
> + cnt->ref_cnt++;
> + return cnt;
> + }
> }
> -#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>
> struct mlx5_flow_counter tmpl = {
> .shared = shared,
> .id = id,
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> + .cs = mlx5_glue->create_counter_set
> + (priv->ctx,
> + &(struct ibv_counters_init_attr){0}),
> +#else
> .cs = mlx5_glue->create_counter_set
> (priv->ctx,
> &(struct ibv_counter_set_init_attr){
> .counter_set_id = id,
> }),
> +#endif
> .hits = 0,
> .bytes = 0,
> };
> @@ -77,17 +82,40 @@
> rte_errno = errno;
> return NULL;
> }
> +
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> + struct ibv_counter_attach_attr attach_attr = {0};
> + int ret;
> +
> + attach_attr.counter_desc = IBV_COUNTER_PACKETS;
> + attach_attr.index = 0;
> + ret = ibv_attach_counters_point_flow(tmpl.cs, &attach_attr, NULL);
> + if (!ret) {
> + attach_attr.counter_desc = IBV_COUNTER_BYTES;
> + attach_attr.index = 1;
> + ret = ibv_attach_counters_point_flow(tmpl.cs,
> + &attach_attr,
> + NULL);
> + }
> + if (ret) {
> + claim_zero(mlx5_glue->destroy_counter_set(tmpl.cs));
> + rte_errno = ret;
> + return NULL;
> + }
> +#endif
> cnt = rte_calloc(__func__, 1, sizeof(*cnt), 0);
> if (!cnt) {
> + claim_zero(mlx5_glue->destroy_counter_set(tmpl.cs));
> rte_errno = ENOMEM;
> return NULL;
> }
> *cnt = tmpl;
> LIST_INSERT_HEAD(&priv->flow_counters, cnt, next);
> return cnt;
> -#endif
> +#else
> rte_errno = ENOTSUP;
> return NULL;
> +#endif
> }
>
> /**
> @@ -947,7 +975,7 @@
> flow->counter = flow_verbs_counter_new(dev, count->shared,
> count->id);
> if (!flow->counter)
> - return rte_flow_error_set(error, ENOTSUP,
> + return rte_flow_error_set(error, rte_errno,
> RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> "cannot get counter"
> @@ -955,7 +983,11 @@
> }
> *action_flags |= MLX5_ACTION_COUNT;
> #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> + counter.counters = flow->counter->cs;
> +#else
> counter.counter_set_handle = flow->counter->cs->handle;
> +#endif
> flow_verbs_spec_add(dev_flow, &counter, size);
> #endif
> return 0;
> diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
> index 48590df..785234c 100644
> --- a/drivers/net/mlx5/mlx5_glue.c
> +++ b/drivers/net/mlx5/mlx5_glue.c
> @@ -211,6 +211,39 @@
> return ibv_dereg_mr(mr);
> }
>
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> +static struct ibv_counters *
> +mlx5_glue_create_counters(struct ibv_context *context,
> + struct ibv_counters_init_attr *init_attr)
> +{
> + return ibv_create_counters(context, init_attr);
> +}
> +
> +static int
> +mlx5_glue_destroy_counters(struct ibv_counters *counters)
> +{
> + return ibv_destroy_counters(counters);
> +}
> +
> +static int
> +mlx5_glue_attach_counters(struct ibv_counters *counters,
> + struct ibv_counter_attach_attr *attr,
> + struct ibv_flow *flow)
> +{
> + return ibv_attach_counters_point_flow(counters, attr, flow);
> +}
> +
> +static int
> +mlx5_glue_query_counters(struct ibv_counters *counters,
> + uint64_t *counters_value,
> + uint32_t ncounters,
> + uint32_t flags)
> +{
> + return ibv_read_counters(counters, counters_value, ncounters, flags);
> +}
> +#endif
> +
> +#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> static struct ibv_counter_set *
> mlx5_glue_create_counter_set(struct ibv_context *context,
> struct ibv_counter_set_init_attr *init_attr)
> @@ -262,6 +295,7 @@
> return ibv_query_counter_set(query_attr, cs_data);
> #endif
> }
> +#endif
>
> static void
> mlx5_glue_ack_async_event(struct ibv_async_event *event)
> @@ -420,10 +454,17 @@
> .modify_qp = mlx5_glue_modify_qp,
> .reg_mr = mlx5_glue_reg_mr,
> .dereg_mr = mlx5_glue_dereg_mr,
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> + .create_counter_set = mlx5_glue_create_counters,
> + .destroy_counter_set = mlx5_glue_destroy_counters,
> + .attach_counter_set = mlx5_glue_attach_counters,
> + .query_counter_set = mlx5_glue_query_counters,
> +#else
> .create_counter_set = mlx5_glue_create_counter_set,
> .destroy_counter_set = mlx5_glue_destroy_counter_set,
> .describe_counter_set = mlx5_glue_describe_counter_set,
> .query_counter_set = mlx5_glue_query_counter_set,
> +#endif
> .ack_async_event = mlx5_glue_ack_async_event,
> .get_async_event = mlx5_glue_get_async_event,
> .port_state_str = mlx5_glue_port_state_str,
> diff --git a/drivers/net/mlx5/mlx5_glue.h b/drivers/net/mlx5/mlx5_glue.h
> index f6e4e38..504d487 100644
> --- a/drivers/net/mlx5/mlx5_glue.h
> +++ b/drivers/net/mlx5/mlx5_glue.h
> @@ -96,6 +96,21 @@ struct mlx5_glue {
> struct ibv_mr *(*reg_mr)(struct ibv_pd *pd, void *addr,
> size_t length, int access);
> int (*dereg_mr)(struct ibv_mr *mr);
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45
> + struct ibv_counters *(*create_counter_set)
> + (struct ibv_context *context,
> + struct ibv_counters_init_attr *init_attr);
> + int (*destroy_counter_set)(struct ibv_counters *cs);
> + int (*attach_counter_set)
> + (struct ibv_counters *cs,
> + struct ibv_counter_attach_attr *attr,
> + struct ibv_flow *flow);
> + int (*query_counter_set)
> + (struct ibv_counters *cs,
> + uint64_t *counters_value,
> + uint32_t ncounters,
> + uint32_t flags);
> +#else
> struct ibv_counter_set *(*create_counter_set)
> (struct ibv_context *context,
> struct ibv_counter_set_init_attr *init_attr);
> @@ -106,6 +121,7 @@ struct mlx5_glue {
> struct ibv_counter_set_description *cs_desc);
> int (*query_counter_set)(struct ibv_query_counter_set_attr *query_attr,
> struct ibv_counter_set_data *cs_data);
> +#endif
> void (*ack_async_event)(struct ibv_async_event *event);
> int (*get_async_event)(struct ibv_context *context,
> struct ibv_async_event *event);
> --
> 1.8.3.1
>
