[dpdk-dev] [PATCH v3 3/4] bnx2x: Enhance stats get

[Rasesh Mody]

> From: Van Haaren, Harry [mailto:harry.van.haaren at intel.com]
> Sent: Wednesday, April 06, 2016 7:33 AM
> 
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Rasesh Mody
> > Subject: [dpdk-dev] [PATCH v3 3/4] bnx2x: Enhance stats get
> 
> Hi Rasesh,
> 
> > +   snprintf(xstats[num].name, sizeof(xstats[num].name),
> "brb_drops");
> 
> I don't understand what a "brb" drop is.
> 
> 
> > +   snprintf(xstats[num].name, sizeof(xstats[num].name), "tx_pfc");
> 
> Similarly here, and with some other of the xstats strings, it doesn't become
> clear to me what exactly the value represents.
> 
> "mac_filter_discard" is descriptive and readable, but the next stat has
> "mf_tag_discard" - these small inconsistencies make it much harder
> (impossible?) to scrap the xstats strings and retrieve useful metadata.
> 
> I'll suggest leaving the xstats implementation part of this patch until the 
> next
> release, and we can align on the names of the stats.
> 
> -Harry

We have re-worked the patches and submitted v4. It incorporates changes to 
rename some of the stats.

Thanks!
Rasesh

[dpdk-dev] [PATCH v3 3/4] bnx2x: Enhance stats get

[Rasesh Mody]

> From: Van Haaren, Harry [mailto:harry.van.haaren at intel.com]
> Sent: Wednesday, April 06, 2016 7:33 AM
> 
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Rasesh Mody
> > Subject: [dpdk-dev] [PATCH v3 3/4] bnx2x: Enhance stats get
> 
> Hi Rasesh,
> 
> > +   snprintf(xstats[num].name, sizeof(xstats[num].name),
> "brb_drops");
> 
> I don't understand what a "brb" drop is.
> 
> 
> > +   snprintf(xstats[num].name, sizeof(xstats[num].name), "tx_pfc");
> 
> Similarly here, and with some other of the xstats strings, it doesn't become
> clear to me what exactly the value represents.
> 
> "mac_filter_discard" is descriptive and readable, but the next stat has
> "mf_tag_discard" - these small inconsistencies make it much harder
> (impossible?) to scrap the xstats strings and retrieve useful metadata.
> 
> I'll suggest leaving the xstats implementation part of this patch until the 
> next
> release, and we can align on the names of the stats.

We'll re-work the patch to accommodate your suggestions. 

> 
> -Harry

[dpdk-dev] [PATCH v3 3/4] bnx2x: Enhance stats get

[Van Haaren, Harry]

> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Rasesh Mody
> Subject: [dpdk-dev] [PATCH v3 3/4] bnx2x: Enhance stats get

Hi Rasesh,

> + snprintf(xstats[num].name, sizeof(xstats[num].name), "brb_drops");

I don't understand what a "brb" drop is.


> + snprintf(xstats[num].name, sizeof(xstats[num].name), "tx_pfc");

Similarly here, and with some other of the xstats strings, it doesn't
become clear to me what exactly the value represents.

"mac_filter_discard" is descriptive and readable, but the next stat
has "mf_tag_discard" - these small inconsistencies make it much harder
(impossible?) to scrap the xstats strings and retrieve useful metadata.

I'll suggest leaving the xstats implementation part of this patch until
the next release, and we can align on the names of the stats.

-Harry

[dpdk-dev] [PATCH v3 3/4] bnx2x: Enhance stats get

[Rasesh Mody]

Enhance the stats_get() routine to display drop counters under
imissed counter.
Added extended stats get support to provide additional info.

Signed-off-by: Rasesh Mody 
---
 drivers/net/bnx2x/bnx2x_ethdev.c |   72 ++
 drivers/net/bnx2x/bnx2x_rxtx.c   |2 ++
 2 files changed, 74 insertions(+)

diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 071b44f..1f38f6d 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -276,6 +276,9 @@ static void
 bnx2x_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
struct bnx2x_softc *sc = dev->data->dev_private;
+   uint32_t brb_truncate_discard;
+   uint64_t brb_drops;
+   uint64_t brb_truncates;

PMD_INIT_FUNC_TRACE();

