Re: [dpdk-dev] [PATCH v2] net/mlx5: flow counters support on the Linux-rdma v19 base

[Shahaf Shuler <shahafs@mellanox.com>]

Hi Slava,

Please see some comments below.
Also - please rebase on top of series https://patches.dpdk.org/project/dpdk/list/?series=1961
It moves the verbs flow query to the mlx5_flow_verbs.c

Wednesday, October 17, 2018 4:54 PM, Slava Ovsiienko:
> Subject: [PATCH v2] net/mlx5: flow counters support on the Linux-rdma v19
> base
> 
> 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.

Counter -> counter

> 
> 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>
> 
> ---
> v2:
> - rebased on top of master-net-mlx branch
> - new compilation flags are introduced:
>     - HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V42, kernel/verbs
>       library provides the flow counter support in style of
>       MLNX_OFED_4.2 to MLNX_OFED_4.4
>     - HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45, kernel/verbs
>       library provides the flow counter support in style of
>       MLNX_OFED_4.5 or higher
> 
> v1:
> - http://patches.dpdk.org/patch/45972/
> ---
>  drivers/net/mlx5/Makefile          |  7 ++-
>  drivers/net/mlx5/meson.build       |  4 +-
>  drivers/net/mlx5/mlx5.c            | 13 +++++-
>  drivers/net/mlx5/mlx5_flow.c       | 27 +++++++++---
>  drivers/net/mlx5/mlx5_flow.h       |  5 +++
>  drivers/net/mlx5/mlx5_flow_verbs.c | 81
> +++++++++++++++++++++++++++--------
>  drivers/net/mlx5/mlx5_glue.c       | 88
> +++++++++++++++++++++++++++++++-------
>  drivers/net/mlx5/mlx5_glue.h       | 22 +++++++++-
>  8 files changed, 202 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile index
> 1e9c0b4..fecb57c 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -157,11 +157,16 @@ mlx5_autoconf.h.new:
> $(RTE_SDK)/buildtools/auto-config-h.sh
>  		enum ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT \
>  		$(AUTOCONF_OUTPUT)
>  	$Q sh -- '$<' '$@' \
> -		HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT \
> +		HAVE_IBV_DEVICE_COUNTERS_SET_V42 \

Better to have this change of MACRO name as a separate commit. 

>  		infiniband/verbs.h \
>  		type 'struct ibv_counter_set_init_attr' \
>  		$(AUTOCONF_OUTPUT)
>  	$Q sh -- '$<' '$@' \
> +		HAVE_IBV_DEVICE_COUNTERS_SET_V45 \
> +		infiniband/verbs.h \
> +		type 'struct ibv_counters_init_attr' \
> +		$(AUTOCONF_OUTPUT)
> +	$Q sh -- '$<' '$@' \
>  		HAVE_RDMA_NL_NLDEV \
>  		rdma/rdma_netlink.h \
>  		enum RDMA_NL_NLDEV \
> diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
> index c192d44..e8cbe3e 100644
> --- a/drivers/net/mlx5/meson.build
> +++ b/drivers/net/mlx5/meson.build
> @@ -79,8 +79,10 @@ if build
>  	has_member_args = [
>  		[ 'HAVE_IBV_MLX5_MOD_SWP', 'infiniband/mlx5dv.h',
>  		'struct mlx5dv_sw_parsing_caps', 'sw_parsing_offloads' ],
> -		[ 'HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT',
> 'infiniband/verbs.h',
> +		[ 'HAVE_IBV_DEVICE_COUNTERS_SET_V42',
> 'infiniband/verbs.h',
>  		'struct ibv_counter_set_init_attr', 'counter_set_id' ],
> +		[ 'HAVE_IBV_DEVICE_COUNTERS_SET_V45',
> 'infiniband/verbs.h',
> +		'struct ibv_counters_init_attr', 'comp_mask' ],
>  	]
>  	# input array for meson symbol search:
>  	# [ "MACRO to define if found", "header for the search", diff --git
> a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> 795a219..7dbe4bc 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -739,7 +739,7 @@
>  	unsigned int mprq_max_stride_size_n = 0;
>  	unsigned int mprq_min_stride_num_n = 0;
>  	unsigned int mprq_max_stride_num_n = 0; -#ifdef
> HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
>  	struct ibv_counter_set_description cs_desc = { .counter_type = 0 };
> #endif
>  	struct ether_addr mac;
> @@ -1009,13 +1009,22 @@
>  	config.hw_csum = !!(attr.device_cap_flags_ex &
> IBV_DEVICE_RAW_IP_CSUM);
>  	DRV_LOG(DEBUG, "checksum offloading is %ssupported",
>  		(config.hw_csum ? "" : "not "));
> -#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> +#if !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) && \
> +	!defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)

