[PATCH] net/gve: Refill all descriptors that were consumed

2023-09-14 Thread Rushil Gupta
Refill all consumed descriptors instead of refilling a set amount of
descriptors everytime. Also, lower the rx-free-thresh to refill more
aggressively. This yields in fewer packet drops for DQO queue format.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/gve_ethdev.h | 2 +-
 drivers/net/gve/gve_rx_dqo.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index c9bcfa553c..fe7095ed76 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -27,7 +27,7 @@
 #define PCI_MSIX_FLAGS 2   /* Message Control */
 #define PCI_MSIX_FLAGS_QSIZE   0x07FF  /* Table size */
 
-#define GVE_DEFAULT_RX_FREE_THRESH  512
+#define GVE_DEFAULT_RX_FREE_THRESH   64
 #define GVE_DEFAULT_TX_FREE_THRESH   32
 #define GVE_DEFAULT_TX_RS_THRESH 32
 #define GVE_TX_MAX_FREE_SZ  512
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 236aefd2a8..7e7ddac48e 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -12,8 +12,8 @@ gve_rx_refill_dqo(struct gve_rx_queue *rxq)
 {
volatile struct gve_rx_desc_dqo *rx_buf_ring;
volatile struct gve_rx_desc_dqo *rx_buf_desc;
-   struct rte_mbuf *nmb[rxq->free_thresh];
-   uint16_t nb_refill = rxq->free_thresh;
+   struct rte_mbuf *nmb[rxq->nb_rx_hold];
+   uint16_t nb_refill = rxq->nb_rx_hold;
uint16_t nb_desc = rxq->nb_rx_desc;
uint16_t next_avail = rxq->bufq_tail;
struct rte_eth_dev *dev;
-- 
2.42.0.459.ge4e396fd5e-goog



Re: [PATCH v3] net/gve: add support for TSO in DQO RDA

2024-08-26 Thread Rushil Gupta
Acked-by: Rushil Gupta 

Thanks!


On Fri, Aug 9, 2024 at 11:49 AM Tathagat Priyadarshi
 wrote:
>
> The patch intends on adding support for TSO in DQO RDA format.
>
> Signed-off-by: Tathagat Priyadarshi 
> Signed-off-by: Varun Lakkur Ambaji Rao 
> ---
>  drivers/net/gve/gve_tx_dqo.c | 26 +++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
> index b9d6d01..731c287 100644
> --- a/drivers/net/gve/gve_tx_dqo.c
> +++ b/drivers/net/gve/gve_tx_dqo.c
> @@ -72,6 +72,17 @@
> txq->complq_tail = next;
>  }
>
I see that we are not populating the flex metadata here like the linux
driver. These metadata fields allow guest vm to send metadata to fxp.
However; that can be a separate change.


> +static inline void
> +gve_tx_fill_seg_desc_dqo(volatile union gve_tx_desc_dqo *desc, struct 
> rte_mbuf *tx_pkt)
> +{
> +   uint32_t hlen = tx_pkt->l2_len + tx_pkt->l3_len + tx_pkt->l4_len;
> +   desc->tso_ctx.cmd_dtype.dtype = GVE_TX_TSO_CTX_DESC_DTYPE_DQO;
> +   desc->tso_ctx.cmd_dtype.tso = 1;
> +   desc->tso_ctx.mss = (uint16_t)tx_pkt->tso_segsz;
> +   desc->tso_ctx.tso_total_len = tx_pkt->pkt_len - hlen;
> +   desc->tso_ctx.header_len = (uint8_t)hlen;
> +}
> +
>  uint16_t
>  gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>  {
> @@ -89,6 +100,7 @@
> uint16_t sw_id;
> uint64_t bytes;
> uint16_t first_sw_id;
> +   uint8_t tso;
> uint8_t csum;
>
> sw_ring = txq->sw_ring;
> @@ -109,15 +121,23 @@
> gve_tx_clean_dqo(txq);
> }
>
> -   if (txq->nb_free < tx_pkt->nb_segs)
> -   break;
> -
> ol_flags = tx_pkt->ol_flags;
> nb_used = tx_pkt->nb_segs;
> first_sw_id = sw_id;
>
> +   tso = !!(ol_flags & RTE_MBUF_F_TX_TCP_SEG);
> csum = !!(ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO);
>
> +   nb_used += tso;
> +   if (txq->nb_free < nb_used)
> +   break;
> +
> +   if (tso) {
> +   txd = &txr[tx_id];
> +   gve_tx_fill_seg_desc_dqo(txd, tx_pkt);
> +   tx_id = (tx_id + 1) & mask;
> +   }
> +
> do {
> if (sw_ring[sw_id] != NULL)
> PMD_DRV_LOG(DEBUG, "Overwriting an entry in 
> sw_ring");
> --
> 1.8.3.1
>


Re: [PATCH 1/2] net/gve: change offloading capability

2023-02-16 Thread Rushil Gupta
Makes sense. The virtual device only does L4 checksum.

Acked-by: Rushil Gupta 

On Thu, Feb 16, 2023 at 10:58 AM Levend Sayar  wrote:

> Google Virtual NIC is not doing IPv4 checksummimg.
> Removed that capability from PMD.
>
> Signed-off-by: Levend Sayar 
> ---
>  drivers/net/gve/gve_ethdev.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
> index 06d1b796c8..fef2458a16 100644
> --- a/drivers/net/gve/gve_ethdev.c
> +++ b/drivers/net/gve/gve_ethdev.c
> @@ -282,7 +282,6 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *dev_info)
> dev_info->rx_offload_capa = 0;
> dev_info->tx_offload_capa =
> RTE_ETH_TX_OFFLOAD_MULTI_SEGS   |
> -   RTE_ETH_TX_OFFLOAD_IPV4_CKSUM   |
> RTE_ETH_TX_OFFLOAD_UDP_CKSUM|
> RTE_ETH_TX_OFFLOAD_TCP_CKSUM|
> RTE_ETH_TX_OFFLOAD_SCTP_CKSUM   |
> --
> 2.37.1 (Apple Git-137.1)
>
>


Re: [PATCH v2] net/gve: add maintainers for GVE

2023-05-07 Thread Rushil Gupta
acked-by: Rushil Gupta 


On Thu, May 4, 2023 at 7:19 PM Junfeng Guo  wrote:
>
> Add maintainers from Google for GVE.
>
> Signed-off-by: Junfeng Guo 
> Signed-off-by: Jeroen de Borst 
> Signed-off-by: Rushil Gupta 
> Signed-off-by: Joshua Washington 
> ---
>  MAINTAINERS | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8df23e5099..08001751b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -713,6 +713,9 @@ F: doc/guides/nics/features/enic.ini
>
>  Google Virtual Ethernet
>  M: Junfeng Guo 
> +M: Jeroen de Borst 
> +M: Rushil Gupta 
> +M: Joshua Washington 
>  F: drivers/net/gve/
>  F: doc/guides/nics/gve.rst
>  F: doc/guides/nics/features/gve.ini
> --
> 2.34.1
>


Re: [PATCH] net/gve: Check whether the driver is compatible with the device presented.

2023-05-07 Thread Rushil Gupta
Thanks for reviewing Ferruh Yigit! I wanted to answer your queries
before sending the next patch.

Please find answers below:

On Thu, May 4, 2023 at 8:06 AM Ferruh Yigit  wrote:
>
> On 4/26/2023 10:37 PM, Rushil Gupta wrote:
> > Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
> > version as OS version, reserving driver_version fields for GVE driver
> > version based on features.
> >
>
> './devtools/check-git-log.sh' is giving some warnings, can you please
> check them?
>
> Contribution guide documentation may help to fix reported issues:
> https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-subject-line
>
I did run ./devtools/checkpatches.sh  but not
./devtools/check-git-log.sh. I will run both before sending the next
patch.

> > Depends-on: series-27687
> >
> > Signed-off-by: Rushil Gupta 
> > Signed-off-by: Joshua Washington 
> > Signed-off-by: Junfeng Guo 
> > Signed-off-by: Jeroen de Borst 
> > ---
> >  drivers/net/gve/base/gve.h|  3 --
> >  drivers/net/gve/base/gve_adminq.c | 19 +
> >  drivers/net/gve/base/gve_adminq.h | 49 ++-
> >  drivers/net/gve/base/gve_osdep.h  | 36 +
> >  drivers/net/gve/gve_ethdev.c  | 65 +--
> >  drivers/net/gve/gve_ethdev.h  |  2 +-
> >  drivers/net/gve/gve_version.c | 14 +++
> >  drivers/net/gve/gve_version.h | 25 
> >  drivers/net/gve/meson.build   |  1 +
> >  9 files changed, 198 insertions(+), 16 deletions(-)
> >  create mode 100644 drivers/net/gve/gve_version.c
> >  create mode 100644 drivers/net/gve/gve_version.h
> >
> > diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
> > index 2dc4507acb..89f9654a72 100644
> > --- a/drivers/net/gve/base/gve.h
> > +++ b/drivers/net/gve/base/gve.h
> > @@ -8,9 +8,6 @@
> >
> >  #include "gve_desc.h"
> >
> > -#define GVE_VERSION  "1.3.0"
> > -#define GVE_VERSION_PREFIX   "GVE-"
> > -
> >  #ifndef GOOGLE_VENDOR_ID
> >  #define GOOGLE_VENDOR_ID 0x1ae0
> >  #endif
> > diff --git a/drivers/net/gve/base/gve_adminq.c 
> > b/drivers/net/gve/base/gve_adminq.c
> > index e745b709b2..2e5099a5b0 100644
> > --- a/drivers/net/gve/base/gve_adminq.c
> > +++ b/drivers/net/gve/base/gve_adminq.c
> > @@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
> >   case GVE_ADMINQ_GET_PTYPE_MAP:
> >   priv->adminq_get_ptype_map_cnt++;
> >   break;
> > + case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
> > + priv->adminq_verify_driver_compatibility_cnt++;
> > + break;
> >   default:
> >   PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
> >   }
> > @@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct 
> > gve_priv *priv,
> >   return gve_adminq_execute_cmd(priv, &cmd);
> >  }
> >
> > +int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
> > +u64 driver_info_len,
> > +dma_addr_t driver_info_addr)
> > +{
> > + union gve_adminq_command cmd;
> > +
> > + memset(&cmd, 0, sizeof(cmd));
> > + cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
> > + cmd.verify_driver_compatibility = (struct 
> > gve_adminq_verify_driver_compatibility) {
> > + .driver_info_len = cpu_to_be64(driver_info_len),
> > + .driver_info_addr = cpu_to_be64(driver_info_addr),
> > + };
> > +
> > + return gve_adminq_execute_cmd(priv, &cmd);
> > +}
> > +
> >  int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
> >  {
> >   union gve_adminq_command cmd;
> > diff --git a/drivers/net/gve/base/gve_adminq.h 
> > b/drivers/net/gve/base/gve_adminq.h
> > index 05550119de..edac32f031 100644
> > --- a/drivers/net/gve/base/gve_adminq.h
> > +++ b/drivers/net/gve/base/gve_adminq.h
> > @@ -1,6 +1,6 @@
> >  /* SPDX-License-Identifier: MIT
> >   * Google Virtual Ethernet (gve) driver
> > - * Copyright (C) 2015-2022 Google, Inc.
> > + * Copyright (C) 2015-2023 Google, Inc.
> >   */
> >
> >  #ifndef _GVE_ADMINQ_H
> > @@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
> >   GVE_ADMINQ_REPORT_STATS = 0xC,
> >   GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
> >   GVE_ADMINQ

[v2] net/gve: check driver compatibility

2023-05-08 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 ++
 drivers/net/gve/base/gve_adminq.h | 46 ++-
 drivers/net/gve/base/gve_osdep.h  | 24 
 drivers/net/gve/gve_ethdev.c  | 61 +--
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 14 +++
 drivers/net/gve/gve_version.h | 25 +
 drivers/net/gve/meson.build   |  7 
 9 files changed, 185 insertions(+), 16 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2dc4507acb..89f9654a72 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -8,9 +8,6 @@
 
 #include "gve_desc.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e745b709b2..2e5099a5b0 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..e30b184913 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,44 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+   __be64 driver_info_addr;
+};
+
+GVE_CHECK_STRUCT_LEN(16,  gve_adminq_verify_driver_compatibility);
+
 
 struct gve_adminq_configure_device_resources {
__be64 counter_array;
@@ -345,6 +384,8 @@ union gve_adminq_command {
 

[v2] net/gve: check driver compatibility

2023-05-08 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 ++
 drivers/net/gve/base/gve_adminq.h | 46 ++-
 drivers/net/gve/base/gve_osdep.h  | 24 
 drivers/net/gve/gve_ethdev.c  | 61 +--
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 14 +++
 drivers/net/gve/gve_version.h | 25 +
 drivers/net/gve/meson.build   |  7 
 9 files changed, 185 insertions(+), 16 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2dc4507acb..89f9654a72 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -8,9 +8,6 @@
 
 #include "gve_desc.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e745b709b2..2e5099a5b0 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..e30b184913 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,44 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+   __be64 driver_info_addr;
+};
+
+GVE_CHECK_STRUCT_LEN(16,  gve_adminq_verify_driver_compatibility);
+
 
 struct gve_adminq_configure_device_resources {
__be64 counter_array;
@@ -345,6 +384,8 @@ union gve_adminq_command {
 

Re: [PATCH] net/gve: Check whether the driver is compatible with the device presented.

2023-05-08 Thread Rushil Gupta
Sounds good. I will document the versioning in a separate patch. For
now, the DPDK driver with DQO changes is reported as 1.0.0.

I think there is some confusion here. The base code is not coming from
the linux kernel driver.
Linux kernel driver is in a different repository and it has nothing to
do with the dpdk driver version. Ideally, we'd want the base code of
both drivers to be the same but there are several higher priority
projects that are taking precedence.

Btw I have addressed your comments:
http://patchwork.dpdk.org/project/dpdk/patch/20230508191552.104540-1-rush...@google.com/


On Mon, May 8, 2023 at 9:00 AM Ferruh Yigit  wrote:
>
> On 5/8/2023 7:23 AM, Rushil Gupta wrote:
> >>> +#ifndef _GVE_VERSION_H_
> >>> +#define _GVE_VERSION_H_
> >>> +
> >>> +#include 
> >>> +
> >>> +#define GVE_VERSION_PREFIX "DPDK-"
> >>> +#define GVE_VERSION_MAJOR 1
> >>> +#define GVE_VERSION_MINOR 0
> >>> +#define GVE_VERSION_SUB 0
> >>> +
> >> Is this GVE base version, or DPDK gve version?
> >> Previous version information was "GVE-1.3.0", now it is becoming
> >> "DPDK-1.0.0",
> >> is this breaking the version link with GVE base version and creating a
> >> new versioning scheme for DPDK GVE driver?
> >>
> >> Btw, documentation still has "v1.3.0. GVE base code", should it be
> >> updated as well?
> >>
> > DPDK-1.0.0 is the DPDK driver version. GVE driver versioning only
> > applies for linux kernel Gvnic driver.
> > Similarly Windows Gvnic driver has different versioning system.
>
> So creating a new versioning scheme for DPDK driver, I guess that is OK.
> Since there is a new version, is it clear how to manage the versioning,
> like when to update major version and when to update minor etc? If so
> can you please document this?
>
> And base code is still coming from the Linux kernel driver, I wonder if
> a mapping between versions of these two is required.
> As mentioned "v1.3.0" is still in the documentation, what happens if
> base code updated and some relevant update is required in the dpdk
> driver code? How to record/manage this?
> Some drivers records the base code version in a readme file in the base
> folder, maybe same can be done.
>


Re: [PATCH] net/gve: support queue start and stop operations

2023-05-15 Thread Rushil Gupta
tested-by: Rushil Gupta 

On Mon, May 8, 2023 at 8:07 PM Junfeng Guo  wrote:

> Add support for queue operations for GQI:
>  - gve_rx_queue_start
>  - gve_tx_queue_start
>  - gve_rx_queue_stop
>  - gve_tx_queue_stop
>
> Add support for queue operations for DQO:
>  - gve_rx_queue_start_dqo
>  - gve_tx_queue_start_dqo
>  - gve_rx_queue_stop_dqo
>  - gve_tx_queue_stop_dqo
>
> Also move the funcs of rxq_mbufs_alloc into the corresponding files.
>
> Signed-off-by: Junfeng Guo 
> ---
>  drivers/net/gve/gve_ethdev.c | 166 +++
>  drivers/net/gve/gve_ethdev.h |  36 
>  drivers/net/gve/gve_rx.c |  96 ++--
>  drivers/net/gve/gve_rx_dqo.c |  97 ++--
>  drivers/net/gve/gve_tx.c |  54 ++--
>  drivers/net/gve/gve_tx_dqo.c |  54 ++--
>  6 files changed, 364 insertions(+), 139 deletions(-)
>
> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
> index 8b6861a24f..1dcb3b3a01 100644
> --- a/drivers/net/gve/gve_ethdev.c
> +++ b/drivers/net/gve/gve_ethdev.c
> @@ -104,81 +104,6 @@ gve_dev_configure(struct rte_eth_dev *dev)
> return 0;
>  }
>
> -static int
> -gve_refill_pages(struct gve_rx_queue *rxq)
> -{
> -   struct rte_mbuf *nmb;
> -   uint16_t i;
> -   int diag;
> -
> -   diag = rte_pktmbuf_alloc_bulk(rxq->mpool, &rxq->sw_ring[0],
> rxq->nb_rx_desc);
> -   if (diag < 0) {
> -   for (i = 0; i < rxq->nb_rx_desc - 1; i++) {
> -   nmb = rte_pktmbuf_alloc(rxq->mpool);
> -   if (!nmb)
> -   break;
> -   rxq->sw_ring[i] = nmb;
> -   }
> -   if (i < rxq->nb_rx_desc - 1)
> -   return -ENOMEM;
> -   }
> -   rxq->nb_avail = 0;
> -   rxq->next_avail = rxq->nb_rx_desc - 1;
> -
> -   for (i = 0; i < rxq->nb_rx_desc; i++) {
> -   if (rxq->is_gqi_qpl) {
> -   rxq->rx_data_ring[i].addr = rte_cpu_to_be_64(i *
> PAGE_SIZE);
> -   } else {
> -   if (i == rxq->nb_rx_desc - 1)
> -   break;
> -   nmb = rxq->sw_ring[i];
> -   rxq->rx_data_ring[i].addr =
> rte_cpu_to_be_64(rte_mbuf_data_iova(nmb));
> -   }
> -   }
> -
> -   rte_write32(rte_cpu_to_be_32(rxq->next_avail), rxq->qrx_tail);
> -
> -   return 0;
> -}
> -
> -static int
> -gve_refill_dqo(struct gve_rx_queue *rxq)
> -{
> -   struct rte_mbuf *nmb;
> -   uint16_t i;
> -   int diag;
> -
> -   diag = rte_pktmbuf_alloc_bulk(rxq->mpool, &rxq->sw_ring[0],
> rxq->nb_rx_desc);
> -   if (diag < 0) {
> -   rxq->stats.no_mbufs_bulk++;
> -   for (i = 0; i < rxq->nb_rx_desc - 1; i++) {
> -   nmb = rte_pktmbuf_alloc(rxq->mpool);
> -   if (!nmb)
> -   break;
> -   rxq->sw_ring[i] = nmb;
> -   }
> -   if (i < rxq->nb_rx_desc - 1) {
> -   rxq->stats.no_mbufs += rxq->nb_rx_desc - 1 - i;
> -   return -ENOMEM;
> -   }
> -   }
> -
> -   for (i = 0; i < rxq->nb_rx_desc; i++) {
> -   if (i == rxq->nb_rx_desc - 1)
> -   break;
> -   nmb = rxq->sw_ring[i];
> -   rxq->rx_ring[i].buf_addr =
> rte_cpu_to_le_64(rte_mbuf_data_iova_default(nmb));
> -   rxq->rx_ring[i].buf_id = rte_cpu_to_le_16(i);
> -   }
> -
> -   rxq->nb_rx_hold = 0;
> -   rxq->bufq_tail = rxq->nb_rx_desc - 1;
> -
> -   rte_write32(rxq->bufq_tail, rxq->qrx_tail);
> -
> -   return 0;
> -}
> -
>  static int
>  gve_link_update(struct rte_eth_dev *dev, __rte_unused int
> wait_to_complete)
>  {
> @@ -208,65 +133,68 @@ gve_link_update(struct rte_eth_dev *dev,
> __rte_unused int wait_to_complete)
>  }
>
>  static int
> -gve_dev_start(struct rte_eth_dev *dev)
> +gve_start_queues(struct rte_eth_dev *dev)
>  {
> -   uint16_t num_queues = dev->data->nb_tx_queues;
> struct gve_priv *priv = dev->data->dev_private;
> -   struct gve_tx_queue *txq;
> -   struct gve_rx_queue *rxq;
> +   uint16_t num_queues;
> uint16_t i;
> -   int err;
> +   int ret;
>
> +   num_queues = dev->data->nb_tx_queues;
>

[v3] net/gve: check driver compatibility

2023-05-18 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 ++
 drivers/net/gve/base/gve_adminq.h | 46 ++-
 drivers/net/gve/base/gve_osdep.h  | 24 
 drivers/net/gve/gve_ethdev.c  | 61 +--
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 13 +++
 drivers/net/gve/gve_version.h | 24 
 drivers/net/gve/meson.build   |  5 ++-
 9 files changed, 179 insertions(+), 18 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2dc4507acb..89f9654a72 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -8,9 +8,6 @@
 
 #include "gve_desc.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e745b709b2..2e5099a5b0 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..e30b184913 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,44 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+   __be64 driver_info_addr;
+};
+
+GVE_CHECK_STRUCT_LEN(16,  gve_adminq_verify_driver_compatibility);
+
 
 struct gve_adminq_configure_device_resources {
__be64 counter_array;
@@ -345,6 +384,8 @@ union gve_adminq_command {
 

Re: [v2] net/gve: check driver compatibility

2023-05-18 Thread Rushil Gupta
Thanks for reviewing. Here is v3:
http://patchwork.dpdk.org/project/dpdk/patch/20230518174005.1377467-1-rush...@google.com/

On Wed, May 17, 2023 at 9:59 AM Ferruh Yigit  wrote:

> On 5/8/2023 8:15 PM, Rushil Gupta wrote:
> > Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
> > version as OS version, reserving driver_version fields for GVE driver
> > version based on features.
> >
> > Signed-off-by: Rushil Gupta 
> > Signed-off-by: Joshua Washington 
> > Signed-off-by: Junfeng Guo 
> > Signed-off-by: Jeroen de Borst 
> > ---
> >  drivers/net/gve/base/gve.h|  3 --
> >  drivers/net/gve/base/gve_adminq.c | 19 ++
> >  drivers/net/gve/base/gve_adminq.h | 46 ++-
> >  drivers/net/gve/base/gve_osdep.h  | 24 
> >  drivers/net/gve/gve_ethdev.c  | 61 +--
> >  drivers/net/gve/gve_ethdev.h  |  2 +-
> >  drivers/net/gve/gve_version.c | 14 +++
> >  drivers/net/gve/gve_version.h | 25 +
> >  drivers/net/gve/meson.build   |  7 
> >  9 files changed, 185 insertions(+), 16 deletions(-)
> >  create mode 100644 drivers/net/gve/gve_version.c
> >  create mode 100644 drivers/net/gve/gve_version.h
> >
>
> 'doc/guides/nics/gve.rst' has following reference to Linux kernel version:
>
> "
> The base code is under MIT license and based on GVE kernel driver v1.3.0.
> "
>
> Previously you mentioned that Linux kernel code has nothing to do with
> DPDK version, should above note removed or updated?
>
> I will address this in the license patch 23.11. We will port all of our
MIT licenses to BSD.
http://patchwork.dpdk.org/project/dpdk/patch/20230411045908.844901-2-rush...@google.com/
For now let's keep this change separate from license update/removal.

> <...>
>
> > +static int
> > +gve_verify_driver_compatibility(struct gve_priv *priv)
> > +{
> > + struct gve_driver_info *driver_info;
> > + int err;
> > +
> > + driver_info = rte_zmalloc("driver info", sizeof(struct
> gve_driver_info), 0);
>
> Can use calloc(), no need to use DPDK memory APIs.
> This memory is not for datapath, and only used for short time.
>
Done

>
> > + if (driver_info == NULL) {
> > + PMD_DRV_LOG(ERR,
> > + "Could not alloc for driver compatibility");
>
> Message can be misleading, adding 'verify' may help.
>
Done

>
> > + return -ENOMEM;
> > + }
> > + *driver_info = (struct gve_driver_info) {
> > + .os_type = 5, /* DPDK */
> > + .driver_major = GVE_VERSION_MAJOR,
> > + .driver_minor = GVE_VERSION_MINOR,
> > + .driver_sub = GVE_VERSION_SUB,
> > + .os_version_major = cpu_to_be32(DPDK_VERSION_MAJOR),
> > + .os_version_minor = cpu_to_be32(DPDK_VERSION_MINOR),
> > + .os_version_sub = cpu_to_be32(DPDK_VERSION_SUB),
> > + .driver_capability_flags = {
> > + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS1),
> > + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS2),
> > + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS3),
> > + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS4),
> > + },
> > + };
> > +
> > + populate_driver_version_strings((char
> *)driver_info->os_version_str1,
> > + (char *)driver_info->os_version_str2);
> > +
>
> Is it accepted by adminq_verify if 'os_version_str1' & 'os_version_str2'
> is not set? If not 'populate_driver_version_strings()' should return
> success/error value.
>
It is acceptable if  'os_version_str1' & 'os_version_str2' are not set.

>
> > + err = gve_adminq_verify_driver_compatibility(priv,
> > + sizeof(struct gve_driver_info), (dma_addr_t)driver_info);
> > +
> > + /* It's ok if the device doesn't support this */
> > + if (err == -EOPNOTSUPP)
> > + err = 0;
> > +
> > + rte_free(driver_info);
> > + return err;
> > +}
> > +
> >  static int
> >  gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info
> *dev_info)
> >  {
> > @@ -672,6 +706,11 @@ gve_init_priv(struct gve_priv *priv, bool
> skip_describe_device)
> >   PMD_DRV_LOG(ERR, "Failed to alloc admin queue: err=%d",
> err);
> >   return err;
> >   }
> > + err

