From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E0F3543909; Sat, 20 Jan 2024 08:08:14 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 61F42402AE; Sat, 20 Jan 2024 08:08:14 +0100 (CET) Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) by mails.dpdk.org (Postfix) with ESMTP id 932CA402AA for ; Sat, 20 Jan 2024 08:08:13 +0100 (CET) Received: by mail-ej1-f47.google.com with SMTP id a640c23a62f3a-a29c4bbb2f4so143759666b.1 for ; Fri, 19 Jan 2024 23:08:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1705734493; x=1706339293; darn=dpdk.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=LyI2EHmweEwW8d7jU9Rjk41khe58I8VGj2MflsEsRpw=; b=yW71duWqvKje2qL3uEVb1ni9kzCmbdpw4uNBSriTYqgs0AtbjLi8q0cca7fKy/oh/W s871aSpmzfs+S9mB30yw16TOyhr/db7gw0+4YhDmAFB0y9Ko5BJ1enRovJf3vtwXYF4E oNBqYd8QMpd+UAW85BYSLGeLX5b/KcFasg3Kra2Cp0NRSskqWS4+ZFNm4/LOC1FESjqM VGHF6M59XQ0T+Xjt9ZmEefEcxMq5a2cm5PRAzon6X8bPn9RuuSOdeWZkDccPNksLa33T dSB6fJvt3b4UgbFci/ZUtk0suJNF651Yud5oEC71VclmcaPj95t37MrxgpwnBtsIrdde qGBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705734493; x=1706339293; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=LyI2EHmweEwW8d7jU9Rjk41khe58I8VGj2MflsEsRpw=; b=TJP96163Dl8g0eQmxdABBM/32ZRuyRohkWYtk5ez+f06jM2MK4ZImmSnAB4uPhftVT qLCrc5AJnn9ZfV6gUzRNBLm8dBQTiEBSH4LDJ4AKOnYcBBGW5M7p1cKtTRC9OXrPMc5G jGEmepogfOoSCQHBst/XOVTK8+qxpXmivOXml5tn1E0iAzY30DCzqxAYkFmC4c/fXkp1 8I1MGa0DBaMDoPaLo4IEiOWMdE1ZmEh5fe1PjvoWGCcLY5I0N7pAkM7LwJEiblgYSNZc mqatQRUtSyj6tvRPhDnU4xzFPno6wIx0J5BpVr4Q+X+ezXpWc+Xqnc2UpSXT1gwYFLFm pAag== X-Gm-Message-State: AOJu0YzBxEGhXox3PPQT1gq1prgYx4602kvS9GUlDgTddf0iT0VCxgHB vM6mH4PAozTUCTxxkweLahvCLg6hB5g2QbPwjSpYcA57n0rJqY3KOFj26h6+JNzcBVd+SkbIRPa omzDq0OZwtGzQZibUi6GmvrUUFRuFiluFdenp X-Google-Smtp-Source: AGHT+IEtmyA81229PpopPJNzxvb5bgtInZEYb1bVDM+pZGtS0g94WmZchhVeBk3Gypftg8Q96ZmLN2OXhe1VSnsCpHw= X-Received: by 2002:a17:906:b88c:b0:a2c:cdd7:bdee with SMTP id hb12-20020a170906b88c00b00a2ccdd7bdeemr506791ejb.132.1705734492903; Fri, 19 Jan 2024 23:08:12 -0800 (PST) MIME-Version: 1.0 References: <20240111120841.136b1e7c@hermes.local> In-Reply-To: <20240111120841.136b1e7c@hermes.local> From: Rushil Gupta Date: Sat, 20 Jan 2024 12:38:00 +0530 Message-ID: Subject: Re: gve: mixes DPDK and Linux versions in compatibility check To: Stephen Hemminger Cc: Joshua Washington , Junfeng Guo , Jeroen de Borst , dev@dpdk.org Content-Type: multipart/alternative; boundary="0000000000008861d0060f5b426c" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --0000000000008861d0060f5b426c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=E2=80=AFAM Stephen Hemminger wrote: > This seems wrong: > *driver_info =3D (struct gve_driver_info) { > .os_type =3D 5, /* DPDK */ > .driver_major =3D GVE_VERSION_MAJOR, > .driver_minor =3D GVE_VERSION_MINOR, > .driver_sub =3D GVE_VERSION_SUB, > .os_version_major =3D cpu_to_be32(DPDK_VERSION_MAJOR), > .os_version_minor =3D cpu_to_be32(DPDK_VERSION_MINOR), > .os_version_sub =3D cpu_to_be32(DPDK_VERSION_SUB), > .driver_capability_flags =3D { > 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 =3D 0; > } > > -static inline void > -populate_driver_version_strings(char *str1, char *str2) > -{ > - struct utsname uts; > - if (uname(&uts) >=3D 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 =3D gve_adminq_verify_driver_compatibility(priv, > sizeof(struct gve_driver_info), > --0000000000008861d0060f5b426c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Stephen
W= e 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 an= d the adminq is working as intended.

On Fri, Jan 12, 2024, 1:38= =E2=80=AFAM Stephen Hemminger <stephen@networkplumber.org> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex">This seems wrong:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 *driver_info =3D (struct gve_driver_info) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .os_type =3D 5, /* = DPDK */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .driver_major =3D G= VE_VERSION_MAJOR,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .driver_minor =3D G= VE_VERSION_MINOR,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .driver_sub =3D GVE= _VERSION_SUB,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .os_version_major = =3D cpu_to_be32(DPDK_VERSION_MAJOR),
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .os_version_minor = =3D cpu_to_be32(DPDK_VERSION_MINOR),
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .os_version_sub =3D= cpu_to_be32(DPDK_VERSION_SUB),
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .driver_capability_= flags =3D {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS1),
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS2),
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS3),
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS4),
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 },
=C2=A0 =C2=A0 =C2=A0 =C2=A0 };

=C2=A0 =C2=A0 =C2=A0 =C2=A0 populate_driver_version_strings((char *)driver_= info->os_version_str1,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 (char *)driver_info->os_version_str2);

The numeric values os_version_major, os_version_minor use DPDK version whic= h
is good.=C2=A0 But the populate_driver_version_strings gets the Linux kerne= l
version number which is problematic.=C2=A0 Looks like a bug to me.

Also technically, the Linux kernel version is not the same as the OS releas= e.

Shouldn't it be more like:

diff --git a/drivers/net/gve/base/gve_osdep.h b/drivers/net/gve/base/gve_os= dep.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)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 mem->pa =3D 0;
=C2=A0}

-static inline void
-populate_driver_version_strings(char *str1, char *str2)
-{
-=C2=A0 =C2=A0 =C2=A0 =C2=A0struct utsname uts;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0if (uname(&uts) >=3D 0) {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* release */
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rte_strscpy(str1, u= ts.release,
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0OS_VERSION_STRLEN);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* version */
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rte_strscpy(str2, u= ts.version,
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0OS_VERSION_STRLEN);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0}
-}
=C2=A0#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)<= br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 },
=C2=A0 =C2=A0 =C2=A0 =C2=A0 };

-=C2=A0 =C2=A0 =C2=A0 =C2=A0populate_driver_version_strings((char *)driver_= info->os_version_str1,
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0(char *)driver_info->os_version_str2);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0rte_strscpy(driver_info.os_version_str1, OS_VER= SION_STRLEN,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rte_v= ersion());

=C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D gve_adminq_verify_driver_compatibility(= priv,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeof(struct gve_d= river_info),
--0000000000008861d0060f5b426c--