[PATCH net 0/3] Fix MTU related issues

[PATCH net 0/3] Fix MTU related issues

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

As suggested by Alexander Lobakin before, I follows the behavior when
computing MTU size as other drivers do. Adding a second VLAN HLEN could
solve the issue. I have some i40e and ixgbe drivers running on the servers,
so I choose to fix both of them.

Besides, I resent the first patch because the third patch is wrote based
on the first patch. It's relatively for maintainers to handle the
patchset, I think.

Jason Xing (3):
  ixgbe: allow to increase MTU to 3K with XDP enabled
  i40e: add double of VLAN header when computing the max MTU
  ixgbe: add double of VLAN header when computing the max MTU

 drivers/net/ethernet/intel/i40e/i40e.h        |  2 ++
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  2 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 28 +++++++++++--------
 4 files changed, 22 insertions(+), 12 deletions(-)

-- 
2.37.3

[PATCH net v3 1/3] ixgbe: allow to increase MTU to 3K with XDP enabled

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Recently I encountered one case where I cannot increase the MTU size
directly from 1500 to a much bigger value with XDP enabled if the
server is equipped with IXGBE card, which happened on thousands of
servers in production environment. After appling the current patch,
we can set the maximum MTU size to 3K.

This patch follows the behavior of changing MTU as i40e/ice does.

Referrences:
[1] commit 23b44513c3e6 ("ice: allow 3k MTU for XDP")
[2] commit 0c8493d90b6b ("i40e: add XDP support for pass and drop actions")

Fixes: fabf1bce103a ("ixgbe: Prevent unsupported configurations with XDP")
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
v3:
1) modify the titile and body message.

v2:
1) change the commit message.
2) modify the logic when changing MTU size suggested by Maciej and Alexander.
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 25 ++++++++++++-------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ab8370c413f3..2c1b6eb60436 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6777,6 +6777,18 @@ static void ixgbe_free_all_rx_resources(struct ixgbe_adapter *adapter)
 			ixgbe_free_rx_resources(adapter->rx_ring[i]);
 }
 
+/**
+ * ixgbe_max_xdp_frame_size - returns the maximum allowed frame size for XDP
+ * @adapter - device handle, pointer to adapter
+ */
+static int ixgbe_max_xdp_frame_size(struct ixgbe_adapter *adapter)
+{
+	if (PAGE_SIZE >= 8192 || adapter->flags2 & IXGBE_FLAG2_RX_LEGACY)
+		return IXGBE_RXBUFFER_2K;
+	else
+		return IXGBE_RXBUFFER_3K;
+}
+
 /**
  * ixgbe_change_mtu - Change the Maximum Transfer Unit
  * @netdev: network interface device structure
@@ -6788,18 +6800,13 @@ static int ixgbe_change_mtu(struct net_device *netdev, int new_mtu)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 
-	if (adapter->xdp_prog) {
+	if (ixgbe_enabled_xdp_adapter(adapter)) {
 		int new_frame_size = new_mtu + ETH_HLEN + ETH_FCS_LEN +
 				     VLAN_HLEN;
-		int i;
-
-		for (i = 0; i < adapter->num_rx_queues; i++) {
-			struct ixgbe_ring *ring = adapter->rx_ring[i];
 
-			if (new_frame_size > ixgbe_rx_bufsz(ring)) {
-				e_warn(probe, "Requested MTU size is not supported with XDP\n");
-				return -EINVAL;
-			}
+		if (new_frame_size > ixgbe_max_xdp_frame_size(adapter)) {
+			e_warn(probe, "Requested MTU size is not supported with XDP\n");
+			return -EINVAL;
 		}
 	}
 
-- 
2.37.3

Re: [PATCH net v3 1/3] ixgbe: allow to increase MTU to 3K with XDP enabled

[Tony Nguyen]

On 2/4/2023 5:35 AM, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>

...

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index ab8370c413f3..2c1b6eb60436 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6777,6 +6777,18 @@ static void ixgbe_free_all_rx_resources(struct ixgbe_adapter *adapter)
>   			ixgbe_free_rx_resources(adapter->rx_ring[i]);
>   }
>   
> +/**
> + * ixgbe_max_xdp_frame_size - returns the maximum allowed frame size for XDP
> + * @adapter - device handle, pointer to adapter
> + */