[v3] net/gve: check driver compatibility

2023-05-19 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 0001-net-gve-check-driver-compatibility.patch | 388 ++
 drivers/net/gve/base/gve.h|   3 -
 drivers/net/gve/base/gve_adminq.c |  19 +
 drivers/net/gve/base/gve_adminq.h |  46 ++-
 drivers/net/gve/base/gve_osdep.h  |  24 ++
 drivers/net/gve/gve_ethdev.c  |  60 ++-
 drivers/net/gve/gve_ethdev.h  |   2 +-
 drivers/net/gve/gve_version.c |  13 +
 drivers/net/gve/gve_version.h |  24 ++
 drivers/net/gve/meson.build   |   5 +-
 10 files changed, 566 insertions(+), 18 deletions(-)
 create mode 100644 0001-net-gve-check-driver-compatibility.patch
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/0001-net-gve-check-driver-compatibility.patch 
b/0001-net-gve-check-driver-compatibility.patch
new file mode 100644
index 00..0187db7dc2
--- /dev/null
+++ b/0001-net-gve-check-driver-compatibility.patch
@@ -0,0 +1,388 @@
+From 1624a08eaa94242286da930b92b227759f906bd9 Mon Sep 17 00:00:00 2001
+From: Rushil Gupta 
+Date: Thu, 13 Apr 2023 22:06:20 -0700
+Subject: [v3] net/gve: check driver compatibility
+
+Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
+version as OS version, reserving driver_version fields for GVE driver
+version based on features.
+
+Signed-off-by: Rushil Gupta 
+Signed-off-by: Joshua Washington 
+Signed-off-by: Junfeng Guo 
+Signed-off-by: Jeroen de Borst 
+---
+ drivers/net/gve/base/gve.h|  3 --
+ drivers/net/gve/base/gve_adminq.c | 19 ++
+ drivers/net/gve/base/gve_adminq.h | 46 ++-
+ drivers/net/gve/base/gve_osdep.h  | 24 
+ drivers/net/gve/gve_ethdev.c  | 61 +--
+ drivers/net/gve/gve_ethdev.h  |  2 +-
+ drivers/net/gve/gve_version.c | 13 +++
+ drivers/net/gve/gve_version.h | 24 
+ drivers/net/gve/meson.build   |  5 ++-
+ 9 files changed, 179 insertions(+), 18 deletions(-)
+ create mode 100644 drivers/net/gve/gve_version.c
+ create mode 100644 drivers/net/gve/gve_version.h
+
+diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
+index 2dc4507acb..89f9654a72 100644
+--- a/drivers/net/gve/base/gve.h
 b/drivers/net/gve/base/gve.h
+@@ -8,9 +8,6 @@
+ 
+ #include "gve_desc.h"
+ 
+-#define GVE_VERSION   "1.3.0"
+-#define GVE_VERSION_PREFIX"GVE-"
+-
+ #ifndef GOOGLE_VENDOR_ID
+ #define GOOGLE_VENDOR_ID  0x1ae0
+ #endif
+diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
+index e745b709b2..2e5099a5b0 100644
+--- a/drivers/net/gve/base/gve_adminq.c
 b/drivers/net/gve/base/gve_adminq.c
+@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
+   case GVE_ADMINQ_GET_PTYPE_MAP:
+   priv->adminq_get_ptype_map_cnt++;
+   break;
++  case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
++  priv->adminq_verify_driver_compatibility_cnt++;
++  break;
+   default:
+   PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
+   }
+@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
+   return gve_adminq_execute_cmd(priv, &cmd);
+ }
+ 
++int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
++ u64 driver_info_len,
++ dma_addr_t driver_info_addr)
++{
++  union gve_adminq_command cmd;
++
++  memset(&cmd, 0, sizeof(cmd));
++  cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
++  cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
++  .driver_info_len = cpu_to_be64(driver_info_len),
++  .driver_info_addr = cpu_to_be64(driver_info_addr),
++  };
++
++  return gve_adminq_execute_cmd(priv, &cmd);
++}
++
+ int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
+ {
+   union gve_adminq_command cmd;
+diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
+index 05550119de..e30b184913 100644
+--- a/drivers/net/gve/base/gve_adminq.h
 b/drivers/net/gve/base/gve_adminq.h
+@@ -1,6 +1,6 @@
+ /* SPDX-License-Identifier: MIT
+  * Google Virtual Ethernet (gve) driver
+- * Copyright (C) 2015-2022 Google, Inc.
++ * Copyright (C) 2015-2023 Google, Inc.
+  */
+ 
+ #ifndef _GVE_ADMINQ_H
+@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
+   GVE_ADMINQ_REPORT_STATS = 0xC,
+   GVE_ADMINQ_REPORT_LIN

[PATCH] net/gve: check driver compatibility

2023-05-19 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 0001-net-gve-check-driver-compatibility.patch | 388 ++
 drivers/net/gve/base/gve.h|   3 -
 drivers/net/gve/base/gve_adminq.c |  19 +
 drivers/net/gve/base/gve_adminq.h |  46 ++-
 drivers/net/gve/base/gve_osdep.h  |  24 ++
 drivers/net/gve/gve_ethdev.c  |  60 ++-
 drivers/net/gve/gve_ethdev.h  |   2 +-
 drivers/net/gve/gve_version.c |  13 +
 drivers/net/gve/gve_version.h |  24 ++
 drivers/net/gve/meson.build   |   5 +-
 10 files changed, 566 insertions(+), 18 deletions(-)
 create mode 100644 0001-net-gve-check-driver-compatibility.patch
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/0001-net-gve-check-driver-compatibility.patch 
b/0001-net-gve-check-driver-compatibility.patch
new file mode 100644
index 00..0187db7dc2
--- /dev/null
+++ b/0001-net-gve-check-driver-compatibility.patch
@@ -0,0 +1,388 @@
+From 1624a08eaa94242286da930b92b227759f906bd9 Mon Sep 17 00:00:00 2001
+From: Rushil Gupta 
+Date: Thu, 13 Apr 2023 22:06:20 -0700
+Subject: [v3] net/gve: check driver compatibility
+
+Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
+version as OS version, reserving driver_version fields for GVE driver
+version based on features.
+
+Signed-off-by: Rushil Gupta 
+Signed-off-by: Joshua Washington 
+Signed-off-by: Junfeng Guo 
+Signed-off-by: Jeroen de Borst 
+---
+ drivers/net/gve/base/gve.h|  3 --
+ drivers/net/gve/base/gve_adminq.c | 19 ++
+ drivers/net/gve/base/gve_adminq.h | 46 ++-
+ drivers/net/gve/base/gve_osdep.h  | 24 
+ drivers/net/gve/gve_ethdev.c  | 61 +--
+ drivers/net/gve/gve_ethdev.h  |  2 +-
+ drivers/net/gve/gve_version.c | 13 +++
+ drivers/net/gve/gve_version.h | 24 
+ drivers/net/gve/meson.build   |  5 ++-
+ 9 files changed, 179 insertions(+), 18 deletions(-)
+ create mode 100644 drivers/net/gve/gve_version.c
+ create mode 100644 drivers/net/gve/gve_version.h
+
+diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
+index 2dc4507acb..89f9654a72 100644
+--- a/drivers/net/gve/base/gve.h
 b/drivers/net/gve/base/gve.h
+@@ -8,9 +8,6 @@
+ 
+ #include "gve_desc.h"
+ 
+-#define GVE_VERSION   "1.3.0"
+-#define GVE_VERSION_PREFIX"GVE-"
+-
+ #ifndef GOOGLE_VENDOR_ID
+ #define GOOGLE_VENDOR_ID  0x1ae0
+ #endif
+diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
+index e745b709b2..2e5099a5b0 100644
+--- a/drivers/net/gve/base/gve_adminq.c
 b/drivers/net/gve/base/gve_adminq.c
+@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
+   case GVE_ADMINQ_GET_PTYPE_MAP:
+   priv->adminq_get_ptype_map_cnt++;
+   break;
++  case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
++  priv->adminq_verify_driver_compatibility_cnt++;
++  break;
+   default:
+   PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
+   }
+@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
+   return gve_adminq_execute_cmd(priv, &cmd);
+ }
+ 
++int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
++ u64 driver_info_len,
++ dma_addr_t driver_info_addr)
++{
++  union gve_adminq_command cmd;
++
++  memset(&cmd, 0, sizeof(cmd));
++  cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
++  cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
++  .driver_info_len = cpu_to_be64(driver_info_len),
++  .driver_info_addr = cpu_to_be64(driver_info_addr),
++  };
++
++  return gve_adminq_execute_cmd(priv, &cmd);
++}
++
+ int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
+ {
+   union gve_adminq_command cmd;
+diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
+index 05550119de..e30b184913 100644
+--- a/drivers/net/gve/base/gve_adminq.h
 b/drivers/net/gve/base/gve_adminq.h
+@@ -1,6 +1,6 @@
+ /* SPDX-License-Identifier: MIT
+  * Google Virtual Ethernet (gve) driver
+- * Copyright (C) 2015-2022 Google, Inc.
++ * Copyright (C) 2015-2023 Google, Inc.
+  */
+ 
+ #ifndef _GVE_ADMINQ_H
+@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
+   GVE_ADMINQ_REPORT_STATS = 0xC,
+   GVE_ADMINQ_REPORT_LIN

[PATCH] net/gve: check driver compatibility

2023-05-19 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 ++
 drivers/net/gve/base/gve_adminq.h | 46 ++-
 drivers/net/gve/base/gve_osdep.h  | 24 
 drivers/net/gve/gve_ethdev.c  | 61 +--
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 13 +++
 drivers/net/gve/gve_version.h | 24 
 drivers/net/gve/meson.build   |  5 ++-
 9 files changed, 179 insertions(+), 18 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2b7cf7d99b..f7b297e759 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -9,9 +9,6 @@
 #include "gve_desc.h"
 #include "gve_desc_dqo.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e963f910a0..41202725e6 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..e30b184913 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,44 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+   __be64 driver_info_addr;
+};
+
+GVE_CHECK_STRUCT_LEN(16,  gve_adminq_verify_driver_compatibility);
+
 
 struct gve_adminq_configure_device_resources {
__be64 counter_array;
@@ -345,6

Re: [PATCH] net/gve: check driver compatibility

2023-05-19 Thread Rushil Gupta
Hi Ferruh
I have updated the latest patch here:
http://patchwork.dpdk.org/project/dpdk/patch/20230519072600.1444309-1-rush...@google.com/
However, using calloc causes issue while executing verify-compatibility command.

sudo dpdk-testpmd -a 00:04.0 -l 0-32 -- --forward-mode=rxonly --txq=16
--rxq=16 --nb-cores=16 --stats-period 5
...
EAL: Using IOMMU type 8 (No-IOMMU)
EAL: Probe PCI driver: net_gve (1ae0:42) device: :00:04.0 (socket -1)
gve_adminq_parse_err(): AQ command failed with status -9
gve_init_priv(): Could not verify driver compatibility: err=-22
EAL: Releasing PCI mapped resource for :00:04.0
...
EAL: Error - exiting with code: 1


This is probably because the adminq command is sharing memory to
report driver-info to the gvnic device and that needs to be in dpdk
memory.



On Fri, May 19, 2023 at 12:26 AM Rushil Gupta  wrote:
>
> Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
> version as OS version, reserving driver_version fields for GVE driver
> version based on features.
>
> Signed-off-by: Rushil Gupta 
> Signed-off-by: Joshua Washington 
> Signed-off-by: Junfeng Guo 
> Signed-off-by: Jeroen de Borst 
> ---
>  drivers/net/gve/base/gve.h|  3 --
>  drivers/net/gve/base/gve_adminq.c | 19 ++
>  drivers/net/gve/base/gve_adminq.h | 46 ++-
>  drivers/net/gve/base/gve_osdep.h  | 24 
>  drivers/net/gve/gve_ethdev.c  | 61 +--
>  drivers/net/gve/gve_ethdev.h  |  2 +-
>  drivers/net/gve/gve_version.c | 13 +++
>  drivers/net/gve/gve_version.h | 24 
>  drivers/net/gve/meson.build   |  5 ++-
>  9 files changed, 179 insertions(+), 18 deletions(-)
>  create mode 100644 drivers/net/gve/gve_version.c
>  create mode 100644 drivers/net/gve/gve_version.h
>
> diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
> index 2b7cf7d99b..f7b297e759 100644
> --- a/drivers/net/gve/base/gve.h
> +++ b/drivers/net/gve/base/gve.h
> @@ -9,9 +9,6 @@
>  #include "gve_desc.h"
>  #include "gve_desc_dqo.h"
>
> -#define GVE_VERSION"1.3.0"
> -#define GVE_VERSION_PREFIX "GVE-"
> -
>  #ifndef GOOGLE_VENDOR_ID
>  #define GOOGLE_VENDOR_ID   0x1ae0
>  #endif
> diff --git a/drivers/net/gve/base/gve_adminq.c 
> b/drivers/net/gve/base/gve_adminq.c
> index e963f910a0..41202725e6 100644
> --- a/drivers/net/gve/base/gve_adminq.c
> +++ b/drivers/net/gve/base/gve_adminq.c
> @@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
> case GVE_ADMINQ_GET_PTYPE_MAP:
> priv->adminq_get_ptype_map_cnt++;
> break;
> +   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
> +   priv->adminq_verify_driver_compatibility_cnt++;
> +   break;
> default:
> PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
> }
> @@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct 
> gve_priv *priv,
> return gve_adminq_execute_cmd(priv, &cmd);
>  }
>
> +int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
> +  u64 driver_info_len,
> +  dma_addr_t driver_info_addr)
> +{
> +   union gve_adminq_command cmd;
> +
> +   memset(&cmd, 0, sizeof(cmd));
> +   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
> +   cmd.verify_driver_compatibility = (struct 
> gve_adminq_verify_driver_compatibility) {
> +   .driver_info_len = cpu_to_be64(driver_info_len),
> +   .driver_info_addr = cpu_to_be64(driver_info_addr),
> +   };
> +
> +   return gve_adminq_execute_cmd(priv, &cmd);
> +}
> +
>  int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
>  {
> union gve_adminq_command cmd;
> diff --git a/drivers/net/gve/base/gve_adminq.h 
> b/drivers/net/gve/base/gve_adminq.h
> index 05550119de..e30b184913 100644
> --- a/drivers/net/gve/base/gve_adminq.h
> +++ b/drivers/net/gve/base/gve_adminq.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: MIT
>   * Google Virtual Ethernet (gve) driver
> - * Copyright (C) 2015-2022 Google, Inc.
> + * Copyright (C) 2015-2023 Google, Inc.
>   */
>
>  #ifndef _GVE_ADMINQ_H
> @@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
> GVE_ADMINQ_REPORT_STATS = 0xC,
> GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
> GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
> +   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
>  };
>
>  /* Admin queue status codes */
> @@ -145,6 +146,44 @@ enum gve_sup_feature_

