net/gve: Enable stats reporting for GQ format

  Read from shared region to retrieve imissed statistics for GQ.
Tested using `show port xstats <port-id>` in interactive mode.
This metric can be triggered by using queues > cores.

Signed-off-by: Rushil Gupta <rushilg@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/base/gve_adminq.h | 11 ++++
 drivers/net/gve/gve_ethdev.c      | 83 +++++++++++++++++++++++++++++++
 drivers/net/gve/gve_ethdev.h      |  6 +++
 3 files changed, 100 insertions(+)
  

> -----Original Message-----
> From: Rushil Gupta <rushilg@google.com>
> Sent: Tuesday, December 19, 2023 10:17
> To: Guo, Junfeng <junfeng.guo@intel.com>; jeroendb@google.com;
> joshwash@google.com; ferruh.yigit@amd.com
> Cc: dev@dpdk.org; Rushil Gupta <rushilg@google.com>
> Subject: [PATCH] net/gve: Enable stats reporting for GQ format
> 
> Read from shared region to retrieve imissed statistics for GQ.
> Tested using `show port xstats <port-id>` in interactive mode.
> This metric can be triggered by using queues > cores.
> 
> Signed-off-by: Rushil Gupta <rushilg@google.com>
> Reviewed-by: Joshua Washington <joshwash@google.com>
> ---
>  drivers/net/gve/base/gve_adminq.h | 11 ++++
>  drivers/net/gve/gve_ethdev.c      | 83
> +++++++++++++++++++++++++++++++
>  drivers/net/gve/gve_ethdev.h      |  6 +++
>  3 files changed, 100 insertions(+)
> 
> diff --git a/drivers/net/gve/base/gve_adminq.h
> b/drivers/net/gve/base/gve_adminq.h
> index e30b184913..f05362f85f 100644
> --- a/drivers/net/gve/base/gve_adminq.h
> +++ b/drivers/net/gve/base/gve_adminq.h
> @@ -314,6 +314,17 @@ struct gve_stats_report {
> 
>  GVE_CHECK_STRUCT_LEN(8, gve_stats_report);
> 
> +/* Numbers of gve tx/rx stats in stats report. */
> +#define GVE_TX_STATS_REPORT_NUM        6
> +#define GVE_RX_STATS_REPORT_NUM        2
> +
> +/* Interval to schedule a stats report update, 20000ms. */
> +#define GVE_STATS_REPORT_TIMER_PERIOD  20000
> +
> +/* Numbers of NIC tx/rx stats in stats report. */
> +#define NIC_TX_STATS_REPORT_NUM        0
> +#define NIC_RX_STATS_REPORT_NUM        4
> +
>  enum gve_stat_names {
>  	/* stats from gve */
>  	TX_WAKE_CNT			= 1,
> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
> index ecd37ff37f..0db612f25c 100644
> --- a/drivers/net/gve/gve_ethdev.c
> +++ b/drivers/net/gve/gve_ethdev.c
> @@ -125,6 +125,70 @@ gve_link_update(struct rte_eth_dev *dev,
> __rte_unused int wait_to_complete)
>  	return rte_eth_linkstatus_set(dev, &link);
>  }
> 
> +static int gve_alloc_stats_report(struct gve_priv *priv,

Minor observation here and for below newly-added functions about the coding style.

In DPDK, the function type is placed on a new line by itself preceding the function, while in the kernel, it is placed on the same line as the function.
But you can keep this kernel coding style for the code under the base folder of the driver.

It's always good to keep the coding style be consistent within each individual file. : )
https://doc.dpdk.org/guides-23.11/contributing/coding_style.html

Regards,
Junfeng

> +		uint16_t nb_tx_queues, uint16_t nb_rx_queues)
> +{
> +	int tx_stats_cnt;
> +	int rx_stats_cnt;
> +
> +	tx_stats_cnt = (GVE_TX_STATS_REPORT_NUM +
> NIC_TX_STATS_REPORT_NUM) *
> +		nb_tx_queues;
> +	rx_stats_cnt = (GVE_RX_STATS_REPORT_NUM +
> NIC_RX_STATS_REPORT_NUM) *
> +		nb_rx_queues;
> +	priv->stats_report_len = sizeof(struct gve_stats_report) +
> +		sizeof(struct stats) * (tx_stats_cnt + rx_stats_cnt);
> +	priv->stats_report_mem =
> rte_memzone_reserve_aligned("report_stats",
> +			priv->stats_report_len,
> +			rte_socket_id(),
> +			RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
> +
> +	if (!priv->stats_report_mem)
> +		return -ENOMEM;
> +
> +	/* offset by skipping stats written by gve. */
> +	priv->stats_start_idx = (GVE_TX_STATS_REPORT_NUM *
> nb_tx_queues) +
> +		(GVE_RX_STATS_REPORT_NUM * nb_rx_queues);
> +	priv->stats_end_idx = priv->stats_start_idx +
> +		(NIC_TX_STATS_REPORT_NUM * nb_tx_queues) +
> +		(NIC_RX_STATS_REPORT_NUM * nb_rx_queues) - 1;
> +
> +	return 0;
> +}
> +
> +static void gve_free_stats_report(struct rte_eth_dev *dev)
> +{
> +	struct gve_priv *priv = dev->data->dev_private;
> +	rte_memzone_free(priv->stats_report_mem);
> +}
> +
> +/* Read Rx NIC stats from shared region */
> +static void gve_get_imissed_from_nic(struct rte_eth_dev *dev)
> +{
> +	struct gve_stats_report *stats_report;
> +	struct gve_rx_queue *rxq;
> +	struct gve_priv *priv;
> +	struct stats stat;
> +	int queue_id;
> +	int stat_id;
> +	int i;
> +
> +	priv = dev->data->dev_private;
> +	stats_report = (struct gve_stats_report *)
> +		priv->stats_report_mem->addr;
> +
> +	for (i = priv->stats_start_idx; i <= priv->stats_end_idx; i++) {
> +		stat = stats_report->stats[i];
> +		queue_id = cpu_to_be32(stat.queue_id);
> +		rxq = dev->data->rx_queues[queue_id];
> +		if (rxq == NULL)
> +			continue;
> +		stat_id = cpu_to_be32(stat.stat_name);
> +		/* Update imissed. */
> +		if (stat_id == RX_NO_BUFFERS_POSTED)
> +			rxq->stats.imissed = cpu_to_be64(stat.value);
> +	}
> +}
> +
>  static int
>  gve_start_queues(struct rte_eth_dev *dev)
>  {
> @@ -176,6 +240,7 @@ gve_start_queues(struct rte_eth_dev *dev)
>  static int
>  gve_dev_start(struct rte_eth_dev *dev)
>  {
> +	struct gve_priv *priv;
>  	int ret;
> 
>  	ret = gve_start_queues(dev);
> @@ -187,6 +252,17 @@ gve_dev_start(struct rte_eth_dev *dev)
>  	dev->data->dev_started = 1;
>  	gve_link_update(dev, 0);
> 
> +	priv = dev->data->dev_private;
> +	/* No stats available yet for Dqo. */
> +	if (gve_is_gqi(priv)) {
> +		gve_alloc_stats_report(priv,
> +			dev->data->nb_tx_queues,
> +			dev->data->nb_rx_queues);
> +		ret = gve_adminq_report_stats(priv, priv-
> >stats_report_len,
> +				priv->stats_report_mem->iova,
> +				GVE_STATS_REPORT_TIMER_PERIOD);
> +	}
> +
>  	return 0;
>  }
> 
> @@ -200,6 +276,9 @@ gve_dev_stop(struct rte_eth_dev *dev)
> 
>  	dev->data->dev_started = 0;
> 
> +	if (gve_is_gqi(dev->data->dev_private))
> +		gve_free_stats_report(dev);
> +
>  	return 0;
>  }
> 
> @@ -352,6 +431,8 @@ static int
>  gve_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  {
>  	uint16_t i;
> +	if (gve_is_gqi(dev->data->dev_private))
> +		gve_get_imissed_from_nic(dev);
> 
>  	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>  		struct gve_tx_queue *txq = dev->data->tx_queues[i];
> @@ -372,6 +453,7 @@ gve_dev_stats_get(struct rte_eth_dev *dev, struct
> rte_eth_stats *stats)
>  		stats->ibytes += rxq->stats.bytes;
>  		stats->ierrors += rxq->stats.errors;
>  		stats->rx_nombuf += rxq->stats.no_mbufs;
> +		stats->imissed += rxq->stats.imissed;
>  	}
> 
>  	return 0;
> @@ -443,6 +525,7 @@ static const struct gve_xstats_name_offset
> rx_xstats_name_offset[] = {
>  	{ "errors",                 RX_QUEUE_STATS_OFFSET(errors) },
>  	{ "mbuf_alloc_errors",      RX_QUEUE_STATS_OFFSET(no_mbufs) },
>  	{ "mbuf_alloc_errors_bulk",
> RX_QUEUE_STATS_OFFSET(no_mbufs_bulk) },
> +	{ "imissed",                RX_QUEUE_STATS_OFFSET(imissed) },
>  };
> 
>  static int
> diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
> index 58d8943e71..9893fcfee6 100644
> --- a/drivers/net/gve/gve_ethdev.h
> +++ b/drivers/net/gve/gve_ethdev.h
> @@ -85,6 +85,7 @@ struct gve_rx_stats {
>  	uint64_t errors;
>  	uint64_t no_mbufs;
>  	uint64_t no_mbufs_bulk;
> +	uint64_t imissed;
>  };
> 
>  struct gve_xstats_name_offset {
> @@ -272,6 +273,11 @@ struct gve_priv {
> 
>  	struct gve_tx_queue **txqs;
>  	struct gve_rx_queue **rxqs;
> +
> +	uint32_t stats_report_len;
> +	const struct rte_memzone *stats_report_mem;
> +	uint16_t stats_start_idx; /* start index of array of stats written by
> NIC */
> +	uint16_t stats_end_idx; /* end index of array of stats written by
> NIC */
>  };
> 
>  static inline bool
> --
> 2.43.0.472.g3155946c3a-goog