netdev - Re: [PATCH 0/7] net: openvswitch: Reduce stack usage

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: <CW60USAI9NQ4.17G447945I9Y@wheely>
Date: Thu, 12 Oct 2023 10:08:08 +1000
From: "Nicholas Piggin" <npiggin@...il.com>
To: "Ilya Maximets" <i.maximets@....org>, <netdev@...r.kernel.org>
Cc: <dev@...nvswitch.org>, "Pravin B Shelar" <pshelar@....org>, "Aaron
 Conole" <aconole@...hat.com>, "Eelco Chaudron" <echaudro@...hat.com>,
 "Flavio Leitner" <fbl@...hat.com>, "Simon Horman" <horms@....org>
Subject: Re: [PATCH 0/7] net: openvswitch: Reduce stack usage

On Wed Oct 11, 2023 at 10:22 PM AEST, Ilya Maximets wrote:
> On 10/11/23 05:43, Nicholas Piggin wrote:
> > Hi,
> > 
> > I'll post this out again to keep discussion going. Thanks all for the
> > testing and comments so far.
>
> Hi, Nicholas.  This patch set still needs performance evaluation
> since it touches very performance-sensitive parts of the stack.
> Did you run any performance tests with this version?

I did, the recipe in the previous thread was in the noise on my
system.

> IIRC, Aaron was still working on testing for the RFC.  I think,
> we should wait for his feedback before re-spinning a new version.

The RFC was a a couple of % slow on the same microbenchmark. I
gave an updated git tree with reworked to avoid the slab allocs
he was looking at, but I thought I'd post it out for others to
see.

> > 
> > Changes since the RFC
> > https://lore.kernel.org/netdev/20230927001308.749910-1-npiggin@gmail.com/
> > 
> > - Replace slab allocations for flow keys with expanding the use
> >   of the per-CPU key allocator to ovs_vport_receive.
>
> While this is likely to work faster than a dynamic memory allocation,
> it is unlikley to be on par with a stack allocation.  Performance
> evaluation is necessary.

Sure.

> > 
> > - Drop patch 1 with Ilya's since they did the same thing (that is
> >   added at patch 3).
>
> The patch is already in net-next, so should not be included in this set.
> For the next version (please, hold) please rebase the set on the
> net-next/main and add the net-next to the subject prefix of the patches.
> They are not simple bug fixes, so should go through net-next, IMO.
>
> You may also see in netdev+bpf patchwork that CI failed trying to guess
> on which tree the patches should be applied and no tests were executed.

I was thinking you might take them through your ovs merge process,
but I'm happy to go whatever way you like. And yes they're not
intended for merge now, I did intend to add RFC v2 prefix.

>
> > 
> > - Change push_nsh stack reduction from slab allocation to per-cpu
> >   buffer.
>
> I still think this change is not needed and will only consume a lot
> of per-CPU memory space for no reason, as NSH is not a frequently
> used thing in OVS and the function is not on the recursive path and
> explicitly not inlined already.

If it's infrequent and you're concerned with per-CPU memory usage, we
could go back to using slab.

It's not in the recursive path but it can be a leaf called from the
recursive path. It could still be function that uses the most stack
in any given scenario, no?

Thanks,
Nick