Re: [PATCH] net/gve: check driver compatibility

2023-05-19 Thread Rushil Gupta
I agree. Other adminq commands like gve_adminq_report_link_speed use
gve_alloc_dma_mem and gve_free_dma_mem (which use
rte_memzone_reserve_aligned under the hood)
so let's stick to that.


On Fri, May 19, 2023 at 3:04 AM Ferruh Yigit  wrote:
>
> On 5/19/2023 8:41 AM, Rushil Gupta wrote:
> > Hi Ferruh
> > I have updated the latest patch here:
> > http://patchwork.dpdk.org/project/dpdk/patch/20230519072600.1444309-1-rush...@google.com/
> > However, using calloc causes issue while executing verify-compatibility 
> > command.
> >
> > sudo dpdk-testpmd -a 00:04.0 -l 0-32 -- --forward-mode=rxonly --txq=16
> > --rxq=16 --nb-cores=16 --stats-period 5
> > ...
> > EAL: Using IOMMU type 8 (No-IOMMU)
> > EAL: Probe PCI driver: net_gve (1ae0:42) device: :00:04.0 (socket -1)
> > gve_adminq_parse_err(): AQ command failed with status -9
> > gve_init_priv(): Could not verify driver compatibility: err=-22
> > EAL: Releasing PCI mapped resource for :00:04.0
> > ...
> > EAL: Error - exiting with code: 1
> >
> >
> > This is probably because the adminq command is sharing memory to
> > report driver-info to the gvnic device and that needs to be in dpdk
> > memory.
> >
> >
>
> Hi Rushil,
>
> That is OK, I missed this requirement.
>
> Admin command passes pointers for device to process, what is the address
> type requirement here?
> Some other commands pass pysical address (iova address) via the command.
>
> Both 'calloc()' and 'rte_zmalloc()' returns (guest) virtual address, why
> one doesn't work but other does?
>
> Initial version was passing 'rte_memzone' pointer, are you updating the
> hyperviser side based on changes on dpdk side?
>
>
> And perhaps better to switch to 'rte_memzone_reserve_aligned()' as done
> initial version and pass (guest) pysical address via adminq, if that is
> the requirement?
>
>
> >
> > On Fri, May 19, 2023 at 12:26 AM Rushil Gupta  wrote:
> >>
> >> Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
> >> version as OS version, reserving driver_version fields for GVE driver
> >> version based on features.
> >>
> >> Signed-off-by: Rushil Gupta 
> >> Signed-off-by: Joshua Washington 
> >> Signed-off-by: Junfeng Guo 
> >> Signed-off-by: Jeroen de Borst 
> >> ---
> >>  drivers/net/gve/base/gve.h|  3 --
> >>  drivers/net/gve/base/gve_adminq.c | 19 ++
> >>  drivers/net/gve/base/gve_adminq.h | 46 ++-
> >>  drivers/net/gve/base/gve_osdep.h  | 24 
> >>  drivers/net/gve/gve_ethdev.c  | 61 +--
> >>  drivers/net/gve/gve_ethdev.h  |  2 +-
> >>  drivers/net/gve/gve_version.c | 13 +++
> >>  drivers/net/gve/gve_version.h | 24 
> >>  drivers/net/gve/meson.build   |  5 ++-
> >>  9 files changed, 179 insertions(+), 18 deletions(-)
> >>  create mode 100644 drivers/net/gve/gve_version.c
> >>  create mode 100644 drivers/net/gve/gve_version.h
> >>
> >> diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
> >> index 2b7cf7d99b..f7b297e759 100644
> >> --- a/drivers/net/gve/base/gve.h
> >> +++ b/drivers/net/gve/base/gve.h
> >> @@ -9,9 +9,6 @@
> >>  #include "gve_desc.h"
> >>  #include "gve_desc_dqo.h"
> >>
> >> -#define GVE_VERSION"1.3.0"
> >> -#define GVE_VERSION_PREFIX "GVE-"
> >> -
> >>  #ifndef GOOGLE_VENDOR_ID
> >>  #define GOOGLE_VENDOR_ID   0x1ae0
> >>  #endif
> >> diff --git a/drivers/net/gve/base/gve_adminq.c 
> >> b/drivers/net/gve/base/gve_adminq.c
> >> index e963f910a0..41202725e6 100644
> >> --- a/drivers/net/gve/base/gve_adminq.c
> >> +++ b/drivers/net/gve/base/gve_adminq.c
> >> @@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
> >> case GVE_ADMINQ_GET_PTYPE_MAP:
> >> priv->adminq_get_ptype_map_cnt++;
> >> break;
> >> +   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
> >> +   priv->adminq_verify_driver_compatibility_cnt++;
> >> +   break;
> >> default:
> >> PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
> >> }
> >> @@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct 
> >> gve_priv *priv,
>

[v4] net/gve: check driver compatibility

2023-05-19 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 +
 drivers/net/gve/base/gve_adminq.h | 46 -
 drivers/net/gve/base/gve_osdep.h  | 24 +++
 drivers/net/gve/gve_ethdev.c  | 68 ++-
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 13 ++
 drivers/net/gve/gve_version.h | 24 +++
 drivers/net/gve/meson.build   |  5 ++-
 9 files changed, 186 insertions(+), 18 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2b7cf7d99b..f7b297e759 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -9,9 +9,6 @@
 #include "gve_desc.h"
 #include "gve_desc_dqo.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e963f910a0..41202725e6 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..e30b184913 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,44 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+   __be64 driver_info_addr;
+};
+
+GVE_CHECK_STRUCT_LEN(16,  gve_adminq_verify_driver_compatibility);
+
 
 struct gve_adminq_configure_device_resources {
__be64 counter_array;
@@ -345,6 +384,8 @@ union gve_adm

[v4] net/gve: check driver compatibility

2023-05-19 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 +
 drivers/net/gve/base/gve_adminq.h | 46 -
 drivers/net/gve/base/gve_osdep.h  | 24 +++
 drivers/net/gve/gve_ethdev.c  | 67 ++-
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 13 ++
 drivers/net/gve/gve_version.h | 24 +++
 drivers/net/gve/meson.build   |  5 ++-
 9 files changed, 185 insertions(+), 18 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2b7cf7d99b..f7b297e759 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -9,9 +9,6 @@
 #include "gve_desc.h"
 #include "gve_desc_dqo.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e963f910a0..41202725e6 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..e30b184913 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,44 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+   __be64 driver_info_addr;
+};
+
+GVE_CHECK_STRUCT_LEN(16,  gve_adminq_verify_driver_compatibility);
+
 
 struct gve_adminq_configure_device_resources {
__be64 counter_array;
@@ -345,6 +384,8 @@ union gve_adm

Re: [v4] net/gve: check driver compatibility

2023-05-20 Thread Rushil Gupta
We are not validating anything.
This is for our internal analysis and product requirements.

On Fri, May 19, 2023 at 1:56 PM Stephen Hemminger <
step...@networkplumber.org> wrote:

> On Fri, 19 May 2023 13:46:18 -0700
> Rushil Gupta  wrote:
>
> > +#include 
> >
> >  #include "../gve_logs.h"
> >
> > +#ifdef __linux__
> > +#include 
> > +#endif
> > +
> >  typedef uint8_t u8;
> >  typedef uint16_t u16;
> >  typedef uint32_t u32;
> > @@ -73,6 +78,12 @@ typedef rte_iova_t dma_addr_t;
> >
> >  #define msleep(ms)   rte_delay_ms(ms)
> >
> > +#define OS_VERSION_STRLEN 128
> > +struct os_version_string {
> > + char os_version_str1[OS_VERSION_STRLEN];
> > + char os_version_str2[OS_VERSION_STRLEN];
> > +};
> > +
>
> Not sure this a good idea. Are you having the host validate
> against DPDK versions. This is a bad idea.
>
> Better to use feature bits like virtio and not be creating
> and validating strings about versions.  For example, ever minor
> stable release changes this.
>


Re: [v4] net/gve: check driver compatibility

2023-05-22 Thread Rushil Gupta
1. This is the excerpt from the google's virtual nic spec:
"In addition to the device-owned register file, vector table, and
doorbells, the gVNIC device uses *DMA* (which in most cases amounts to
ordinary memory access by host software since we're dealing with a virtual
device, but guests must assume the device could be backed by actual
hardware) to access physical memory. The following are all located in
physical memory: Admin queue - 4096-byte command queue used for configuring
gVNIC.
Some commands require an additional dma memory region to be passed to the
device. These memory regions are allocated to execute the command and freed
when the command completes."
The calloc by default doesn't allow memory to be shared between the dpdk
process and hypervisor (where virtual device lives); so that's the reason
it doesn't work.

2. I also have a query: RHEL8 compilation in ci/Intel-compilation context
fails due to; is this because of if `not is_linux`

meson.build:67:0: ERROR: Include dir lib/eal/linux/include does not exist.

Passes: 
http://patchwork.dpdk.org/project/dpdk/patch/20230508191552.104540-1-rush...@google.com/

Fails: 
http://patchwork.dpdk.org/project/dpdk/patch/20230519204618.1507956-1-rush...@google.com/


On Mon, May 22, 2023 at 1:52 AM Ferruh Yigit  wrote:

> On 5/19/2023 9:46 PM, Rushil Gupta wrote:
> > +static int
> > +gve_verify_driver_compatibility(struct gve_priv *priv)
> > +{
> > + const struct rte_memzone *driver_info_mem;
> > + struct gve_driver_info *driver_info;
> > + int err;
> > +
> > + driver_info_mem =
> rte_memzone_reserve_aligned("verify_driver_compatibility",
> > + sizeof(struct gve_driver_info),
> > + rte_socket_id(),
> > + RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
> > +
> > + if (driver_info_mem == NULL) {
> > + PMD_DRV_LOG(ERR,
> > + "Could not alloc memzone for driver
> compatibility");
> > + return -ENOMEM;
> > + }
> > + driver_info = (struct gve_driver_info *)driver_info_mem->addr;
> > +
> > + *driver_info = (struct gve_driver_info) {
> > + .os_type = 5, /* DPDK */
> > + .driver_major = GVE_VERSION_MAJOR,
> > + .driver_minor = GVE_VERSION_MINOR,
> > + .driver_sub = GVE_VERSION_SUB,
> > + .os_version_major = cpu_to_be32(DPDK_VERSION_MAJOR),
> > + .os_version_minor = cpu_to_be32(DPDK_VERSION_MINOR),
> > + .os_version_sub = cpu_to_be32(DPDK_VERSION_SUB),
> > + .driver_capability_flags = {
> > + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS1),
> > + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS2),
> > + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS3),
> > + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS4),
> > + },
> > + };
> > +
> > + populate_driver_version_strings((char
> *)driver_info->os_version_str1,
> > + (char *)driver_info->os_version_str2);
> > +
> > + err = gve_adminq_verify_driver_compatibility(priv,
> > + sizeof(struct gve_driver_info), (dma_addr_t)driver_info);
>
> Back to previous discussion, other commands pass physical address to the
> admin command, but this pass virtual address.
> To follow the same semantic, shouldn't above be 'driver_info_mem.iova'?
>
> I asked before but not able to get an answer, what is the memory type
> requirement for device?
> Why virtual address obtained via 'calloc()' is not working, but virtual
> address from hugepages are working?
>


[v5] net/gve: check driver compatibility

2023-05-24 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 +
 drivers/net/gve/base/gve_adminq.h | 46 -
 drivers/net/gve/base/gve_osdep.h  | 24 +++
 drivers/net/gve/gve_ethdev.c  | 67 ++-
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 13 ++
 drivers/net/gve/gve_version.h | 24 +++
 drivers/net/gve/meson.build   |  5 ++-
 9 files changed, 185 insertions(+), 18 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2b7cf7d99b..f7b297e759 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -9,9 +9,6 @@
 #include "gve_desc.h"
 #include "gve_desc_dqo.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e963f910a0..41202725e6 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..e30b184913 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,44 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+   __be64 driver_info_addr;
