DPDK patches and discussions
 help / color / mirror / Atom feed
From: Yongseok Koh <yskoh@mellanox.com>
To: Slava Ovsiienko <viacheslavo@mellanox.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Shahaf Shuler <shahafs@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: flow counters support on the Linux-rdma v19 base
Date: Wed, 3 Oct 2018 23:48:18 +0000	[thread overview]
Message-ID: <20181003234810.GH26206@mtidpdk.mti.labs.mlnx> (raw)
In-Reply-To: <1538580540-2224-1-git-send-email-viacheslavo@mellanox.com>

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 <viacheslavo@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
> 

  reply	other threads:[~2018-10-03 23:48 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-20 13:34 [dpdk-dev] [RFC] mlx5: flow counters support on the linux-rdma " viacheslavo
2018-09-27 23:40 ` Yongseok Koh
2018-10-03 15:29 ` [dpdk-dev] [PATCH] net/mlx5: flow counters support on the Linux-rdma " Slava Ovsiienko
2018-10-03 23:48   ` Yongseok Koh [this message]
2018-10-09 13:45     ` Shahaf Shuler
2018-10-11 14:51     ` Slava Ovsiienko
2018-10-14  5:32       ` Shahaf Shuler
2018-10-17 13:53   ` [dpdk-dev] [PATCH v2] " Viacheslav Ovsiienko
2018-10-18  6:34     ` Shahaf Shuler
2018-10-19 15:21     ` [dpdk-dev] [PATCH v3 0/6] net/mlx5: flow counters support for Linux-rdma v19 Viacheslav Ovsiienko
2018-10-19 15:21       ` [dpdk-dev] [PATCH v3 1/6] net/mlx5: flow counters object create function bugfix Viacheslav Ovsiienko
2018-10-21  9:20         ` Shahaf Shuler
2018-10-19 15:21       ` [dpdk-dev] [PATCH v3 2/6] net/mlx5: flow counters new configuration flags Viacheslav Ovsiienko
2018-10-21  9:20         ` Shahaf Shuler
2018-10-19 15:21       ` [dpdk-dev] [PATCH v3 3/6] net/mlx5: flow counters simplifying runtime support check Viacheslav Ovsiienko
2018-10-21  9:20         ` Shahaf Shuler
2018-10-19 15:21       ` [dpdk-dev] [PATCH v3 4/6] net/mlx5: flow counters mlx5 glue library update Viacheslav Ovsiienko
2018-10-21  9:20         ` Shahaf Shuler
2018-10-19 15:21       ` [dpdk-dev] [PATCH v3 5/6] net/mlx5: flow counters query function move and rename Viacheslav Ovsiienko
2018-10-21  9:20         ` Shahaf Shuler
2018-10-19 15:21       ` [dpdk-dev] [PATCH v3 6/6] net/mlx5: flow counters Verbs interface functions update Viacheslav Ovsiienko
2018-10-21  9:20         ` Shahaf Shuler
2018-10-21  9:21       ` [dpdk-dev] [PATCH v3 0/6] net/mlx5: flow counters support for Linux-rdma v19 Shahaf Shuler
2018-10-23 10:04       ` [dpdk-dev] " Slava Ovsiienko
2018-10-23 10:04         ` [dpdk-dev] [PATCH v4 1/8] net/mlx5: fix flow counters creation Slava Ovsiienko
2018-10-23 10:04         ` [dpdk-dev] [PATCH v4 2/8] net/mlx5: rename flow counter configuration macro Slava Ovsiienko
2018-10-23 10:04         ` [dpdk-dev] [PATCH v4 3/8] net/mlx5: introduce new flow counters " Slava Ovsiienko
2018-10-23 10:04         ` [dpdk-dev] [PATCH v4 4/8] net/mlx5: simplify flow counters support check Slava Ovsiienko
2018-10-23 10:04         ` [dpdk-dev] [PATCH v4 5/8] net/mlx5: relocate flow counters query function Slava Ovsiienko
2018-10-23 10:04         ` [dpdk-dev] [PATCH v4 6/8] net/mlx5: add new flow counter Verbs API to glue library Slava Ovsiienko
2018-10-23 10:04         ` [dpdk-dev] [PATCH v4 7/8] net/mlx5: remove unnecessary structure initializers Slava Ovsiienko
2018-10-23 10:04         ` [dpdk-dev] [PATCH v4 8/8] net/mlx5: support new flow counter API Slava Ovsiienko
2018-10-24 16:31           ` Ferruh Yigit
2018-10-24 16:35             ` Ferruh Yigit
2018-10-24 17:25               ` Shahaf Shuler
2018-10-25  8:59                 ` Ferruh Yigit
2018-10-27 10:54           ` [dpdk-dev] [PATCH] net/mlx5: fix flow counters deletion in Verbs Slava Ovsiienko
2018-10-28 12:53             ` Shahaf Shuler
     [not found]       ` <1540289541-30019-1-git-send-email-viacheslavo@mellanox.com>
2018-10-24 11:02         ` [dpdk-dev] [PATCH v4 0/8] net/mlx5: flow counters support for Linux-rdma v19 Shahaf Shuler

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=20181003234810.GH26206@mtidpdk.mti.labs.mlnx \
    --to=yskoh@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=shahafs@mellanox.com \
    --cc=viacheslavo@mellanox.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).