> -----Original Message-----
> From: Jiawei Wang <jiaw...@mellanox.com>
> Sent: Monday, March 30, 2020 6:02
> To: Slava Ovsiienko <viachesl...@mellanox.com>; Matan Azrad
> <ma...@mellanox.com>; Jiawei(Jonny) Wang <jiaw...@mellanox.com>
> Cc: Raslan Darawsheh <rasl...@mellanox.com>; dev@dpdk.org;
> sta...@dpdk.org
> Subject: [PATCH] net/mlx5: fix imissed counter overflow issue
>
> The Hw counters is defined as 32bit unsigned value and read from the sysfs.
> Firstly read the base value while application start, then fetch the new value
> while do query and minus the base value.
> If the new value is less than base value, will result in the a negative value
> and
> convert to the big value as unsigned 64bit.
>
> PMD add xstats field to store the last successfully read counter, use it if
> failed
> to read hw counter from sysfs.
> PMD also record the last output value to handle the wrap around case, if
> overflow happened, increase the wrap count by 1 and save into the higher
> 32bit, and update the new value into lower 32bit, finally return the 64bit
> counter value.
>
> Fixes: ce9494d76c4 ("net/mlx5: report imissed statistics")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Jiawei Wang <jiaw...@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
> ---
> drivers/net/mlx5/mlx5.h | 3 ++
> drivers/net/mlx5/mlx5_stats.c | 57 ++++++++++++++++++++++++++++++-----
> 2 files changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> d7c519bae0..c01fae8c8e 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -112,12 +112,15 @@ struct mlx5_xstats_ctrl {
> /* Index in the device counters table. */
> uint16_t dev_table_idx[MLX5_MAX_XSTATS];
> uint64_t base[MLX5_MAX_XSTATS];
> + uint64_t xstats[MLX5_MAX_XSTATS];
> + uint64_t hw_stats[MLX5_MAX_XSTATS];
> struct mlx5_counter_ctrl info[MLX5_MAX_XSTATS]; };
>
> struct mlx5_stats_ctrl {
> /* Base for imissed counter. */
> uint64_t imissed_base;
> + uint64_t imissed;
> };
>
> /* Flow list . */
> diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
> index 7603502967..5bc6fa6aa1 100644
> --- a/drivers/net/mlx5/mlx5_stats.c
> +++ b/drivers/net/mlx5/mlx5_stats.c
> @@ -139,7 +139,7 @@ static const struct mlx5_counter_ctrl
> mlx5_counters_init[] = {
>
> static const unsigned int xstats_n = RTE_DIM(mlx5_counters_init);
>
> -static inline void
> +static inline int
> mlx5_read_ib_stat(struct mlx5_priv *priv, const char *ctr_name, uint64_t
> *stat) {
> FILE *file;
> @@ -155,10 +155,11 @@ mlx5_read_ib_stat(struct mlx5_priv *priv, const
> char *ctr_name, uint64_t *stat)
>
> fclose(file);
> if (n == 1)
> - return;
> + return 0;
> }
> }
> *stat = 0;
> + return 1;
> }
>
> /**
> @@ -197,8 +198,14 @@ mlx5_read_dev_counters(struct rte_eth_dev *dev,
> uint64_t *stats)
> }
> for (i = 0; i != xstats_ctrl->mlx5_stats_n; ++i) {
> if (xstats_ctrl->info[i].ib) {
> - mlx5_read_ib_stat(priv, xstats_ctrl->info[i].ctr_name,
> - &stats[i]);
> + ret = mlx5_read_ib_stat(priv,
> + xstats_ctrl->info[i].ctr_name,
> + &stats[i]);
> + /* return last xstats counter if fail to read. */
> + if (ret == 0)
> + xstats_ctrl->xstats[i] = stats[i];
> + else
> + stats[i] = xstats_ctrl->xstats[i];
> } else {
> stats[i] = (uint64_t)
> et_stats->data[xstats_ctrl->dev_table_idx[i]];
> @@ -304,6 +311,7 @@ mlx5_stats_init(struct rte_eth_dev *dev)
> unsigned int idx = xstats_ctrl->mlx5_stats_n++;
>
> xstats_ctrl->info[idx] = mlx5_counters_init[i];
> + xstats_ctrl->hw_stats[idx] = 0;
> }
> }
> MLX5_ASSERT(xstats_ctrl->mlx5_stats_n <= MLX5_MAX_XSTATS); @@
> -314,6 +322,7 @@ mlx5_stats_init(struct rte_eth_dev *dev)
> DRV_LOG(ERR, "port %u cannot read device counters: %s",
> dev->data->port_id, strerror(rte_errno));
> mlx5_read_ib_stat(priv, "out_of_buffer", &stats_ctrl->imissed_base);
> + stats_ctrl->imissed = 0;
> free:
> rte_free(strings);
> }
> @@ -356,7 +365,23 @@ mlx5_xstats_get(struct rte_eth_dev *dev, struct
> rte_eth_xstat *stats,
> return ret;
> for (i = 0; i != mlx5_stats_n; ++i) {
> stats[i].id = i;
> - stats[i].value = (counters[i] - xstats_ctrl->base[i]);
> + if (xstats_ctrl->info[i].ib) {
> + uint64_t wrap_n;
> + uint64_t hw_stat = xstats_ctrl->hw_stats[i];
> +
> + stats[i].value = (counters[i] -
> + xstats_ctrl->base[i]) &
> + (uint64_t)UINT32_MAX;
> + wrap_n = hw_stat >> 32;
> + if (stats[i].value <
> + (hw_stat &
> (uint64_t)UINT32_MAX))
> + wrap_n++;
> + stats[i].value |= (wrap_n) << 32;
> + xstats_ctrl->hw_stats[i] = stats[i].value;
> + } else {
> + stats[i].value =
> + (counters[i] - xstats_ctrl->base[i]);
> + }
> }
> }
> return mlx5_stats_n;
> @@ -378,9 +403,12 @@ int
> mlx5_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) {
> struct mlx5_priv *priv = dev->data->dev_private;
> + struct mlx5_stats_ctrl *stats_ctrl = &priv->stats_ctrl;
> struct rte_eth_stats tmp;
> unsigned int i;
> unsigned int idx;
> + uint64_t wrap_n;
> + int ret;
>
> memset(&tmp, 0, sizeof(tmp));
> /* Add software counters. */
> @@ -423,8 +451,18 @@ mlx5_stats_get(struct rte_eth_dev *dev, struct
> rte_eth_stats *stats) #endif
> tmp.oerrors += txq->stats.oerrors;
> }
> - mlx5_read_ib_stat(priv, "out_of_buffer", &tmp.imissed);
> - tmp.imissed -= priv->stats_ctrl.imissed_base;
> + ret = mlx5_read_ib_stat(priv, "out_of_buffer", &tmp.imissed);
> + if (ret == 0) {
> + tmp.imissed = (tmp.imissed - stats_ctrl->imissed_base) &
> + (uint64_t)UINT32_MAX;
> + wrap_n = stats_ctrl->imissed >> 32;
> + if (tmp.imissed < (stats_ctrl->imissed &
> (uint64_t)UINT32_MAX))
> + wrap_n++;
> + tmp.imissed |= (wrap_n) << 32;
> + stats_ctrl->imissed = tmp.imissed;
> + } else {
> + tmp.imissed = stats_ctrl->imissed;
> + }
> #ifndef MLX5_PMD_SOFT_COUNTERS
> /* FIXME: retrieve and add hardware counters. */ #endif @@ -461,6
> +499,7 @@ mlx5_stats_reset(struct rte_eth_dev *dev)
> sizeof(struct mlx5_txq_stats));
> }
> mlx5_read_ib_stat(priv, "out_of_buffer", &stats_ctrl->imissed_base);
> + stats_ctrl->imissed = 0;
> #ifndef MLX5_PMD_SOFT_COUNTERS
> /* FIXME: reset hardware counters. */
> #endif
> @@ -503,8 +542,10 @@ mlx5_xstats_reset(struct rte_eth_dev *dev)
> dev->data->port_id, strerror(rte_errno));
> return ret;
> }
> - for (i = 0; i != n; ++i)
> + for (i = 0; i != n; ++i) {
> xstats_ctrl->base[i] = counters[i];
> + xstats_ctrl->hw_stats[i] = 0;
> + }
>
> return 0;
> }
> --
> 2.21.0