+};
+
+GVE_CHECK_STRUCT_LEN(16,  gve_adminq_verify_driver_compatibility);
+
 
 struct gve_adminq_configure_device_resources {
__be64 counter_array;
@@ -345,6 +384,8 @@ union gve_adm

Re: [v4] net/gve: check driver compatibility

2023-05-24 Thread Rushil Gupta
As noted from spec: "Some commands require an additional dma memory
region to be passed to the device"
We are passing virtual addresses to devices (please look at link-speed
and describe-device adminq commands):
describe device:
https://github.com/DPDK/dpdk/blob/main/drivers/net/gve/base/gve_adminq.c#L704
link-speed: 
https://github.com/DPDK/dpdk/blob/main/drivers/net/gve/base/gve_adminq.c#L869

These adminq commands rely on gve_dma_alloc which presents mz->addr
and not iova as memory region:
https://github.com/DPDK/dpdk/blob/main/drivers/net/gve/base/gve_osdep.h#L139

Here is the new patch:
http://patchwork.dpdk.org/project/dpdk/patch/20230524171324.2072742-1-rush...@google.com/



On Tue, May 23, 2023 at 3:22 AM Ferruh Yigit  wrote:
>
> On 5/19/2023 9:46 PM, Rushil Gupta wrote:
> > diff --git a/drivers/net/gve/base/gve_osdep.h 
> > b/drivers/net/gve/base/gve_osdep.h
> > index abf3d379ae..5e8ae1eac6 100644
> > --- a/drivers/net/gve/base/gve_osdep.h
> > +++ b/drivers/net/gve/base/gve_osdep.h
> > @@ -21,9 +21,14 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include "../gve_logs.h"
> >
> > +#ifdef __linux__
> > +#include 
> > +#endif
> > +
>
> Can you please use 'RTE_EXEC_ENV_LINUX' macro instead of '__linux__'?


Re: [v4] net/gve: check driver compatibility

2023-05-31 Thread Rushil Gupta
http://patchwork.dpdk.org/project/dpdk/patch/20230524171324.2072742-1-rush...@google.com/
Does this look good to you?

On Wed, May 24, 2023 at 10:14 AM Rushil Gupta  wrote:
>
> As noted from spec: "Some commands require an additional dma memory
> region to be passed to the device"
> We are passing virtual addresses to devices (please look at link-speed
> and describe-device adminq commands):
> describe device:
> https://github.com/DPDK/dpdk/blob/main/drivers/net/gve/base/gve_adminq.c#L704
> link-speed: 
> https://github.com/DPDK/dpdk/blob/main/drivers/net/gve/base/gve_adminq.c#L869
>
> These adminq commands rely on gve_dma_alloc which presents mz->addr
> and not iova as memory region:
> https://github.com/DPDK/dpdk/blob/main/drivers/net/gve/base/gve_osdep.h#L139
>
> Here is the new patch:
> http://patchwork.dpdk.org/project/dpdk/patch/20230524171324.2072742-1-rush...@google.com/
>
>
>
> On Tue, May 23, 2023 at 3:22 AM Ferruh Yigit  wrote:
> >
> > On 5/19/2023 9:46 PM, Rushil Gupta wrote:
> > > diff --git a/drivers/net/gve/base/gve_osdep.h 
> > > b/drivers/net/gve/base/gve_osdep.h
> > > index abf3d379ae..5e8ae1eac6 100644
> > > --- a/drivers/net/gve/base/gve_osdep.h
> > > +++ b/drivers/net/gve/base/gve_osdep.h
> > > @@ -21,9 +21,14 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >
> > >  #include "../gve_logs.h"
> > >
> > > +#ifdef __linux__
> > > +#include 
> > > +#endif
> > > +
> >
> > Can you please use 'RTE_EXEC_ENV_LINUX' macro instead of '__linux__'?


[PATCH] net/gve: fix bug in verify driver compatibility

2023-05-31 Thread Rushil Gupta
gVNIC requires physical address to be passed in the adminq command.
This was initially rightly pointed by ferruh.yigit@.
Fixed by passing 'driver_info_mem->iova'.

Signed-off-by: Rushil Gupta 
---
 drivers/net/gve/gve_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 2c1e73d07a..1c5ce930a4 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv)
(char *)driver_info->os_version_str2);
 
err = gve_adminq_verify_driver_compatibility(priv,
-   sizeof(struct gve_driver_info), (dma_addr_t)driver_info);
+   sizeof(struct gve_driver_info),
+(dma_addr_t)driver_info_mem->iova);
/* It's ok if the device doesn't support this */
if (err == -EOPNOTSUPP)
err = 0;
-- 
2.41.0.rc2.161.g9c6817b8e7-goog



[PATCH] net/gve: fix bug in verify driver compatibility

2023-05-31 Thread Rushil Gupta
gVNIC requires physical address to be passed in the adminq command.
This was initially rightly pointed by ferruh.yigit@.
Fixed by passing 'driver_info_mem->iova'.

Signed-off-by: Rushil Gupta 
---
 drivers/net/gve/gve_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 2c1e73d07a..aa75abe102 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv)
(char *)driver_info->os_version_str2);
 
err = gve_adminq_verify_driver_compatibility(priv,
-   sizeof(struct gve_driver_info), (dma_addr_t)driver_info);
+   sizeof(struct gve_driver_info),
+   (dma_addr_t)driver_info_mem->iova);
/* It's ok if the device doesn't support this */
if (err == -EOPNOTSUPP)
err = 0;
-- 
2.41.0.rc2.161.g9c6817b8e7-goog



Re: [PATCH] net/gve: fix bug in verify driver compatibility

2023-06-01 Thread Rushil Gupta
Thanks for the quick response!
Just for my own knowledge, what Junfeng described is the process to
fix the bug if a bug is present in the main dpdk repo?

On Thu, Jun 1, 2023 at 3:24 AM Ferruh Yigit  wrote:
>
> On 6/1/2023 9:26 AM, Ferruh Yigit wrote:
> > On 6/1/2023 5:49 AM, Rushil Gupta wrote:
> >> gVNIC requires physical address to be passed in the adminq command.
> >> This was initially rightly pointed by ferruh.yigit@.
> >> Fixed by passing 'driver_info_mem->iova'.
> >>
> >> Signed-off-by: Rushil Gupta 
> >> ---
> >>  drivers/net/gve/gve_ethdev.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
> >> index 2c1e73d07a..aa75abe102 100644
> >> --- a/drivers/net/gve/gve_ethdev.c
> >> +++ b/drivers/net/gve/gve_ethdev.c
> >> @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv)
> >>  (char *)driver_info->os_version_str2);
> >>
> >>  err = gve_adminq_verify_driver_compatibility(priv,
> >> -sizeof(struct gve_driver_info), (dma_addr_t)driver_info);
> >> +sizeof(struct gve_driver_info),
> >> +(dma_addr_t)driver_info_mem->iova);
> >>
> >
> > Yep, this was my point, let me squashed onto original patch in next-net.
> >
> >
>
> Squashed into relevant commit in next-net, thanks.
> Can you please verify latest code in next-net?


Re: [PATCH] net/gve: fix bug in verify driver compatibility

2023-06-02 Thread Rushil Gupta
Thanks for the clarification and for squashing the bug into the main commit.
I just tested the net-next branch and it seems to log the right
message on the device.

On Fri, Jun 2, 2023 at 7:49 AM Ferruh Yigit  wrote:
>
> On 6/1/2023 5:32 PM, Rushil Gupta wrote:
> > Thanks for the quick response!
> > Just for my own knowledge, what Junfeng described is the process to
> > fix the bug if a bug is present in the main dpdk repo?
> >
>
> Correct.
>
> As main dpdk repo doesn't rewrite git history, any bug there needs to be
> fixed with an incremental commit, and fix commit should have some
> metadata to describe what it is fixing (this is to help LTS maintainers
> and developers tracing down a change).
>
> But sub-trees (like next-net) allowed to rewrite git history, so allowed
>  to squash/rebase commits when it is more practical to do so as this case.
> But changing directly in the git repo has its problems, like tracing,
> justifying source of the change and missing consensus etc. So, even it
> is squashed, the patch should be publicly available in the mail list first.
>
> For developers it is best to send incremental patch as if it will be
> merged as incremental commit, but can request a squash if the commit
> still not pulled by main repo, or maintainer can prefer to squash the
> fix to original commit, to reduce the churn & process overhead.
>
> > On Thu, Jun 1, 2023 at 3:24 AM Ferruh Yigit  wrote:
> >>
> >> On 6/1/2023 9:26 AM, Ferruh Yigit wrote:
> >>> On 6/1/2023 5:49 AM, Rushil Gupta wrote:
> >>>> gVNIC requires physical address to be passed in the adminq command.
> >>>> This was initially rightly pointed by ferruh.yigit@.
> >>>> Fixed by passing 'driver_info_mem->iova'.
> >>>>
> >>>> Signed-off-by: Rushil Gupta 
> >>>> ---
> >>>>  drivers/net/gve/gve_ethdev.c | 3 ++-
> >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
> >>>> index 2c1e73d07a..aa75abe102 100644
> >>>> --- a/drivers/net/gve/gve_ethdev.c
> >>>> +++ b/drivers/net/gve/gve_ethdev.c
> >>>> @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv 
> >>>> *priv)
> >>>>  (char *)driver_info->os_version_str2);
> >>>>
> >>>>  err = gve_adminq_verify_driver_compatibility(priv,
> >>>> -sizeof(struct gve_driver_info), (dma_addr_t)driver_info);
> >>>> +sizeof(struct gve_driver_info),
> >>>> +(dma_addr_t)driver_info_mem->iova);
> >>>>
> >>>
> >>> Yep, this was my point, let me squashed onto original patch in next-net.
> >>>
> >>>
> >>
> >> Squashed into relevant commit in next-net, thanks.
> >> Can you please verify latest code in next-net?
>


[PATCH] net/gve: fix dqo bug for chained descriptors

2024-02-15 Thread Rushil Gupta
net/gve: fix completion path for chained segments

Dqo Tx path had a bug where driver was overwriting mbufs in sw-ring.
We fixed this bug by cleaning slots for all segments.

Fixes: 4022f9 ("net/gve: support basic Tx data path for DQO")
Cc: sta...@dpdk.org

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/gve_tx_dqo.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index 16101de..d060664 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -13,7 +13,7 @@ gve_tx_clean_dqo(struct gve_tx_queue *txq)
struct gve_tx_compl_desc *compl_desc;
struct gve_tx_queue *aim_txq;
uint16_t nb_desc_clean;
-   struct rte_mbuf *txe;
+   struct rte_mbuf *txe, *txe_next;
uint16_t compl_tag;
uint16_t next;
 
@@ -43,10 +43,15 @@ gve_tx_clean_dqo(struct gve_tx_queue *txq)
PMD_DRV_LOG(DEBUG, "GVE_COMPL_TYPE_DQO_REINJECTION !!!");
/* FALLTHROUGH */
case GVE_COMPL_TYPE_DQO_PKT:
+   /* free all segments. */
txe = aim_txq->sw_ring[compl_tag];
-   if (txe != NULL) {
+   while (txe != NULL) {
+   txe_next = txe->next;
rte_pktmbuf_free_seg(txe);
-   txe = NULL;
+   if (aim_txq->sw_ring[compl_tag] == txe)
+   aim_txq->sw_ring[compl_tag] = NULL;
+   txe = txe_next;
+   compl_tag = (compl_tag + 1) & (aim_txq->sw_size - 1);
}
break;
case GVE_COMPL_TYPE_DQO_MISS:
@@ -83,6 +88,7 @@ gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf **tx_pkts, 
uint16_t nb_pkts)
uint16_t tx_id;
uint16_t sw_id;
uint64_t bytes;
+   uint16_t first_sw_id;
 
sw_ring = txq->sw_ring;
txr = txq->tx_ring;
@@ -107,23 +113,26 @@ gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf 
**tx_pkts, uint16_t nb_pkts)
 
ol_flags = tx_pkt->ol_flags;
nb_used = tx_pkt->nb_segs;
-
+   first_sw_id = sw_id;
do {
-   txd = &txr[tx_id];
+   if (sw_ring[sw_id] != NULL) {
+   PMD_DRV_LOG(ERR, "Overwriting an entry in 
sw_ring");
+   }
 
+   txd = &txr[tx_id];
sw_ring[sw_id] = tx_pkt;
 
/* fill Tx descriptor */
txd->pkt.buf_addr = 
rte_cpu_to_le_64(rte_mbuf_data_iova(tx_pkt));
txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
-   txd->pkt.compl_tag = rte_cpu_to_le_16(sw_id);
+   txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, 
GVE_TX_MAX_BUF_SIZE_DQO);
 
/* size of desc_ring and sw_ring could be different */
tx_id = (tx_id + 1) & mask;
sw_id = (sw_id + 1) & sw_mask;
 
-   bytes += tx_pkt->pkt_len;
+   bytes += tx_pkt->data_len;
tx_pkt = tx_pkt->next;
} while (tx_pkt);
 
-- 
2.44.0.rc0.258.g7320e95886-goog



[PATCH] net/gve: fix dqo bug for chained descriptors

2024-02-15 Thread Rushil Gupta
Dqo Tx path had a bug where driver was overwriting mbufs in sw-ring.
We fixed this bug by cleaning slots for all segments.

Fixes: 4022f9 ("net/gve: support basic Tx data path for DQO")
Cc: sta...@dpdk.org

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/gve_tx_dqo.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index 16101de84f..1a8eb96ea9 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -13,7 +13,7 @@ gve_tx_clean_dqo(struct gve_tx_queue *txq)
struct gve_tx_compl_desc *compl_desc;
struct gve_tx_queue *aim_txq;
uint16_t nb_desc_clean;
-   struct rte_mbuf *txe;
+   struct rte_mbuf *txe, *txe_next;
uint16_t compl_tag;
uint16_t next;
 
@@ -43,10 +43,15 @@ gve_tx_clean_dqo(struct gve_tx_queue *txq)
PMD_DRV_LOG(DEBUG, "GVE_COMPL_TYPE_DQO_REINJECTION !!!");
/* FALLTHROUGH */
case GVE_COMPL_TYPE_DQO_PKT:
+   /* free all segments. */
txe = aim_txq->sw_ring[compl_tag];
-   if (txe != NULL) {
+   while (txe != NULL) {
+   txe_next = txe->next;
rte_pktmbuf_free_seg(txe);
-   txe = NULL;
+   if (aim_txq->sw_ring[compl_tag] == txe)
+   aim_txq->sw_ring[compl_tag] = NULL;
+   txe = txe_next;
+   compl_tag = (compl_tag + 1) & (aim_txq->sw_size - 1);
}
break;
case GVE_COMPL_TYPE_DQO_MISS:
@@ -83,6 +88,7 @@ gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf **tx_pkts, 
uint16_t nb_pkts)
uint16_t tx_id;
uint16_t sw_id;
uint64_t bytes;
+   uint16_t first_sw_id;
 
sw_ring = txq->sw_ring;
txr = txq->tx_ring;
@@ -107,23 +113,25 @@ gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf 
**tx_pkts, uint16_t nb_pkts)
 
ol_flags = tx_pkt->ol_flags;
nb_used = tx_pkt->nb_segs;
-
+   first_sw_id = sw_id;
do {
-   txd = &txr[tx_id];
+   if (sw_ring[sw_id] != NULL)
+   PMD_DRV_LOG(ERR, "Overwriting an entry in 
sw_ring");
 
+   txd = &txr[tx_id];
sw_ring[sw_id] = tx_pkt;
 
/* fill Tx descriptor */
txd->pkt.buf_addr = 
rte_cpu_to_le_64(rte_mbuf_data_iova(tx_pkt));
txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
-   txd->pkt.compl_tag = rte_cpu_to_le_16(sw_id);
+   txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, 
GVE_TX_MAX_BUF_SIZE_DQO);
 
/* size of desc_ring and sw_ring could be different */
tx_id = (tx_id + 1) & mask;
sw_id = (sw_id + 1) & sw_mask;
 
-   bytes += tx_pkt->pkt_len;
+   bytes += tx_pkt->data_len;
tx_pkt = tx_pkt->next;
} while (tx_pkt);
 
-- 
2.44.0.rc0.258.g7320e95886-goog



[PATCH] net/gve: Change ERR to DEBUG to prevent flooding of logs for Tx-Dqo.

2024-02-18 Thread Rushil Gupta
This was causing failure for testpmd runs (for queues >=15)
presumably due to flooding of logs due to descriptor ring being
overwritten.

