[PATCH] net/gve: Update max_rx_pktlen to be based on MTU

[PATCH] net/gve: Update max_rx_pktlen to be based on MTU

[Joshua Washington]

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

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  |	\
-- 
2.42.0.655.g421f12c284-goog

Re: [PATCH] net/gve: Update max_rx_pktlen to be based on MTU

[Stephen Hemminger]

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.

Re: [PATCH] net/gve: Update max_rx_pktlen to be based on MTU

[Stephen Hemminger]

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;
}

Re: [PATCH] net/gve: Update max_rx_pktlen to be based on MTU

[Ferruh Yigit]

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.