Like we discussed over the phone. Since this condition (has any of the flow counter) repeats multiple times on this commit, it is better to define it as a macro.
Something like
#define MLX5_VERBS_HAS_FLOW_COUNTER (defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45))

> +	DRV_LOG(DEBUG, "no flow counters support found, OFED 4.2+ "
> +		       "is required for counters support"); #endif #ifdef
> +HAVE_IBV_DEVICE_COUNTERS_SET_V42
>  	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);

I never liked this log, it is too developer specific. Also the flwo_counter_en seems redundant, assuming we have the APIs we can just try to create a counter and if it fails then there is not counter support on the kernel level (this is actually the logic on the new counter approach).  
How about the following logic (following previous commit):
1. remove the flow_counter_en flag (separate commit)
2. the logic here will be:
#ifdef MLX5_VERBS_HAS_FLOW_COUNTER
	DRV_LOG(DEBUG, "Flow counters are supported")  //the user doesn't really care if those are the new/old
#else
	DRV_LOG(DEBUG, "Flow counters are not supported")
#endif

?

>  #endif
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V45
> +	DRV_LOG(DEBUG, "Flow counters support OFED 4.5+/upstream
> assumed");
> +	config.flow_counter_en = 1;
> +#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 bd70fce..4df114d 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -2352,15 +2352,17 @@ struct rte_flow *
>   * @return
>   *   0 on success, a negative errno value otherwise and rte_errno is set.
>   */
> +#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || \
> +	defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)

Better to insert the ifdef inside the function, so that you will not need to declare it two times. 
Then it will be like:
mlx5_flow_query_count(...) {
#ifdef MLX5_VERBS_HAS_FLOW_COUNTER
	Function logic
#else
	Return not supported
}

>  static int
> -mlx5_flow_query_count(struct rte_flow *flow __rte_unused,
> -		      void *data __rte_unused,
> +mlx5_flow_query_count(struct rte_flow *flow,
> +		      void *data,
>  		      struct rte_flow_error *error)
>  {
> -#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>  	if (flow->actions & MLX5_FLOW_ACTION_COUNT) {
>  		struct rte_flow_query_count *qc = data;
>  		uint64_t counters[2] = {0, 0};

Better to have the logic from here..

> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
>  		struct ibv_query_counter_set_attr query_cs_attr = {
>  			.cs = flow->counter->cs,
>  			.query_flags = IBV_COUNTER_SET_FORCE_UPDATE,
> @@ -2371,7 +2373,14 @@ struct rte_flow *
>  		};
>  		int err = mlx5_glue->query_counter_set(&query_cs_attr,
>  						       &query_out);
> -
> +#endif
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V45
> +		int err = mlx5_glue->query_counter_set(
> +				flow->counter->cs,
> +				counters,
> +				RTE_DIM(counters),
> +
> 	IBV_READ_COUNTERS_ATTR_PREFER_CACHED);
> +#endif

Till here in a separate function. 
Also use:
#Ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
		Logic
#else ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V45
		Logic
#endif

It is safer and the code is more clear. 


>  		if (err)
>  			return rte_flow_error_set
>  				(error, err,
> @@ -2392,12 +2401,20 @@ struct rte_flow *
>  				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>  				  NULL,
>  				  "flow does not have counter");
> -#endif
> +}
> +#else
> +static int
> +mlx5_flow_query_count(struct rte_flow *flow __rte_unused,
> +		      void *data __rte_unused,
> +		      struct rte_flow_error *error)
> +{
>  	return rte_flow_error_set(error, ENOTSUP,
>  				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>  				  NULL,
>  				  "counters are not available");
>  }
> +#endif /* HAVE_IBV_DEVICE_COUNTERS_SET_Vxx */
> +
> 
>  /**
>   * Query a flows.
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 094f666..d2856c3 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -224,7 +224,12 @@ 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_V42
>  	struct ibv_counter_set *cs; /**< Holds the counters for the rule. */
> +#endif
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V45
> +	struct ibv_counters *cs; /**< Holds the counters for the rule. */
> +#endif

Ditto. 
If -> else if -> 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 65c849c..6d5f900 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -46,30 +46,37 @@
>   * @return
>   *   A pointer to the counter, NULL otherwise and rte_errno is set.
>   */
> +#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || \
> +	defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)

