[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250915145206.74a59699@kernel.org>
Date: Mon, 15 Sep 2025 14:52:06 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Bhargava Chenna Marreddy <bhargava.marreddy@...adcom.com>
Cc: davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
andrew+netdev@...n.ch, horms@...nel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, michael.chan@...adcom.com,
pavan.chebbi@...adcom.com, vsrama-krishna.nemani@...adcom.com,
vikas.gupta@...adcom.com, Rajashekar Hudumula
<rajashekar.hudumula@...adcom.com>
Subject: Re: [v7, net-next 06/10] bng_en: Allocate packet buffers
On Mon, 15 Sep 2025 23:26:07 +0530 Bhargava Chenna Marreddy wrote:
> > You should have some sort of minimal fill level of the Rx rings.
> > Right now ndo_open will succeed even when Rx rings are completely empty.
> > Looks like you made even more functions void since v6, this is going in
> I changed those functions to void only because in this patchset they can’t fail.
> > the wrong direction. Most drivers actually expect the entire ring to be
> > filled. You can have a partial fill, but knowing bnxt I'm worried the
> > driver will actually never try to fill the rings back up.
> I believe the driver should return an error if any buffer allocation
> fails and handle the unwinding accordingly.
Yes, that's also my preference. I think allowing Rx buffer lists to not
be completely filled is okay if the driver author prefers that, but in
that case there needs to be some minimal "fill level" which makes the
device operational.
Speaking of Rx fill -- bnxt drops packets when it can't allocate a
replacement buffer. This used to be the recommended way of handling
allocation failures years ago. In modern drivers I believe it's better
to let the queue run dry and have a watchdog / service tasks which
periodically checks for complete depletion and kicks NAPI in.
Getting constantly interrupted with new packets when machine is trying
to recover from a hard OOM is not very helpful.
That's just a future note, I don't think this series itself contains
much of Rx.
Powered by blists - more mailing lists