gve: mixes DPDK and Linux versions in compatibility check

gve: mixes DPDK and Linux versions in compatibility check

[Stephen Hemminger]

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),

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

[Rushil Gupta]

[-- Attachment #1: Type: text/plain, Size: 3196 bytes --]

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 <stephen@networkplumber.org>
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),
>

[-- Attachment #2: Type: text/html, Size: 4027 bytes --]