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]

Date: Wed, 11 Oct 2023 14:22:40 +0200
From: Ilya Maximets <i.maximets@....org>
To: Nicholas Piggin <npiggin@...il.com>, 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>, i.maximets@....org,
 Simon Horman <horms@....org>
Subject: Re: [PATCH 0/7] net: openvswitch: Reduce stack usage

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?

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

> 
> 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.

> 
> - 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.

> 
> - 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.

Best regards, Ilya Maximets.

P.S.  Please use my ovn.org email instead.

> 
> - Drop the ovs_fragment stack usage reduction for now sinc it used
>   slab and was a bit more complicated.
> 
> I posted an initial version of the per-cpu flow allocator patch in
> the RFC thread. Since then I cleaned up some debug code and increased
> the allocator size to accommodate the additional user of it.
> 
> Thanks,
> Nick
> 
> Ilya Maximets (1):
>   openvswitch: reduce stack usage in do_execute_actions
> 
> Nicholas Piggin (6):
>   net: openvswitch: generalise the per-cpu flow key allocation stack
>   net: openvswitch: Use flow key allocator in ovs_vport_receive
>   net: openvswitch: Reduce push_nsh stack usage
>   net: openvswitch: uninline action execution
>   net: openvswitch: uninline ovs_fragment to control stack usage
>   net: openvswitch: Reduce stack usage in ovs_dp_process_packet
> 
>  net/openvswitch/actions.c  | 208 +++++++++++++++++++++++--------------
>  net/openvswitch/datapath.c |  56 +++++-----
>  net/openvswitch/flow.h     |   3 +
>  net/openvswitch/vport.c    |  27 +++--
>  4 files changed, 185 insertions(+), 109 deletions(-)
>