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 A2BB8431E7 for ; Tue, 24 Oct 2023 00:01:54 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9AE15402CB; Tue, 24 Oct 2023 00:01:54 +0200 (CEST) Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by mails.dpdk.org (Postfix) with ESMTP id EEBC440298 for ; Tue, 24 Oct 2023 00:01:40 +0200 (CEST) Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-4078fe6a063so10775e9.1 for ; Mon, 23 Oct 2023 15:01:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1698098500; x=1698703300; 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=A9VHPVEHXMkHIZVEzbXFk45R6MeW2iL+xWGTnTH1pLM=; b=R175lrxrogb9baN/ayU569JX3swowP2/pqK1mERJZ0hWa/OEAa/BxuC/52O5ZLX4BG AQ0AvYW/Zv60jFgsETcENa3X8kuILe58jOKxmk6WNdmWWZrZWEChklI313NqS8u/XTdr ZRl/MYpt594VjlUj2E5Dfno+lOJfQobv+W70B2oNmTRHDNORyHiTKpHvpTMPkYsJf3ya 4cAAP5nOL4v/VnPuFbc43RF6beBPqDaXqKnK0b9mPiCfJg4U7q4zmO/KCOnRw4VrzqB6 89L+3iWxQfqK3ubWKrdywPaFXYqDLf4gdw/3zzMuaxfy8Uut9B7mdoh5QLi+8IpwNsK9 ENig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698098500; x=1698703300; 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=A9VHPVEHXMkHIZVEzbXFk45R6MeW2iL+xWGTnTH1pLM=; b=Kj6JwJSf2qn0oZt0sks0tzmsTxP9QOl76+sUl3u2YUO0petWAMZrxGE34xLtT4Wdcx 4fAE48QnjEUtBKeYIVnseJmKritKCOqI0i82FpCPy3RmUlC3Sy4nCh1gjcOpTkykFhdv syeeHRyXkMu7CydbgV/n+/LOpim3l4hF/040jg1J2sSGMfKHqc1kb8AhNM76gDYvml5E Js7XjfWfC2Ls9tmmKc6JJ3sZSh4A/jNULp2PUTY1uCWLys49cMJANaNc1JUcboh0uz1o nE245PcjNgzuQQDHppDcfqyZmjKCj32utiQDbuCtlpCINGrfSGefuJOYsDs0iysLymi1 sdjg== X-Gm-Message-State: AOJu0YxKj+eweVnk+mxpibwcEnDIUCPIDkJEfOZFSjVhS3JI7thdsrkW CAI7dLucX3q1io34irCKopsWdGc6lxcIpnYZUIOS3fp95lyRRNg0VBZzwsBG X-Google-Smtp-Source: AGHT+IHrdy0bgeTGLDit557cuNEj07HLPpgZ1KEwe/1pSKMaglu73dJELT/6TNlrsHtArwbdajsVMMLjmLzEYQdb7yI= X-Received: by 2002:a05:600c:510d:b0:405:38d1:e146 with SMTP id o13-20020a05600c510d00b0040538d1e146mr44320wms.4.1698098500310; Mon, 23 Oct 2023 15:01:40 -0700 (PDT) MIME-Version: 1.0 References: <20231022142250.10324-1-xuemingl@nvidia.com> <20231022142250.10324-37-xuemingl@nvidia.com> In-Reply-To: <20231022142250.10324-37-xuemingl@nvidia.com> From: Joshua Washington Date: Mon, 23 Oct 2023 15:01:29 -0700 Message-ID: Subject: Re: patch 'net/gve: fix max MTU limit' has been queued to stable release 22.11.4 To: Xueming Li Cc: dpdk stable Content-Type: multipart/alternative; boundary="000000000000e815800608695d46" X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org --000000000000e815800608695d46 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hello, This change, while having the correct behavior, breaks the testpmd program, as the default MTU for testpmd is 1500B, which is higher than the default Google Cloud MTU. When specifying the max packet length, which dictates the MTU set by testpmd, the max packet length field set by the GVE driver is incorrect, causing testpmd to attempt to set the MTU to an invalid value due to underflow. This is fixed in https://patchwork.dpdk.org/project/dpdk/patch/20231016205948.2252342-1-josh= wash@google.com/, and I believe these should go in together to ensure there is no point where testpmd is unusable for the GVE driver. Thanks, Josh On Sun, Oct 22, 2023 at 7:26=E2=80=AFAM Xueming Li wr= ote: > Hi, > > FYI, your patch has been queued to stable release 22.11.4 > > Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. > It will be pushed if I get no objections before 11/15/23. So please > shout if anyone has objections. > > Also note that after the patch there's a diff of the upstream commit vs t= he > patch applied to the branch. This will indicate if there was any rebasing > needed to apply to the stable branch. If there were code changes for > rebasing > (ie: not only metadata diffs), please double check that the rebase was > correctly done. > > Queued patches are on a temporary branch at: > https://git.dpdk.org/dpdk-stable/log/?h=3D22.11-staging > > This queued commit can be viewed at: > > https://git.dpdk.org/dpdk-stable/commit/?h=3D22.11-staging&id=3D8ef9e184c= a867c0ed9aed5728364aed6e7dd47e3 > > Thanks. > > Xueming Li > > --- > From 8ef9e184ca867c0ed9aed5728364aed6e7dd47e3 Mon Sep 17 00:00:00 2001 > From: Joshua Washington > Date: Fri, 29 Sep 2023 13:38:25 -0700 > Subject: [PATCH] net/gve: fix max MTU limit > Cc: Xueming Li > > [ upstream commit 030025b74202896e85a72b1e75049866800dd3f7 ] > > This patch corrects the MTU setting behavior in the GVE DPDK driver to > remove the artificial upper limit of RTE_ETHER_MTU. Instead, the max MTU > is dictated by the default value of the MTU that the device sends during > initialization, which will always be the maximum supported MTU. > > Fixes: 71dea04cdf9a ("net/gve: support device info and configure") > > Signed-off-by: Joshua Washington > --- > drivers/net/gve/gve_ethdev.c | 5 +++-- > drivers/net/gve/gve_ethdev.h | 3 --- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c > index e357f16e16..806f45b0a7 100644 > --- a/drivers/net/gve/gve_ethdev.c > +++ b/drivers/net/gve/gve_ethdev.c > @@ -5,6 +5,7 @@ > #include "gve_ethdev.h" > #include "base/gve_adminq.h" > #include "base/gve_register.h" > +#include "rte_ether.h" > > const char gve_version_str[] =3D GVE_VERSION; > static const char gve_version_prefix[] =3D GVE_VERSION_PREFIX; > @@ -276,8 +277,8 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct > rte_eth_dev_info *dev_info) > dev_info->max_tx_queues =3D priv->max_nb_txq; > dev_info->min_rx_bufsize =3D GVE_MIN_BUF_SIZE; > dev_info->max_rx_pktlen =3D GVE_MAX_RX_PKTLEN; > - dev_info->max_mtu =3D GVE_MAX_MTU; > - dev_info->min_mtu =3D GVE_MIN_MTU; > + dev_info->max_mtu =3D priv->max_mtu; > + dev_info->min_mtu =3D RTE_ETHER_MIN_MTU; > > dev_info->rx_offload_capa =3D 0; > dev_info->tx_offload_capa =3D > diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h > index 235e55899e..975318938d 100644 > --- a/drivers/net/gve/gve_ethdev.h > +++ b/drivers/net/gve/gve_ethdev.h > @@ -31,9 +31,6 @@ > #define GVE_MIN_BUF_SIZE 1024 > #define GVE_MAX_RX_PKTLEN 65535 > > -#define GVE_MAX_MTU RTE_ETHER_MTU > -#define GVE_MIN_MTU RTE_ETHER_MIN_MTU > - > /* A list of pages registered with the device during setup and used by a > queue > * as buffers > */ > -- > 2.25.1 > > --- > Diff of the applied patch vs upstream commit (please double-check if > non-empty: > --- > --- - 2023-10-22 22:17:35.768883300 +0800 > +++ 0036-net-gve-fix-max-MTU-limit.patch 2023-10-22 > 22:17:34.206723700 +0800 > @@ -1 +1 @@ > -From 030025b74202896e85a72b1e75049866800dd3f7 Mon Sep 17 00:00:00 2001 > +From 8ef9e184ca867c0ed9aed5728364aed6e7dd47e3 Mon Sep 17 00:00:00 2001 > @@ -4,0 +5,3 @@ > +Cc: Xueming Li > + > +[ upstream commit 030025b74202896e85a72b1e75049866800dd3f7 ] > @@ -12 +14,0 @@ > -Cc: stable@dpdk.org > @@ -21 +23 @@ > -index 9b25f3036b..b441f96623 100644 > +index e357f16e16..806f45b0a7 100644 > @@ -24 +26,3 @@ > -@@ -7,6 +7,7 @@ > +@@ -5,6 +5,7 @@ > + #include "gve_ethdev.h" > + #include "base/gve_adminq.h" > @@ -26,2 +29,0 @@ > - #include "base/gve_osdep.h" > - #include "gve_version.h" > @@ -30,3 +32,3 @@ > - static void > - gve_write_version(uint8_t *driver_version_register) > -@@ -297,8 +298,8 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct > rte_eth_dev_info *dev_info) > + const char gve_version_str[] =3D GVE_VERSION; > + static const char gve_version_prefix[] =3D GVE_VERSION_PREFIX; > +@@ -276,8 +277,8 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct > rte_eth_dev_info *dev_info) > @@ -44 +46 @@ > -index ca94a09a2f..1cba282128 100644 > +index 235e55899e..975318938d 100644 > @@ -47 +49 @@ > -@@ -23,9 +23,6 @@ > +@@ -31,9 +31,6 @@ > @@ -54,3 +56,3 @@ > - #define GVE_TX_CKSUM_OFFLOAD_MASK ( \ > - RTE_MBUF_F_TX_L4_MASK | \ > - RTE_MBUF_F_TX_TCP_SEG) > + /* A list of pages registered with the device during setup and used by = a > queue > + * as buffers > + */ > --=20 Joshua Washington | Software Engineer | joshwash@google.com | (414) 366-442= 3 --000000000000e815800608695d46 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hello,

