| Message ID | 20230209180111.2214872-1-i.maximets@ovn.org |
|---|---|
| Headers | show |
| Series | northd: Hash locks and lflow creation with dp groups. | expand |
I merged this series to main. Thanks Ilya! On 2/9/23 13:01, 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 sense 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. > > > Version 3: > * Fixed the bitmap leak in case only first 3 patches are applied. > * Changed the if condition in patch 7/8 to (!reject || build_non_meter) [Mark] > * Added Ack from Mark to patches that didn't change (patch 4/8 didn't > really change, but it needed a slight rebase after changes in 3/8). > > > 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(-) >
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 sense 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. Version 3: * Fixed the bitmap leak in case only first 3 patches are applied. * Changed the if condition in patch 7/8 to (!reject || build_non_meter) [Mark] * Added Ack from Mark to patches that didn't change (patch 4/8 didn't really change, but it needed a slight rebase after changes in 3/8). 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(-)