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-npig...@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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
