From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shahaf Shuler Subject: Re: [PATCH v2] mlx5: Report imissed stat Date: Tue, 27 Nov 2018 08:09:24 +0000 Message-ID: References: <1542960217-29436-1-git-send-email-barbette@kth.se> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: Yongseok Koh , swx_dpdk To: Tom Barbette , "dev@dpdk.org" Return-path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-eopbgr140058.outbound.protection.outlook.com [40.107.14.58]) by dpdk.org (Postfix) with ESMTP id 918D8262E for ; Tue, 27 Nov 2018 09:09:27 +0100 (CET) In-Reply-To: <1542960217-29436-1-git-send-email-barbette@kth.se> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Friday, November 23, 2018 10:04 AM, Friday, November 23, 2018: > Subject: [PATCH v2] mlx5: Report imissed stat >=20 > The imissed counters (number of packets dropped because the queues were > full) were actually reported through xstats as "rx_out_of_buffer" > but was not reported through stats. >=20 > Following a recent discussion on the ML, as there is no way to tell the u= ser if a > counter is implemented or not, this should be considered a bug. Eg, user > looking at imissed will think the packets are lost before reaching the de= vice. >=20 > As for xstats, I added a base counter to be able to "reset" imissed. >=20 > v2: > - Add rx_discards_phy xstats counter > - Add documentation > - Follow suggestions from Shahaf Shuler as per > v1 comments >=20 > Signed-off-by: Tom Barbette Applied to next-net-mlx, with small modifications due to rebase issues. Tom - please have a second look and confirm. > --- > doc/guides/nics/mlx5.rst | 2 +- > drivers/net/mlx5/mlx5.h | 8 ++++++- > drivers/net/mlx5/mlx5_stats.c | 53 ++++++++++++++++++++++++++++---- > --------- > drivers/net/mlx5/mlx5_trigger.c | 2 +- > 4 files changed, 46 insertions(+), 19 deletions(-) >=20 > diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index > 52e1213..ddc0fdd 100644 > --- a/doc/guides/nics/mlx5.rst > +++ b/doc/guides/nics/mlx5.rst > @@ -141,7 +141,7 @@ Statistics >=20 > MLX5 supports various of methods to report statistics: >=20 > -Port statistics can be queried using ``rte_eth_stats_get()``. The port s= tatistics > are through SW only and counts the number of packets received or sent > successfully by the PMD. > +Port statistics can be queried using ``rte_eth_stats_get()``. The receiv= ed > and sent statistics are through SW only and counts the number of packets > received or sent successfully by the PMD. The imissed counter is the amou= nt > of packets that could not be delivered to SW because a queue was full. > Packets not received due to congestion in the bus or on the NIC can be > queried via the rx_discards_phy xstats counter. >=20 > Extended statistics can be queried using ``rte_eth_xstats_get()``. The > extended statistics expose a wider set of counters counted by the device. > The extended port statistics counts the number of packets received or sen= t > successfully by the port. As Mellanox NICs are using the :ref:`Bifurcated= Linux > Driver ` those counters counts also packet recei= ved > or sent by the Linux kernel. The counters with ``_phy`` suffix counts the= total > events on the physical port, therefore not valid for VF. >=20 > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index > a3a34cf..1aaaafe 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -79,6 +79,11 @@ struct mlx5_xstats_ctrl { > uint64_t base[MLX5_MAX_XSTATS]; > }; >=20 > +struct mlx5_stats_ctrl { > + /* Base for imissed counter. */ > + uint64_t imissed_base; > +}; > + > /* Flow list . */ > TAILQ_HEAD(mlx5_flows, rte_flow); >=20 > @@ -214,6 +219,7 @@ struct priv { > LIST_HEAD(ind_tables, mlx5_ind_table_ibv) ind_tbls; > uint32_t link_speed_capa; /* Link speed capabilities. */ > struct mlx5_xstats_ctrl xstats_ctrl; /* Extended stats control. */ > + struct mlx5_stats_ctrl stats_ctrl; /* Stats control. */ > int primary_socket; /* Unix socket for primary process. */ > void *uar_base; /* Reserved address space for UAR mapping */ > struct rte_intr_handle intr_handle_socket; /* Interrupt handler. */ > @@ -309,7 +315,7 @@ void mlx5_allmulticast_disable(struct rte_eth_dev > *dev); >=20 > /* mlx5_stats.c */ >=20 > -void mlx5_xstats_init(struct rte_eth_dev *dev); > +void mlx5_stats_init(struct rte_eth_dev *dev); > int mlx5_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)= ; > void mlx5_stats_reset(struct rte_eth_dev *dev); int mlx5_xstats_get(stru= ct > rte_eth_dev *dev, struct rte_eth_xstat *stats, diff --git > a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c index > 91f3d47..323f8fb 100644 > --- a/drivers/net/mlx5/mlx5_stats.c > +++ b/drivers/net/mlx5/mlx5_stats.c > @@ -108,6 +108,14 @@ static const struct mlx5_counter_ctrl > mlx5_counters_init[] =3D { > .ctr_name =3D "rx_packets_phy", > }, > { > + .dpdk_name =3D "tx_discards_phy", > + .ctr_name =3D "tx_discards_phy", > + }, > + { > + .dpdk_name =3D "rx_discards_phy", > + .ctr_name =3D "rx_discards_phy", > + }, > + { > .dpdk_name =3D "tx_bytes_phy", > .ctr_name =3D "tx_bytes_phy", > }, > @@ -119,6 +127,24 @@ static const struct mlx5_counter_ctrl > mlx5_counters_init[] =3D { >=20 > static const unsigned int xstats_n =3D RTE_DIM(mlx5_counters_init); >=20 > +static inline void > +mlx5_read_ib_stat(struct priv *priv, const char *ctr_name, uint64_t > +*stat) { > + FILE *file; > + MKSTR(path, "%s/ports/1/hw_counters/%s", > + priv->ibdev_path, > + ctr_name); > + > + file =3D fopen(path, "rb"); > + if (file) { > + int n =3D fscanf(file, "%" SCNu64, stat); > + > + fclose(file); > + if (n !=3D 1) > + stat =3D 0; > + } > +} > + > /** > * Read device counters table. > * > @@ -155,19 +181,8 @@ mlx5_read_dev_counters(struct rte_eth_dev *dev, > uint64_t *stats) > } > for (i =3D 0; i !=3D xstats_n; ++i) { > if (mlx5_counters_init[i].ib) { > - FILE *file; > - MKSTR(path, "%s/ports/1/hw_counters/%s", > - priv->ibdev_path, > - mlx5_counters_init[i].ctr_name); > - > - file =3D fopen(path, "rb"); > - if (file) { > - int n =3D fscanf(file, "%" SCNu64, &stats[i]); > - > - fclose(file); > - if (n !=3D 1) > - stats[i] =3D 0; > - } > + mlx5_read_ib_stat(priv, > mlx5_counters_init[i].ctr_name, > + &stats[i]); > } else { > stats[i] =3D (uint64_t) > et_stats->data[xstats_ctrl- > >dev_table_idx[i]]; > @@ -210,10 +225,11 @@ mlx5_ethtool_get_stats_n(struct rte_eth_dev > *dev) { > * Pointer to Ethernet device. > */ > void > -mlx5_xstats_init(struct rte_eth_dev *dev) > +mlx5_stats_init(struct rte_eth_dev *dev) > { > struct priv *priv =3D dev->data->dev_private; > struct mlx5_xstats_ctrl *xstats_ctrl =3D &priv->xstats_ctrl; > + struct mlx5_stats_ctrl *stats_ctrl =3D &priv->stats_ctrl; > unsigned int i; > unsigned int j; > struct ifreq ifr; > @@ -281,6 +297,7 @@ mlx5_xstats_init(struct rte_eth_dev *dev) > if (ret) > 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); > free: > rte_free(strings); > } > @@ -316,7 +333,7 @@ mlx5_xstats_get(struct rte_eth_dev *dev, struct > rte_eth_xstat *stats, > if (stats_n < 0) > return stats_n; > if (xstats_ctrl->stats_n !=3D stats_n) > - mlx5_xstats_init(dev); > + mlx5_stats_init(dev); > ret =3D mlx5_read_dev_counters(dev, counters); > if (ret) > return ret; > @@ -389,6 +406,8 @@ mlx5_stats_get(struct rte_eth_dev *dev, struct > rte_eth_stats *stats) #endif > tmp.oerrors +=3D txq->stats.oerrors; > } > + mlx5_read_ib_stat(priv, "out_of_buffer", &tmp.imissed); > + tmp.imissed -=3D priv->stats_ctrl.imissed_base; > #ifndef MLX5_PMD_SOFT_COUNTERS > /* FIXME: retrieve and add hardware counters. */ #endif @@ -406,6 > +425,7 @@ void mlx5_stats_reset(struct rte_eth_dev *dev) { > struct priv *priv =3D dev->data->dev_private; > + struct mlx5_stats_ctrl *stats_ctrl =3D &priv->stats_ctrl; > unsigned int i; > unsigned int idx; >=20 > @@ -423,6 +443,7 @@ mlx5_stats_reset(struct rte_eth_dev *dev) > (*priv->txqs)[i]->stats =3D > (struct mlx5_txq_stats){ .idx =3D idx }; > } > + mlx5_read_ib_stat(priv, "out_of_buffer", &stats_ctrl- > >imissed_base); > #ifndef MLX5_PMD_SOFT_COUNTERS > /* FIXME: reset hardware counters. */ > #endif > @@ -452,7 +473,7 @@ mlx5_xstats_reset(struct rte_eth_dev *dev) > return; > } > if (xstats_ctrl->stats_n !=3D stats_n) > - mlx5_xstats_init(dev); > + mlx5_stats_init(dev); > ret =3D mlx5_read_dev_counters(dev, counters); > if (ret) { > DRV_LOG(ERR, "port %u cannot read device counters: %s", > diff --git a/drivers/net/mlx5/mlx5_trigger.c > b/drivers/net/mlx5/mlx5_trigger.c index e2a9bb7..3015edd 100644 > --- a/drivers/net/mlx5/mlx5_trigger.c > +++ b/drivers/net/mlx5/mlx5_trigger.c > @@ -181,7 +181,7 @@ mlx5_dev_start(struct rte_eth_dev *dev) > dev->data->port_id); > goto error; > } > - mlx5_xstats_init(dev); > + mlx5_stats_init(dev); > ret =3D mlx5_traffic_enable(dev); > if (ret) { > DRV_LOG(DEBUG, "port %u failed to set defaults flows", > -- > 2.7.4