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

Message ID 20230414051454.1245936-1-rushilg@google.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/gve: Check whether the driver is compatible with the device presented. |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

Rushil Gupta April 14, 2023, 5:14 a.m. UTC
  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 <rushilg@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
Signed-off-by: Jeroen de Borst <jeroendb@google.com>
---
 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
  

Comments

Junfeng Guo April 14, 2023, 9:05 a.m. UTC | #1
Hi Ferruh,

Here is the summary of the dependency of related patches:

P1: necessary headers update
    https://patchwork.dpdk.org/project/dpdk/list/?series=27647&state=*
P2: minor update for base code
    https://patchwork.dpdk.org/project/dpdk/list/?series=27653&state=*
P3: gve PMD enhancement
    https://patchwork.dpdk.org/project/dpdk/list/?series=27687&state=*
P4: add new AdminQ cmd to verify drv cap
    https://patchwork.dpdk.org/project/dpdk/list/?series=27703&state=*

The merging order could be: P1 > P2 > P3 > P4, due to the dependency.
Just to inform you about this here. Thanks for the review!

Regards,
Junfeng

> -----Original Message-----
> From: Rushil Gupta <rushilg@google.com>
> Sent: Friday, April 14, 2023 13:15
> To: ferruh.yigit@amd.com; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>
> Cc: Guo, Junfeng <junfeng.guo@intel.com>; joshwash@google.com;
> dev@dpdk.org; Rushil Gupta <rushilg@google.com>; Jeroen de Borst
> <jeroendb@google.com>
> Subject: [PATCH] net/gve: Check whether the driver is compatible with
> the device presented.
> 
> 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 <rushilg@google.com>
> Signed-off-by: Joshua Washington <joshwash@google.com>
> Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
> Signed-off-by: Jeroen de Borst <jeroendb@google.com>
> ---
>  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_compatibility);
> +
> 
>  struct gve_adminq_configure_device_resources {
>  	__be64 counter_array;
> @@ -345,6 +387,8 @@ union gve_adminq_command {
>  			struct gve_adminq_report_stats report_stats;
>  			struct gve_adminq_report_link_speed
> report_link_speed;
>  			struct gve_adminq_get_ptype_map
> get_ptype_map;
> +			struct gve_adminq_verify_driver_compatibility
> +				verify_driver_compatibility;
>  		};
>  	};
>  	u8 reserved[64];
> @@ -378,4 +422,7 @@ struct gve_ptype_lut;
>  int gve_adminq_get_ptype_map_dqo(struct gve_priv *priv,
>  				 struct gve_ptype_lut *ptype_lut);
> 
> +int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
> +					   u64 driver_info_len,
> +					   dma_addr_t driver_info_addr);
>  #endif /* _GVE_ADMINQ_H */
> diff --git a/drivers/net/gve/base/gve_osdep.h
> b/drivers/net/gve/base/gve_osdep.h
> index 7cb73002f4..a5efa8543e 100644
> --- a/drivers/net/gve/base/gve_osdep.h
> +++ b/drivers/net/gve/base/gve_osdep.h
> @@ -21,9 +21,14 @@
>  #include <rte_malloc.h>
>  #include <rte_memcpy.h>
>  #include <rte_memzone.h>
> +#include <rte_version.h>
> 
>  #include "../gve_logs.h"
> 
> +#ifdef __linux__
> +#include <sys/utsname.h>
> +#endif
> +
>  typedef uint8_t u8;
>  typedef uint16_t u16;
>  typedef uint32_t u32;
> @@ -69,6 +74,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];
> +};
> +
>  /* These macros are used to generate compilation errors if a
> struct/union
>   * is not exactly the correct length. It gives a divide by zero error if
>   * the struct/union is not of the correct size, otherwise it creates an
> @@ -79,6 +90,12 @@ typedef rte_iova_t dma_addr_t;
>  #define GVE_CHECK_UNION_LEN(n, X) enum
> gve_static_asset_enum_##X \
>  	{ gve_static_assert_##X = (n) / ((sizeof(union X) == (n)) ? 1 : 0) }
> 
> +#ifndef LINUX_VERSION_MAJOR
> +#define LINUX_VERSION_MAJOR (((LINUX_VERSION_CODE) >> 16) &
> 0xff)
> +#define LINUX_VERSION_SUBLEVEL (((LINUX_VERSION_CODE) >> 8) &
> 0xff)
> +#define LINUX_VERSION_PATCHLEVEL ((LINUX_VERSION_CODE) & 0xff)
> +#endif
> +
>  static __rte_always_inline u8
>  readb(volatile void *addr)
>  {
> @@ -156,4 +173,23 @@ gve_free_dma_mem(struct gve_dma_mem
> *mem)
>  	mem->pa = 0;
>  }
> 
> +static inline void
> +populate_driver_version_strings(struct os_version_string
> *os_version_str)
> +{
> +#ifdef __linux__
> +	struct utsname uts;
> +	if (uname(&uts) >= 0) {
> +		/* release */
> +		rte_strscpy(os_version_str->os_version_str1, uts.release,
> +			sizeof(os_version_str->os_version_str1));
> +		/* version */
> +		rte_strscpy(os_version_str->os_version_str2, uts.version,
> +			sizeof(os_version_str->os_version_str2));
> +	}
> +#else
> +	/* gVNIC is currently not supported on OS like FreeBSD */
> +	RTE_SET_USED(os_version_str);
> +#endif
> +}
> +
>  #endif /* _GVE_OSDEP_H_ */
> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
> index cf28a4a3b7..1d6e134fff 100644
> --- a/drivers/net/gve/gve_ethdev.c
> +++ b/drivers/net/gve/gve_ethdev.c
> @@ -5,21 +5,13 @@
>  #include "gve_ethdev.h"
>  #include "base/gve_adminq.h"
>  #include "base/gve_register.h"
> -
> -const char gve_version_str[] = GVE_VERSION;
> -static const char gve_version_prefix[] = GVE_VERSION_PREFIX;
> +#include "base/gve_osdep.h"
> +#include "gve_version.h"
> 
>  static void
>  gve_write_version(uint8_t *driver_version_register)
>  {
> -	const char *c = gve_version_prefix;
> -
> -	while (*c) {
> -		writeb(*c, driver_version_register);
> -		c++;
> -	}
> -
> -	c = gve_version_str;
> +	const char *c = gve_version_string();
>  	while (*c) {
>  		writeb(*c, driver_version_register);
>  		c++;
> @@ -265,6 +257,52 @@ gve_dev_close(struct rte_eth_dev *dev)
>  	return err;
>  }
> 
> +static int
> +gve_verify_driver_compatibility(struct gve_priv *priv)
> +{
> +	const struct rte_memzone *driver_info_bus;
> +	struct gve_driver_info *driver_info;
> +	int err;
> +
> +	driver_info_bus =
> rte_memzone_reserve_aligned("verify_driver_compatibility",
> +						      sizeof(struct
> gve_driver_info),
> +						      rte_socket_id(),
> +
> RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
> +	if (driver_info_bus == NULL) {
> +		PMD_DRV_LOG(ERR,
> +			    "Could not alloc memzone for driver
> compatibility");
> +		return -ENOMEM;
> +	}
> +	driver_info = (struct gve_driver_info *)driver_info_bus->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((struct os_version_string
> *)&driver_info->os_version_str1);
> +
> +	err = gve_adminq_verify_driver_compatibility(priv,
> +		sizeof(struct gve_driver_info),
> (dma_addr_t)driver_info_bus);
> +
> +	/* It's ok if the device doesn't support this */
> +	if (err == -EOPNOTSUPP)
> +		err = 0;
> +
> +	rte_memzone_free(driver_info_bus);
> +	return err;
> +}
> +
>  static int
>  gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info
> *dev_info)
>  {
> @@ -672,6 +710,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 = gve_verify_driver_compatibility(priv);
> +	if (err) {
> +		PMD_DRV_LOG(ERR, "Could not verify driver
> compatibility: err=%d", err);
> +		goto free_adminq;
> +	}
> 
>  	if (skip_describe_device)
>  		goto setup_device;
> diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
> index 42a02cf5d4..23ccff37d3 100644
> --- a/drivers/net/gve/gve_ethdev.h
> +++ b/drivers/net/gve/gve_ethdev.h
> @@ -222,7 +222,7 @@ struct gve_priv {
>  	uint32_t adminq_report_stats_cnt;
>  	uint32_t adminq_report_link_speed_cnt;
>  	uint32_t adminq_get_ptype_map_cnt;
> -
> +	uint32_t adminq_verify_driver_compatibility_cnt;
>  	volatile uint32_t state_flags;
> 
>  	/* Gvnic device link speed from hypervisor. */
> diff --git a/drivers/net/gve/gve_version.c
> b/drivers/net/gve/gve_version.c
> new file mode 100644
> index 0000000000..7eaa689d90
> --- /dev/null
> +++ b/drivers/net/gve/gve_version.c
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: MIT
> + * Google Virtual Ethernet (gve) driver
> + * Copyright (C) 2015-2023 Google, Inc.
> + */
> +#include "gve_version.h"
> +
> +const char *gve_version_string(void)
> +{
> +	static char gve_version[10];
> +	snprintf(gve_version, sizeof(gve_version), "%s%d.%d.%d",
> +		GVE_VERSION_PREFIX, GVE_VERSION_MAJOR,
> GVE_VERSION_MINOR,
> +		GVE_VERSION_SUB);
> +	return gve_version;
> +}
> diff --git a/drivers/net/gve/gve_version.h
> b/drivers/net/gve/gve_version.h
> new file mode 100644
> index 0000000000..9af6a00568
> --- /dev/null
> +++ b/drivers/net/gve/gve_version.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: MIT
> + * Google Virtual Ethernet (gve) driver
> + * Copyright (C) 2015-2023 Google, Inc.
> + */
> +
> +#ifndef _GVE_VERSION_H_
> +#define _GVE_VERSION_H_
> +
> +#include <rte_version.h>
> +
> +#define GVE_VERSION_PREFIX "DPDK-"
> +#define GVE_VERSION_MAJOR 1
> +#define GVE_VERSION_MINOR 0
> +#define GVE_VERSION_SUB 0
> +
> +#define DPDK_VERSION_MAJOR (100 * RTE_VER_YEAR +
> RTE_VER_MONTH)
> +#define DPDK_VERSION_MINOR RTE_VER_MINOR
> +#define DPDK_VERSION_SUB RTE_VER_RELEASE
> +
> +
> +const char *
> +gve_version_string(void);
> +
> +
> +#endif /* GVE_VERSION_H */
> diff --git a/drivers/net/gve/meson.build b/drivers/net/gve/meson.build
> index af0010c01c..a37c4bafa2 100644
> --- a/drivers/net/gve/meson.build
> +++ b/drivers/net/gve/meson.build
> @@ -12,5 +12,6 @@ sources = files(
>          'gve_rx.c',
>          'gve_tx.c',
>          'gve_ethdev.c',
> +        'gve_version.c',
>  )
>  includes += include_directories('base')
> --
> 2.40.0.634.g4ca3ef3211-goog
  
Ferruh Yigit April 14, 2023, 3:40 p.m. UTC | #2
On 4/14/2023 10:05 AM, Guo, Junfeng wrote:
> Hi Ferruh,
> 
> Here is the summary of the dependency of related patches:
> 
> P1: necessary headers update
>     https://patchwork.dpdk.org/project/dpdk/list/?series=27647&state=*
> P2: minor update for base code
>     https://patchwork.dpdk.org/project/dpdk/list/?series=27653&state=*
> P3: gve PMD enhancement
>     https://patchwork.dpdk.org/project/dpdk/list/?series=27687&state=*
> P4: add new AdminQ cmd to verify drv cap
>     https://patchwork.dpdk.org/project/dpdk/list/?series=27703&state=*
> 
> The merging order could be: P1 > P2 > P3 > P4, due to the dependency.
> Just to inform you about this here. Thanks for the review!
> 

Thanks Junfeng, this is useful.

Btw, we have a defined syntax for this [1], having it in commit log may
help automated CI to parse it and apply in correct order (when it is
fully supported in CI :) )


[1]
https://doc.dpdk.org/guides/contributing/patches.html#patch-dependencies

Depends-on: series-NNNNN ("Title of the series")
or
Depends-on: patch-NNNNN ("Title of the patch")

And I just recognized that syntax is not clear for multiple dependency
case, we should define it better.
  
Junfeng Guo April 18, 2023, 5:30 a.m. UTC | #3
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Friday, April 14, 2023 23:40
> To: Guo, Junfeng <junfeng.guo@intel.com>; Rushil Gupta
> <rushilg@google.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>
> Cc: joshwash@google.com; dev@dpdk.org; Jeroen de Borst
> <jeroendb@google.com>; Aaron Conole <aconole@redhat.com>;
> dpdklab <dpdklab@iol.unh.edu>
> Subject: Re: [PATCH] net/gve: Check whether the driver is compatible
> with the device presented.
> 
> On 4/14/2023 10:05 AM, Guo, Junfeng wrote:
> > Hi Ferruh,
> >
> > Here is the summary of the dependency of related patches:
> >
> > P1: necessary headers update
> >     https://patchwork.dpdk.org/project/dpdk/list/?series=27647&state=*
> > P2: minor update for base code
> >     https://patchwork.dpdk.org/project/dpdk/list/?series=27653&state=*
> > P3: gve PMD enhancement
> >     https://patchwork.dpdk.org/project/dpdk/list/?series=27687&state=*
> > P4: add new AdminQ cmd to verify drv cap
> >     https://patchwork.dpdk.org/project/dpdk/list/?series=27703&state=*
> >
> > The merging order could be: P1 > P2 > P3 > P4, due to the dependency.
> > Just to inform you about this here. Thanks for the review!
> >
> 
> Thanks Junfeng, this is useful.
> 
> Btw, we have a defined syntax for this [1], having it in commit log may
> help automated CI to parse it and apply in correct order (when it is
> fully supported in CI :) )
> 
> 
> [1]
> https://doc.dpdk.org/guides/contributing/patches.html#patch-
> dependencies
> 
> Depends-on: series-NNNNN ("Title of the series")
> or
> Depends-on: patch-NNNNN ("Title of the patch")
> 
> And I just recognized that syntax is not clear for multiple dependency
> case, we should define it better.

Sure, we will try this in the coming version patches.

Really a good solution for the patch dependencies!
Thanks for the enhancement!
  

Patch

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_compatibility);
+
 
 struct gve_adminq_configure_device_resources {
 	__be64 counter_array;
@@ -345,6 +387,8 @@  union gve_adminq_command {
 			struct gve_adminq_report_stats report_stats;
 			struct gve_adminq_report_link_speed report_link_speed;
 			struct gve_adminq_get_ptype_map get_ptype_map;
+			struct gve_adminq_verify_driver_compatibility
+				verify_driver_compatibility;
 		};
 	};
 	u8 reserved[64];
@@ -378,4 +422,7 @@  struct gve_ptype_lut;
 int gve_adminq_get_ptype_map_dqo(struct gve_priv *priv,
 				 struct gve_ptype_lut *ptype_lut);
 
+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
+					   u64 driver_info_len,
+					   dma_addr_t driver_info_addr);
 #endif /* _GVE_ADMINQ_H */