Please use ':' instead of '-' for kdoc

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:6785: warning: Function 
parameter or member 'adapter' not described in 'ixgbe_max_xdp_frame_size'

i.e.

@adapter: device handle, pointer to adapter

> +static int ixgbe_max_xdp_frame_size(struct ixgbe_adapter *adapter)
> +{
> +	if (PAGE_SIZE >= 8192 || adapter->flags2 & IXGBE_FLAG2_RX_LEGACY)
> +		return IXGBE_RXBUFFER_2K;
> +	else
> +		return IXGBE_RXBUFFER_3K;
> +}
> +

Re: [PATCH net v3 1/3] ixgbe: allow to increase MTU to 3K with XDP enabled

[Jason Xing]

On Wed, Feb 8, 2023 at 3:03 AM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
> On 2/4/2023 5:35 AM, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
>
> ...
>
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index ab8370c413f3..2c1b6eb60436 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -6777,6 +6777,18 @@ static void ixgbe_free_all_rx_resources(struct ixgbe_adapter *adapter)
> >                       ixgbe_free_rx_resources(adapter->rx_ring[i]);
> >   }
> >
> > +/**
> > + * ixgbe_max_xdp_frame_size - returns the maximum allowed frame size for XDP
> > + * @adapter - device handle, pointer to adapter
> > + */
>
> Please use ':' instead of '-' for kdoc
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:6785: warning: Function
> parameter or member 'adapter' not described in 'ixgbe_max_xdp_frame_size'
>
> i.e.
>
> @adapter: device handle, pointer to adapter

Thanks for correcting the format. Now I understand.

Thanks,
Jason

>
> > +static int ixgbe_max_xdp_frame_size(struct ixgbe_adapter *adapter)
> > +{
> > +     if (PAGE_SIZE >= 8192 || adapter->flags2 & IXGBE_FLAG2_RX_LEGACY)
> > +             return IXGBE_RXBUFFER_2K;
> > +     else
> > +             return IXGBE_RXBUFFER_3K;
> > +}
> > +

[PATCH net 2/3] i40e: add double of VLAN header when computing the max MTU

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Include the second VLAN HLEN into account when computing the maximum
MTU size as other drivers do.

Fixes: 0c8493d90b6b ("i40e: add XDP support for pass and drop actions")
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h      | 2 ++
 drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 60e351665c70..e03853d3c706 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -107,6 +107,8 @@
 #define I40E_BW_MBPS_DIVISOR		125000 /* rate / (1000000 / 8) Mbps */
 #define I40E_MAX_BW_INACTIVE_ACCUM	4 /* accumulate 4 credits max */
 
