On 2/8/23 22:55, Mark Michelson wrote:
> Hi Ilya,
>
> Aside from the finding you made in patch 3, I couldn't find anything wrong
> with the series. I have a minor suggestion on patch 7, but it's nothing that
> should hold up merging the series.
>
> For the series,
> Acked-by: Mark Michelson <[email protected]>
>
> Thanks for the improvement!
Thanks, Mark! I'll fix the mentioned issues and post v3.
Best regards, Ilya Maximets.
>
> On 2/7/23 06:42, Ilya Maximets wrote:
>> While running tests with ovn-heater it was observed that locking and
>> unlocking dpg_lock can take more than 20% of CPU cycles in northd even
>> without any contention.
>>
>> This series is trying to address that issue and add thread safety
>> static analysis. While doing that, the code is re-worked to use and
>> better utilize datapath group bitmaps.
>>
>> Why the version 2 has 6 more patches... ?
>>
>> Attempt to rebase v1 on top of a current main branch revealed a significant
>> conflict with the previous commit:
>> 7b94d212f694 ("northd: Refactor build_lrouter_nat_flows_for_lb function")
>>
>> This refactor made it hard to utilize hash locks in an efficient way for the
>> LR NAT flows generation. So, in v2 the first commit doesn't attempt to
>> optimize this part anymore and just locking and unlocking the hash lock each
>> time. Patch #3 was introduced to re-work the aforementioned function in a
>> way
>> that it is possible to lock only one hash by pre-calculating datapath groups
>> for each type of flows. However, that doesn't make a significant performance
>> difference due to the added extra work. So, the code is further re-factored
>> to better utilize datapath group bitmaps. It's hard to do that without
>> touching a lot of code though. So, in order to keep the codebase uniform and
>> elegant, bitmap usage is re-worked all the way up to generation of ls/lr
>> lists
>> for load balancers and introduction of a new wrapper that creates a new
>> logical flow with a desired datapath group from the beginning.
>>
>> Patches are structured to make sence even without following ones. So, some
>> parts of the code are changed multiple times throughout the set.
>>
>> This re-work also ended up with a nice code deduplication and shortening
>> of many functions.
>>
>> Final result shows up to 37% performance improvement and up to 30% memory
>> consumption reduction in ovn-heater's density-heavy 500 node scenario.
>>
>>
>> Ilya Maximets (8):
>> northd: Use larger hash bucket locks instead of dp group ones.
>> northd: Add thread safety analysis to lflow hash locks.
>> northd: Optimize locking pattern for GW LR NAT flows for LB.
>> northd: Use bitmaps for LB affinity flows.
>> northd: Use bitmaps for LB lists of switches and routers.
>> northd: Create logical flows with datapath groups.
>> northd: Create metered flows with dp groups if CoPP is not configured.
>> northd: Don't collect datapath groups for LB affinity if disabled.
>>
>> lib/lb.c | 26 +--
>> lib/lb.h | 9 +-
>> northd/northd.c | 593 ++++++++++++++++++++++--------------------------
>> 3 files changed, 286 insertions(+), 342 deletions(-)
>>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
