diff mbox series

[v2,net] ixgbe: allow to increase MTU to some extent with XDP enabled

Message ID 20230127122018.2839-1-kerneljasonxing@gmail.com
State New
Headers
Series [v2,net] ixgbe: allow to increase MTU to some extent with XDP enabled |

Commit Message

Jason Xing Jan. 27, 2023, 12:20 p.m. UTC 
  From: Jason Xing <kernelxing@tencent.com>

I encountered one case where I cannot increase the MTU size directly
from 1500 to 2000 with XDP enabled if the server is equipped with
IXGBE card, which happened on thousands of servers in production
environment.

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

Referrences:
commit 23b44513c3e6f ("ice: allow 3k MTU for XDP")
commit 0c8493d90b6bb ("i40e: add XDP support for pass and drop actions")

Link: https://lore.kernel.org/lkml/20230121085521.9566-1-kerneljasonxing@gmail.com/
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
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(-)

Comments

Jason Xing Jan. 27, 2023, 2:54 p.m. UTC | #1 
My bad. It's not that right. Please ignore the v2 patch. I need some
time to do more studies and tests on this part.

Thanks,
Jason

On Fri, Jan 27, 2023 at 8:20 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> I encountered one case where I cannot increase the MTU size directly
> from 1500 to 2000 with XDP enabled if the server is equipped with
> IXGBE card, which happened on thousands of servers in production
> environment.
>
> This patch follows the behavior of changing MTU as i40e/ice does.
>
> Referrences:
> commit 23b44513c3e6f ("ice: allow 3k MTU for XDP")
> commit 0c8493d90b6bb ("i40e: add XDP support for pass and drop actions")
>
> Link: https://lore.kernel.org/lkml/20230121085521.9566-1-kerneljasonxing@gmail.com/
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> 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
>
Jason Xing Jan. 27, 2023, 3:08 p.m. UTC | #2 
On Fri, Jan 27, 2023 at 10:54 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> My bad. It's not that right. Please ignore the v2 patch. I need some
> time to do more studies and tests on this part.
>

In the meantime, any suggestions and help are appreciated :) I'm still
working on it.


> Thanks,
> Jason
>
> On Fri, Jan 27, 2023 at 8:20 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > I encountered one case where I cannot increase the MTU size directly
> > from 1500 to 2000 with XDP enabled if the server is equipped with
> > IXGBE card, which happened on thousands of servers in production
> > environment.
> >
> > This patch follows the behavior of changing MTU as i40e/ice does.
> >
> > Referrences:
> > commit 23b44513c3e6f ("ice: allow 3k MTU for XDP")
> > commit 0c8493d90b6bb ("i40e: add XDP support for pass and drop actions")
> >
> > Link: https://lore.kernel.org/lkml/20230121085521.9566-1-kerneljasonxing@gmail.com/
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> > 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
> >
Jason Xing Jan. 30, 2023, 5:01 a.m. UTC | #3 
On Fri, Jan 27, 2023 at 11:08 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> On Fri, Jan 27, 2023 at 10:54 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >
> > My bad. It's not that right. Please ignore the v2 patch. I need some
> > time to do more studies and tests on this part.
> >
>
> In the meantime, any suggestions and help are appreciated :) I'm still
> working on it.
>

Dear maintainers,  after several tests I did during this time as much
as possible,  I couldn't find anything wrong though I am not that
familiar with the whole ixgbe code. I decided to 'reopen' this patch
v2. Please help me review the current patch.

Thanks,
Jason


>
> > Thanks,
> > Jason
> >
> > On Fri, Jan 27, 2023 at 8:20 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
> > >
> > > From: Jason Xing <kernelxing@tencent.com>
> > >
> > > I encountered one case where I cannot increase the MTU size directly
> > > from 1500 to 2000 with XDP enabled if the server is equipped with
> > > IXGBE card, which happened on thousands of servers in production
> > > environment.
> > >
> > > This patch follows the behavior of changing MTU as i40e/ice does.
> > >
> > > Referrences:
> > > commit 23b44513c3e6f ("ice: allow 3k MTU for XDP")
> > > commit 0c8493d90b6bb ("i40e: add XDP support for pass and drop actions")
> > >
> > > Link: https://lore.kernel.org/lkml/20230121085521.9566-1-kerneljasonxing@gmail.com/
> > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > ---
> > > 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
> > >
Maciej Fijalkowski Jan. 30, 2023, 3:07 p.m. UTC | #4 
On Fri, Jan 27, 2023 at 08:20:18PM +0800, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> I encountered one case where I cannot increase the MTU size directly
> from 1500 to 2000 with XDP enabled if the server is equipped with
> IXGBE card, which happened on thousands of servers in production
> environment.

