Message-ID: <ZDWB1zRNlxpTN1IK@shell.armlinux.org.uk>
Date:   Tue, 11 Apr 2023 16:50:47 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Marek Behún <kabel@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        Paolo Abeni <pabeni@...hat.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH RFC net-next 0/6] net: mvneta: reduce size of TSO header
 allocation

Hi,

I think the Turris folk are waiting for me to get this into the kernel
and backported to stable before they merge it into their tree and we
therefore end up with it being tested.

We are now at -rc7, and this series is in danger of missing the
upcoming merge window.

So, I think it's time that I posted a wake-up call here to say that no,
that's not going to happen until such time that we know whether these
patches solve the problem that they identified. I'm not bunging patches
into the kernel for problems people have without those people testing
the proposed changes.

I think if the Turris folk want to engage with mainline for assistance
in resolving issues, they need to do their part and find a way to
provide kernels to test out proposed fixes for their problems.

On Mon, Apr 03, 2023 at 07:29:59PM +0100, Russell King (Oracle) wrote:
> Hi,
> 
> With reference to
> https://forum.turris.cz/t/random-kernel-exceptions-on-hbl-tos-7-0/18865/
> 
> It appears that mvneta attempts an order-6 allocation for the TSO
> header memory. While this succeeds early on in the system's life time,
> trying order-6 allocations later can result in failure due to memory
> fragmentation.
> 
> Firstly, the reason it's so large is that we take the number of
> transmit descriptors, and allocate a TSO header buffer for each, and
> each TSO header is 256 bytes. The driver uses a simple mechanism to
> determine the address - it uses the transmit descriptor index as an
> index into the TSO header memory.
> 
> 	(The first obvious question is: do there need to be this
> 	many? Won't each TSO header always have at least one bit
> 	of data to go with it? In other words, wouldn't the maximum
> 	number of TSO headers that a ring could accept be the number
> 	of ring entries divided by 2?)
> 
> There is no real need for this memory to be an order-6 allocation,
> since nothing in hardware requires this buffer to be contiguous.
> 
> Therefore, this series splits this order-6 allocation up into 32
> order-1 allocations (8k pages on 4k page platforms), each giving
> 32 TSO headers per page.
> 
> In order to do this, these patches:
> 
> 1) fix a horrible transmit path error-cleanup bug - the existing
>    code unmaps from the first descriptor that was allocated at
>    interface bringup, not the first descriptor that the packet
>    is using, resulting in the wrong descriptors being unmapped.
> 
> 2) since xdp support was added, we now have buf->type which indicates
>    what this transmit buffer contains. Use this to mark TSO header
>    buffers.
> 
> 3) get rid of IS_TSO_HEADER(), instead using buf->type to determine
>    whether this transmit buffer needs to be DMA-unmapped.
> 
> 4) move tso_build_hdr() into mvneta_tso_put_hdr() to keep all the
>    TSO header building code together.
> 
> 5) split the TSO header allocation into chunks of order-1 pages.
> 
>  drivers/net/ethernet/marvell/mvneta.c | 166 +++++++++++++++++++++++-----------
>  1 file changed, 115 insertions(+), 51 deletions(-)
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists