From: Jason Xing <[email protected]>
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
From: Jason Xing <[email protected]>
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 <[email protected]>
---
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
From: Jason Xing <[email protected]>
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 <[email protected]>
---
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
From: Jason Xing <[email protected]>
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 <[email protected]>
---
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
On 2/4/2023 5:35 AM, Jason Xing wrote:
> From: Jason Xing <[email protected]>
...
> 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;
> +}
> +
On 2/4/2023 5:35 AM, Jason Xing wrote:
> From: Jason Xing <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
On Wed, Feb 8, 2023 at 3:03 AM Tony Nguyen <[email protected]> wrote:
>
>
>
> On 2/4/2023 5:35 AM, Jason Xing wrote:
> > From: Jason Xing <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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.
On Wed, Feb 8, 2023 at 3:03 AM Tony Nguyen <[email protected]> wrote:
>
> On 2/4/2023 5:35 AM, Jason Xing wrote:
> > From: Jason Xing <[email protected]>
>
> ...
>
> > 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;
> > +}
> > +
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 <[email protected]> wrote:
>
> From: Jason Xing <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
On Wed, Feb 8, 2023 at 4:47 PM Jason Xing <[email protected]> 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 <[email protected]>
On Thu, Feb 9, 2023 at 9:08 AM Alexander Duyck
<[email protected]> wrote:
>
> On Wed, Feb 8, 2023 at 4:47 PM Jason Xing <[email protected]> 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/[email protected]/.
> 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 <[email protected]>
Anyway, I'm going to send a v2 version with your reviewed-by label.
Thank you, Alexander :)
Jason
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/[email protected]/.
>
> > 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.