Fixes: a01854 ("net/gve: fix dqo bug for chained descriptors")
Cc: sta...@dpdk.org

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/gve_tx_dqo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index 1a8eb96ea9..30a1455b20 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -116,7 +116,7 @@ gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf **tx_pkts, 
uint16_t nb_pkts)
first_sw_id = sw_id;
do {
if (sw_ring[sw_id] != NULL)
-   PMD_DRV_LOG(ERR, "Overwriting an entry in 
sw_ring");
+   PMD_DRV_LOG(DEBUG, "Overwriting an entry in 
sw_ring");
 
txd = &txr[tx_id];
sw_ring[sw_id] = tx_pkt;
-- 
2.44.0.rc0.258.g7320e95886-goog



Re: [PATCH] net/gve: Change ERR to DEBUG to prevent flooding of logs for Tx-Dqo.

2024-02-19 Thread Rushil Gupta
I agree.
This bug has manifested for a while before I fixed it partially in "[PATCH]
net/gve: fix dqo bug for chained descriptors"
However, for higher queue counts (> 13); we still see this behavior. I'll
add a statistic.

On Mon, Feb 19, 2024, 10:56 PM Stephen Hemminger 
wrote:

> On Mon, 19 Feb 2024 02:44:35 +
> Rushil Gupta  wrote:
>
> > This was causing failure for testpmd runs (for queues >=15)
> > presumably due to flooding of logs due to descriptor ring being
> > overwritten.
> >
> > Fixes: a01854 ("net/gve: fix dqo bug for chained descriptors")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Rushil Gupta 
> > Reviewed-by: Joshua Washington 
>
> Isn't this still an error. What about the descriptor overwritten is there
> an mbuf leak?
> Maybe a statistic would be better than a message?
>


Re: [PATCH] net/gve: Change ERR to DEBUG to prevent flooding of logs for Tx-Dqo.

2024-02-20 Thread Rushil Gupta
These are very useful insights Ferruh. I think RTE_LOG_DP() is something
that would have been more suitable here.
Also, the DEBUG statement combined with a statistic will be more useful
than ERR from developer perspective if they see a potential memory leak in
their program.

On Mon, Feb 19, 2024, 3:03 PM Ferruh Yigit  wrote:

> On 2/19/2024 2:44 AM, Rushil Gupta wrote:
> > This was causing failure for testpmd runs (for queues >=15)
> > presumably due to flooding of logs due to descriptor ring being
> > overwritten.
> >
> > Fixes: a01854 ("net/gve: fix dqo bug for chained descriptors")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Rushil Gupta 
> > Reviewed-by: Joshua Washington 
> > ---
> >  drivers/net/gve/gve_tx_dqo.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
> > index 1a8eb96ea9..30a1455b20 100644
> > --- a/drivers/net/gve/gve_tx_dqo.c
> > +++ b/drivers/net/gve/gve_tx_dqo.c
> > @@ -116,7 +116,7 @@ gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf
> **tx_pkts, uint16_t nb_pkts)
> >   first_sw_id = sw_id;
> >   do {
> >   if (sw_ring[sw_id] != NULL)
> > - PMD_DRV_LOG(ERR, "Overwriting an entry in
> sw_ring");
> > + PMD_DRV_LOG(DEBUG, "Overwriting an entry
> in sw_ring");
> >
> >   txd = &txr[tx_id];
> >   sw_ring[sw_id] = tx_pkt;
>
> Hi Rushil,
>
> I will continue with this patch, BUT
> logging in the datapath has potential problems like this, also it may
> have performance impact even log is not printed, because of additional
> check in the log function.
>
> For datapath, you can prefer:
> - Add log macros controlled by RTE_ETHDEV_DEBUG_RX & RTE_ETHDEV_DEBUG_TX
> flags
> - Or use RTE_LOG_DP() macro which is compiled out if default log level
> is less than the log uses
>
>
> Also another perspective for the logs, when end-user observes this bloat
> of logs, what she can do?
> Can she change some driver arguments or environment variables to fix the
> issue, if not what is the point of the log?
> If this is a condition that can occur dynamically based on received
> traffic and user doesn't have control on it, maybe driver should handle
> the error without log?
> And if this is a log for driver developer, perhaps it should be assert
> or similar that is disabled in the release build?
>


[PATCH] dpdk: Enable GQ stats reporting

2023-12-18 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ.

Change-Id: I8123d70ec5019b12f368e67b858c382bb7812e5c
---
 dpdk/base/gve_adminq.h | 11 ++
 dpdk/gve_ethdev.c  | 84 ++
 dpdk/gve_ethdev.h  |  6 +++
 3 files changed, 101 insertions(+)

diff --git a/drivers/net/gve/base/gve_adminq.h b/dpdk/base/gve_adminq.h
index e30b184..f05362f 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 enum gve_stat_names {
/* stats from gve */
TX_WAKE_CNT = 1,
diff --git a/drivers/net/gve/gve_ethdev.c b/dpdk/gve_ethdev.c
index 77f6634..bfc704f 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 @@ err_tx:
 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_pri

[PATCH] net/gve: Enable stats reporting for GQ format

2023-12-18 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ.
Tested using `show port xstats ` in interactive mode.
This metric can be triggered by using queues > cores.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 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-&g

[PATCH] net/gve: Enable stats reporting for GQ format

2023-12-18 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ.
Tested using `show port xstats ` in interactive mode.
This metric can be triggered by using queues > cores.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 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-&g

[PATCH] net/gve: Enable stats reporting for GQ format

2023-12-22 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ.
Tested using `show port xstats ` in interactive mode.
This metric can be triggered by using queues > cores.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 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

[PATCH] net/gve: Enable stats reporting for GQ format

2023-12-22 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ from device.
Tested using `show port xstats ` 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 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

[PATCH] net/gve: Enable stats reporting for GQ format

2023-12-22 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ from device.
Tested using `show port xstats ` in interactive mode.
This metric can be triggered by using queues > cores.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 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

[PATCH] net/gve: Enable stats reporting for GQ format

2023-12-22 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ from device.
Tested using `show port xstats ` in interactive mode.
This metric can be triggered by using queues > cores.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 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

Re: [PATCH] net/gve: Enable stats reporting for GQ format

2024-01-15 Thread Rushil Gupta
On Fri, Jan 12, 2024 at 8:36 PM Ferruh Yigit  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 ` 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.


[v2] net/gve: enable imissed stats for GQ format

2024-01-15 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ from device.
Tested using `show port xstats ` in interactive mode.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/base/gve_adminq.h |  11 
 drivers/net/gve/gve_ethdev.c  | 100 ++
 drivers/net/gve/gve_ethdev.h  |   6 ++
 3 files changed, 117 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 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..fe91e790bc 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -125,6 +125,78 @@ 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->gve_stats_report_mem = rte_memzone_reserve_aligned(z_name,
+   priv->stats_report_len,
+   rte_socket_id(),
+   RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
+
+   if (!priv->gve_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->gve_stats_report_mem);
+   priv->gve_stats_report_mem = NULL;
+}
+
+/* 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;
+   if (!priv->gve_stats_report_mem)
+   return;
+   stats_report = (struct gve_stats_report *)
+   priv->gve_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 +248,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 +260,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,
+

Re: [PATCH] net/gve: Enable stats reporting for GQ format

2024-01-19 Thread Rushil Gupta
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  wrote:

> On 1/16/2024 6:18 AM, Rushil Gupta wrote:
> >
> >
> > On Fri, Jan 12, 2024 at 8:36 PM Ferruh Yigit  > <mailto:ferruh.yi...@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 ` 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.
>
>


[v3] net/gve: enable imissed stats for GQ format

2024-01-19 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ from device.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/base/gve_adminq.h |  11 
 drivers/net/gve/gve_ethdev.c  | 103 ++
 drivers/net/gve/gve_ethdev.h  |   6 ++
 3 files changed, 120 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 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..723b9332b8 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -125,6 +125,78 @@ 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->gve_stats_report_mem = rte_memzone_reserve_aligned(z_name,
+   priv->stats_report_len,
+   rte_socket_id(),
+   RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
+
+   if (!priv->gve_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->gve_stats_report_mem);
+   priv->gve_stats_report_mem = NULL;
+}
+
+/* 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;
+   if (!priv->gve_stats_report_mem)
+   return;
+   stats_report = (struct gve_stats_report *)
+   priv->gve_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 +248,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 +260,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 st

Re: [v3] net/gve: enable imissed stats for GQ format

2024-01-19 Thread Rushil Gupta
I misinterpreted your comment earlier and prefixed memzone variable name
with "gve" instead of memzone name. Will fix it in v4. Thanks!

On Fri, Jan 19, 2024 at 8:55 PM Ferruh Yigit  wrote:

> On 1/19/2024 2:26 PM, Rushil Gupta wrote:
> > Read from shared region to retrieve imissed statistics for GQ from
> device.
> >
> > Signed-off-by: Rushil Gupta 
> > Reviewed-by: Joshua Washington 
> >
>
> <...>
>
> > +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->gve_stats_report_mem = rte_memzone_reserve_aligned(z_name,
> > + priv->stats_report_len,
> > + rte_socket_id(),
> > + RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
> >
>
> Adding 'gve_' prefix to memzone name comment seems missed.
>
>


[v4] net/gve: enable imissed stats for GQ format

2024-01-19 Thread Rushil Gupta
Read from shared region to retrieve imissed statistics for GQ from device.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/base/gve_adminq.h |  11 
 drivers/net/gve/gve_ethdev.c  | 104 ++
 drivers/net/gve/gve_ethdev.h  |   6 ++
 3 files changed, 121 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_NUM6
+#define GVE_RX_STATS_REPORT_NUM2
+
+/* Interval to schedule a stats report update, 2ms. */
+#define GVE_STATS_REPORT_TIMER_PERIOD  2
+
+/* Numbers of NIC tx/rx stats in stats report. */
+#define NIC_TX_STATS_REPORT_NUM0
+#define NIC_RX_STATS_REPORT_NUM4
+
 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..d162fd3864 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -125,6 +125,79 @@ 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), "gve_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);
+   priv->stats_report_mem = NULL;
+}
+
+/* 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;
+   if (!priv->stats_report_mem)
+   return;
+   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 +249,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 +261,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 st

Re: gve: mixes DPDK and Linux versions in compatibility check

2024-01-19 Thread Rushil Gupta
Hi Stephen
We wish to capture both rte version and linux kernel version.

The current implementation is needed to answer questions like how many
customers are on Dpdk 23.11 for Ubuntu 22 vs Debian 11.
Therefore, we need the uts library and the adminq is working as intended.

On Fri, Jan 12, 2024, 1:38 AM Stephen Hemminger 
wrote:

> This seems wrong:
> *driver_info = (struct gve_driver_info) {
> .os_type = 5, /* DPDK */
> .driver_major = GVE_VERSION_MAJOR,
> .driver_minor = GVE_VERSION_MINOR,
> .driver_sub = GVE_VERSION_SUB,
> .os_version_major = cpu_to_be32(DPDK_VERSION_MAJOR),
> .os_version_minor = cpu_to_be32(DPDK_VERSION_MINOR),
> .os_version_sub = cpu_to_be32(DPDK_VERSION_SUB),
> .driver_capability_flags = {
> cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS1),
> cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS2),
> cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS3),
> cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS4),
> },
> };
>
> populate_driver_version_strings((char
> *)driver_info->os_version_str1,
> (char *)driver_info->os_version_str2);
>
> The numeric values os_version_major, os_version_minor use DPDK version
> which
> is good.  But the populate_driver_version_strings gets the Linux kernel
> version number which is problematic.  Looks like a bug to me.
>
> Also technically, the Linux kernel version is not the same as the OS
> release.
>
> Shouldn't it be more like:
>
> diff --git a/drivers/net/gve/base/gve_osdep.h
> b/drivers/net/gve/base/gve_osdep.h
> index a3702f4b8c8d..914746c8d226 100644
> --- a/drivers/net/gve/base/gve_osdep.h
> +++ b/drivers/net/gve/base/gve_osdep.h
> @@ -171,17 +171,4 @@ gve_free_dma_mem(struct gve_dma_mem *mem)
> mem->pa = 0;
>  }
>
> -static inline void
> -populate_driver_version_strings(char *str1, char *str2)
> -{
> -   struct utsname uts;
> -   if (uname(&uts) >= 0) {
> -   /* release */
> -   rte_strscpy(str1, uts.release,
> -   OS_VERSION_STRLEN);
> -   /* version */
> -   rte_strscpy(str2, uts.version,
> -   OS_VERSION_STRLEN);
> -   }
> -}
>  #endif /* _GVE_OSDEP_H_ */
> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
> index ecd37ff37f55..b8c48fc657b9 100644
> --- a/drivers/net/gve/gve_ethdev.c
> +++ b/drivers/net/gve/gve_ethdev.c
> @@ -273,8 +273,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv)
> },
> };
>
> -   populate_driver_version_strings((char
> *)driver_info->os_version_str1,
> -   (char *)driver_info->os_version_str2);
> +   rte_strscpy(driver_info.os_version_str1, OS_VERSION_STRLEN,
> +   rte_version());
>
> err = gve_adminq_verify_driver_compatibility(priv,
> sizeof(struct gve_driver_info),
>


[PATCH] net/gve: add support for 4K ring size only for DQO

2023-11-20 Thread Rushil Gupta
Bump up the dpdk dqo driver ring size to 4096.
The queue size is controlled by queue_setup method.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/base/gve_adminq.c | 8 
 drivers/net/gve/gve_ethdev.c  | 4 ++--
 drivers/net/gve/gve_ethdev.h  | 1 +
 drivers/net/gve/gve_rx_dqo.c  | 6 --
 drivers/net/gve/gve_tx_dqo.c  | 6 --
 5 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index 41202725e6..343bd13d67 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -516,11 +516,11 @@ static int gve_adminq_create_tx_queue(struct gve_priv 
*priv, u32 queue_index)
cmd.create_tx_queue.queue_page_list_id = cpu_to_be32(qpl_id);
} else {
cmd.create_tx_queue.tx_ring_size =
-   cpu_to_be16(priv->tx_desc_cnt);
+   cpu_to_be16(txq->nb_tx_desc);
cmd.create_tx_queue.tx_comp_ring_addr =
cpu_to_be64(txq->compl_ring_phys_addr);
cmd.create_tx_queue.tx_comp_ring_size =
-   cpu_to_be16(priv->tx_compq_size * DQO_TX_MULTIPLIER);
+   cpu_to_be16(txq->sw_size);
}
 
return gve_adminq_issue_cmd(priv, &cmd);
@@ -566,7 +566,7 @@ static int gve_adminq_create_rx_queue(struct gve_priv 
*priv, u32 queue_index)
cmd.create_rx_queue.packet_buffer_size = 
cpu_to_be16(rxq->rx_buf_len);
} else {
cmd.create_rx_queue.rx_ring_size =
-   cpu_to_be16(priv->rx_desc_cnt);
+   cpu_to_be16(rxq->nb_rx_desc);
cmd.create_rx_queue.rx_desc_ring_addr =
cpu_to_be64(rxq->compl_ring_phys_addr);
cmd.create_rx_queue.rx_data_ring_addr =
@@ -574,7 +574,7 @@ static int gve_adminq_create_rx_queue(struct gve_priv 
*priv, u32 queue_index)
cmd.create_rx_queue.packet_buffer_size =
cpu_to_be16(rxq->rx_buf_len);
cmd.create_rx_queue.rx_buff_ring_size =
-   cpu_to_be16(priv->rx_bufq_size);
+   cpu_to_be16(rxq->nb_rx_desc);
cmd.create_rx_queue.enable_rsc = !!(priv->enable_rsc);
}
 
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 001cae2b98..ecd37ff37f 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -333,14 +333,14 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
 
dev_info->default_rxportconf.ring_size = priv->rx_desc_cnt;
dev_info->rx_desc_lim = (struct rte_eth_desc_lim) {
-   .nb_max = priv->rx_desc_cnt,
+   .nb_max = gve_is_gqi(priv) ? priv->rx_desc_cnt : 
GVE_MAX_QUEUE_SIZE_DQO,
.nb_min = priv->rx_desc_cnt,
.nb_align = 1,
};
 
dev_info->default_txportconf.ring_size = priv->tx_desc_cnt;
dev_info->tx_desc_lim = (struct rte_eth_desc_lim) {
-   .nb_max = priv->tx_desc_cnt,
+   .nb_max = gve_is_gqi(priv) ? priv->tx_desc_cnt : 
GVE_MAX_QUEUE_SIZE_DQO,
.nb_min = priv->tx_desc_cnt,
.nb_align = 1,
};
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 37f2b60845..58d8943e71 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -23,6 +23,7 @@
 #define GVE_RX_BUF_ALIGN_DQO128
 #define GVE_RX_MIN_BUF_SIZE_DQO1024
 #define GVE_RX_MAX_BUF_SIZE_DQO((16 * 1024) - GVE_RX_BUF_ALIGN_DQO)
+#define GVE_MAX_QUEUE_SIZE_DQO 4096
 
 #define GVE_RX_BUF_ALIGN_GQI   2048
 #define GVE_RX_MIN_BUF_SIZE_GQI2048
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 422784e7e0..7c7a8c48d0 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -223,12 +223,6 @@ gve_rx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t 
queue_id,
uint32_t mbuf_len;
int err = 0;
 
-   if (nb_desc != hw->rx_desc_cnt) {
-   PMD_DRV_LOG(WARNING, "gve doesn't support nb_desc config, use 
hw nb_desc %u.",
-   hw->rx_desc_cnt);
-   }
-   nb_desc = hw->rx_desc_cnt;
-
/* Free memory if needed */
if (dev->data->rx_queues[queue_id]) {
gve_rx_queue_release_dqo(dev, queue_id);
diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index e0d144835b..16101de84f 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -268,12 +268,6 @@ gve_tx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t 
queue_id,
uint16_t sw_size;
int err = 0;
 
-   if (nb_desc != hw

[RFC] net/gve: add IPv4 checksum offloading capability

2024-03-05 Thread Rushil Gupta
Gvnic's DQO format allows offloading IPv4 checksum.
Made changes to Tx and Rx path to translate DPDK flags
to descriptor for offloading (and vice-versa).
Added ptype adminq support to only add this flags for
supported L3/L4 packet-types.
---
 drivers/net/gve/gve_ethdev.c | 29 +--
 drivers/net/gve/gve_ethdev.h |  5 +
 drivers/net/gve/gve_rx_dqo.c | 38 ++--
 drivers/net/gve/gve_tx_dqo.c |  2 +-
 4 files changed, 69 insertions(+), 5 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 3b8ec5872d..ef0116218e 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -434,8 +434,14 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
RTE_ETH_TX_OFFLOAD_SCTP_CKSUM   |
RTE_ETH_TX_OFFLOAD_TCP_TSO;
 
-   if (priv->queue_format == GVE_DQO_RDA_FORMAT)
-   dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_LRO;
+   if (!gve_is_gqi(priv)) {
+   dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+   dev_info->rx_offload_capa |=
+   RTE_ETH_RX_OFFLOAD_IPV4_CKSUM   |
+   RTE_ETH_RX_OFFLOAD_UDP_CKSUM|
+   RTE_ETH_RX_OFFLOAD_TCP_CKSUM|
+   RTE_ETH_RX_OFFLOAD_TCP_LRO;
+   }
 
dev_info->default_rxconf = (struct rte_eth_rxconf) {
.rx_free_thresh = GVE_DEFAULT_RX_FREE_THRESH,
@@ -938,6 +944,8 @@ gve_teardown_device_resources(struct gve_priv *priv)
if (err)
PMD_DRV_LOG(ERR, "Could not deconfigure device 
resources: err=%d", err);
}
+   rte_free(priv->ptype_lut_dqo);
+   priv->ptype_lut_dqo = NULL;
gve_free_counter_array(priv);
gve_free_irq_db(priv);
gve_clear_device_resources_ok(priv);
@@ -997,8 +1005,25 @@ gve_setup_device_resources(struct gve_priv *priv)
PMD_DRV_LOG(ERR, "Could not config device resources: err=%d", 
err);
goto free_irq_dbs;
}
+
+   priv->ptype_lut_dqo = rte_zmalloc("gve_ptype_lut_dqo",
+   sizeof(struct gve_ptype_lut), 0);
+   if (priv->ptype_lut_dqo == NULL) {
+   PMD_DRV_LOG(ERR, "Failed to alloc ptype lut.");
+   err = -ENOMEM;
+   goto free_irq_dbs;
+   }
+   err = gve_adminq_get_ptype_map_dqo(priv, priv->ptype_lut_dqo);
+   if (unlikely(err)) {
+   PMD_DRV_LOG(ERR, "Failed to get ptype map: err=%d", err);
+   goto free_ptype_lut;
+   }
+
return 0;
 
+free_ptype_lut:
+   rte_free(priv->ptype_lut_dqo);
+   priv->ptype_lut_dqo = NULL;
 free_irq_dbs:
gve_free_irq_db(priv);
 free_cnt_array:
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index d713657d10..9b19fc55e3 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -36,6 +36,10 @@
RTE_MBUF_F_TX_L4_MASK  |\
RTE_MBUF_F_TX_TCP_SEG)
 
+#define GVE_TX_CKSUM_OFFLOAD_MASK_DQO (\
+   GVE_TX_CKSUM_OFFLOAD_MASK | \
+   RTE_MBUF_F_TX_IP_CKSUM)
+
 #define GVE_RTE_RSS_OFFLOAD_ALL (  \
RTE_ETH_RSS_IPV4 |  \
RTE_ETH_RSS_NONFRAG_IPV4_TCP |  \
@@ -295,6 +299,7 @@ struct gve_priv {
uint16_t stats_end_idx; /* end index of array of stats written by NIC */
 
struct gve_rss_config rss_config;
+   struct gve_ptype_lut *ptype_lut_dqo;
 };
 
 static inline bool
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 7c7a8c48d0..1c37c54cb7 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -75,6 +75,40 @@ gve_rx_refill_dqo(struct gve_rx_queue *rxq)
rxq->bufq_tail = next_avail;
 }
 