Like before, move the macro to the function body. 
And need to return NULL and errno = ENOTSUP in case there is no support. 

>  static struct mlx5_flow_counter *
>  flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared,
> uint32_t id)  {
>  	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;
> +		}

This logic change should be on a separate commit as it is not part of the new counter support. 

>  	}
> -#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> 
>  	struct mlx5_flow_counter tmpl = {
>  		.shared = shared,
>  		.id = id,

Better to have all the logic of the counter creation along with the macros for the diff implementation inside a separate function. 

> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
>  		.cs = mlx5_glue->create_counter_set
>  			(priv->ctx,
>  			 &(struct ibv_counter_set_init_attr){
>  				 .counter_set_id = id,
>  			 }),
> +#endif
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V45
> +		.cs = mlx5_glue->create_counter_set
> +			(priv->ctx,
> +			 &(struct ibv_counters_init_attr){0}), #endif
>  		.hits = 0,
>  		.bytes = 0,
>  	};
> @@ -78,17 +85,36 @@
>  		rte_errno = errno;
>  		return NULL;
>  	}
> +
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_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));

Bug fix? 
If yes then separate commit. For bug fixes it is super important so that we can have it backported to the stable releases. 

>  		rte_errno = ENOMEM;
>  		return NULL;
>  	}
>  	*cnt = tmpl;
>  	LIST_INSERT_HEAD(&priv->flow_counters, cnt, next);
>  	return cnt;
> -#endif
> -	rte_errno = ENOTSUP;
> -	return NULL;
>  }
> 
>  /**
> @@ -106,6 +132,7 @@
>  		rte_free(counter);
>  	}
>  }
> +#endif /* HAVE_IBV_DEVICE_COUNTERS_SET_Vxx */
> 
>  /**
>   * Add a verbs item specification into @p flow.
> @@ -927,6 +954,8 @@
>   * @return
>   *   0 On success else a negative errno value is returned and rte_errno is set.
>   */
> +#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || \
> +	defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)

Ditto. 
Macro inside and on the else part return notsup. 

