From: "Guo, Junfeng" <junfeng.guo@intel.com>
To: Rushil Gupta <rushilg@google.com>,
"jeroendb@google.com" <jeroendb@google.com>,
"joshwash@google.com" <joshwash@google.com>,
"ferruh.yigit@amd.com" <ferruh.yigit@amd.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: RE: [PATCH] net/gve: Enable stats reporting for GQ format
Date: Wed, 20 Dec 2023 02:51:09 +0000 [thread overview]
Message-ID: <CY5PR11MB64862BD76361FBD4E3DA3DC4E796A@CY5PR11MB6486.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20231219021650.549227-1-rushilg@google.com>
> -----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
next prev parent reply other threads:[~2023-12-20 2:51 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-19 2:16 Rushil Gupta
2023-12-20 2:51 ` Guo, Junfeng [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-12-22 15:39 Rushil Gupta
2024-01-12 15:06 ` Ferruh Yigit
2024-01-16 6:18 ` Rushil Gupta
2024-01-17 7:06 ` Joshua Washington
2024-01-17 9:40 ` Ferruh Yigit
2024-01-19 13:37 ` Rushil Gupta
2023-12-22 15:30 Rushil Gupta
2023-12-22 15:23 Rushil Gupta
2023-12-22 15:05 Rushil Gupta
2023-12-19 2:13 Rushil Gupta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CY5PR11MB64862BD76361FBD4E3DA3DC4E796A@CY5PR11MB6486.namprd11.prod.outlook.com \
--to=junfeng.guo@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=jeroendb@google.com \
--cc=joshwash@google.com \
--cc=rushilg@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).