[v2,19/29] net/ena: add Tx drops statistic

Message ID 20200401142127.13715-20-mk@semihalf.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series Update ENA driver to v2.1.0 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Michal Krawczyk April 1, 2020, 2:21 p.m. UTC
  ENA device can report in the AENQ handler amount of Tx packets that were
dropped and not sent.

This statistic is showing global value for the device and because
rte_eth_stats is missing field that could indicate this value (it
isn't the Tx error), it is being presented as a extended statistic.

As the current design of extended statistics prevents tx_drops from
being an atomic variable and both tx_drops and rx_drops are only updated
from the AENQ handler, both were set as non-atomic for the alignment.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 11 ++++++++---
 drivers/net/ena/ena_ethdev.h |  8 +++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)
  

Comments

Ferruh Yigit April 2, 2020, 1:05 p.m. UTC | #1
On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> ENA device can report in the AENQ handler amount of Tx packets that were
> dropped and not sent.
> 
> This statistic is showing global value for the device and because
> rte_eth_stats is missing field that could indicate this value (it
> isn't the Tx error), it is being presented as a extended statistic.

What is the reason of Tx drop if it is not error?

> 
> As the current design of extended statistics prevents tx_drops from
> being an atomic variable and both tx_drops and rx_drops are only updated
> from the AENQ handler, both were set as non-atomic for the alignment.
> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>

<...>
  
Michal Krawczyk April 3, 2020, 12:30 p.m. UTC | #2
czw., 2 kwi 2020 o 15:05 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> > ENA device can report in the AENQ handler amount of Tx packets that were
> > dropped and not sent.
> >
> > This statistic is showing global value for the device and because
> > rte_eth_stats is missing field that could indicate this value (it
> > isn't the Tx error), it is being presented as a extended statistic.
>
> What is the reason of Tx drop if it is not error?
>

It's connected with the device architecture (I'm sorry - I don't know
the details).

> >
> > As the current design of extended statistics prevents tx_drops from
> > being an atomic variable and both tx_drops and rx_drops are only updated
> > from the AENQ handler, both were set as non-atomic for the alignment.
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
>
> <...>
>
  

Patch

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index eb97c80660..b7cdc5711c 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -96,6 +96,7 @@  static const struct ena_stats ena_stats_global_strings[] = {
 	ENA_STAT_GLOBAL_ENTRY(wd_expired),
 	ENA_STAT_GLOBAL_ENTRY(dev_start),
 	ENA_STAT_GLOBAL_ENTRY(dev_stop),
+	ENA_STAT_GLOBAL_ENTRY(tx_drops),
 };
 
 static const struct ena_stats ena_stats_tx_strings[] = {
@@ -938,7 +939,7 @@  static void ena_stats_restart(struct rte_eth_dev *dev)
 	rte_atomic64_init(&adapter->drv_stats->ierrors);
 	rte_atomic64_init(&adapter->drv_stats->oerrors);
 	rte_atomic64_init(&adapter->drv_stats->rx_nombuf);
-	rte_atomic64_init(&adapter->drv_stats->rx_drops);
+	adapter->drv_stats->rx_drops = 0;
 }
 
 static int ena_stats_get(struct rte_eth_dev *dev,
@@ -972,7 +973,7 @@  static int ena_stats_get(struct rte_eth_dev *dev,
 					ena_stats.tx_bytes_low);
 
 	/* Driver related stats */
-	stats->imissed = rte_atomic64_read(&adapter->drv_stats->rx_drops);
+	stats->imissed = adapter->drv_stats->rx_drops;
 	stats->ierrors = rte_atomic64_read(&adapter->drv_stats->ierrors);
 	stats->oerrors = rte_atomic64_read(&adapter->drv_stats->oerrors);
 	stats->rx_nombuf = rte_atomic64_read(&adapter->drv_stats->rx_nombuf);
@@ -2785,12 +2786,16 @@  static void ena_keep_alive(void *adapter_data,
 	struct ena_adapter *adapter = adapter_data;
 	struct ena_admin_aenq_keep_alive_desc *desc;
 	uint64_t rx_drops;
+	uint64_t tx_drops;
 
 	adapter->timestamp_wd = rte_get_timer_cycles();
 
 	desc = (struct ena_admin_aenq_keep_alive_desc *)aenq_e;
 	rx_drops = ((uint64_t)desc->rx_drops_high << 32) | desc->rx_drops_low;
-	rte_atomic64_set(&adapter->drv_stats->rx_drops, rx_drops);
+	tx_drops = ((uint64_t)desc->tx_drops_high << 32) | desc->tx_drops_low;
+
+	adapter->drv_stats->rx_drops = rx_drops;
+	adapter->dev_stats.tx_drops = tx_drops;
 }
 
 /**
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 0e82c894f4..9558ee072f 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -134,13 +134,19 @@  struct ena_driver_stats {
 	rte_atomic64_t ierrors;
 	rte_atomic64_t oerrors;
 	rte_atomic64_t rx_nombuf;
-	rte_atomic64_t rx_drops;
+	u64 rx_drops;
 };
 
 struct ena_stats_dev {
 	u64 wd_expired;
 	u64 dev_start;
 	u64 dev_stop;
+	/*
+	 * Tx drops cannot be reported as the driver statistic, because DPDK
+	 * rte_eth_stats structure isn't providing appropriate field for that.
+	 * As a workaround it is being published as an extended statistic.
+	 */
+	u64 tx_drops;
 };
 
 struct ena_offloads {