You said in this thread that you've done several tests - what were they?
Now that you're following logic from other drivers, have you tested 3k MTU
against XDP? Because your commit msg still refer to 2k as your target. If
3k is fine then i would reflect that in the subject of the patch.

> 
> This patch follows the behavior of changing MTU as i40e/ice does.
> 
> Referrences:
> commit 23b44513c3e6f ("ice: allow 3k MTU for XDP")
> commit 0c8493d90b6bb ("i40e: add XDP support for pass and drop actions")
> 
> Link: https://lore.kernel.org/lkml/20230121085521.9566-1-kerneljasonxing@gmail.com/

Why do you share a link to v1 here?

You're also missing Fixes: tag, as you're targetting the net tree.

> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> 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
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Jason Xing Jan. 31, 2023, 3 a.m. UTC | #5 
On Mon, Jan 30, 2023 at 11:09 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Fri, Jan 27, 2023 at 08:20:18PM +0800, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > I encountered one case where I cannot increase the MTU size directly
> > from 1500 to 2000 with XDP enabled if the server is equipped with
> > IXGBE card, which happened on thousands of servers in production
> > environment.
>

> You said in this thread that you've done several tests - what were they?

Tests against XDP are running on the server side when MTU varies from
1500 to 3050 (not including ETH_HLEN, ETH_FCS_LEN and VLAN_HLEN) for a
few days.
I choose the iperf tool to test the maximum throughput and observe the
behavior when the machines are under greater pressure. Also, I use
netperf to send different size packets to the server side with
different modes (TCP_RR/_STREAM) applied.

> Now that you're following logic from other drivers, have you tested 3k MTU

Sure, the maximum MTU size users could set is 3050 (which is 3072 - 14
- 4 - 4 in ixgbe_change_mtu() function).

> against XDP? Because your commit msg still refer to 2k as your target. If
> 3k is fine then i would reflect that in the subject of the patch.

I will modify the title and body message both.

>
> >
> > This patch follows the behavior of changing MTU as i40e/ice does.
> >
> > Referrences:
> > commit 23b44513c3e6f ("ice: allow 3k MTU for XDP")
> > commit 0c8493d90b6bb ("i40e: add XDP support for pass and drop actions")
> >
> > Link: https://lore.kernel.org/lkml/20230121085521.9566-1-kerneljasonxing@gmail.com/
>
> Why do you share a link to v1 here?

I originally intended to let maintainers trace the previous
discussion. Well, I'm going to remove the link.

>
> You're also missing Fixes: tag, as you're targetting the net tree.
>

I'll do it in the v3 patch.

Thanks,
Jason

> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> > 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
> >
> > _______________________________________________
> > Intel-wired-lan mailing list
> > Intel-wired-lan@osuosl.org
> > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Alexander Lobakin Jan. 31, 2023, 11:07 a.m. UTC | #6 
From: Jason Xing <kerneljasonxing@gmail.com>
Date: Tue, 31 Jan 2023 11:00:05 +0800

> On Mon, Jan 30, 2023 at 11:09 PM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
>>
>> On Fri, Jan 27, 2023 at 08:20:18PM +0800, Jason Xing wrote:
>>> From: Jason Xing <kernelxing@tencent.com>
>>>
>>> I encountered one case where I cannot increase the MTU size directly
>>> from 1500 to 2000 with XDP enabled if the server is equipped with
>>> IXGBE card, which happened on thousands of servers in production
>>> environment.
>>
> 
>> You said in this thread that you've done several tests - what were they?
> 
> Tests against XDP are running on the server side when MTU varies from
> 1500 to 3050 (not including ETH_HLEN, ETH_FCS_LEN and VLAN_HLEN) for a