+static inline uint16_t
+gve_parse_csum_ol_flags(volatile struct gve_rx_compl_desc_dqo *rx_desc,
+   struct gve_priv *priv) {
+   uint64_t ol_flags = 0;
+   struct gve_ptype ptype =
+   priv->ptype_lut_dqo->ptypes[rx_desc->packet_type];
+
+   if(!rx_desc->l3_l4_processed)
+   return ol_flags;
+
+   if (ptype.l3_type == GVE_L3_TYPE_IPV4) {
+   if (rx_desc->csum_ip_err)
+   ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
+   else
+   ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
+   }
+
+   if (rx_desc->csum_l4_err) {
+   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
+   return ol_flags;
+   }
+   switch (ptype.l4_type) {
+   case GVE_L4_TYPE_TCP:
+   case GVE_L4_TYPE_UDP:
+   case GVE_L4_TYPE_ICMP:
+   case GVE_L4_TYPE_SCTP:
+   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
+   break;
+   default:
+   break;
+   }
+   return ol_flags;
+}
+
 uint16_t
 g

[PATCH] drivers/net/gve: add IPv4 checksum offloading capability

2024-03-13 Thread Rushil Gupta
Gvnic's DQO format allows offloading IPv4 checksum.
Made changes to Tx and Rx path to translate DPDK flags
to descriptor for offloading (and vice-versa).
Add ptype adminq support to only add this flags for
supported L3/L4 packet-types.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/gve_ethdev.c | 34 +++---
 drivers/net/gve/gve_ethdev.h |  5 +
 drivers/net/gve/gve_rx_dqo.c | 38 --
 drivers/net/gve/gve_tx_dqo.c |  2 +-
 4 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 3b8ec58..475745b 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -434,8 +434,14 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
RTE_ETH_TX_OFFLOAD_SCTP_CKSUM   |
RTE_ETH_TX_OFFLOAD_TCP_TSO;
 
-   if (priv->queue_format == GVE_DQO_RDA_FORMAT)
-   dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_LRO;
+   if (!gve_is_gqi(priv)) {
+   dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+   dev_info->rx_offload_capa |=
+   RTE_ETH_RX_OFFLOAD_IPV4_CKSUM   |
+   RTE_ETH_RX_OFFLOAD_UDP_CKSUM|
+   RTE_ETH_RX_OFFLOAD_TCP_CKSUM|
+   RTE_ETH_RX_OFFLOAD_TCP_LRO;
+   }
 
dev_info->default_rxconf = (struct rte_eth_rxconf) {
.rx_free_thresh = GVE_DEFAULT_RX_FREE_THRESH,
@@ -938,6 +944,11 @@ gve_teardown_device_resources(struct gve_priv *priv)
if (err)
PMD_DRV_LOG(ERR, "Could not deconfigure device 
resources: err=%d", err);
}
+
+   if (!gve_is_gqi(priv)) {
+   rte_free(priv->ptype_lut_dqo);
+   priv->ptype_lut_dqo = NULL;
+   }
gve_free_counter_array(priv);
gve_free_irq_db(priv);
gve_clear_device_resources_ok(priv);
@@ -997,8 +1008,25 @@ gve_setup_device_resources(struct gve_priv *priv)
PMD_DRV_LOG(ERR, "Could not config device resources: err=%d", 
err);
goto free_irq_dbs;
}
-   return 0;
+   if (!gve_is_gqi(priv)) {
+   priv->ptype_lut_dqo = rte_zmalloc("gve_ptype_lut_dqo",
+   sizeof(struct gve_ptype_lut), 0);
+   if (priv->ptype_lut_dqo == NULL) {
+   PMD_DRV_LOG(ERR, "Failed to alloc ptype lut.");
+   err = -ENOMEM;
+   goto free_irq_dbs;
+   }
+   err = gve_adminq_get_ptype_map_dqo(priv, priv->ptype_lut_dqo);
+   if (unlikely(err)) {
+   PMD_DRV_LOG(ERR, "Failed to get ptype map: err=%d", 
err);
+   goto free_ptype_lut;
+   }
+   }
 
+   return 0;
+free_ptype_lut:
+   rte_free(priv->ptype_lut_dqo);
+   priv->ptype_lut_dqo = NULL;
 free_irq_dbs:
gve_free_irq_db(priv);
 free_cnt_array:
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index d713657..9b19fc5 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -36,6 +36,10 @@
RTE_MBUF_F_TX_L4_MASK  |\
RTE_MBUF_F_TX_TCP_SEG)
 
+#define GVE_TX_CKSUM_OFFLOAD_MASK_DQO (\
+   GVE_TX_CKSUM_OFFLOAD_MASK | \
+   RTE_MBUF_F_TX_IP_CKSUM)
+
 #define GVE_RTE_RSS_OFFLOAD_ALL (  \
RTE_ETH_RSS_IPV4 |  \
RTE_ETH_RSS_NONFRAG_IPV4_TCP |  \
@@ -295,6 +299,7 @@ struct gve_priv {
uint16_t stats_end_idx; /* end index of array of stats written by NIC */
 
struct gve_rss_config rss_config;
+   struct gve_ptype_lut *ptype_lut_dqo;
 };
 
 static inline bool
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 7c7a8c4..1c37c54 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -75,6 +75,40 @@ gve_rx_refill_dqo(struct gve_rx_queue *rxq)
rxq->bufq_tail = next_avail;
 }
 
+static inline uint16_t
+gve_parse_csum_ol_flags(volatile struct gve_rx_compl_desc_dqo *rx_desc,
+   struct gve_priv *priv) {
+   uint64_t ol_flags = 0;
+   struct gve_ptype ptype =
+   priv->ptype_lut_dqo->ptypes[rx_desc->packet_type];
+
+   if (!rx_desc->l3_l4_processed)
+   return ol_flags;
+
+   if (ptype.l3_type == GVE_L3_TYPE_IPV4) {
+   if (rx_desc->csum_ip_err)
+   ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
+   else
+   ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
+   }
+
+   if (rx_desc->csum_l4_err) {
+   ol_flags |= RTE_MBUF_F_RX_L4_CKS

[PATCH] drivers/net/gve: add IPv4 checksum offloading capability

2024-03-13 Thread Rushil Gupta
Gvnic's DQO format allows offloading IPv4 checksum.
Made changes to Tx and Rx path to translate DPDK flags
to descriptor for offloading (and vice-versa).
Add ptype adminq support to only add this flags for
supported L3/L4 packet-types.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/gve_ethdev.c | 34 +++---
 drivers/net/gve/gve_ethdev.h |  5 +
 drivers/net/gve/gve_rx_dqo.c | 38 --
 drivers/net/gve/gve_tx_dqo.c |  2 +-
 4 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 3b8ec58..475745b 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -434,8 +434,14 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
RTE_ETH_TX_OFFLOAD_SCTP_CKSUM   |
RTE_ETH_TX_OFFLOAD_TCP_TSO;
 
-   if (priv->queue_format == GVE_DQO_RDA_FORMAT)
-   dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_LRO;
+   if (!gve_is_gqi(priv)) {
+   dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+   dev_info->rx_offload_capa |=
+   RTE_ETH_RX_OFFLOAD_IPV4_CKSUM   |
+   RTE_ETH_RX_OFFLOAD_UDP_CKSUM|
+   RTE_ETH_RX_OFFLOAD_TCP_CKSUM|
+   RTE_ETH_RX_OFFLOAD_TCP_LRO;
+   }
 
dev_info->default_rxconf = (struct rte_eth_rxconf) {
.rx_free_thresh = GVE_DEFAULT_RX_FREE_THRESH,
@@ -938,6 +944,11 @@ gve_teardown_device_resources(struct gve_priv *priv)
if (err)
PMD_DRV_LOG(ERR, "Could not deconfigure device 
resources: err=%d", err);
}
+
+   if (!gve_is_gqi(priv)) {
+   rte_free(priv->ptype_lut_dqo);
+   priv->ptype_lut_dqo = NULL;
+   }
gve_free_counter_array(priv);
gve_free_irq_db(priv);
gve_clear_device_resources_ok(priv);
@@ -997,8 +1008,25 @@ gve_setup_device_resources(struct gve_priv *priv)
PMD_DRV_LOG(ERR, "Could not config device resources: err=%d", 
err);
goto free_irq_dbs;
}
-   return 0;
+   if (!gve_is_gqi(priv)) {
+   priv->ptype_lut_dqo = rte_zmalloc("gve_ptype_lut_dqo",
+   sizeof(struct gve_ptype_lut), 0);
+   if (priv->ptype_lut_dqo == NULL) {
+   PMD_DRV_LOG(ERR, "Failed to alloc ptype lut.");
+   err = -ENOMEM;
+   goto free_irq_dbs;
+   }
+   err = gve_adminq_get_ptype_map_dqo(priv, priv->ptype_lut_dqo);
+   if (unlikely(err)) {
+   PMD_DRV_LOG(ERR, "Failed to get ptype map: err=%d", 
err);
+   goto free_ptype_lut;
+   }
+   }
 
+   return 0;
+free_ptype_lut:
+   rte_free(priv->ptype_lut_dqo);
+   priv->ptype_lut_dqo = NULL;
 free_irq_dbs:
gve_free_irq_db(priv);
 free_cnt_array:
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index d713657..9b19fc5 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -36,6 +36,10 @@
RTE_MBUF_F_TX_L4_MASK  |\
RTE_MBUF_F_TX_TCP_SEG)
 
+#define GVE_TX_CKSUM_OFFLOAD_MASK_DQO (\
+   GVE_TX_CKSUM_OFFLOAD_MASK | \
+   RTE_MBUF_F_TX_IP_CKSUM)
+
 #define GVE_RTE_RSS_OFFLOAD_ALL (  \
RTE_ETH_RSS_IPV4 |  \
RTE_ETH_RSS_NONFRAG_IPV4_TCP |  \
@@ -295,6 +299,7 @@ struct gve_priv {
uint16_t stats_end_idx; /* end index of array of stats written by NIC */
 
struct gve_rss_config rss_config;
+   struct gve_ptype_lut *ptype_lut_dqo;
 };
 
 static inline bool
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 7c7a8c4..1c37c54 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -75,6 +75,40 @@ gve_rx_refill_dqo(struct gve_rx_queue *rxq)
rxq->bufq_tail = next_avail;
 }
 
+static inline uint16_t
+gve_parse_csum_ol_flags(volatile struct gve_rx_compl_desc_dqo *rx_desc,
+   struct gve_priv *priv) {
+   uint64_t ol_flags = 0;
+   struct gve_ptype ptype =
+   priv->ptype_lut_dqo->ptypes[rx_desc->packet_type];
+
+   if (!rx_desc->l3_l4_processed)
+   return ol_flags;
+
+   if (ptype.l3_type == GVE_L3_TYPE_IPV4) {
+   if (rx_desc->csum_ip_err)
+   ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
+   else
+   ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
+   }
+
+   if (rx_desc->csum_l4_err) {
+   ol_flags |= RTE_MBUF_F_RX_L4_CKS

[PATCH] net/gve: add IPv4 checksum offloading capability

2024-03-14 Thread Rushil Gupta
Gvnic's DQO format allows offloading IPv4 checksum.
Made changes to Tx and Rx path to translate DPDK flags
to descriptor for offloading (and vice-versa).
Add ptype adminq support to only add this flags for
supported L3/L4 packet-types.

Signed-off-by: Rushil Gupta 
Reviewed-by: Joshua Washington 
---
 drivers/net/gve/gve_ethdev.c | 34 +++---
 drivers/net/gve/gve_ethdev.h |  5 +
 drivers/net/gve/gve_rx_dqo.c | 38 --
 drivers/net/gve/gve_tx_dqo.c |  2 +-
 4 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 3b8ec58..475745b 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -434,8 +434,14 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
RTE_ETH_TX_OFFLOAD_SCTP_CKSUM   |
RTE_ETH_TX_OFFLOAD_TCP_TSO;
 
-   if (priv->queue_format == GVE_DQO_RDA_FORMAT)
-   dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_LRO;
+   if (!gve_is_gqi(priv)) {
+   dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+   dev_info->rx_offload_capa |=
+   RTE_ETH_RX_OFFLOAD_IPV4_CKSUM   |
+   RTE_ETH_RX_OFFLOAD_UDP_CKSUM|
+   RTE_ETH_RX_OFFLOAD_TCP_CKSUM|
+   RTE_ETH_RX_OFFLOAD_TCP_LRO;
+   }
 
dev_info->default_rxconf = (struct rte_eth_rxconf) {
.rx_free_thresh = GVE_DEFAULT_RX_FREE_THRESH,
@@ -938,6 +944,11 @@ gve_teardown_device_resources(struct gve_priv *priv)
if (err)
PMD_DRV_LOG(ERR, "Could not deconfigure device 
resources: err=%d", err);
}
+
+   if (!gve_is_gqi(priv)) {
+   rte_free(priv->ptype_lut_dqo);
+   priv->ptype_lut_dqo = NULL;
+   }
gve_free_counter_array(priv);
gve_free_irq_db(priv);
gve_clear_device_resources_ok(priv);
@@ -997,8 +1008,25 @@ gve_setup_device_resources(struct gve_priv *priv)
PMD_DRV_LOG(ERR, "Could not config device resources: err=%d", 
err);
goto free_irq_dbs;
}
-   return 0;
+   if (!gve_is_gqi(priv)) {
+   priv->ptype_lut_dqo = rte_zmalloc("gve_ptype_lut_dqo",
+   sizeof(struct gve_ptype_lut), 0);
+   if (priv->ptype_lut_dqo == NULL) {
+   PMD_DRV_LOG(ERR, "Failed to alloc ptype lut.");
+   err = -ENOMEM;
+   goto free_irq_dbs;
+   }
+   err = gve_adminq_get_ptype_map_dqo(priv, priv->ptype_lut_dqo);
+   if (unlikely(err)) {
+   PMD_DRV_LOG(ERR, "Failed to get ptype map: err=%d", 
err);
+   goto free_ptype_lut;
+   }
+   }
 
+   return 0;
+free_ptype_lut:
+   rte_free(priv->ptype_lut_dqo);
+   priv->ptype_lut_dqo = NULL;
 free_irq_dbs:
gve_free_irq_db(priv);
 free_cnt_array:
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index d713657..9b19fc5 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -36,6 +36,10 @@
RTE_MBUF_F_TX_L4_MASK  |\
RTE_MBUF_F_TX_TCP_SEG)
 
+#define GVE_TX_CKSUM_OFFLOAD_MASK_DQO (\
+   GVE_TX_CKSUM_OFFLOAD_MASK | \
+   RTE_MBUF_F_TX_IP_CKSUM)
+
 #define GVE_RTE_RSS_OFFLOAD_ALL (  \
RTE_ETH_RSS_IPV4 |  \
RTE_ETH_RSS_NONFRAG_IPV4_TCP |  \
@@ -295,6 +299,7 @@ struct gve_priv {
uint16_t stats_end_idx; /* end index of array of stats written by NIC */
 
struct gve_rss_config rss_config;
+   struct gve_ptype_lut *ptype_lut_dqo;
 };
 
 static inline bool
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 7c7a8c4..1c37c54 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -75,6 +75,40 @@ gve_rx_refill_dqo(struct gve_rx_queue *rxq)
rxq->bufq_tail = next_avail;
 }
 
+static inline uint16_t
+gve_parse_csum_ol_flags(volatile struct gve_rx_compl_desc_dqo *rx_desc,
+   struct gve_priv *priv) {
+   uint64_t ol_flags = 0;
+   struct gve_ptype ptype =
+   priv->ptype_lut_dqo->ptypes[rx_desc->packet_type];
+
+   if (!rx_desc->l3_l4_processed)
+   return ol_flags;
+
+   if (ptype.l3_type == GVE_L3_TYPE_IPV4) {
+   if (rx_desc->csum_ip_err)
+   ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
+   else
+   ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
+   }
+
+   if (rx_desc->csum_l4_err) {
+   ol_flags |= RTE_MBUF_F_RX_L4_CKS

Re: [PATCH v3 2/3] net/gve: update copyright holders

2023-03-28 Thread Rushil Gupta
On Tue, Mar 28, 2023 at 3:37 AM Ferruh Yigit  wrote:

> On 3/28/2023 10:45 AM, Junfeng Guo wrote:
> > Add Google LLC as one of the copyright holders for GVE.
> >
> > Signed-off-by: Rushil Gupta 
> > Signed-off-by: Joshua Washington 
> > Signed-off-by: Junfeng Guo 
> > Signed-off-by: Jeroen de Borst 
>
> Acked-by: Ferruh Yigit 
>

Acked-by: Rushil Gupta 


Re: [PATCH 2/2] net/gve: update copyright holders

2023-03-30 Thread Rushil Gupta
We were just trying to comply with the BSD license to get
rid of the exception. You have the MIT license for control path/admin-queue
code. Since admin-queue path is similar across linux, freebsd and dpdk the
code is similar but not exactly the same,
We are about to upstream driver code to FreeBSD under BSD license as well
so you will see this code under BSD license soon. I will consult the
lawyers on my end as well.

On Thu, Mar 30, 2023 at 6:14 AM Thomas Monjalon  wrote:

> 30/03/2023 09:20, Guo, Junfeng:
> > From: Thomas Monjalon 
> > > 28/03/2023 11:35, Guo, Junfeng:
> > > > The background is that, in the past (DPDK 22.11) we didn't get the
> > > approval
> > > > of license from Google, thus chose the MIT License for the base code,
> > > and
> > > > BSD-3 License for GVE common code (without the files in /base
> folder).
> > > > We also left the copyright holder of base code just to Google Inc,
> and
> > > made
> > > > Intel as the copyright holder of GVE common code (without /base
> > > folder).
> > > >
> > > > Today we are working together for GVE dev and maintaining. And we
> > > got
> > > > the approval of BSD-3 License from Google for the base code.
> > > > Thus we dicided to 1) switch the License of GVE base code from MIT to
> > > BSD-3;
> > > > 2) add Google LLC as one of the copyright holders for GVE common
> > > code.
> > >
> > > Do you realize we had lenghty discussions in the Technical Board,
> > > the Governing Board, and with lawyers, just for this unneeded
> exception?
> > >
> > > Now looking at the patches, there seem to be some big mistakes like
> > > removing some copyright. I don't understand how it can be taken so
> > > lightly.
> > >
> > > I regret how fast we were, next time we will surely operate
> differently.
> > > If you want to improve the reputation of this driver,
> > > please ask other copyright holders to be more active and responsive.
> > >
> >
> > Really sorry for causing such severe trouble.
> >
> > Yes, we did take lots of efforts in the Technical Board and the Governing
> > Board about this MIT exception. We really appreciate that.
> >
> > About this patch set, it is my severe mistake to switch the MIT License
> > directly for the upstream-ed code in community, in the wrong way.
> > In the past we upstream-ed this driver with MIT License followed from
> > the kernel community's gve driver base code. And now we want to
> > use the code with BSD-3 License (approved by Google).
> > So I suppose that the correct way may be 1) first remove all these code
> > under MIT License and 2) then add the new files under BSD-3 License.
>
> The code under BSD is different of the MIT code?
> If it is the same with a new approved license, you can just change the
> license.
>
> > Please correct me if there are still misunderstanding in my statement.
> > Thanks Thomas for pointing out my mistake. I'll be careful to fix this.
> >
> > Copyright holder for the gve base code will stay unchanged. Google LLC
> > will be added as one of the copyright holders for the gve common code.
> > @Rushil Gupta Please also be more active and responsive for the code
> > review and contribution in the community. Thanks!
>
>
>
>