+#define I40E_PACKET_HDR_PAD (ETH_HLEN + ETH_FCS_LEN + (VLAN_HLEN * 2))
+
 /* driver state flags */
 enum i40e_state_t {
 	__I40E_TESTING,
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 53d0083e35da..d039928f3646 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -2921,7 +2921,7 @@ static int i40e_change_mtu(struct net_device *netdev, int new_mtu)
 	struct i40e_pf *pf = vsi->back;
 
 	if (i40e_enabled_xdp_vsi(vsi)) {
-		int frame_size = new_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
+		int frame_size = new_mtu + I40E_PACKET_HDR_PAD;
 
 		if (frame_size > i40e_max_xdp_frame_size(vsi))
 			return -EINVAL;
-- 
2.37.3

Re: [PATCH net 2/3] i40e: add double of VLAN header when computing the max MTU

[Tony Nguyen]

On 2/4/2023 5:35 AM, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Include the second VLAN HLEN into account when computing the maximum
> MTU size as other drivers do.
> 
> Fixes: 0c8493d90b6b ("i40e: add XDP support for pass and drop actions")
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e.h      | 2 ++
>   drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +-
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index 60e351665c70..e03853d3c706 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -107,6 +107,8 @@
>   #define I40E_BW_MBPS_DIVISOR		125000 /* rate / (1000000 / 8) Mbps */
>   #define I40E_MAX_BW_INACTIVE_ACCUM	4 /* accumulate 4 credits max */
>   
> +#define I40E_PACKET_HDR_PAD (ETH_HLEN + ETH_FCS_LEN + (VLAN_HLEN * 2))

This already exists:
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/intel/i40e/i40e_txrx.h#L112

Re: [PATCH net 2/3] i40e: add double of VLAN header when computing the max MTU

[Jason Xing]

On Wed, Feb 8, 2023 at 3:03 AM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
>
>
> On 2/4/2023 5:35 AM, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Include the second VLAN HLEN into account when computing the maximum
> > MTU size as other drivers do.
> >
> > Fixes: 0c8493d90b6b ("i40e: add XDP support for pass and drop actions")
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >   drivers/net/ethernet/intel/i40e/i40e.h      | 2 ++
> >   drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +-
> >   2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> > index 60e351665c70..e03853d3c706 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e.h
> > +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> > @@ -107,6 +107,8 @@
> >   #define I40E_BW_MBPS_DIVISOR                125000 /* rate / (1000000 / 8) Mbps */
> >   #define I40E_MAX_BW_INACTIVE_ACCUM  4 /* accumulate 4 credits max */
> >
> > +#define I40E_PACKET_HDR_PAD (ETH_HLEN + ETH_FCS_LEN + (VLAN_HLEN * 2))
>

> This already exists:
> https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/intel/i40e/i40e_txrx.h#L112

Thanks for pointing out the duplication definition. I'll drop this in
the i40e.h file.

[PATCH net 3/3] ixgbe: add double of VLAN header when computing the max MTU

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Include the second VLAN HLEN into account when computing the maximum
MTU size as other drivers do.

Fixes: fabf1bce103a ("ixgbe: Prevent unsupported configurations with XDP")
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      | 2 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index bc68b8f2176d..8736ca4b2628 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -73,6 +73,8 @@
 #define IXGBE_RXBUFFER_4K    4096
 #define IXGBE_MAX_RXBUFFER  16384  /* largest size for a single descriptor */
 
+#define IXGBE_PKT_HDR_PAD   (ETH_HLEN + ETH_FCS_LEN + (VLAN_HLEN * 2))
+
 /* Attempt to maximize the headroom available for incoming frames.  We
  * use a 2K buffer for receives and need 1536/1534 to store the data for
  * the frame.  This leaves us with 512 bytes of room.  From that we need
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 2c1b6eb60436..149f7baf40fe 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6801,8 +6801,7 @@ static int ixgbe_change_mtu(struct net_device *netdev, int new_mtu)
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 
 	if (ixgbe_enabled_xdp_adapter(adapter)) {
-		int new_frame_size = new_mtu + ETH_HLEN + ETH_FCS_LEN +
-				     VLAN_HLEN;
+		int new_frame_size = new_mtu + IXGBE_PKT_HDR_PAD;
 
 		if (new_frame_size > ixgbe_max_xdp_frame_size(adapter)) {
 			e_warn(probe, "Requested MTU size is not supported with XDP\n");
-- 
2.37.3

Re: [PATCH net 3/3] ixgbe: add double of VLAN header when computing the max MTU

[Jason Xing]

CC Alexander Duyck

Hello Alexander, thanks for reviewing the other two patches of this
patchset last night. Would you mind reviewing the last one? :)

Thanks,
Jason

On Sat, Feb 4, 2023 at 9:36 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> Include the second VLAN HLEN into account when computing the maximum
> MTU size as other drivers do.
>
> Fixes: fabf1bce103a ("ixgbe: Prevent unsupported configurations with XDP")
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h      | 2 ++
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +--
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index bc68b8f2176d..8736ca4b2628 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -73,6 +73,8 @@
>  #define IXGBE_RXBUFFER_4K    4096
>  #define IXGBE_MAX_RXBUFFER  16384  /* largest size for a single descriptor */
>
> +#define IXGBE_PKT_HDR_PAD   (ETH_HLEN + ETH_FCS_LEN + (VLAN_HLEN * 2))
> +
>  /* Attempt to maximize the headroom available for incoming frames.  We
>   * use a 2K buffer for receives and need 1536/1534 to store the data for
>   * the frame.  This leaves us with 512 bytes of room.  From that we need
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 2c1b6eb60436..149f7baf40fe 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6801,8 +6801,7 @@ static int ixgbe_change_mtu(struct net_device *netdev, int new_mtu)
>         struct ixgbe_adapter *adapter = netdev_priv(netdev);
>
>         if (ixgbe_enabled_xdp_adapter(adapter)) {
> -               int new_frame_size = new_mtu + ETH_HLEN + ETH_FCS_LEN +
> -                                    VLAN_HLEN;
> +               int new_frame_size = new_mtu + IXGBE_PKT_HDR_PAD;
>
>                 if (new_frame_size > ixgbe_max_xdp_frame_size(adapter)) {
>                         e_warn(probe, "Requested MTU size is not supported with XDP\n");
> --
> 2.37.3
>

Re: [PATCH net 3/3] ixgbe: add double of VLAN header when computing the max MTU

[Alexander Duyck]

On Wed, Feb 8, 2023 at 4:47 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> CC Alexander Duyck
>
> Hello Alexander, thanks for reviewing the other two patches of this
> patchset last night. Would you mind reviewing the last one? :)
>
> Thanks,
> Jason

It looks like this patch isn't in the patch queue at:
https://patchwork.kernel.org/project/netdevbpf/list/

I believe you will need to resubmit it to get it accepted upstream.

The patch itself looks fine. Feel free to add my reviewed by when you submit it.

Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>

Re: [PATCH net 3/3] ixgbe: add double of VLAN header when computing the max MTU

[Jason Xing]

On Thu, Feb 9, 2023 at 9:08 AM Alexander Duyck
<alexander.duyck@gmail.com> wrote:
>
> On Wed, Feb 8, 2023 at 4:47 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >
> > CC Alexander Duyck
> >
> > Hello Alexander, thanks for reviewing the other two patches of this
> > patchset last night. Would you mind reviewing the last one? :)
> >
> > Thanks,
> > Jason
>
> It looks like this patch isn't in the patch queue at:
> https://patchwork.kernel.org/project/netdevbpf/list/
>

I got it.

I have no clue on how patchwork works, I searched the current email,
see https://patchwork.kernel.org/project/netdevbpf/patch/20230204133535.99921-4-kerneljasonxing@gmail.com/.

> I believe you will need to resubmit it to get it accepted upstream.
>
> The patch itself looks fine. Feel free to add my reviewed by when you submit it.
>
> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>

Anyway, I'm going to send a v2 version with your reviewed-by label.

Thank you, Alexander :)

Jason

Re: [PATCH net 3/3] ixgbe: add double of VLAN header when computing the max MTU

[Jakub Kicinski]

On Thu, 9 Feb 2023 10:22:19 +0800 Jason Xing wrote:
> > It looks like this patch isn't in the patch queue at:
> > https://patchwork.kernel.org/project/netdevbpf/list/
> 
> I got it.
> 
> I have no clue on how patchwork works, I searched the current email,
> see https://patchwork.kernel.org/project/netdevbpf/patch/20230204133535.99921-4-kerneljasonxing@gmail.com/.
> 
> > I believe you will need to resubmit it to get it accepted upstream.

The patches are marked as Awaiting Upstream, that's why they don't show
up. We usually let Tony pick up patches for Intel's drivers rather than
applying directly.