From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-4.sys.kth.se (smtp-4.sys.kth.se [130.237.48.193]) by dpdk.org (Postfix) with ESMTP id A6FD34F9C for ; Wed, 14 Nov 2018 17:18:22 +0100 (CET) Received: from smtp-4.sys.kth.se (localhost.localdomain [127.0.0.1]) by smtp-4.sys.kth.se (Postfix) with ESMTP id 56ACF63E; Wed, 14 Nov 2018 17:18:22 +0100 (CET) X-Virus-Scanned: by amavisd-new at kth.se Received: from smtp-4.sys.kth.se ([127.0.0.1]) by smtp-4.sys.kth.se (smtp-4.sys.kth.se [127.0.0.1]) (amavisd-new, port 10024) with LMTP id uVKz3vxJaxs6; Wed, 14 Nov 2018 17:18:21 +0100 (CET) Received: from exdb01.ug.kth.se (unknown [192.168.32.111]) by smtp-4.sys.kth.se (Postfix) with ESMTPS id D6F6B9B; Wed, 14 Nov 2018 17:18:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kth.se; s=default; t=1542212301; bh=92EQKBt+eEKyC4ExuEmp0IL6AkIWfECEsOOO6AnzXmE=; h=From:To:CC:Subject:Date:References:In-Reply-To; b=FPPByFHV9n2YlFBng/vltyfE7iKaTKUXie/ZZLoxzcgmisw0CxVQKijavI9JyZKei sHLQ4CjDLQlHJRz6g04mNBRY3Cx5bcnHZGDWlt0nGhtiQBNLRCHcB7xWWuAPBzNdYt rsMdNEUZz37xghDwnelYc/bFq7huzKO84JOU4KLU= Received: from exdb06.ug.kth.se (192.168.32.116) by exdb01.ug.kth.se (192.168.32.111) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Wed, 14 Nov 2018 17:18:20 +0100 Received: from exdb05.ug.kth.se (192.168.32.115) by exdb06.ug.kth.se (192.168.32.116) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Wed, 14 Nov 2018 17:18:20 +0100 Received: from exdb05.ug.kth.se ([192.168.32.115]) by exdb05.ug.kth.se ([192.168.32.115]) with mapi id 15.00.1367.000; Wed, 14 Nov 2018 17:18:20 +0100 From: Tom Barbette To: Shahaf Shuler , "dev@dpdk.org" CC: Yongseok Koh Thread-Topic: [PATCH] mlx5: Report imissed stat Thread-Index: AQHUezn5OM3QSjl7Zk+8Amsio/TB0KVPUwGAgAAd+v0= Date: Wed, 14 Nov 2018 16:18:20 +0000 Message-ID: <1542212300053.31332@kth.se> References: <1542104199-55746-1-git-send-email-barbette@kth.se>, In-Reply-To: Accept-Language: fr-FR, sv-SE, en-US Content-Language: fr-FR X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [130.237.20.142] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] mlx5: Report imissed stat X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Nov 2018 16:18:22 -0000 Hi Shahaf,=0A= =0A= Yes we learned this distinction with rx_discards_phy the hard way. I would = expect imissed to be only rx_out_of_buffer actually. I'd say people look a= t imissed to see if they consume packets fast enough. If the problem is pur= e CPU power. And that is more the definition of imissed, it is "ie RX queue= s are full", not "eg".=0A= The *_phy counters are somehow unusual, 82599, xl710 and others would simpl= y never receive other packets. But I'll follow your lead.=0A= =0A= If documentation of the limitations is okay for you, I can propose a v2, al= so adding rx_discards_phy no matter what in xstats as it is needed for the = full picture. Then the documentation can mention that.=0A= =0A= Tom=0A= =0A= =0A= =0A= ________________________________________=0A= De : Shahaf Shuler =0A= Envoy=E9 : mercredi 14 novembre 2018 16:17=0A= =C0 : Tom Barbette; dev@dpdk.org=0A= Cc : Yongseok Koh=0A= Objet : RE: [PATCH] mlx5: Report imissed stat=0A= =0A= Hi Again Tom,=0A= =0A= Tuesday, November 13, 2018 12:17 PM, Tom Barbette:=0A= > Subject: [PATCH] mlx5: Report imissed stat=0A= >=0A= > The imissed counters (number of packets dropped because the queues were= =0A= > full) were actually reported through xstats as "rx_out_of_buffer"=0A= > but was not reported through stats.=0A= >=0A= > Following a recent discussion on the ML, as there is no way to tell the u= ser if a=0A= > counter is implemented or not, this should be considered a bug. Eg, user= =0A= > looking at imissed will think the packets are lost before reaching the de= vice.=0A= >=0A= > As for xstats, I added a base counter to be able to "reset" imissed.=0A= >=0A= =0A= Yes, a bit of extra work is needed here.=0A= the full definition of the missed is=0A= "=0A= uint64_t imissed;=0A= /**< Total of RX packets dropped by the HW,=0A= * because there are no available buffer (i.e. RX queues are full).=0A= */=0A= "=0A= =0A= It is not well defined (this is other topic), because it assumes the only r= eason to drop packets is because there is no avail buffer on the Rx queue.= =0A= It is not entirely correct, for example if you have large latency on your s= ystem it is possible that packets would be dropped on the physical port, no= t because the application is slow and not replenish the buffers fast enough= , rather because the NIC is not processing them on the needed rate.=0A= =0A= I guess what application seek on imissed is the sum of two. It can be done = by summing up two counters: the out_of_buffer and the rx_discard_phy (which= exists on ethtool -S and need to be added to mlx5_counters_init a= rray).=0A= =0A= Still, the output will be incorrect on the following cases:=0A= 1. from VF, since VF cannot read the phy port counters=0A= 2. in the presence of representors, as they all share the same phy port cou= nter.=0A= =0A= I guess if we want to get such patch in, we need first to make the calculat= ion correct, and document the limitations.=0A= =0A= =0A= > Signed-off-by: Tom Barbette =0A= > ---=0A= > drivers/net/mlx5/mlx5.h | 2 ++=0A= > drivers/net/mlx5/mlx5_stats.c | 36 +++++++++++++++++++++++-------------= =0A= > 2 files changed, 25 insertions(+), 13 deletions(-)=0A= >=0A= > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index=0A= > a3a34cf..61054a8 100644=0A= > --- a/drivers/net/mlx5/mlx5.h=0A= > +++ b/drivers/net/mlx5/mlx5.h=0A= > @@ -77,6 +77,8 @@ struct mlx5_xstats_ctrl {=0A= > /* Index in the device counters table. */=0A= > uint16_t dev_table_idx[MLX5_MAX_XSTATS];=0A= > uint64_t base[MLX5_MAX_XSTATS];=0A= > + /* Base for imissed counter. */=0A= > + uint64_t imissed_base;=0A= > };=0A= >=0A= > /* Flow list . */=0A= > diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.= c=0A= > index 91f3d47..1e75e85 100644=0A= > --- a/drivers/net/mlx5/mlx5_stats.c=0A= > +++ b/drivers/net/mlx5/mlx5_stats.c=0A= > @@ -119,6 +119,24 @@ static const struct mlx5_counter_ctrl=0A= > mlx5_counters_init[] =3D {=0A= >=0A= > static const unsigned int xstats_n =3D RTE_DIM(mlx5_counters_init);=0A= >=0A= > +static inline void=0A= > +mlx5_read_ib_stat(struct priv *priv, unsigned int idx, uint64_t *stat)= =0A= > +{=0A= > + FILE *file;=0A= > + MKSTR(path, "%s/ports/1/hw_counters/%s",=0A= > + priv->ibdev_path,=0A= > + mlx5_counters_init[idx].ctr_name);=0A= > +=0A= > + file =3D fopen(path, "rb");=0A= > + if (file) {=0A= > + int n =3D fscanf(file, "%" SCNu64, stat);=0A= > +=0A= > + fclose(file);=0A= > + if (n !=3D 1)=0A= > + stat =3D 0;=0A= > + }=0A= > +}=0A= > +=0A= > /**=0A= > * Read device counters table.=0A= > *=0A= > @@ -155,19 +173,7 @@ mlx5_read_dev_counters(struct rte_eth_dev *dev,=0A= > uint64_t *stats)=0A= > }=0A= > for (i =3D 0; i !=3D xstats_n; ++i) {=0A= > if (mlx5_counters_init[i].ib) {=0A= > - FILE *file;=0A= > - MKSTR(path, "%s/ports/1/hw_counters/%s",=0A= > - priv->ibdev_path,=0A= > - mlx5_counters_init[i].ctr_name);=0A= > -=0A= > - file =3D fopen(path, "rb");=0A= > - if (file) {=0A= > - int n =3D fscanf(file, "%" SCNu64, &stats[i= ]);=0A= > -=0A= > - fclose(file);=0A= > - if (n !=3D 1)=0A= > - stats[i] =3D 0;=0A= > - }=0A= > + mlx5_read_ib_stat(priv, i, &stats[i]);=0A= > } else {=0A= > stats[i] =3D (uint64_t)=0A= > et_stats->data[xstats_ctrl-=0A= > >dev_table_idx[i]];=0A= > @@ -281,6 +287,7 @@ mlx5_xstats_init(struct rte_eth_dev *dev)=0A= > if (ret)=0A= > DRV_LOG(ERR, "port %u cannot read device counters: %s",=0A= > dev->data->port_id, strerror(rte_errno));=0A= > + mlx5_read_ib_stat(priv, 17, &xstats_ctrl->imissed_base);=0A= > free:=0A= > rte_free(strings);=0A= > }=0A= > @@ -389,6 +396,8 @@ mlx5_stats_get(struct rte_eth_dev *dev, struct=0A= > rte_eth_stats *stats) #endif=0A= > tmp.oerrors +=3D txq->stats.oerrors;=0A= > }=0A= > + mlx5_read_ib_stat(priv, 17, &tmp.imissed);=0A= > + tmp.imissed -=3D priv->xstats_ctrl.imissed_base;=0A= > #ifndef MLX5_PMD_SOFT_COUNTERS=0A= > /* FIXME: retrieve and add hardware counters. */ #endif @@ -461,6= =0A= > +470,7 @@ mlx5_xstats_reset(struct rte_eth_dev *dev)=0A= > }=0A= > for (i =3D 0; i !=3D n; ++i)=0A= > xstats_ctrl->base[i] =3D counters[i];=0A= > + mlx5_read_ib_stat(priv, 17, &xstats_ctrl->imissed_base);=0A= > }=0A= >=0A= > /**=0A= > --=0A= > 2.7.4=0A= =0A=