diff mbox series

net/gve: Update max_rx_pktlen to be based on MTU

Message ID 20231016205948.2252342-1-joshwash@google.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series net/gve: Update max_rx_pktlen to be based on MTU |

Checks

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

Commit Message

Joshua Washington Oct. 16, 2023, 8:59 p.m. UTC 
  Before this patch, max_rx_pktlen was always set to UINT16_MAX. This, in
conjunction with the MTU fix, causes problems with testpmd, as setting the
packet length with the --max-pkt-len flag causes the MTU to be set
higher than possible due to underflow.

As an example, setting --max-pkt-len=1460 (the default MTU on Google
Cloud VMs) causes testpmd to set the following:
    mtu = 1460 - eth_overhead,

where eth_overhead = dev->max_rx_pktlen - dev->max_mtu = 65535 - 1460.

Thus, mtu = 1460 - 65535 + 1460 = 2921 due to underflow.

Signed-off-by: Joshua Washington <joshwash@google.com>

Fixes: 030025b74202 ("net/gve: fix max MTU limit")
Cc: joshwash@google.com
---
 drivers/net/gve/gve_ethdev.c | 2 +-
 drivers/net/gve/gve_ethdev.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

Comments

Stephen Hemminger Oct. 16, 2023, 9:20 p.m. UTC | #1 
On Mon, 16 Oct 2023 13:59:48 -0700
Joshua Washington <joshwash@google.com> wrote:

> Before this patch, max_rx_pktlen was always set to UINT16_MAX. This, in
> conjunction with the MTU fix, causes problems with testpmd, as setting the
> packet length with the --max-pkt-len flag causes the MTU to be set
> higher than possible due to underflow.
> 
> As an example, setting --max-pkt-len=1460 (the default MTU on Google
> Cloud VMs) causes testpmd to set the following:
>     mtu = 1460 - eth_overhead,
> 
> where eth_overhead = dev->max_rx_pktlen - dev->max_mtu = 65535 - 1460.
> 
> Thus, mtu = 1460 - 65535 + 1460 = 2921 due to underflow.
> 
> Signed-off-by: Joshua Washington <joshwash@google.com>

This seems wrong and is not what other drivers do.
Other drivers give the absolute maximum in the dev_info so that
application can size accordingly.
Stephen Hemminger Oct. 16, 2023, 10:58 p.m. UTC | #2 
On Mon, 16 Oct 2023 13:59:48 -0700
Joshua Washington <joshwash@google.com> wrote:

> conjunction with the MTU fix, causes problems with testpmd, as setting the
> packet length with the --max-pkt-len flag causes the MTU to be set
> higher than possible due to underflow.
> 
> As an example, setting --max-pkt-len=1460 (the default MTU on Google
> Cloud VMs) causes testpmd to set the following:
>     mtu = 1460 - eth_overhead,
> 
> where eth_overhead = dev->max_rx_pktlen - dev->max_mtu = 65535 - 1460.
> 
> Thus, mtu = 1460 - 65535 + 1460 = 2921 due to underflow.
> 
> Signed-off-by: Joshua Washington <joshwash@google.com>
> 
> Fixes: 030025b74202 ("net/gve: fix max MTU limit")
> Cc: joshwash@google.com

Never mind, previous comment.
You are correctly doing the inverse of this common code pattern.

In testpmd:
static uint32_t
eth_dev_get_overhead_len(uint32_t max_rx_pktlen, uint16_t max_mtu)
{
	uint32_t overhead_len;

	if (max_mtu != UINT16_MAX && max_rx_pktlen > max_mtu)
		overhead_len = max_rx_pktlen - max_mtu;
	else
		overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;

	return overhead_len;
}
Ferruh Yigit Oct. 26, 2023, 5:38 p.m. UTC | #3 
On 10/16/2023 9:59 PM, Joshua Washington wrote:
> Before this patch, max_rx_pktlen was always set to UINT16_MAX. This, in
> conjunction with the MTU fix, causes problems with testpmd, as setting the
> packet length with the --max-pkt-len flag causes the MTU to be set
> higher than possible due to underflow.
> 
> As an example, setting --max-pkt-len=1460 (the default MTU on Google
> Cloud VMs) causes testpmd to set the following:
>     mtu = 1460 - eth_overhead,
> 
> where eth_overhead = dev->max_rx_pktlen - dev->max_mtu = 65535 - 1460.
> 
> Thus, mtu = 1460 - 65535 + 1460 = 2921 due to underflow.
> 
> Signed-off-by: Joshua Washington <joshwash@google.com>
> 
> Fixes: 030025b74202 ("net/gve: fix max MTU limit")
> Cc: joshwash@google.com
> 

   Cc: stable@dpdk.org

Applied to dpdk-next-net/main, thanks.
diff mbox series

Patch

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index b441f96623..eb3bc7e151 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -297,7 +297,7 @@  gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_queues = priv->max_nb_rxq;
 	dev_info->max_tx_queues = priv->max_nb_txq;
 	dev_info->min_rx_bufsize = GVE_MIN_BUF_SIZE;
-	dev_info->max_rx_pktlen = GVE_MAX_RX_PKTLEN;
+	dev_info->max_rx_pktlen = priv->max_mtu + RTE_ETHER_HDR_LEN;
 	dev_info->max_mtu = priv->max_mtu;
 	dev_info->min_mtu = RTE_ETHER_MIN_MTU;
 
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 1cba282128..755ee8ad15 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -21,7 +21,6 @@ 
 #define GVE_TX_MAX_FREE_SZ          512
 
 #define GVE_MIN_BUF_SIZE	    1024
-#define GVE_MAX_RX_PKTLEN	    65535
 
 #define GVE_TX_CKSUM_OFFLOAD_MASK (		\
 		RTE_MBUF_F_TX_L4_MASK  |	\