[v3,3/6] net/mlx5: flow counters simplifying runtime support check
Checks
Commit Message
This part of patchset removes the redundant check of counters
support in runtime. The flag flow_counter_en is eliminated
from the code. The Verbs create counter function just returns
an error if no counter support presented in kernel.
Some log messages regarding the counter support type and
presence are added.
mlx5_flow_validate_action_count() is also updated due to
flow_counter_en flag removal.
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
drivers/net/mlx5/mlx5.c | 15 ++++++++++-----
drivers/net/mlx5/mlx5.h | 1 -
drivers/net/mlx5/mlx5_flow.c | 16 +++++++++-------
3 files changed, 19 insertions(+), 13 deletions(-)
Comments
Friday, October 19, 2018 6:21 PM, Slava Ovsiienko:
> Subject: [PATCH v3 3/6] net/mlx5: flow counters simplifying runtime support
> check
How about "net/mlx5: simplify flow counters support check
>
> This part of patchset removes the redundant check of counters support in
> runtime. The flag flow_counter_en is eliminated from the code. The Verbs
> create counter function just returns an error if no counter support presented
> in kernel.
>
> Some log messages regarding the counter support type and presence are
> added.
>
> mlx5_flow_validate_action_count() is also updated due to flow_counter_en
> flag removal.
In continue to previous patch comments, this patch should only make the needed preparation for the new counters.
The new counters macro along with implementation should be on a separate commit.
>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
> drivers/net/mlx5/mlx5.c | 15 ++++++++++-----
> drivers/net/mlx5/mlx5.h | 1 -
> drivers/net/mlx5/mlx5_flow.c | 16 +++++++++-------
> 3 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> bb19085..8a33639 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -1009,12 +1009,17 @@
> 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_V42
> - config.flow_counter_en = !!attr.max_counter_sets;
> +#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
> 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);
> + DRV_LOG(DEBUG, "Flow counters are supported (4.2), "
> + "type = %d, num of cs = %ld, attr = %d",
> + cs_desc.counter_type,
> + cs_desc.num_of_cs,
> + cs_desc.attributes);
I would refine on saying the counters are supported (since you cannot be sure).
I would recommend to have a message log only if the macro is not defined to say "flow counters are not supported". This is the only thing we know for sure.
> +#elif defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
> + DRV_LOG(DEBUG, "Flow counters are supported (4.5)"); #else
> + DRV_LOG(DEBUG, "Flow counters are not supported");
> #endif
> config.ind_table_max_size =
> attr.rss_caps.max_rwq_indirection_table_size;
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> d14239c..74d87c0 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -114,7 +114,6 @@ struct mlx5_dev_config {
> unsigned int tunnel_en:1;
> /* Whether tunnel stateless offloads are supported. */
> unsigned int mpls_en:1; /* MPLS over GRE/UDP is enabled. */
> - unsigned int flow_counter_en:1; /* Whether flow counter is
> supported. */
> unsigned int cqe_comp:1; /* CQE compression is enabled. */
> unsigned int tso:1; /* Whether TSO is supported. */
> unsigned int tx_vec_en:1; /* Tx vector is enabled. */ diff --git
> a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index
> fcabab0..c15722d 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -921,22 +921,24 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> * 0 on success, a negative errno value otherwise and rte_ernno is set.
> */
> int
> -mlx5_flow_validate_action_count(struct rte_eth_dev *dev,
> +mlx5_flow_validate_action_count(struct rte_eth_dev *dev __rte_unused,
> const struct rte_flow_attr *attr,
> struct rte_flow_error *error)
> {
> - struct priv *priv = dev->data->dev_private;
> -
> - if (!priv->config.flow_counter_en)
> - return rte_flow_error_set(error, ENOTSUP,
> - RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> - "flow counters are not supported.");
> +#if !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) && \
> + !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
> + (void)attr;
> + return rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> + "flow counters are not supported."); #else
This ifdef addition is not needed. Just let It fall on the counter creation (like you commit log says).
> if (attr->egress)
> return rte_flow_error_set(error, ENOTSUP,
>
> RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL,
> "count action not supported for "
> "egress");
> return 0;
> +#endif
> }
>
> /**
> --
> 1.8.3.1
@@ -1009,12 +1009,17 @@
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_V42
- config.flow_counter_en = !!attr.max_counter_sets;
+#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
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);
+ DRV_LOG(DEBUG, "Flow counters are supported (4.2), "
+ "type = %d, num of cs = %ld, attr = %d",
+ cs_desc.counter_type,
+ cs_desc.num_of_cs,
+ cs_desc.attributes);
+#elif defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
+ DRV_LOG(DEBUG, "Flow counters are supported (4.5)");
+#else
+ DRV_LOG(DEBUG, "Flow counters are not supported");
#endif
config.ind_table_max_size =
attr.rss_caps.max_rwq_indirection_table_size;
@@ -114,7 +114,6 @@ struct mlx5_dev_config {
unsigned int tunnel_en:1;
/* Whether tunnel stateless offloads are supported. */
unsigned int mpls_en:1; /* MPLS over GRE/UDP is enabled. */
- unsigned int flow_counter_en:1; /* Whether flow counter is supported. */
unsigned int cqe_comp:1; /* CQE compression is enabled. */
unsigned int tso:1; /* Whether TSO is supported. */
unsigned int tx_vec_en:1; /* Tx vector is enabled. */
@@ -921,22 +921,24 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
* 0 on success, a negative errno value otherwise and rte_ernno is set.
*/
int
-mlx5_flow_validate_action_count(struct rte_eth_dev *dev,
+mlx5_flow_validate_action_count(struct rte_eth_dev *dev __rte_unused,
const struct rte_flow_attr *attr,
struct rte_flow_error *error)
{
- struct priv *priv = dev->data->dev_private;
-
- if (!priv->config.flow_counter_en)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ACTION, NULL,
- "flow counters are not supported.");
+#if !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) && \
+ !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
+ (void)attr;
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+ "flow counters are not supported.");
+#else
if (attr->egress)
return rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL,
"count action not supported for "
"egress");
return 0;
+#endif
}
/**