diff --git a/drivers/net/gve/base/gve_osdep.h b/drivers/net/gve/base/gve_osdep.h
index 7cb73002f4..a5efa8543e 100644
--- a/drivers/net/gve/base/gve_osdep.h
+++ b/drivers/net/gve/base/gve_osdep.h
@@ -21,9 +21,14 @@ 
 #include <rte_malloc.h>
 #include <rte_memcpy.h>
 #include <rte_memzone.h>
+#include <rte_version.h>
 
 #include "../gve_logs.h"
 
+#ifdef __linux__
+#include <sys/utsname.h>
+#endif
+
 typedef uint8_t u8;
 typedef uint16_t u16;
 typedef uint32_t u32;
@@ -69,6 +74,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];
+};
+
 /* These macros are used to generate compilation errors if a struct/union
  * is not exactly the correct length. It gives a divide by zero error if
  * the struct/union is not of the correct size, otherwise it creates an
@@ -79,6 +90,12 @@  typedef rte_iova_t dma_addr_t;
 #define GVE_CHECK_UNION_LEN(n, X) enum gve_static_asset_enum_##X \
 	{ gve_static_assert_##X = (n) / ((sizeof(union X) == (n)) ? 1 : 0) }
 
+#ifndef LINUX_VERSION_MAJOR
+#define LINUX_VERSION_MAJOR (((LINUX_VERSION_CODE) >> 16) & 0xff)
+#define LINUX_VERSION_SUBLEVEL (((LINUX_VERSION_CODE) >> 8) & 0xff)
+#define LINUX_VERSION_PATCHLEVEL ((LINUX_VERSION_CODE) & 0xff)
+#endif
+
 static __rte_always_inline u8
 readb(volatile void *addr)
 {
@@ -156,4 +173,23 @@  gve_free_dma_mem(struct gve_dma_mem *mem)
 	mem->pa = 0;
 }
 
+static inline void
+populate_driver_version_strings(struct os_version_string *os_version_str)
+{
+#ifdef __linux__
+	struct utsname uts;
+	if (uname(&uts) >= 0) {
+		/* release */
+		rte_strscpy(os_version_str->os_version_str1, uts.release,
+			sizeof(os_version_str->os_version_str1));
+		/* version */
+		rte_strscpy(os_version_str->os_version_str2, uts.version,
+			sizeof(os_version_str->os_version_str2));
+	}
+#else
+	/* gVNIC is currently not supported on OS like FreeBSD */
+	RTE_SET_USED(os_version_str);
+#endif
+}
+
 #endif /* _GVE_OSDEP_H_ */
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index cf28a4a3b7..1d6e134fff 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -5,21 +5,13 @@ 
 #include "gve_ethdev.h"
 #include "base/gve_adminq.h"
 #include "base/gve_register.h"
