netdev - Re: [PATCH net-next 1/3] net: netpoll: Defer skb

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: <20241106-gecko-of-sheer-opposition-dde586@leitao>
Date: Wed, 6 Nov 2024 07:06:06 -0800
From: Breno Leitao <leitao@...ian.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: horms@...nel.org, davem@...emloft.net, edumazet@...gle.com,
	pabeni@...hat.com, thepacketgeek@...il.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, davej@...emonkey.org.uk,
	vlad.wing@...il.com, max@...sevol.com, kernel-team@...a.com,
	jiri@...nulli.us, jv@...sburgh.net, andy@...yhouse.net,
	aehkn@...hub.one, Rik van Riel <riel@...riel.com>,
	Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH net-next 1/3] net: netpoll: Defer skb_pool population
 until setup success

Hello Jakub,

On Tue, Nov 05, 2024 at 05:00:29PM -0800, Jakub Kicinski wrote:
> On Mon, 4 Nov 2024 12:40:00 -0800 Breno Leitao wrote:
> > Let's assume the pool is full and we start getting OOMs. It doesn't
> > matter if alloc_skb() will fail in the critical path or in the work
> > thread, netpoll will have MAX_SKBS skbs buffered to use, and none will
> > be allocated, thus, just 32 SKBs will be used until a -ENOMEM returns.
> 
> Do you assume the worker thread will basically keep up with the output?
> Vadim was showing me a system earlier today where workqueue workers
> didn't get scheduled in for minutes :( That's a bit extreme but doesn't
> inspire confidence in worker replenishing the pool quickly.

Interesting. Thanks for the data point.

> > On the other side, let's suppose there is a bunch of OOM pressure for a
> > while (10 SKBs are consumed for instance), and then some free memory
> > show up, causing the pool to be replenished. It is better
> > to do it in the workthread other than in the hot path.
> 
> We could cap how much we replenish in one go?

If we keep the replenish in the hot path, I think it is worth doing it,
for sure.

> > In both cases, the chance of not having SKBs to send the packet seems to
> > be the same, unless I am not modeling the problem correctly.
> 
> Maybe I misunderstood the proposal, I think you said earlier that you
> want to consume from the pool instead of calling alloc(). If you mean
> that we'd still alloc in the fast path but not replenish the pool
> that's different.

To clarify, let me take a step back and outline what this patchset proposes:

The patchset enhances SKB pool management in three key ways:

	a) It delays populating the skb pool until the target is active.
	b) It releases the skb pool when there are no more active users.
	c) It creates a separate pool for each target.

The third point (c) is the one that's open to discussion, as I
understand.

I proposed that having an individualized skb pool that users can control
would be beneficial. For example, users could define the number of skbs
in the pool. This could lead to additional advantages, such as allowing
netpoll to directly consume from the pool instead of relying on alloc()
in the optimal scenario, thereby speeding up the critical path.

--breno