Re: [PATCH 2/2] net/gve: update copyright holders

2023-03-30 Thread Rushil Gupta
On Thu, Mar 30, 2023 at 6:14 AM Thomas Monjalon  wrote:

> 30/03/2023 09:20, Guo, Junfeng:
> > From: Thomas Monjalon 
> > > 28/03/2023 11:35, Guo, Junfeng:
> > > > The background is that, in the past (DPDK 22.11) we didn't get the
> > > approval
> > > > of license from Google, thus chose the MIT License for the base code,
> > > and
> > > > BSD-3 License for GVE common code (without the files in /base
> folder).
> > > > We also left the copyright holder of base code just to Google Inc,
> and
> > > made
> > > > Intel as the copyright holder of GVE common code (without /base
> > > folder).
> > > >
> > > > Today we are working together for GVE dev and maintaining. And we
> > > got
> > > > the approval of BSD-3 License from Google for the base code.
> > > > Thus we dicided to 1) switch the License of GVE base code from MIT to
> > > BSD-3;
> > > > 2) add Google LLC as one of the copyright holders for GVE common
> > > code.
> > >
> > > Do you realize we had lenghty discussions in the Technical Board,
> > > the Governing Board, and with lawyers, just for this unneeded
> exception?
> > >
> > > Now looking at the patches, there seem to be some big mistakes like
> > > removing some copyright. I don't understand how it can be taken so
> > > lightly.
> > >
> > > I regret how fast we were, next time we will surely operate
> differently.
> > > If you want to improve the reputation of this driver,
> > > please ask other copyright holders to be more active and responsive.
> > >
> >
> > Really sorry for causing such severe trouble.
> >
> > Yes, we did take lots of efforts in the Technical Board and the Governing
> > Board about this MIT exception. We really appreciate that.
> >
> > About this patch set, it is my severe mistake to switch the MIT License
> > directly for the upstream-ed code in community, in the wrong way.
> > In the past we upstream-ed this driver with MIT License followed from
> > the kernel community's gve driver base code. And now we want to
> > use the code with BSD-3 License (approved by Google).
> > So I suppose that the correct way may be 1) first remove all these code
> > under MIT License and 2) then add the new files under BSD-3 License.
>
> The code under BSD is different of the MIT code?
> If it is the same with a new approved license, you can just change the
> license.
>
> > Please correct me if there are still misunderstanding in my statement.
> > Thanks Thomas for pointing out my mistake. I'll be careful to fix this.
> >
> > Copyright holder for the gve base code will stay unchanged. Google LLC
> > will be added as one of the copyright holders for the gve common code.
> > @Rushil Gupta Please also be more active and responsive for the code
> > review and contribution in the community. Thanks!
>
>
>
> We were just trying to comply with the BSD license to get
rid of the exception. You have the MIT license for control path/admin-queue
code. Since admin-queue path is similar across linux, freebsd and dpdk the
code is similar but not exactly the same,
We are about to upstream driver code to FreeBSD under BSD license as well
so you will see this code under BSD license soon. I will consult the
lawyers on my end as well.


[PATCH 0/1] *** Update drivers/net/gve/base code for DQO ***

2023-04-10 Thread Rushil Gupta
This patch is dependent on 
https://patchwork.dpdk.org/project/dpdk/patch/20230410064724.2094392-1-junfeng@intel.com/

Rushil Gupta (1):
  net/gve: update base code for DQO

 drivers/net/gve/base/gve.h  |  1 +
 drivers/net/gve/base/gve_adminq.c   | 10 +-
 drivers/net/gve/base/gve_desc_dqo.h |  4 
 3 files changed, 6 insertions(+), 9 deletions(-)

-- 
2.40.0.577.gac1e443424-goog



[PATCH 1/1] net/gve: update base code for DQO

2023-04-10 Thread Rushil Gupta
Update gve base code to support DQO.

This patch is based on this:
https://patchwork.dpdk.org/project/dpdk/list/?series=27647&state=*

Signed-off-by: Rushil Gupta 
Signed-off-by: Junfeng Guo 
---
 drivers/net/gve/base/gve.h  |  1 +
 drivers/net/gve/base/gve_adminq.c   | 10 +-
 drivers/net/gve/base/gve_desc_dqo.h |  4 
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2dc4507acb..2b7cf7d99b 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -7,6 +7,7 @@
 #define _GVE_H_
 
 #include "gve_desc.h"
+#include "gve_desc_dqo.h"
 
 #define GVE_VERSION"1.3.0"
 #define GVE_VERSION_PREFIX "GVE-"
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e745b709b2..e963f910a0 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -497,11 +497,11 @@ static int gve_adminq_create_tx_queue(struct gve_priv 
*priv, u32 queue_index)
cmd.create_tx_queue.queue_page_list_id = cpu_to_be32(qpl_id);
} else {
cmd.create_tx_queue.tx_ring_size =
-   cpu_to_be16(txq->nb_tx_desc);
+   cpu_to_be16(priv->tx_desc_cnt);
cmd.create_tx_queue.tx_comp_ring_addr =
-   cpu_to_be64(txq->complq->tx_ring_phys_addr);
+   cpu_to_be64(txq->compl_ring_phys_addr);
cmd.create_tx_queue.tx_comp_ring_size =
-   cpu_to_be16(priv->tx_compq_size);
+   cpu_to_be16(priv->tx_compq_size * DQO_TX_MULTIPLIER);
}
 
return gve_adminq_issue_cmd(priv, &cmd);
@@ -549,9 +549,9 @@ static int gve_adminq_create_rx_queue(struct gve_priv 
*priv, u32 queue_index)
cmd.create_rx_queue.rx_ring_size =
cpu_to_be16(priv->rx_desc_cnt);
cmd.create_rx_queue.rx_desc_ring_addr =
-   cpu_to_be64(rxq->rx_ring_phys_addr);
+   cpu_to_be64(rxq->compl_ring_phys_addr);
cmd.create_rx_queue.rx_data_ring_addr =
-   cpu_to_be64(rxq->bufq->rx_ring_phys_addr);
+   cpu_to_be64(rxq->rx_ring_phys_addr);
cmd.create_rx_queue.packet_buffer_size =
cpu_to_be16(rxq->rx_buf_len);
cmd.create_rx_queue.rx_buff_ring_size =
diff --git a/drivers/net/gve/base/gve_desc_dqo.h 
b/drivers/net/gve/base/gve_desc_dqo.h
index ee1afdecb8..bb4a18d4d1 100644
--- a/drivers/net/gve/base/gve_desc_dqo.h
+++ b/drivers/net/gve/base/gve_desc_dqo.h
@@ -13,10 +13,6 @@
 #define GVE_TX_MAX_HDR_SIZE_DQO 255
 #define GVE_TX_MIN_TSO_MSS_DQO 88
 
-#ifndef __LITTLE_ENDIAN_BITFIELD
-#error "Only little endian supported"
-#endif
-
 /* Basic TX descriptor (DTYPE 0x0C) */
 struct gve_tx_pkt_desc_dqo {
__le64 buf_addr;
-- 
2.40.0.577.gac1e443424-goog



Re: [PATCH 1/1] net/gve: update base code for DQO

2023-04-12 Thread Rushil Gupta
On Wed, Apr 12, 2023 at 2:41 AM Guo, Junfeng  wrote:

>
>
> > -Original Message-
> > From: Ferruh Yigit 
> > Sent: Wednesday, April 12, 2023 17:35
> > To: Guo, Junfeng ; Richardson, Bruce
> > 
> > Cc: dev@dpdk.org; Zhang, Qi Z ; Rushil Gupta
> > 
> > Subject: Re: [PATCH 1/1] net/gve: update base code for DQO
> >
> > On 4/12/2023 10:09 AM, Guo, Junfeng wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: Ferruh Yigit 
> > >> Sent: Wednesday, April 12, 2023 16:50
> > >> To: Guo, Junfeng ; Richardson, Bruce
> > >> 
> > >> Cc: dev@dpdk.org; Zhang, Qi Z ; Rushil Gupta
> > >> 
> > >> Subject: Re: [PATCH 1/1] net/gve: update base code for DQO
> > >>
> > >> On 4/11/2023 7:51 AM, Guo, Junfeng wrote:
> > >>
> > >> Hi Junfeng, message moved down.
> > >>
> > >>>
> > >>>> -Original Message-
> > >>>> From: Rushil Gupta 
> > >>>> Sent: Tuesday, April 11, 2023 12:59
> > >>>> To: Zhang, Qi Z ; ferruh.yi...@amd.com
> > >>>> Cc: Richardson, Bruce ;
> > dev@dpdk.org;
> > >>>> Rushil Gupta ; Guo, Junfeng
> > >>>> 
> > >>>> Subject: [PATCH 1/1] net/gve: update base code for DQO
> > >>>>
> > >>>> Update gve base code to support DQO.
> > >>>>
> > >>>> This patch is based on this:
> > >>>>
> > https://patchwork.dpdk.org/project/dpdk/list/?series=27647&state=*
> > >>>>
> > >>>> Signed-off-by: Rushil Gupta 
> > >>>> Signed-off-by: Junfeng Guo 
> > >>> Hi Ferruh & Bruce,
> > >>>
> > >>> This patch contains few lines change for the MIT licensed gve base
> > code.
> > >>> Note that there is no new files added, just some minor code update.
> > >>>
> > >>> Do we need to ask for special approval from the Tech Board for this?
> > >>> Please help give some advice and also help review this patch. Thanks!
> > >>>
> > >>
> > >> Once the MIT license exception is in place, as far as I know no more
> > >> approval is required per change.
> > >
> > > Got it, thanks the comment!
> > >
> > > Then we may also need your help to review, as well as the coming patch
> > > set for GVE PMD enhancement for DPDK 23.07. Thanks in advance!
> > >
> > >>
> > >>> BTW, Google will also help replace all the base code under MIT
> > license
> > >>> with the ones under BSD-3 license soon, which would make things
> > more
> > >>> easier.
> > >>>
> > >>
> > >> Is this different from base code under DPDK is changing license [1] ?
> > >>
> > >>
> > >> [1]
> > >>
> > https://patches.dpdk.org/project/dpdk/list/?series=27570&state=%2A&ar
> > >> chive=both
> > >>
> > >
> > > The patch set of the above link only contains the processing of replace
> > the
> > > MIT licensed base code with the BSD-3 licensed base code. After some
> > > discussion, we think Google is in the right place to do that work. And
> > they
> > > are working on that now.
> > >
> >
> > Is the Google GVE driver [2] in the process of changing license from MIT
> > to BSD-3?
> >
> >
> > [2]
> > https://github.com/GoogleCloudPlatform/compute-virtual-ethernet-
> > linux/tree/v1.3.0/google/gve
> >
>
> I'm not sure, I don't know much about Google's plans.
> Maybe they could provide some info here. Thanks!
>
> @Rushil Gupta
>
> >
> >
> > > This patch is mainly for the feature upstreaming of DPDK 23.07. It
> > contains
> > > only the code part, following previous license statements, without any
> > > license change.
> > >
> > > This patch is separated and sent by Google, to ensure there is no
> license
> > > violation.
> > >
> > > BTW, about the feature of GVE PMD enhancement, the rest code are all
> > > about BSD-3 licensed files, and that patch set will be sent out soon.
> > >
> > > Thanks!
>
> I have got the green light internally to switch to BSD-3 license for code
under base directory. If it is ok with the tech board, I can send a patch
right away with all of the base files changed to BSD-3 which can be merged
after this patch. Please let me know what you think.
We are also about to upstream driver code:
https://github.com/GoogleCloudPlatform/compute-virtual-ethernet-freebsd to
FreeBSD as well so you will see similar code under BSD license soon in
freebsd repo.


Re: [PATCH 1/1] net/gve: update base code for DQO

2023-04-12 Thread Rushil Gupta
Sorry for the confusion. I was talking about the same patch (titled
net/gve: update copyright holders); however, I am not able to find it on
patchwork.


On Wed, Apr 12, 2023 at 9:03 AM Ferruh Yigit  wrote:

> On 4/12/2023 4:42 PM, Rushil Gupta wrote:
> >
> >
> > On Wed, Apr 12, 2023 at 2:41 AM Guo, Junfeng  > <mailto:junfeng@intel.com>> wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Ferruh Yigit  > <mailto:ferruh.yi...@amd.com>>
> > > Sent: Wednesday, April 12, 2023 17:35
> > > To: Guo, Junfeng  > <mailto:junfeng@intel.com>>; Richardson, Bruce
> > > mailto:bruce.richard...@intel.com>>
> > > Cc: dev@dpdk.org <mailto:dev@dpdk.org>; Zhang, Qi Z
> > mailto:qi.z.zh...@intel.com>>; Rushil Gupta
> > > mailto:rush...@google.com>>
> > > Subject: Re: [PATCH 1/1] net/gve: update base code for DQO
> > >
> > > On 4/12/2023 10:09 AM, Guo, Junfeng wrote:
> > > >
> > > >
> > > >> -Original Message-
> > > >> From: Ferruh Yigit  > <mailto:ferruh.yi...@amd.com>>
> > > >> Sent: Wednesday, April 12, 2023 16:50
> > > >> To: Guo, Junfeng  > <mailto:junfeng@intel.com>>; Richardson, Bruce
> > > >> mailto:bruce.richard...@intel.com
> >>
> > > >> Cc: dev@dpdk.org <mailto:dev@dpdk.org>; Zhang, Qi Z
> > mailto:qi.z.zh...@intel.com>>; Rushil Gupta
> > > >> mailto:rush...@google.com>>
> > > >> Subject: Re: [PATCH 1/1] net/gve: update base code for DQO
> > > >>
> > > >> On 4/11/2023 7:51 AM, Guo, Junfeng wrote:
> > > >>
> > > >> Hi Junfeng, message moved down.
> >     > >>
> > > >>>
> > > >>>> -Original Message-
> > > >>>> From: Rushil Gupta  > <mailto:rush...@google.com>>
> > > >>>> Sent: Tuesday, April 11, 2023 12:59
> > > >>>> To: Zhang, Qi Z  > <mailto:qi.z.zh...@intel.com>>; ferruh.yi...@amd.com
> > <mailto:ferruh.yi...@amd.com>
> > > >>>> Cc: Richardson, Bruce  >     <mailto:bruce.richard...@intel.com>>;
> > > dev@dpdk.org <mailto:dev@dpdk.org>;
> > > >>>> Rushil Gupta  > <mailto:rush...@google.com>>; Guo, Junfeng
> > > >>>> mailto:junfeng@intel.com>>
> > > >>>> Subject: [PATCH 1/1] net/gve: update base code for DQO
> > > >>>>
> > > >>>> Update gve base code to support DQO.
> > > >>>>
> > > >>>> This patch is based on this:
> > > >>>>
> > > https://patchwork.dpdk.org/project/dpdk/list/?series=27647&state=*
> > <https://patchwork.dpdk.org/project/dpdk/list/?series=27647&state=*>
> > > >>>>
> > > >>>> Signed-off-by: Rushil Gupta  > <mailto:rush...@google.com>>
> > > >>>> Signed-off-by: Junfeng Guo  > <mailto:junfeng@intel.com>>
> > > >>> Hi Ferruh & Bruce,
> > > >>>
> > > >>> This patch contains few lines change for the MIT licensed gve
> base
> > > code.
> > > >>> Note that there is no new files added, just some minor code
> > update.
> > > >>>
> > > >>> Do we need to ask for special approval from the Tech Board for
> > this?
> > > >>> Please help give some advice and also help review this patch.
> > Thanks!
> > > >>>
> > > >>
> > > >> Once the MIT license exception is in place, as far as I know no
> > more
> > > >> approval is required per change.
> > > >
> > > > Got it, thanks the comment!
> > > >
> > > > Then we may also need your help to review, as well as the coming
> > patch
> > > > set for GVE PMD enhancement for DPDK 23.07. Thanks in advance!
> > > >
> > > >>
> > > >>> BTW, Google will also help replace all the base code under MIT
> > > license
> > 

[PATCH] Check whether the driver is compatible with the device presented.

2023-04-13 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

This patch is dependent on  
https://patchwork.dpdk.org/project/dpdk/list/?series=27687&state=*

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 +
 drivers/net/gve/base/gve_adminq.h | 49 ++-
 drivers/net/gve/base/gve_osdep.h  | 36 +
 drivers/net/gve/gve_ethdev.c  | 65 +--
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 14 +++
 drivers/net/gve/gve_version.h | 25 
 drivers/net/gve/meson.build   |  1 +
 9 files changed, 198 insertions(+), 16 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2dc4507acb..89f9654a72 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -8,9 +8,6 @@
 
 #include "gve_desc.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e745b709b2..2e5099a5b0 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..edac32f031 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,47 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+#define GVE_CAP2(a) BIT(((int)a) - 64)
+#define GVE_CAP3(a) BIT(((int)a) - 128)
+#define GVE_CAP4(a) BIT(((int)a) - 192)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+

[PATCH] net/gve: Check whether the driver is compatible with the device presented.

2023-04-13 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

This patch is dependent on  
https://patchwork.dpdk.org/project/dpdk/list/?series=27687&state=*

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 +
 drivers/net/gve/base/gve_adminq.h | 49 ++-
 drivers/net/gve/base/gve_osdep.h  | 36 +
 drivers/net/gve/gve_ethdev.c  | 65 +--
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 14 +++
 drivers/net/gve/gve_version.h | 25 
 drivers/net/gve/meson.build   |  1 +
 9 files changed, 198 insertions(+), 16 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2dc4507acb..89f9654a72 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -8,9 +8,6 @@
 
 #include "gve_desc.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e745b709b2..2e5099a5b0 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..edac32f031 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,47 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+#define GVE_CAP2(a) BIT(((int)a) - 64)
+#define GVE_CAP3(a) BIT(((int)a) - 128)
+#define GVE_CAP4(a) BIT(((int)a) - 192)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+

Re: [PATCH 1/1] net/gve: update base code for DQO

2023-04-13 Thread Rushil Gupta
I want to highlight that we wish to keep license changes separate from this
patch (probably for 23.11). This patch is to simply support basic
structures for the DQO data path.

On Wed, Apr 12, 2023 at 11:04 AM Rushil Gupta  wrote:

> Sorry for the confusion. I was talking about the same patch (titled
> net/gve: update copyright holders); however, I am not able to find it on
> patchwork.
>
>
> On Wed, Apr 12, 2023 at 9:03 AM Ferruh Yigit  wrote:
>
>> On 4/12/2023 4:42 PM, Rushil Gupta wrote:
>> >
>> >
>> > On Wed, Apr 12, 2023 at 2:41 AM Guo, Junfeng > > <mailto:junfeng@intel.com>> wrote:
>> >
>> >
>> >
>> > > -Original Message-
>> > > From: Ferruh Yigit > > <mailto:ferruh.yi...@amd.com>>
>> > > Sent: Wednesday, April 12, 2023 17:35
>> > > To: Guo, Junfeng > > <mailto:junfeng@intel.com>>; Richardson, Bruce
>> > > mailto:bruce.richard...@intel.com>>
>> > > Cc: dev@dpdk.org <mailto:dev@dpdk.org>; Zhang, Qi Z
>> > mailto:qi.z.zh...@intel.com>>; Rushil Gupta
>> > > mailto:rush...@google.com>>
>> > > Subject: Re: [PATCH 1/1] net/gve: update base code for DQO
>> > >
>> > > On 4/12/2023 10:09 AM, Guo, Junfeng wrote:
>> > > >
>> > > >
>> > > >> -Original Message-
>> > > >> From: Ferruh Yigit > > <mailto:ferruh.yi...@amd.com>>
>> > > >> Sent: Wednesday, April 12, 2023 16:50
>> > > >> To: Guo, Junfeng > > <mailto:junfeng@intel.com>>; Richardson, Bruce
>> > > >> mailto:bruce.richard...@intel.com
>> >>
>> > > >> Cc: dev@dpdk.org <mailto:dev@dpdk.org>; Zhang, Qi Z
>> > mailto:qi.z.zh...@intel.com>>; Rushil Gupta
>> > > >> mailto:rush...@google.com>>
>> > > >> Subject: Re: [PATCH 1/1] net/gve: update base code for DQO
>> > > >>
>> > > >> On 4/11/2023 7:51 AM, Guo, Junfeng wrote:
>> > > >>
>> > > >> Hi Junfeng, message moved down.
>> > > >>
>> > > >>>
>> > > >>>> -Original Message-
>> > > >>>> From: Rushil Gupta > > <mailto:rush...@google.com>>
>> > > >>>> Sent: Tuesday, April 11, 2023 12:59
>> > > >>>> To: Zhang, Qi Z > > <mailto:qi.z.zh...@intel.com>>; ferruh.yi...@amd.com
>> > <mailto:ferruh.yi...@amd.com>
>> > > >>>> Cc: Richardson, Bruce > > <mailto:bruce.richard...@intel.com>>;
>> > > dev@dpdk.org <mailto:dev@dpdk.org>;
>> > > >>>> Rushil Gupta > > <mailto:rush...@google.com>>; Guo, Junfeng
>> > > >>>> mailto:junfeng@intel.com>>
>> > > >>>> Subject: [PATCH 1/1] net/gve: update base code for DQO
>> > > >>>>
>> > > >>>> Update gve base code to support DQO.
>> > > >>>>
>> > > >>>> This patch is based on this:
>> > > >>>>
>> > >
>> https://patchwork.dpdk.org/project/dpdk/list/?series=27647&state=*
>> > <https://patchwork.dpdk.org/project/dpdk/list/?series=27647&state=*
>> >
>> > > >>>>
>> > > >>>> Signed-off-by: Rushil Gupta > > <mailto:rush...@google.com>>
>> > > >>>> Signed-off-by: Junfeng Guo > > <mailto:junfeng@intel.com>>
>> > > >>> Hi Ferruh & Bruce,
>> > > >>>
>> > > >>> This patch contains few lines change for the MIT licensed gve
>> base
>> > > code.
>> > > >>> Note that there is no new files added, just some minor code
>> > update.
>> > > >>>
>> > > >>> Do we need to ask for special approval from the Tech Board for
>> > this?
>> > > >>> Please help give some advice and also help review this patch.
>> > Thanks!
>> > > >>>
>> > > >>
>> > > >> Once the MIT lic

[PATCH] net/gve: Check whether the driver is compatible with the device presented.

2023-04-26 Thread Rushil Gupta
Change gve_driver_info fields to report DPDK as OS type and DPDK RTE
version as OS version, reserving driver_version fields for GVE driver
version based on features.

Depends-on: series-27687

Signed-off-by: Rushil Gupta 
Signed-off-by: Joshua Washington 
Signed-off-by: Junfeng Guo 
Signed-off-by: Jeroen de Borst 
---
 drivers/net/gve/base/gve.h|  3 --
 drivers/net/gve/base/gve_adminq.c | 19 +
 drivers/net/gve/base/gve_adminq.h | 49 ++-
 drivers/net/gve/base/gve_osdep.h  | 36 +
 drivers/net/gve/gve_ethdev.c  | 65 +--
 drivers/net/gve/gve_ethdev.h  |  2 +-
 drivers/net/gve/gve_version.c | 14 +++
 drivers/net/gve/gve_version.h | 25 
 drivers/net/gve/meson.build   |  1 +
 9 files changed, 198 insertions(+), 16 deletions(-)
 create mode 100644 drivers/net/gve/gve_version.c
 create mode 100644 drivers/net/gve/gve_version.h

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 2dc4507acb..89f9654a72 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -8,9 +8,6 @@
 
 #include "gve_desc.h"
 
-#define GVE_VERSION"1.3.0"
-#define GVE_VERSION_PREFIX "GVE-"
-
 #ifndef GOOGLE_VENDOR_ID
 #define GOOGLE_VENDOR_ID   0x1ae0
 #endif
diff --git a/drivers/net/gve/base/gve_adminq.c 
b/drivers/net/gve/base/gve_adminq.c
index e745b709b2..2e5099a5b0 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -401,6 +401,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_GET_PTYPE_MAP:
priv->adminq_get_ptype_map_cnt++;
break;
+   case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
+   priv->adminq_verify_driver_compatibility_cnt++;
+   break;
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -465,6 +468,22 @@ int gve_adminq_configure_device_resources(struct gve_priv 
*priv,
return gve_adminq_execute_cmd(priv, &cmd);
 }
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+  u64 driver_info_len,
+  dma_addr_t driver_info_addr)
+{
+   union gve_adminq_command cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
+   cmd.verify_driver_compatibility = (struct 
gve_adminq_verify_driver_compatibility) {
+   .driver_info_len = cpu_to_be64(driver_info_len),
+   .driver_info_addr = cpu_to_be64(driver_info_addr),
+   };
+
+   return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv)
 {
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h 
b/drivers/net/gve/base/gve_adminq.h
index 05550119de..edac32f031 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: MIT
  * Google Virtual Ethernet (gve) driver
- * Copyright (C) 2015-2022 Google, Inc.
+ * Copyright (C) 2015-2023 Google, Inc.
  */
 
 #ifndef _GVE_ADMINQ_H
@@ -23,6 +23,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_STATS = 0xC,
GVE_ADMINQ_REPORT_LINK_SPEED= 0xD,
GVE_ADMINQ_GET_PTYPE_MAP= 0xE,
+   GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY  = 0xF,
 };
 
 /* Admin queue status codes */