This change, wh= ile having the correct behavior, breaks the testpmd program, as the default= MTU for testpmd is 1500B, which is higher than the default Google Cloud MT= U. When specifying the max packet length, which dictates the MTU set by tes= tpmd, the max packet length field set by the GVE driver=C2=A0is incorrect, = causing testpmd to attempt to set the MTU to an invalid value due to underf= low. This is fixed in=C2=A0h= ttps://patchwork.dpdk.org/project/dpdk/patch/20231016205948.2252342-1-joshw= ash@google.com/, and I believe these should go in together to ensure th= ere is no point where testpmd is unusable for the GVE driver.
Thanks,
Josh

On Sun, Oct 22, 2023 at 7:26=E2=80= =AFAM Xueming Li <xuemingl@nvidia= .com> wrote:
Hi,

FYI, your patch has been queued to stable release 22.11.4

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stab= le yet.
It will be pushed if I get no objections before 11/15/23. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs= the
patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasi= ng
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://git.dpdk.org/dpdk-stable/log/?h=3D22.= 11-staging

This queued commit can be viewed at:
https://git.dpdk.org/dpdk-stable/commit/?h=3D22.11-staging&id= =3D8ef9e184ca867c0ed9aed5728364aed6e7dd47e3

Thanks.

Xueming Li <xue= mingl@nvidia.com>

