From: Rushil Gupta <rushilg@google.com>
To: ferruh.yigit@amd.com, qi.z.zhang@intel.com, jingjing.wu@intel.com
Cc: junfeng.guo@intel.com, joshwash@google.com, dev@dpdk.org,
Jeroen de Borst <jeroendb@google.com>
Subject: Re: [PATCH] net/gve: check driver compatibility
Date: Fri, 19 May 2023 00:41:31 -0700 [thread overview]
Message-ID: <CANzqiF4O2YdP2PRo6tDrjC-45=jvvC+9PFXezttPkSNFvLcK8w@mail.gmail.com> (raw)
In-Reply-To: <20230519072600.1444309-1-rushilg@google.com>
Hi Ferruh
I have updated the latest patch here:
http://patchwork.dpdk.org/project/dpdk/patch/20230519072600.1444309-1-rushilg@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: 0000: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 0000: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 <rushilg@google.com> 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 <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 | 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 +384,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 +419,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 abf3d379ae..5e8ae1eac6 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;
> @@ -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];
> +};
> +
> /* 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
> @@ -160,4 +171,17 @@ 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 1dcb3b3a01..71d131fb41 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++;
> @@ -245,6 +237,48 @@ gve_dev_close(struct rte_eth_dev *dev)
> return err;
> }
>
> +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);
> + if (driver_info == NULL) {
> + PMD_DRV_LOG(ERR,
> + "Could not alloc for verify driver compatibility");
> + 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);
> +
> + 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)
> {
> @@ -679,6 +713,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 cd62debd22..c9bcfa553c 100644
> --- a/drivers/net/gve/gve_ethdev.h
> +++ b/drivers/net/gve/gve_ethdev.h
> @@ -268,7 +268,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..5fe34dc179
> --- /dev/null
> +++ b/drivers/net/gve/gve_version.c
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2015-2023 Google, Inc.
> + */
> +#include "gve_version.h"
> +
> +const char *gve_version_string(void)
> +{
> + static char gve_version[20];
> + 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..4dd998dca1
> --- /dev/null
> +++ b/drivers/net/gve/gve_version.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * 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 c9d87903f9..61d195009c 100644
> --- a/drivers/net/gve/meson.build
> +++ b/drivers/net/gve/meson.build
> @@ -1,9 +1,9 @@
> # SPDX-License-Identifier: BSD-3-Clause
> # Copyright(C) 2022 Intel Corporation
>
> -if is_windows
> +if not is_linux
> build = false
> - reason = 'not supported on Windows'
> + reason = 'only supported on Linux'
> subdir_done()
> endif
>
> @@ -14,5 +14,6 @@ sources = files(
> 'gve_rx_dqo.c',
> 'gve_tx_dqo.c',
> 'gve_ethdev.c',
> + 'gve_version.c',
> )
> includes += include_directories('base')
> --
> 2.40.1.698.g37aff9b760-goog
>
next prev parent reply other threads:[~2023-05-19 7:41 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-14 5:14 [PATCH] net/gve: Check whether the driver is compatible with the device presented Rushil Gupta
2023-04-14 9:05 ` Guo, Junfeng
2023-04-14 15:40 ` Ferruh Yigit
2023-04-18 5:30 ` Guo, Junfeng
2023-04-26 21:37 ` Rushil Gupta
2023-05-04 15:06 ` Ferruh Yigit
2023-05-08 6:23 ` Rushil Gupta
2023-05-08 16:00 ` Ferruh Yigit
2023-05-08 19:22 ` Rushil Gupta
2023-05-08 19:15 ` [v2] net/gve: check driver compatibility Rushil Gupta
2023-05-17 16:58 ` Ferruh Yigit
2023-05-18 17:41 ` Rushil Gupta
2023-05-19 7:15 ` [v3] " Rushil Gupta
2023-05-19 7:23 ` [PATCH] " Rushil Gupta
2023-05-19 7:26 ` Rushil Gupta
2023-05-19 7:41 ` Rushil Gupta [this message]
2023-05-19 10:04 ` Ferruh Yigit
2023-05-19 16:15 ` Rushil Gupta
2023-05-19 19:54 ` [v4] " Rushil Gupta
2023-05-19 20:46 ` Rushil Gupta
2023-05-19 20:56 ` Stephen Hemminger
2023-05-20 16:44 ` Rushil Gupta
2023-05-22 8:52 ` Ferruh Yigit
2023-05-22 15:45 ` Rushil Gupta
2023-05-23 10:20 ` Ferruh Yigit
2023-05-23 10:21 ` Ferruh Yigit
2023-05-24 17:14 ` Rushil Gupta
2023-05-31 16:40 ` Rushil Gupta
2023-05-24 17:13 ` [v5] " Rushil Gupta
2023-05-31 17:04 ` Ferruh Yigit
2023-05-18 17:40 ` [v3] " Rushil Gupta
2023-05-08 19:14 ` [v2] " Rushil Gupta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANzqiF4O2YdP2PRo6tDrjC-45=jvvC+9PFXezttPkSNFvLcK8w@mail.gmail.com' \
--to=rushilg@google.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=jeroendb@google.com \
--cc=jingjing.wu@intel.com \
--cc=joshwash@google.com \
--cc=junfeng.guo@intel.com \
--cc=qi.z.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).