@@ -145,6 +146,47 @@ enum gve_sup_feature_mask {
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
+enum gve_driver_capbility {
+   gve_driver_capability_gqi_qpl = 0,
+   gve_driver_capability_gqi_rda = 1,
+   gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
+   gve_driver_capability_dqo_rda = 3,
+};
+
+#define GVE_CAP1(a) BIT((int)a)
+#define GVE_CAP2(a) BIT(((int)a) - 64)
+#define GVE_CAP3(a) BIT(((int)a) - 128)
+#define GVE_CAP4(a) BIT(((int)a) - 192)
+
+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
+   (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
+GVE_CAP1(gve_driver_capability_gqi_rda) | \
+GVE_CAP1(gve_driver_capability_dqo_rda))
+
+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
+
+struct gve_driver_info {
+   u8 os_type; /* 0x05 = DPDK */
+   u8 driver_major;
+   u8 driver_minor;
+   u8 driver_sub;
+   __be32 os_version_major;
+   __be32 os_version_minor;
+   __be32 os_version_sub;
+   __be64 driver_capability_flags[4];
+   u8 os_version_str1[OS_VERSION_STRLEN];
+   u8 os_version_str2[OS_VERSION_STRLEN];
+};
+
+struct gve_adminq_verify_driver_compatibility {
+   __be64 driver_info_len;
+   __be64 driver_info_addr;
+};
+
+GVE_CHECK_STRUCT_LEN(16,  gve_adminq_verify_driver_compati

Re: [PATCH] doc: fix missing release note for GVE PMD DQO

2023-07-03 Thread Rushil Gupta
On Sun, Jul 2, 2023 at 8:00 PM Junfeng Guo  wrote:
>
> Add missing release note for GVE PMD enabling for DQO queue format.
>
> Fixes: a14d391c7d99 ("net/gve: add Tx queue setup for DQO")
>
> Signed-off-by: Junfeng Guo 
> ---
>  doc/guides/rel_notes/release_23_07.rst | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_23_07.rst 
> b/doc/guides/rel_notes/release_23_07.rst
> index 4459144140..28c538acc9 100644
> --- a/doc/guides/rel_notes/release_23_07.rst
> +++ b/doc/guides/rel_notes/release_23_07.rst
> @@ -200,6 +200,9 @@ New Features
>
>Enhanced the GRO library to support TCP packets over IPv6 network.
>
> +* **Added GVE PMD enabling for DQO.**
> +
> +  Added GVE PMD enabling for DQO queue format.
>
>  Removed Items
>  -
> --
> 2.34.1
>
Acked-by: Rushil Gupta 


Re: [PATCH] maintainers: update for gve

2022-11-10 Thread Rushil Gupta
Thanks a lot Junfeng!

On Tue, Nov 8, 2022 at 11:26 PM Junfeng Guo  wrote:

> Add co-maintainers from Google team for gve (Google Virtual Ethernet).
>
> Signed-off-by: Junfeng Guo 
> ---
>  MAINTAINERS | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1c9922123e..d8c1d5272b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -698,6 +698,9 @@ F: doc/guides/nics/features/enic.ini
>
>  Google Virtual Ethernet
>  M: Junfeng Guo 
> +M: Jeroen de Borst 
> +M: Rushil Gupta 
> +M: Jordan Kimbrough 
>  F: drivers/net/gve/
>  F: doc/guides/nics/gve.rst
>  F: doc/guides/nics/features/gve.ini
> --
> 2.34.1
>
>


Re: [PATCH] maintainers: update for gve

2022-11-15 Thread Rushil Gupta
Done. Thanks Junfeng!



On Tue, Nov 15, 2022 at 1:22 AM Guo, Junfeng  wrote:
>
>
>
> > -Original Message-
> > From: Thomas Monjalon 
> > Sent: Tuesday, November 15, 2022 16:33
> > To: Guo, Junfeng 
> > Cc: dev@dpdk.org; Zhang, Qi Z ; Wu, Jingjing
> > ; ferruh.yi...@xilinx.com; Xing, Beilei
> > ; dev@dpdk.org; jeroe...@google.com;
> > jr...@google.com; Zhang, Helin ; Rushil Gupta
> > ; Jeroen de Borst ;
> > Jordan Kimbrough 
> > Subject: Re: [PATCH] maintainers: update for gve
> >
> > 09/11/2022 20:37, Rushil Gupta:
> > > Thanks a lot Junfeng!
> > >
> > > On Tue, Nov 8, 2022 at 11:26 PM Junfeng Guo 
> > wrote:
> > >
> > > > Add co-maintainers from Google team for gve (Google Virtual
> > Ethernet).
> > > >
> > > > Signed-off-by: Junfeng Guo 
> > > > ---
> > > >  Google Virtual Ethernet
> > > >  M: Junfeng Guo 
> > > > +M: Jeroen de Borst 
> > > > +M: Rushil Gupta 
> > > > +M: Jordan Kimbrough 
> >
> > They were not part of the patch review process in the mailing list,
> > why do you want them to become maintainers?
> > I think it would be saner to have them involved first.
>
> Yes, make sense! Thanks for reminding this!
>
> Hi @Rushil Gupta @Jeroen de Borst @Jordan Kimbrough,
> could you help register for dev@dpdk.org
> at https://www.dpdk.org/contribute/#Mailing-Lists as well as for
> the patchwork at http://patchwork.dpdk.org/project/dpdk/list/ first?
> Then you can add more info & explanation for your maintaining plan here.
> Thanks!
>
> >
>
>


Re: [PATCH] net/gve: add support for basic stats

2022-11-25 Thread Rushil Gupta
Makes sense.




On Thu, Nov 24, 2022 at 9:26 AM Ferruh Yigit  wrote:
>
> On 11/24/2022 4:59 PM, Stephen Hemminger wrote:
> > On Thu, 24 Nov 2022 15:33:35 +0800
> > Junfeng Guo  wrote:
> >
> >> +static int
> >> +gve_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
> >> +{
> >> +uint16_t i;
> >> +
> >> +for (i = 0; i < dev->data->nb_tx_queues; i++) {
> >> +struct gve_tx_queue *txq = dev->data->tx_queues[i];
> >> +if (txq == NULL)
> >> +continue;
> >> +
> >> +stats->opackets += txq->packets;
> >> +stats->obytes += txq->bytes;
> >> +stats->oerrors += txq->errors;
> >> +}
> >> +
> >> +for (i = 0; i < dev->data->nb_rx_queues; i++) {
> >> +struct gve_rx_queue *rxq = dev->data->rx_queues[i];
> >> +if (rxq == NULL)
> >> +continue;
> >> +
> >> +stats->ipackets += rxq->packets;
> >> +stats->ibytes += rxq->bytes;
> >> +stats->ierrors += rxq->errors;
> >> +stats->rx_nombuf += rxq->no_mbufs;
> >> +}
> >> +
> >> +return 0;
> >> +}
> >> +
> >
> > The driver should be filling in the per-queue stats as well.
> > q_ipackets, q_opackets, q_ibytes, q_obytes, q_errors
>
>
> Hi Stephen,
>
> Queue stats moved to xstats, and there is a long term goal to move all
> queue stats from basic stats to xstats for all PMDs, and remove interim
> 'RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS' flag.
>
> That is why request to new PMDs is to not introduce queue stats in basic
> stats, but in xstats.


Re: [PATCH] net/gve: add support for basic stats

2022-12-19 Thread Rushil Gupta
Hi all
Josh just found out some inconsistencies in the Tx/Rx statistics sum
for all ports. Not sure if we can screenshot here but it goes like
this:
Tx-dropped for port0: 447034
Tx-dropped for port1: 0
Accumulated forward statistics for all ports: 807630

Please note that this issue is only with Tx-dropped (not Tx-packets/Tx-total).


On Wed, Dec 7, 2022 at 8:39 AM Stephen Hemminger
 wrote:
>
> On Wed, 7 Dec 2022 15:09:08 +
> Ferruh Yigit  wrote:
>
> > On 11/24/2022 7:33 AM, Junfeng Guo wrote:
> > > Add support for dev_ops of stats_get and stats_reset.
> > >
> > > Queue stats update will be moved into xstat [1], but the basic stats
> > > items may still be required. So we just keep the remaining ones and
> > > will implement the queue stats via xstats in the coming release.
> > >
> > > [1]
> > > https://elixir.bootlin.com/dpdk/v22.07/ \
> > > source/doc/guides/rel_notes/deprecation.rst#L118
> > >
> > > Signed-off-by: Xiaoyun Li 
> > > Signed-off-by: Junfeng Guo 
> >
> > <...>
> >
> > > +static int
> > > +gve_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
> > > +{
> > > +   uint16_t i;
> > > +
> > > +   for (i = 0; i < dev->data->nb_tx_queues; i++) {
> > > +   struct gve_tx_queue *txq = dev->data->tx_queues[i];
> > > +   if (txq == NULL)
> > > +   continue;
> > > +
> > > +   stats->opackets += txq->packets;
> > > +   stats->obytes += txq->bytes;
> > > +   stats->oerrors += txq->errors;
> >
> > Hi Junfeng, Qi, Jingjing, Beilei,
> >
> > Above logic looks wrong to me, did you test it?
> >
> > If the 'gve_dev_stats_get()' called multiple times (without stats reset
> > in between), same values will be keep added to stats.
> > Some hw based implementations does this, because reading from stats
> > registers automatically reset those registers but this shouldn't be case
> > for this driver.
> >
> > I expect it to be something like:
> >
> > local_stats = 0
> > foreach queue
> >   local_stats += queue->stats
> > stats = local_stats
>
> The zero of local_stats is unnecessary.
> The only caller of the PMD stats_get is rte_ethdev_stats_get
> and it zeros the stats structure before calling the PMD.
>
>
> int
> rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats)
> {
> struct rte_eth_dev *dev;
>
> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> dev = &rte_eth_devices[port_id];
>
> memset(stats, 0, sizeof(*stats));
> ...
> stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed;
> return eth_err(port_id, (*dev->dev_ops->stats_get)(dev, stats));
>
> If any PMD has extra memset in their stats get that could be removed.