BTW, if ixgbe allows you to set MTU of 3050, it needs to be fixed. Intel
drivers at some point didn't include the second VLAN tag into account,
thus it was possible to trigger issues on Q-in-Q setups. AICS, not all
of them were fixed.

> few days.
> I choose the iperf tool to test the maximum throughput and observe the
> behavior when the machines are under greater pressure. Also, I use
> netperf to send different size packets to the server side with
> different modes (TCP_RR/_STREAM) applied.
[...]

Thanks,
Olek
Jason Xing Jan. 31, 2023, 11:23 a.m. UTC | #7 
On Tue, Jan 31, 2023 at 7:08 PM Alexander Lobakin
<alexandr.lobakin@intel.com> wrote:
>
> From: Jason Xing <kerneljasonxing@gmail.com>
> Date: Tue, 31 Jan 2023 11:00:05 +0800
>
> > On Mon, Jan 30, 2023 at 11:09 PM Maciej Fijalkowski
> > <maciej.fijalkowski@intel.com> wrote:
> >>
> >> On Fri, Jan 27, 2023 at 08:20:18PM +0800, Jason Xing wrote:
> >>> From: Jason Xing <kernelxing@tencent.com>
> >>>
> >>> I encountered one case where I cannot increase the MTU size directly
> >>> from 1500 to 2000 with XDP enabled if the server is equipped with
> >>> IXGBE card, which happened on thousands of servers in production
> >>> environment.
> >>
> >
> >> You said in this thread that you've done several tests - what were they?
> >
> > Tests against XDP are running on the server side when MTU varies from
> > 1500 to 3050 (not including ETH_HLEN, ETH_FCS_LEN and VLAN_HLEN) for a
>

> BTW, if ixgbe allows you to set MTU of 3050, it needs to be fixed. Intel
> drivers at some point didn't include the second VLAN tag into account,

Yes, I noticed that.

It should be like "int new_frame_size = new_mtu + ETH_HLEN +
ETH_FCS_LEN + (VLAN_HLEN * 2)" instead of only one VLAN_HLEN, which is
used to compute real size in ixgbe_change_mtu() function.
I'm wondering if I could submit another patch to fix the issue you
mentioned because the current patch tells a different issue. Does it
make sense?

If you're available, please help me review the v3 patch I've already
sent to the mailing-list. Thanks anyway.
The Link is https://lore.kernel.org/lkml/20230131032357.34029-1-kerneljasonxing@gmail.com/
.

Thanks,
Jason

> thus it was possible to trigger issues on Q-in-Q setups. AICS, not all
> of them were fixed.
>
> > few days.
> > I choose the iperf tool to test the maximum throughput and observe the
> > behavior when the machines are under greater pressure. Also, I use
> > netperf to send different size packets to the server side with
> > different modes (TCP_RR/_STREAM) applied.
> [...]
>
> Thanks,
> Olek
Alexander Lobakin Feb. 1, 2023, 11:15 a.m. UTC | #8 
From: Jason Xing <kerneljasonxing@gmail.com>
Date: Tue, 31 Jan 2023 19:23:59 +0800

> On Tue, Jan 31, 2023 at 7:08 PM Alexander Lobakin
> <alexandr.lobakin@intel.com> wrote:

[...]

>>>> You said in this thread that you've done several tests - what were they?
>>>
>>> Tests against XDP are running on the server side when MTU varies from
>>> 1500 to 3050 (not including ETH_HLEN, ETH_FCS_LEN and VLAN_HLEN) for a
>>
> 
>> BTW, if ixgbe allows you to set MTU of 3050, it needs to be fixed. Intel
>> drivers at some point didn't include the second VLAN tag into account,
> 
> Yes, I noticed that.
> 
> It should be like "int new_frame_size = new_mtu + ETH_HLEN +
> ETH_FCS_LEN + (VLAN_HLEN * 2)" instead of only one VLAN_HLEN, which is
> used to compute real size in ixgbe_change_mtu() function.
> I'm wondering if I could submit another patch to fix the issue you
> mentioned because the current patch tells a different issue. Does it
> make sense?

Yes, please send as a separate patch. It's somewhat related to the
topic, but better to keep commits atomic.

