netdev - Re: [PATCH RFC net-next 0/6] net: mvneta: reduce size of TSO header allocation

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]

Date:   Tue, 4 Apr 2023 07:11:04 +0200
From:   Eric Dumazet <edumazet@...gle.com>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Marek Behún <kabel@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        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

On Mon, Apr 3, 2023 at 8:30 PM Russell King (Oracle)
<linux@...linux.org.uk> 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.
>

Series looks very good to me, thanks.

Reviewed-by: Eric Dumazet <edumazet@...gle.com>