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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y+Pxq10w/4e1HT+4@boxer>
Date:   Wed, 8 Feb 2023 20:02:03 +0100
From:   Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To:     Alexander H Duyck <alexander.duyck@...il.com>
CC:     Jason Xing <kerneljasonxing@...il.com>,
        <jesse.brandeburg@...el.com>, <anthony.l.nguyen@...el.com>,
        <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <richardcochran@...il.com>, <ast@...nel.org>,
        <daniel@...earbox.net>, <hawk@...nel.org>,
        <john.fastabend@...il.com>, <alexandr.lobakin@...el.com>,
        <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <bpf@...r.kernel.org>,
        Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net v4 1/3] ixgbe: allow to increase MTU to 3K with XDP
 enabled

On Wed, Feb 08, 2023 at 10:57:22AM -0800, Alexander H Duyck wrote:
> On Wed, 2023-02-08 at 17:27 +0100, Maciej Fijalkowski wrote:
> > On Wed, Feb 08, 2023 at 07:37:57AM -0800, Alexander H Duyck wrote:
> > > On Wed, 2023-02-08 at 10:43 +0800, Jason Xing wrote:
> > > > From: Jason Xing <kernelxing@...cent.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@...cent.com>
> > > 
> > > This is based on the broken premise that w/ XDP we are using a 4K page.
> > > The ixgbe driver isn't using page pool and is therefore running on
> > > different limitations. The ixgbe driver is only using 2K slices of the
> > > 4K page. In addition that is reduced to 1.5K to allow for headroom and
> > > the shared info in the buffer.
> > > 
> > > Currently the only way a 3K buffer would work is if FCoE is enabled and
> > > in that case the driver is using order 1 pages and still using the
> > > split buffer approach.
> > 
> > Hey Alex, interesting, we based this on the following logic from
> > ixgbe_set_rx_buffer_len() I guess:
> > 
> > #if (PAGE_SIZE < 8192)
> > 		if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)
> > 			set_bit(__IXGBE_RX_3K_BUFFER, &rx_ring->state);
> > 
> > 		if (IXGBE_2K_TOO_SMALL_WITH_PADDING ||
> > 		    (max_frame > (ETH_FRAME_LEN + ETH_FCS_LEN)))
> > 			set_bit(__IXGBE_RX_3K_BUFFER, &rx_ring->state);
> > #endif
> > 
> > so we assumed that ixgbe is no different than i40e/ice in these terms, but
> > we ignored whole overhead of LRO/RSC that ixgbe carries.
> 
> If XDP is already enabled the LRO/RSC cannot be enabled. I think that
> is already disabled if we have XDP enabled.
> 
> > I am not actively working with ixgbe but I know that you were the main dev
> > of it, so without premature dive into the datasheet and codebase, are you
> > really sure that 3k mtu for XDP is a no go?
> 
> I think I mixed up fm10k and ixgbe, either that or I was thinking of
> the legacy setup. They all kind of blur together as I had worked on
> pretty much all the Intel drivers up to i40e the last time I was
> updating them for all the Rx path stuff. :)
> 
> So if I am reading things right the issue is that if XDP is enabled you
> cannot set a 3K MTU, but if you set the 3K MTU first then you can
> enable XDP after the fact right?

Yes and vice versa - when XDP is on then you should be able to work with
3k mtus.

> 
> Looking it over again after re-reading the code this looks good to me.
> 
> Reviewed-by: Alexander Duyck <alexanderduyck@...com>

Awesome :)

> 
> 
> 
> 
>