>  static int
>  flow_verbs_translate_action_count(struct rte_eth_dev *dev,
>  				  const struct rte_flow_action *action, @@ -
> 936,13 +965,11 @@  {
>  	const struct rte_flow_action_count *count = action->conf;
>  	struct rte_flow *flow = dev_flow->flow; -#ifdef
> HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>  	unsigned int size = sizeof(struct ibv_flow_spec_counter_action);
>  	struct ibv_flow_spec_counter_action counter = {
>  		.type = IBV_FLOW_SPEC_ACTION_COUNT,
>  		.size = size,
>  	};
> -#endif
> 
>  	if (!flow->counter) {
>  		flow->counter = flow_verbs_counter_new(dev, count-
> >shared, @@ -955,12 +982,16 @@
>  						  " context.");
>  	}
>  	*action_flags |= MLX5_FLOW_ACTION_COUNT; -#ifdef
> HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
>  	counter.counter_set_handle = flow->counter->cs->handle;
> -	flow_verbs_spec_add(dev_flow, &counter, size);
>  #endif
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V45
> +	counter.counters = flow->counter->cs;
> +#endif
> +	flow_verbs_spec_add(dev_flow, &counter, size);
>  	return 0;
>  }
> +#endif /* HAVE_IBV_DEVICE_COUNTERS_SET_Vxx */
> 
>  /**
>   * Internal validation function. For validating both actions and items.
> @@ -1157,12 +1188,15 @@
>  				return ret;
>  			action_flags |= MLX5_FLOW_ACTION_RSS;
>  			break;
> +#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || \
> +	defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)

This is not needed. It is OK to validate the count action, the code will fail to create the counters after with errno of not supported.
On the current logic you just ignore the counter action request instead of rejecting it as not supported. 

>  		case RTE_FLOW_ACTION_TYPE_COUNT:
>  			ret = mlx5_flow_validate_action_count(dev, attr,
> error);
>  			if (ret < 0)
>  				return ret;
>  			action_flags |= MLX5_FLOW_ACTION_COUNT;
>  			break;
> +#endif
>  		default:
>  			return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ACTION,
> @@ -1219,11 +1253,12 @@
>  		case RTE_FLOW_ACTION_TYPE_RSS:
>  			detected_actions |= MLX5_FLOW_ACTION_RSS;
>  			break;
> +#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || \
> +	defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
>  		case RTE_FLOW_ACTION_TYPE_COUNT:
> -#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>  			size += sizeof(struct ibv_flow_spec_counter_action);
> -#endif
>  			detected_actions |= MLX5_FLOW_ACTION_COUNT;
> +#endif

Keep the ifdef as before (only on the size+=). The counter creation will fail on the translate. 

>  			break;
>  		default:
>  			break;
> @@ -1406,7 +1441,6 @@
>  	if (priority == MLX5_FLOW_PRIO_RSVD)
>  		priority = priv->config.flow_prio - 1;
>  	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
> -		int ret;
>  		switch (actions->type) {
>  		case RTE_FLOW_ACTION_TYPE_VOID:
>  			break;
> @@ -1434,7 +1468,11 @@
>  							&action_flags,
>  							dev_flow);
>  			break;
> -		case RTE_FLOW_ACTION_TYPE_COUNT:
> +#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || \
> +	defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)

Same. No need for this macro, you have one inside the translate function. 

> +		case RTE_FLOW_ACTION_TYPE_COUNT: {
> +			int ret;
> +
>  			ret = flow_verbs_translate_action_count(dev,
>  								actions,
> 
> 	&action_flags,
> @@ -1443,6 +1481,8 @@
>  			if (ret < 0)
>  				return ret;
>  			break;
> +		}
> +#endif
>  		default:
>  			return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ACTION,
> @@ -1538,10 +1578,13 @@
>  			verbs->hrxq = NULL;
>  		}
>  	}
> +#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || \
> +	defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)

Why the macro is needed? If no counter support flow->counter will always be NULL. 

>  	if (flow->counter) {
>  		flow_verbs_counter_release(flow->counter);
>  		flow->counter = NULL;
>  	}
> +#endif
>  }
> 
>  /**
> diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
> index 48590df..d77adb8 100644
> --- a/drivers/net/mlx5/mlx5_glue.c
> +++ b/drivers/net/mlx5/mlx5_glue.c

For the whole glue part, you didn't address my comments from the V1. Attaching those again:
" Pay attention to how the old counter supported was added to the mlx5_glue lib.
There are no ifdefs of the function pointer declaration nor on the function declaration. This was because it was causing link issues between this lib to verbs. 

Same should be for the new counters. Need to declare all needed structs if not declared on the mlx5_glue.h file, and always the function pointer and the declaration. 
The ifdef is only inside the function implementation."

> @@ -211,28 +211,84 @@
>  	return ibv_dereg_mr(mr);
>  }
> 
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_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 /* HAVE_IBV_DEVICE_COUNTERS_SET_V45 */
> +
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42 static struct
> ibv_counter_set *
> +mlx5_glue_create_counter_set(struct ibv_context *context,
> +			     struct ibv_counter_set_init_attr *init_attr) {
> +	return ibv_create_counter_set(context, init_attr); }
> +
> +static int
> +mlx5_glue_destroy_counter_set(struct ibv_counter_set *cs) {
> +	return ibv_destroy_counter_set(cs);
> +}
> +
> +static int
> +mlx5_glue_describe_counter_set(struct ibv_context *context,
> +			       uint16_t counter_set_id,
> +			       struct ibv_counter_set_description *cs_desc) {
> +	return ibv_describe_counter_set(context, counter_set_id, cs_desc);
> }
> +
> +static int
> +mlx5_glue_query_counter_set(struct ibv_query_counter_set_attr
> *query_attr,
> +			    struct ibv_counter_set_data *cs_data) {
> +	return ibv_query_counter_set(query_attr, cs_data); } #endif /*
> +HAVE_IBV_DEVICE_COUNTERS_SET_V42 */
> +
> +#if !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) && \
> +	!defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
>  static struct ibv_counter_set *
>  mlx5_glue_create_counter_set(struct ibv_context *context,
>  			     struct ibv_counter_set_init_attr *init_attr)  { -
> #ifndef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>  	(void)context;
>  	(void)init_attr;
>  	return NULL;
> -#else
> -	return ibv_create_counter_set(context, init_attr);
> -#endif
>  }
> 
>  static int
>  mlx5_glue_destroy_counter_set(struct ibv_counter_set *cs)  { -#ifndef
> HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>  	(void)cs;
>  	return ENOTSUP;
> -#else
> -	return ibv_destroy_counter_set(cs);
> -#endif
>  }
> 
>  static int
> @@ -240,28 +296,21 @@
>  			       uint16_t counter_set_id,
>  			       struct ibv_counter_set_description *cs_desc)  { -
> #ifndef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>  	(void)context;
>  	(void)counter_set_id;
>  	(void)cs_desc;
>  	return ENOTSUP;
> -#else
> -	return ibv_describe_counter_set(context, counter_set_id, cs_desc);
> -#endif
>  }
> 
>  static int
>  mlx5_glue_query_counter_set(struct ibv_query_counter_set_attr
> *query_attr,
>  			    struct ibv_counter_set_data *cs_data)  { -#ifndef
> HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>  	(void)query_attr;
>  	(void)cs_data;
>  	return ENOTSUP;
> -#else
> -	return ibv_query_counter_set(query_attr, cs_data);
> -#endif
>  }
> +#endif /* HAVE_IBV_DEVICE_COUNTERS_SET_Vxx */
> 
>  static void
>  mlx5_glue_ack_async_event(struct ibv_async_event *event) @@ -420,10
> +469,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_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..b9033de 100644
> --- a/drivers/net/mlx5/mlx5_glue.h
> +++ b/drivers/net/mlx5/mlx5_glue.h
> @@ -23,7 +23,11 @@
>  #define MLX5_GLUE_VERSION ""
>  #endif
> 
> -#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> +#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V45 struct ibv_counters; struct
> +ibv_counters_attach_data; struct ibv_counters_init_attr; #else
>  struct ibv_counter_set;
>  struct ibv_counter_set_data;
>  struct ibv_counter_set_description;
> @@ -96,6 +100,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_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
> +125,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