@@ -316,6 +319,73 @@ bnx2x_dev_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *stats)
stats->rx_nombuf =
HILO_U64(sc->eth_stats.no_buff_discard_hi,
sc->eth_stats.no_buff_discard_lo);
+
+   brb_drops =
+   HILO_U64(sc->eth_stats.brb_drop_hi,
+sc->eth_stats.brb_drop_lo);
+
+   brb_truncates =
+   HILO_U64(sc->eth_stats.brb_truncate_hi,
+sc->eth_stats.brb_truncate_lo);
+
+   brb_truncate_discard = sc->eth_stats.brb_truncate_discard;
+
+   stats->imissed = brb_drops + brb_truncates +
+brb_truncate_discard + stats->rx_nombuf;
+}
+
+#define BNX2X_EXTENDED_STATS 9
+
+static int
+bnx2x_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats,
+unsigned n)
+{
+   struct bnx2x_softc *sc = dev->data->dev_private;
+   unsigned num = BNX2X_EXTENDED_STATS;
+
+   if (n < num)
+   return num;
+
+   num = 0;
+
+   bnx2x_stats_handle(sc, STATS_EVENT_UPDATE);
+
+   snprintf(xstats[num].name, sizeof(xstats[num].name), "brb_drops");
+   xstats[num++].value = HILO_U64(sc->eth_stats.brb_drop_hi,
+  sc->eth_stats.brb_drop_lo);
+
+   snprintf(xstats[num].name, sizeof(xstats[num].name), "brb_truncates");
+   xstats[num++].value = HILO_U64(sc->eth_stats.brb_truncate_hi,
+  sc->eth_stats.brb_truncate_lo);
+
+   snprintf(xstats[num].name, sizeof(xstats[num].name),
+"brb_truncate_discard");
+   xstats[num++].value = sc->eth_stats.brb_truncate_discard;
+
+   snprintf(xstats[num].name, sizeof(xstats[num].name),
+"mac_filter_discard");
+   xstats[num++].value = sc->eth_stats.mac_filter_discard;
+
+   snprintf(xstats[num].name, sizeof(xstats[num].name), "mf_tag_discard");
+   xstats[num++].value = sc->eth_stats.mf_tag_discard;
+
+   snprintf(xstats[num].name, sizeof(xstats[num].name), "tx_pause");
+   xstats[num++].value = HILO_U64(sc->eth_stats.pause_frames_sent_hi,
+  sc->eth_stats.pause_frames_sent_lo);
+
+   snprintf(xstats[num].name, sizeof(xstats[num].name), "rx_pause");
+   xstats[num++].value = HILO_U64(sc->eth_stats.pause_frames_received_hi,
+  sc->eth_stats.pause_frames_received_lo);
+
+   snprintf(xstats[num].name, sizeof(xstats[num].name), "tx_pfc");
+   xstats[num++].value = HILO_U64(sc->eth_stats.pfc_frames_sent_hi,
+  sc->eth_stats.pfc_frames_sent_lo);
+
+   snprintf(xstats[num].name, sizeof(xstats[num].name), "rx_pfc");
+   xstats[num++].value = HILO_U64(sc->eth_stats.pfc_frames_received_hi,
+  sc->eth_stats.pfc_frames_received_lo);
+
+   return num;
 }

 static void
@@ -360,6 +430,7 @@ static const struct eth_dev_ops bnx2x_eth_dev_ops = {
.allmulticast_disable = bnx2x_dev_allmulticast_disable,
.link_update  = bnx2x_dev_link_update,
.stats_get= bnx2x_dev_stats_get,
+   .xstats_get   = bnx2x_dev_xstats_get,
.dev_infos_get= bnx2x_dev_infos_get,
.rx_queue_setup   = bnx2x_dev_rx_queue_setup,
.rx_queue_release = bnx2x_dev_rx_queue_release,
@@ -383,6 +454,7 @@ static const struct eth_dev_ops bnx2xvf_eth_dev_ops = {
.allmulticast_disable = bnx2x_dev_allmulticast_disable,
.link_update  = bnx2xvf_dev_link_update,
.stats_get= bnx2x_dev_stats_get,
+   .xstats_get   = bnx2x_dev_xstats_get,
.dev_infos_get= bnx2x_dev_infos_get,
.rx_queue_setup   = bnx2x_dev_rx_queue_setup,
.rx_queue_release = bnx2x_dev_rx_queue_release,
diff --git a/drivers/net/bnx2x/bnx2x_rxtx.c b/drivers/net/bnx2x/bnx2x_rxtx.c
index 8b047d4..60bd08b 100644
--- a/drivers/net/bnx2x/bnx2x_rxtx.c
+++