* [PATCH] net/gve: Enable stats reporting for GQ format
@ 2023-12-22 15:05 Rushil Gupta
0 siblings, 0 replies; 12+ messages in thread
From: Rushil Gupta @ 2023-12-22 15:05 UTC (permalink / raw)
To: qi.z.zhang, ferruh.yigit
Cc: junfeng.guo, dev, Rushil Gupta, Joshua Washington
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 | 142 ++++++++++++++++++++++++++++--
drivers/net/gve/gve_ethdev.h | 20 ++++-
3 files changed, 167 insertions(+), 6 deletions(-)
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..bb535a863f 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -125,6 +125,73 @@ 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,
+ uint16_t nb_tx_queues, uint16_t nb_rx_queues)
+{
+ char z_name[RTE_MEMZONE_NAMESIZE];
+ 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);
+
+ snprintf(z_name, sizeof(z_name), "stats_report_%s", priv->pci_dev->device.name);
+ priv->stats_report_mem = rte_memzone_reserve_aligned(z_name,
+ 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 +243,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 +255,27 @@ 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))
+ {
+ ret = gve_alloc_stats_report(priv,
+ dev->data->nb_tx_queues,
+ dev->data->nb_rx_queues);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR,
+ "Failed to allocate region for stats reporting.");
+ return ret;
+ }
+ ret = gve_adminq_report_stats(priv, priv->stats_report_len,
+ priv->stats_report_mem->iova,
+ GVE_STATS_REPORT_TIMER_PERIOD);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR, "gve_adminq_report_stats command failed.");
+ return ret;
+ }
+ }
+
return 0;
}
@@ -200,6 +289,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 +444,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 +466,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 +538,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
@@ -614,17 +710,53 @@ gve_teardown_device_resources(struct gve_priv *priv)
gve_clear_device_resources_ok(priv);
}
+static uint8_t
+pci_dev_find_capability(struct rte_pci_device *pdev, int cap)
+{
+ uint8_t pos, id;
+ uint16_t ent;
+ int loops;
+ int ret;
+
+ ret = rte_pci_read_config(pdev, &pos, sizeof(pos), PCI_CAPABILITY_LIST);
+ if (ret != sizeof(pos))
+ return 0;
+
+ loops = (PCI_CFG_SPACE_SIZE - PCI_STD_HEADER_SIZEOF) / PCI_CAP_SIZEOF;
+
+ while (pos && loops--) {
+ ret = rte_pci_read_config(pdev, &ent, sizeof(ent), pos);
+ if (ret != sizeof(ent))
+ return 0;
+
+ id = ent & 0xff;
+ if (id == 0xff)
+ break;
+
+ if (id == cap)
+ return pos;
+
+ pos = (ent >> 8);
+ }
+
+ return 0;
+}
+
static int
pci_dev_msix_vec_count(struct rte_pci_device *pdev)
{
- off_t msix_pos = rte_pci_find_capability(pdev, RTE_PCI_CAP_ID_MSIX);
+ uint8_t msix_cap = pci_dev_find_capability(pdev, PCI_CAP_ID_MSIX);
uint16_t control;
+ int ret;
- if (msix_pos > 0 && rte_pci_read_config(pdev, &control, sizeof(control),
- msix_pos + RTE_PCI_MSIX_FLAGS) == sizeof(control))
- return (control & RTE_PCI_MSIX_FLAGS_QSIZE) + 1;
+ if (!msix_cap)
+ return 0;
- return 0;
+ ret = rte_pci_read_config(pdev, &control, sizeof(control), msix_cap + PCI_MSIX_FLAGS);
+ if (ret != sizeof(control))
+ return 0;
+
+ return (control & PCI_MSIX_FLAGS_QSIZE) + 1;
}
static int
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 58d8943e71..0e1bfec871 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -8,13 +8,25 @@
#include <ethdev_driver.h>
#include <ethdev_pci.h>
#include <rte_ether.h>
-#include <rte_pci.h>
#include "base/gve.h"
/* TODO: this is a workaround to ensure that Tx complq is enough */
#define DQO_TX_MULTIPLIER 4
+/*
+ * Following macros are derived from linux/pci_regs.h, however,
+ * we can't simply include that header here, as there is no such
+ * file for non-Linux platform.
+ */
+#define PCI_CFG_SPACE_SIZE 256
+#define PCI_CAPABILITY_LIST 0x34 /* Offset of first capability list entry */
+#define PCI_STD_HEADER_SIZEOF 64
+#define PCI_CAP_SIZEOF 4
+#define PCI_CAP_ID_MSIX 0x11 /* MSI-X */
+#define PCI_MSIX_FLAGS 2 /* Message Control */
+#define PCI_MSIX_FLAGS_QSIZE 0x07FF /* Table size */
+
#define GVE_DEFAULT_RX_FREE_THRESH 64
#define GVE_DEFAULT_TX_FREE_THRESH 32
#define GVE_DEFAULT_TX_RS_THRESH 32
@@ -85,6 +97,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 +285,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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net/gve: Enable stats reporting for GQ format
2024-01-17 9:40 ` Ferruh Yigit
@ 2024-01-19 13:37 ` Rushil Gupta
0 siblings, 0 replies; 12+ messages in thread
From: Rushil Gupta @ 2024-01-19 13:37 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: junfeng.guo, jeroendb, joshwash, dev
[-- Attachment #1: Type: text/plain, Size: 3212 bytes --]
Those are fair points. I'll fix this by simply
calling gve_get_imissed_from_nic from gve_xstats_get in the v3 patch.
On Wed, Jan 17, 2024 at 3:10 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> On 1/16/2024 6:18 AM, Rushil Gupta wrote:
> >
> >
> > On Fri, Jan 12, 2024 at 8:36 PM Ferruh Yigit <ferruh.yigit@amd.com
> > <mailto:ferruh.yigit@amd.com>> wrote:
> >
> > On 12/22/2023 3:39 PM, Rushil Gupta wrote:
> > > Read from shared region to retrieve imissed statistics for GQ from
> > device.
> > > Tested using `show port xstats <port-id>` in interactive mode.
> > > This metric can be triggered by using queues > cores.
> > >
> >
> > Looks good but please check following comments:
> >
> > Checkpatch gives warning on the patch title, and this patch adds
> > 'imissed' support so it can be added to the patch title, something
> like:
> > "net/gve: enable imissed stats for GQ format"
> >
> > <...>
> >
> > > +static int gve_alloc_stats_report(struct gve_priv *priv,
> > > + uint16_t nb_tx_queues, uint16_t nb_rx_queues)
> > > +{
> > > + char z_name[RTE_MEMZONE_NAMESIZE];
> > > + 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);
> > > +
> > > + snprintf(z_name, sizeof(z_name), "stats_report_%s",
> > priv->pci_dev->device.name <http://device.name>);
> > >
> >
> > Can you please add 'gve_' prefix to the memzone name, to prevent any
> > possible collision.
> >
> > Done.
> >
> >
> > <...>
> >
> > > +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);
> > >
> >
> > What will happen if user asks stats/xstats after port stopped?
> >
> > Good catch. I have added a null check so that the driver doesn't try to
> > read stats from memory region that doesn't exist.
> >
> >
> > <...>
> >
> > > 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);
> > >
> >
> > This updates imissed in RxQ struct for all queues for basic stats,
> but
> > what if user only calls xstats, I guess in that case stat won't be
> > updated.
> >
> >
> > Yes; that is expected. Since imissed is a member of rte_eth_stats;
> > calling gve_dev_stats_get is the right way to get this stat.
> >
>
> I don't think it is expected.
> xstats contains the basic stats too, if users calls xstats API,
> expectation is to get correct values.
>
>
[-- Attachment #2: Type: text/html, Size: 4599 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net/gve: Enable stats reporting for GQ format
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
1 sibling, 1 reply; 12+ messages in thread
From: Ferruh Yigit @ 2024-01-17 9:40 UTC (permalink / raw)
To: Rushil Gupta; +Cc: junfeng.guo, jeroendb, joshwash, dev
On 1/16/2024 6:18 AM, Rushil Gupta wrote:
>
>
> On Fri, Jan 12, 2024 at 8:36 PM Ferruh Yigit <ferruh.yigit@amd.com
> <mailto:ferruh.yigit@amd.com>> wrote:
>
> On 12/22/2023 3:39 PM, Rushil Gupta wrote:
> > Read from shared region to retrieve imissed statistics for GQ from
> device.
> > Tested using `show port xstats <port-id>` in interactive mode.
> > This metric can be triggered by using queues > cores.
> >
>
> Looks good but please check following comments:
>
> Checkpatch gives warning on the patch title, and this patch adds
> 'imissed' support so it can be added to the patch title, something like:
> "net/gve: enable imissed stats for GQ format"
>
> <...>
>
> > +static int gve_alloc_stats_report(struct gve_priv *priv,
> > + uint16_t nb_tx_queues, uint16_t nb_rx_queues)
> > +{
> > + char z_name[RTE_MEMZONE_NAMESIZE];
> > + 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);
> > +
> > + snprintf(z_name, sizeof(z_name), "stats_report_%s",
> priv->pci_dev->device.name <http://device.name>);
> >
>
> Can you please add 'gve_' prefix to the memzone name, to prevent any
> possible collision.
>
> Done.
>
>
> <...>
>
> > +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);
> >
>
> What will happen if user asks stats/xstats after port stopped?
>
> Good catch. I have added a null check so that the driver doesn't try to
> read stats from memory region that doesn't exist.
>
>
> <...>
>
> > 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);
> >
>
> This updates imissed in RxQ struct for all queues for basic stats, but
> what if user only calls xstats, I guess in that case stat won't be
> updated.
>
>
> Yes; that is expected. Since imissed is a member of rte_eth_stats;
> calling gve_dev_stats_get is the right way to get this stat.
>
I don't think it is expected.
xstats contains the basic stats too, if users calls xstats API,
expectation is to get correct values.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net/gve: Enable stats reporting for GQ format
2024-01-16 6:18 ` Rushil Gupta
@ 2024-01-17 7:06 ` Joshua Washington
2024-01-17 9:40 ` Ferruh Yigit
1 sibling, 0 replies; 12+ messages in thread
From: Joshua Washington @ 2024-01-17 7:06 UTC (permalink / raw)
To: Rushil Gupta; +Cc: Ferruh Yigit, junfeng.guo, jeroendb, dev
[-- Attachment #1: Type: text/plain, Size: 3454 bytes --]
>
>
>> <...>
>>
>> > 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);
>> >
>>
>> This updates imissed in RxQ struct for all queues for basic stats, but
>> what if user only calls xstats, I guess in that case stat won't be
>> updated.
>>
>
> Yes; that is expected. Since imissed is a member of rte_eth_stats; calling
> gve_dev_stats_get is the right way to get this stat.
>
I actually think that this should be a counter backed by an xstat as well.
As far as I can tell xstats is meant to be a more flexible version of
stats, and ultimately should be able to handle a superset of what basic
stats can handle, for the purposes of querying, etc.
On Mon, Jan 15, 2024 at 10:18 PM Rushil Gupta <rushilg@google.com> wrote:
>
>
> On Fri, Jan 12, 2024 at 8:36 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
>> On 12/22/2023 3:39 PM, Rushil Gupta wrote:
>> > Read from shared region to retrieve imissed statistics for GQ from
>> device.
>> > Tested using `show port xstats <port-id>` in interactive mode.
>> > This metric can be triggered by using queues > cores.
>> >
>>
>> Looks good but please check following comments:
>>
>> Checkpatch gives warning on the patch title, and this patch adds
>> 'imissed' support so it can be added to the patch title, something like:
>> "net/gve: enable imissed stats for GQ format"
>>
>> <...>
>>
>> > +static int gve_alloc_stats_report(struct gve_priv *priv,
>> > + uint16_t nb_tx_queues, uint16_t nb_rx_queues)
>> > +{
>> > + char z_name[RTE_MEMZONE_NAMESIZE];
>> > + 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);
>> > +
>> > + snprintf(z_name, sizeof(z_name), "stats_report_%s",
>> priv->pci_dev->device.name);
>> >
>>
>> Can you please add 'gve_' prefix to the memzone name, to prevent any
>> possible collision.
>>
> Done.
>
>>
>> <...>
>>
>> > +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);
>> >
>>
>> What will happen if user asks stats/xstats after port stopped?
>>
> Good catch. I have added a null check so that the driver doesn't try to
> read stats from memory region that doesn't exist.
>
>>
>> <...>
>>
>> > 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);
>> >
>>
>> This updates imissed in RxQ struct for all queues for basic stats, but
>> what if user only calls xstats, I guess in that case stat won't be
>> updated.
>>
>
> Yes; that is expected. Since imissed is a member of rte_eth_stats; calling
> gve_dev_stats_get is the right way to get this stat.
>
--
Joshua Washington | Software Engineer | joshwash@google.com | (414) 366-4423
[-- Attachment #2: Type: text/html, Size: 5973 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net/gve: Enable stats reporting for GQ format
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
0 siblings, 2 replies; 12+ messages in thread
From: Rushil Gupta @ 2024-01-16 6:18 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: junfeng.guo, jeroendb, joshwash, dev
[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]
On Fri, Jan 12, 2024 at 8:36 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> On 12/22/2023 3:39 PM, Rushil Gupta wrote:
> > Read from shared region to retrieve imissed statistics for GQ from
> device.
> > Tested using `show port xstats <port-id>` in interactive mode.
> > This metric can be triggered by using queues > cores.
> >
>
> Looks good but please check following comments:
>
> Checkpatch gives warning on the patch title, and this patch adds
> 'imissed' support so it can be added to the patch title, something like:
> "net/gve: enable imissed stats for GQ format"
>
> <...>
>
> > +static int gve_alloc_stats_report(struct gve_priv *priv,
> > + uint16_t nb_tx_queues, uint16_t nb_rx_queues)
> > +{
> > + char z_name[RTE_MEMZONE_NAMESIZE];
> > + 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);
> > +
> > + snprintf(z_name, sizeof(z_name), "stats_report_%s", priv->pci_dev->
> device.name);
> >
>
> Can you please add 'gve_' prefix to the memzone name, to prevent any
> possible collision.
>
Done.
>
> <...>
>
> > +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);
> >
>
> What will happen if user asks stats/xstats after port stopped?
>
Good catch. I have added a null check so that the driver doesn't try to
read stats from memory region that doesn't exist.
>
> <...>
>
> > 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);
> >
>
> This updates imissed in RxQ struct for all queues for basic stats, but
> what if user only calls xstats, I guess in that case stat won't be updated.
>
Yes; that is expected. Since imissed is a member of rte_eth_stats; calling
gve_dev_stats_get is the right way to get this stat.
[-- Attachment #2: Type: text/html, Size: 3409 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net/gve: Enable stats reporting for GQ format
2023-12-22 15:39 Rushil Gupta
@ 2024-01-12 15:06 ` Ferruh Yigit
2024-01-16 6:18 ` Rushil Gupta
0 siblings, 1 reply; 12+ messages in thread
From: Ferruh Yigit @ 2024-01-12 15:06 UTC (permalink / raw)
To: Rushil Gupta, junfeng.guo, jeroendb, joshwash; +Cc: dev
On 12/22/2023 3:39 PM, Rushil Gupta wrote:
> Read from shared region to retrieve imissed statistics for GQ from device.
> Tested using `show port xstats <port-id>` in interactive mode.
> This metric can be triggered by using queues > cores.
>
Looks good but please check following comments:
Checkpatch gives warning on the patch title, and this patch adds
'imissed' support so it can be added to the patch title, something like:
"net/gve: enable imissed stats for GQ format"
<...>
> +static int gve_alloc_stats_report(struct gve_priv *priv,
> + uint16_t nb_tx_queues, uint16_t nb_rx_queues)
> +{
> + char z_name[RTE_MEMZONE_NAMESIZE];
> + 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);
> +
> + snprintf(z_name, sizeof(z_name), "stats_report_%s", priv->pci_dev->device.name);
>
Can you please add 'gve_' prefix to the memzone name, to prevent any
possible collision.
<...>
> +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);
>
What will happen if user asks stats/xstats after port stopped?
<...>
> 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);
>
This updates imissed in RxQ struct for all queues for basic stats, but
what if user only calls xstats, I guess in that case stat won't be updated.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] net/gve: Enable stats reporting for GQ format
@ 2023-12-22 15:39 Rushil Gupta
2024-01-12 15:06 ` Ferruh Yigit
0 siblings, 1 reply; 12+ messages in thread
From: Rushil Gupta @ 2023-12-22 15:39 UTC (permalink / raw)
To: junfeng.guo, jeroendb, joshwash, ferruh.yigit; +Cc: dev, Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ from device.
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 | 95 +++++++++++++++++++++++++++++++
drivers/net/gve/gve_ethdev.h | 6 ++
3 files changed, 112 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..836136d993 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -125,6 +125,73 @@ 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,
+ uint16_t nb_tx_queues, uint16_t nb_rx_queues)
+{
+ char z_name[RTE_MEMZONE_NAMESIZE];
+ 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);
+
+ snprintf(z_name, sizeof(z_name), "stats_report_%s", priv->pci_dev->device.name);
+ priv->stats_report_mem = rte_memzone_reserve_aligned(z_name,
+ 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 +243,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 +255,26 @@ 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)) {
+ ret = gve_alloc_stats_report(priv,
+ dev->data->nb_tx_queues,
+ dev->data->nb_rx_queues);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR,
+ "Failed to allocate region for stats reporting.");
+ return ret;
+ }
+ ret = gve_adminq_report_stats(priv, priv->stats_report_len,
+ priv->stats_report_mem->iova,
+ GVE_STATS_REPORT_TIMER_PERIOD);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR, "gve_adminq_report_stats command failed.");
+ return ret;
+ }
+ }
+
return 0;
}
@@ -200,6 +288,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 +443,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 +465,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 +537,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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] net/gve: Enable stats reporting for GQ format
@ 2023-12-22 15:30 Rushil Gupta
0 siblings, 0 replies; 12+ messages in thread
From: Rushil Gupta @ 2023-12-22 15:30 UTC (permalink / raw)
To: junfeng.guo, jeroendb, joshwash, ferruh.yigit; +Cc: dev, Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ from device.
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 | 95 +++++++++++++++++++++++++++++++
drivers/net/gve/gve_ethdev.h | 6 ++
3 files changed, 112 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..986418cf5b 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -125,6 +125,73 @@ 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,
+ uint16_t nb_tx_queues, uint16_t nb_rx_queues)
+{
+ char z_name[RTE_MEMZONE_NAMESIZE];
+ 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);
+
+ snprintf(z_name, sizeof(z_name), "stats_report_%s", priv->pci_dev->device.name);
+ priv->stats_report_mem = rte_memzone_reserve_aligned(z_name,
+ 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 +243,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 +255,26 @@ 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) {
+ ret = gve_alloc_stats_report(priv,
+ dev->data->nb_tx_queues,
+ dev->data->nb_rx_queues);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR,
+ "Failed to allocate region for stats reporting.");
+ return ret;
+ }
+ ret = gve_adminq_report_stats(priv, priv->stats_report_len,
+ priv->stats_report_mem->iova,
+ GVE_STATS_REPORT_TIMER_PERIOD);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR, "gve_adminq_report_stats command failed.");
+ return ret;
+ }
+ }
+
return 0;
}
@@ -200,6 +288,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 +443,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 +465,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 +537,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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] net/gve: Enable stats reporting for GQ format
@ 2023-12-22 15:23 Rushil Gupta
0 siblings, 0 replies; 12+ messages in thread
From: Rushil Gupta @ 2023-12-22 15:23 UTC (permalink / raw)
To: junfeng.guo, jeroendb, joshwash, ferruh.yigit; +Cc: dev, Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ from device.
Tested using `show port xstats <port-id>` in interactive mode.
This metric can be triggered by using queues > cores.
---
drivers/net/gve/base/gve_adminq.h | 11 +++
drivers/net/gve/gve_ethdev.c | 117 ++++++++++++++++++++++++++++++
drivers/net/gve/gve_ethdev.h | 6 ++
3 files changed, 134 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..8e9596bb83 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -125,6 +125,73 @@ 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,
+ uint16_t nb_tx_queues, uint16_t nb_rx_queues)
+{
+ char z_name[RTE_MEMZONE_NAMESIZE];
+ 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);
+
+ snprintf(z_name, sizeof(z_name), "stats_report_%s", priv->pci_dev->device.name);
+ priv->stats_report_mem = rte_memzone_reserve_aligned(z_name,
+ 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 +243,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 +255,26 @@ 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) {
+ ret = gve_alloc_stats_report(priv,
+ dev->data->nb_tx_queues,
+ dev->data->nb_rx_queues);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR,
+ "Failed to allocate region for stats reporting.");
+ return ret;
+ }
+ ret = gve_adminq_report_stats(priv, priv->stats_report_len,
+ priv->stats_report_mem->iova,
+ GVE_STATS_REPORT_TIMER_PERIOD);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR, "gve_adminq_report_stats command failed.");
+ return ret;
+ }
+ }
+
return 0;
}
@@ -200,6 +288,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 +443,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 +465,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 +537,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
@@ -627,6 +722,28 @@ pci_dev_msix_vec_count(struct rte_pci_device *pdev)
return 0;
}
+static int
+pci_dev_msix_vec_count(struct rte_pci_device *pdev)
+{
+ off_t msix_pos = rte_pci_find_capability(pdev, RTE_PCI_CAP_ID_MSIX);
+ uint8_t msix_cap = pci_dev_find_capability(pdev, PCI_CAP_ID_MSIX);
+ uint16_t control;
+ int ret;
+
+ if (msix_pos > 0 && rte_pci_read_config(pdev, &control, sizeof(control),
+ msix_pos + RTE_PCI_MSIX_FLAGS) == sizeof(control))
+ return (control & RTE_PCI_MSIX_FLAGS_QSIZE) + 1;
+ if (!msix_cap)
+ return 0;
+
+ return 0;
+ ret = rte_pci_read_config(pdev, &control, sizeof(control), msix_cap + PCI_MSIX_FLAGS);
+ if (ret != sizeof(control))
+ return 0;
+
+ return (control & PCI_MSIX_FLAGS_QSIZE) + 1;
+}
+
static int
gve_setup_device_resources(struct gve_priv *priv)
{
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] net/gve: Enable stats reporting for GQ format
2023-12-19 2:16 Rushil Gupta
@ 2023-12-20 2:51 ` Guo, Junfeng
0 siblings, 0 replies; 12+ messages in thread
From: Guo, Junfeng @ 2023-12-20 2:51 UTC (permalink / raw)
To: Rushil Gupta, jeroendb, joshwash, ferruh.yigit; +Cc: dev
> -----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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] net/gve: Enable stats reporting for GQ format
@ 2023-12-19 2:16 Rushil Gupta
2023-12-20 2:51 ` Guo, Junfeng
0 siblings, 1 reply; 12+ messages in thread
From: Rushil Gupta @ 2023-12-19 2:16 UTC (permalink / raw)
To: junfeng.guo, jeroendb, joshwash, ferruh.yigit; +Cc: dev, Rushil Gupta
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,
+ 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] net/gve: Enable stats reporting for GQ format
@ 2023-12-19 2:13 Rushil Gupta
0 siblings, 0 replies; 12+ messages in thread
From: Rushil Gupta @ 2023-12-19 2:13 UTC (permalink / raw)
To: junfeng.guo, jeroendb, joshwash, ferruh.yigit; +Cc: dev, Rushil Gupta
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 | 84 +++++++++++++++++++++++++++++++
drivers/net/gve/gve_ethdev.h | 6 +++
3 files changed, 101 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..0829c17fc2 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,
+ 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,18 @@ 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 +277,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 +432,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 +454,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 +526,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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-01-19 13:37 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-22 15:05 [PATCH] net/gve: Enable stats reporting for GQ format Rushil Gupta
-- 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-19 2:16 Rushil Gupta
2023-12-20 2:51 ` Guo, Junfeng
2023-12-19 2:13 Rushil Gupta
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).