-
-const char gve_version_str[] = GVE_VERSION;
-static const char gve_version_prefix[] = GVE_VERSION_PREFIX;
+#include "base/gve_osdep.h"
+#include "gve_version.h"
 
 static void
 gve_write_version(uint8_t *driver_version_register)
 {
-	const char *c = gve_version_prefix;
-
-	while (*c) {
-		writeb(*c, driver_version_register);
-		c++;
-	}
-
-	c = gve_version_str;
+	const char *c = gve_version_string();
 	while (*c) {
 		writeb(*c, driver_version_register);
 		c++;
@@ -265,6 +257,52 @@  gve_dev_close(struct rte_eth_dev *dev)
 	return err;
 }
 
+static int
+gve_verify_driver_compatibility(struct gve_priv *priv)
+{
+	const struct rte_memzone *driver_info_bus;
+	struct gve_driver_info *driver_info;
+	int err;
+
+	driver_info_bus = rte_memzone_reserve_aligned("verify_driver_compatibility",
+						      sizeof(struct gve_driver_info),
+						      rte_socket_id(),
+						      RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
+	if (driver_info_bus == NULL) {
+		PMD_DRV_LOG(ERR,
+			    "Could not alloc memzone for driver compatibility");
+		return -ENOMEM;
+	}
+	driver_info = (struct gve_driver_info *)driver_info_bus->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((struct os_version_string *)&driver_info->os_version_str1);
+
+	err = gve_adminq_verify_driver_compatibility(priv,
+		sizeof(struct gve_driver_info), (dma_addr_t)driver_info_bus);
+
+	/* It's ok if the device doesn't support this */
+	if (err == -EOPNOTSUPP)
+		err = 0;
+
+	rte_memzone_free(driver_info_bus);
+	return err;
+}
+
 static int
 gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
@@ -672,6 +710,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 = gve_verify_driver_compatibility(priv);
+	if (err) {
+		PMD_DRV_LOG(ERR, "Could not verify driver compatibility: err=%d", err);
+		goto free_adminq;
+	}
 
 	if (skip_describe_device)
 		goto setup_device;
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 42a02cf5d4..23ccff37d3 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -222,7 +222,7 @@  struct gve_priv {
 	uint32_t adminq_report_stats_cnt;
 	uint32_t adminq_report_link_speed_cnt;
 	uint32_t adminq_get_ptype_map_cnt;
-
+	uint32_t adminq_verify_driver_compatibility_cnt;
 	volatile uint32_t state_flags;
 
 	/* Gvnic device link speed from hypervisor. */
diff --git a/drivers/net/gve/gve_version.c b/drivers/net/gve/gve_version.c
new file mode 100644
index 0000000000..7eaa689d90
--- /dev/null
+++ b/drivers/net/gve/gve_version.c
@@ -0,0 +1,14 @@ 
+/* SPDX-License-Identifier: MIT
+ * Google Virtual Ethernet (gve) driver
+ * Copyright (C) 2015-2023 Google, Inc.
+ */
+#include "gve_version.h"
+
+const char *gve_version_string(void)
+{
+	static char gve_version[10];
+	snprintf(gve_version, sizeof(gve_version), "%s%d.%d.%d",
+		GVE_VERSION_PREFIX, GVE_VERSION_MAJOR, GVE_VERSION_MINOR,
+		GVE_VERSION_SUB);
+	return gve_version;
+}
diff --git a/drivers/net/gve/gve_version.h b/drivers/net/gve/gve_version.h
new file mode 100644
index 0000000000..9af6a00568
--- /dev/null
+++ b/drivers/net/gve/gve_version.h
@@ -0,0 +1,25 @@ 
+/* SPDX-License-Identifier: MIT
+ * Google Virtual Ethernet (gve) driver
+ * Copyright (C) 2015-2023 Google, Inc.
+ */
+
+#ifndef _GVE_VERSION_H_
+#define _GVE_VERSION_H_
+
+#include <rte_version.h>
+
+#define GVE_VERSION_PREFIX "DPDK-"
+#define GVE_VERSION_MAJOR 1
+#define GVE_VERSION_MINOR 0
+#define GVE_VERSION_SUB 0
+
+#define DPDK_VERSION_MAJOR (100 * RTE_VER_YEAR + RTE_VER_MONTH)
+#define DPDK_VERSION_MINOR RTE_VER_MINOR
+#define DPDK_VERSION_SUB RTE_VER_RELEASE
+
+
+const char *
+gve_version_string(void);
+
+
+#endif /* GVE_VERSION_H */
diff --git a/drivers/net/gve/meson.build b/drivers/net/gve/meson.build
index af0010c01c..a37c4bafa2 100644
--- a/drivers/net/gve/meson.build
+++ b/drivers/net/gve/meson.build
@@ -12,5 +12,6 @@  sources = files(
         'gve_rx.c',
         'gve_tx.c',
         'gve_ethdev.c',
+        'gve_version.c',
 )
 includes += include_directories('base')