> 
> If you're available, please help me review the v3 patch I've already
> sent to the mailing-list. Thanks anyway.
> The Link is https://lore.kernel.org/lkml/20230131032357.34029-1-kerneljasonxing@gmail.com/
> .
> 
> Thanks,
> Jason

Thanks,
Olek
Jason Xing Feb. 1, 2023, 1:28 p.m. UTC | #9 
On Wed, Feb 1, 2023 at 7:15 PM Alexander Lobakin
<alexandr.lobakin@intel.com> wrote:
>
> From: Jason Xing <kerneljasonxing@gmail.com>
> Date: Tue, 31 Jan 2023 19:23:59 +0800
>
> > On Tue, Jan 31, 2023 at 7:08 PM Alexander Lobakin
> > <alexandr.lobakin@intel.com> wrote:
>
> [...]
>
> >>>> You said in this thread that you've done several tests - what were they?
> >>>
> >>> Tests against XDP are running on the server side when MTU varies from
> >>> 1500 to 3050 (not including ETH_HLEN, ETH_FCS_LEN and VLAN_HLEN) for a
> >>
> >
> >> BTW, if ixgbe allows you to set MTU of 3050, it needs to be fixed. Intel
> >> drivers at some point didn't include the second VLAN tag into account,
> >
> > Yes, I noticed that.
> >
> > It should be like "int new_frame_size = new_mtu + ETH_HLEN +
> > ETH_FCS_LEN + (VLAN_HLEN * 2)" instead of only one VLAN_HLEN, which is
> > used to compute real size in ixgbe_change_mtu() function.
> > I'm wondering if I could submit another patch to fix the issue you
> > mentioned because the current patch tells a different issue. Does it
> > make sense?
>

> Yes, please send as a separate patch. It's somewhat related to the
> topic, but better to keep commits atomic.
>

Roger that, I will write another patch with your suggestions (labeled
'suggested-by your email address').

Thanks,
Jason

> >
> > If you're available, please help me review the v3 patch I've already
> > sent to the mailing-list. Thanks anyway.
> > The Link is https://lore.kernel.org/lkml/20230131032357.34029-1-kerneljasonxing@gmail.com/
> > .
> >
> > Thanks,
> > Jason
>
> Thanks,
> Olek
>
Jason Xing Feb. 2, 2023, 1:11 p.m. UTC | #10 
On Wed, Feb 1, 2023 at 7:15 PM Alexander Lobakin
<alexandr.lobakin@intel.com> wrote:
>
> From: Jason Xing <kerneljasonxing@gmail.com>
> Date: Tue, 31 Jan 2023 19:23:59 +0800
>
> > On Tue, Jan 31, 2023 at 7:08 PM Alexander Lobakin
> > <alexandr.lobakin@intel.com> wrote:
>
> [...]
>
> >>>> You said in this thread that you've done several tests - what were they?
> >>>
> >>> Tests against XDP are running on the server side when MTU varies from
> >>> 1500 to 3050 (not including ETH_HLEN, ETH_FCS_LEN and VLAN_HLEN) for a
> >>
> >
> >> BTW, if ixgbe allows you to set MTU of 3050, it needs to be fixed. Intel
> >> drivers at some point didn't include the second VLAN tag into account,
> >
> > Yes, I noticed that.
> >
> > It should be like "int new_frame_size = new_mtu + ETH_HLEN +
> > ETH_FCS_LEN + (VLAN_HLEN * 2)" instead of only one VLAN_HLEN, which is
> > used to compute real size in ixgbe_change_mtu() function.
> > I'm wondering if I could submit another patch to fix the issue you
> > mentioned because the current patch tells a different issue. Does it
> > make sense?
>
> Yes, please send as a separate patch. It's somewhat related to the
> topic, but better to keep commits atomic.

Hi Alexander,

I'm not sure if I should wait for the current patch to get reviewed,
then I'll write the vlan related patch we talked about based on the
current patch?

Thanks,
Jason

>
> >
> > If you're available, please help me review the v3 patch I've already
> > sent to the mailing-list. Thanks anyway.
> > The Link is https://lore.kernel.org/lkml/20230131032357.34029-1-kerneljasonxing@gmail.com/
> > .
> >
> > Thanks,
> > Jason
>
> Thanks,
> Olek
>
diff mbox series

Patch

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