---
>From 8ef9e184ca867c0ed9aed5728364aed6e7dd47e3 Mon Sep 17 00:00:00 2001
From: Joshua Washington <joshwash@google.com>
Date: Fri, 29 Sep 2023 13:38:25 -0700
Subject: [PATCH] net/gve: fix max MTU limit
Cc: Xueming Li <xuemingl@nvidia.com>

[ upstream commit 030025b74202896e85a72b1e75049866800dd3f7 ]

This patch corrects the MTU setting behavior in the GVE DPDK driver to
remove the artificial upper limit of RTE_ETHER_MTU. Instead, the max MTU is dictated by the default value of the MTU that the device sends during initialization, which will always be the maximum supported MTU.

Fixes: 71dea04cdf9a ("net/gve: support device info and configure"= )

Signed-off-by: Joshua Washington <joshwash@google.com>
---
=C2=A0drivers/net/gve/gve_ethdev.c | 5 +++--
=C2=A0drivers/net/gve/gve_ethdev.h | 3 ---
=C2=A02 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c index e357f16e16..806f45b0a7 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -5,6 +5,7 @@
=C2=A0#include "gve_ethdev.h"
=C2=A0#include "base/gve_adminq.h"
=C2=A0#include "base/gve_register.h"
+#include "rte_ether.h"

