netdev - Re: [PATCH RFC net-next 2/6] cadence: macb/gem: handle multi-descriptor frame reception

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] [day] [month] [year] [list]

Message-Id: <DETKB07L5M29.W6ES6AZIA9AQ@bootlin.com>
Date: Tue, 09 Dec 2025 10:01:31 +0100
From: Théo Lebrun <theo.lebrun@...tlin.com>
To: "Paolo Valerio" <pvalerio@...hat.com>, Théo Lebrun
 <theo.lebrun@...tlin.com>, <netdev@...r.kernel.org>
Cc: "Nicolas Ferre" <nicolas.ferre@...rochip.com>, "Claudiu Beznea"
 <claudiu.beznea@...on.dev>, "Andrew Lunn" <andrew+netdev@...n.ch>, "David
 S. Miller" <davem@...emloft.net>, "Eric Dumazet" <edumazet@...gle.com>,
 "Jakub Kicinski" <kuba@...nel.org>, "Paolo Abeni" <pabeni@...hat.com>,
 "Lorenzo Bianconi" <lorenzo@...nel.org>, Grégory Clement
 <gregory.clement@...tlin.com>, Benoît Monin
 <benoit.monin@...tlin.com>, "Thomas Petazzoni"
 <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH RFC net-next 2/6] cadence: macb/gem: handle
 multi-descriptor frame reception

On Mon Dec 8, 2025 at 1:53 PM CET, Paolo Valerio wrote:
> On 08 Dec 2025 at 11:21:00 AM, Théo Lebrun <theo.lebrun@...tlin.com> wrote:
>> On Tue Dec 2, 2025 at 6:32 PM CET, Paolo Valerio wrote:
>>> On 27 Nov 2025 at 02:38:45 PM, Théo Lebrun <theo.lebrun@...tlin.com> wrote:
>>>> I forgot about it in [PATCH 1/6], but the error handling if
>>>> gem_create_page_pool() fails is odd. We set queue->page_pool to NULL
>>>> and keep on going. Then once opened we'll fail allocating any buffer
>>>> but still be open. Shouldn't we fail the link up operation?
>>>>
>>>> If we want to support this codepath (page pool not allocated), then we
>>>> should unmask Rx interrupts only if alloc succeeded. I don't know if
>>>> we'd want that though.
>>>>
>>>> 	queue_writel(queue, IER, bp->rx_intr_mask | ...);
>>>
>>> Makes sense to fail the link up.
>>> Doesn't this imply to move the page pool creation and refill into
>>> macb_open()?
>>>
>>> I didn't look into it, I'm not sure if this can potentially become a
>>> bigger change.
>>
>> So I looked into it. Indeed buffer alloc should be done at open, doing
>> it at link up (that cannot fail) makes no sense. It is probably
>> historical, because on MACB it is mog_alloc_rx_buffers() that does all
>> the alloc. On GEM it only does the ring buffer but not each individual
>> slot buffer, which is done by ->mog_init_rings() / gem_rx_refill().
>>
>> I am linking a patch that applies before your series. Then the rebase
>> conflict resolution is pretty simple, and the gem_create_page_pool()
>> function should be adapted something like:
>
> Théo, thanks for looking into this. I was pretty much working on
> something similar for my next respin.
>
> Do you prefer to post the patch separately, or are you ok if I pick this
> up and send it on your behalf as part of this set?

You can pick it up if you agree with the patch; it'll avoid series
dependencies which is annoying to deal with for everyone.

Thanks Paolo,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com