=C2=A0const char gve_version_str[] =3D GVE_VERSION;
=C2=A0static const char gve_version_prefix[] =3D GVE_VERSION_PREFIX;
@@ -276,8 +277,8 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct rte_et= h_dev_info *dev_info)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_info->max_tx_queues =3D priv->max_nb_= txq;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_info->min_rx_bufsize =3D GVE_MIN_BUF_SIZ= E;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_info->max_rx_pktlen =3D GVE_MAX_RX_PKTLE= N;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0dev_info->max_mtu =3D GVE_MAX_MTU;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0dev_info->min_mtu =3D GVE_MIN_MTU;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0dev_info->max_mtu =3D priv->max_mtu;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0dev_info->min_mtu =3D RTE_ETHER_MIN_MTU;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_info->rx_offload_capa =3D 0;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_info->tx_offload_capa =3D
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h index 235e55899e..975318938d 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -31,9 +31,6 @@
=C2=A0#define GVE_MIN_BUF_SIZE=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A01024=
=C2=A0#define GVE_MAX_RX_PKTLEN=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 65535

-#define GVE_MAX_MTU=C2=A0 =C2=A0 RTE_ETHER_MTU
-#define GVE_MIN_MTU=C2=A0 =C2=A0 RTE_ETHER_MIN_MTU
-
=C2=A0/* A list of pages registered with the device during setup and used b= y a queue
=C2=A0 * as buffers
=C2=A0 */
--
2.25.1

---
=C2=A0 Diff of the applied patch vs upstream commit (please double-check if= non-empty:
---
--- -=C2=A0 =C2=A02023-10-22 22:17:35.768883300 +0800
+++ 0036-net-gve-fix-max-MTU-limit.patch=C2=A0 =C2=A0 =C2=A0 =C2=A0 2023-10= -22 22:17:34.206723700 +0800
@@ -1 +1 @@
-From 030025b74202896e85a72b1e75049866800dd3f7 Mon Sep 17 00:00:00 2001
+From 8ef9e184ca867c0ed9aed5728364aed6e7dd47e3 Mon Sep 17 00:00:00 2001
@@ -4,0 +5,3 @@
+Cc: Xueming Li <xuemingl@nvidia.com>
+
+[ upstream commit 030025b74202896e85a72b1e75049866800dd3f7 ]
@@ -12 +14,0 @@
-Cc: stable@dpdk.org
@@ -21 +23 @@
-index 9b25f3036b..b441f96623 100644
+index e357f16e16..806f45b0a7 100644
@@ -24 +26,3 @@
-@@ -7,6 +7,7 @@
+@@ -5,6 +5,7 @@
+ #include "gve_ethdev.h"
+ #include "base/gve_adminq.h"
@@ -26,2 +29,0 @@
- #include "base/gve_osdep.h"
- #include "gve_version.h"
@@ -30,3 +32,3 @@
- static void
- gve_write_version(uint8_t *driver_version_register)
-@@ -297,8 +298,8 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct rte_e= th_dev_info *dev_info)
+ const char gve_version_str[] =3D GVE_VERSION;
+ static const char gve_version_prefix[] =3D GVE_VERSION_PREFIX;
+@@ -276,8 +277,8 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct rte_e= th_dev_info *dev_info)
@@ -44 +46 @@
-index ca94a09a2f..1cba282128 100644
+index 235e55899e..975318938d 100644
@@ -47 +49 @@
-@@ -23,9 +23,6 @@
+@@ -31,9 +31,6 @@
@@ -54,3 +56,3 @@
- #define GVE_TX_CKSUM_OFFLOAD_MASK (=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=A0RTE_MBUF_F_TX_L4_MA= SK=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=A0RTE_MBUF_F_TX_TCP_S= EG)
+ /* A list of pages registered with the device during setup and used by a = queue
+=C2=A0 * as buffers
+=C2=A0 */


--

=C2=A0
<= /div> --000000000000e815800608695d46--