On Tue, Jul 11, 2023 at 10:41 PM Numan Siddique <[email protected]> wrote: > > On Sat, Jul 8, 2023 at 1:27 AM Mark Michelson <[email protected]> wrote: > > > > Hi Numan, > > > > I gave the series a look. I've looked at the code but haven't yet run > > any tests with it. The main reason for this is that the series does not > > apply cleanly to OVN main. > > > > Overall, I only have small notes. I've replied to patches 2 and 3 with > > the specifics. > > Thanks for the review Mark. I'll address those comments and any other in v3. > I'll wait for a bit before submitting v3 if you or others have more comments. > Thanks Numan. I spent some time understanding the overall dependencies and the strategy, and I completed reviewing for the first 3 patches. Mostly look good except some nits. But I am still working on the rest of the patches. Sorry for my slowness!
I am also curious if you have done any scale tests and if there is any performance data. Or is it going to be more meaningful when I-P in the lflow node is implemented for LB changes (to achieve end-to-end I-P for LB)? > I did rebase before submitting. Maybe some mistake from my side. > But looks like ovsrobot was able to successfully apply the patches and > run the tests. > I didn't have any problem applying patches to main. It was clean. Thanks, Han > If you want to continue reviewing can you please pull it from here > - https://github.com/numansiddique/ovn/tree/northd_ip_refactor_lb_ip_v2_p8 > or > - https://github.com/ovsrobot/ovn/tree/series_362800 > > Thanks > Numan > > > > > > > On 7/7/23 01:51, [email protected] wrote: > > > From: Numan Siddique <[email protected]> > > > > > > This patch series adds the support to handle load balancer and > > > load balancer group changes incrementally in the "northd" engine > > > node. "flow" engine node doesn't support I-P yet and falls back > > > to full recompute. > > > > > > Note: I still need to do some scale testing and measure the > > > improvements. I'll first attempt adding I-P in the "flow" > > > engine node before attempting some scale tests. Neverthless > > > this patch series is good enough for reviews. > > > > > > > > > v1 -> v2 > > > -------- > > > * Resolved the conflicts and rebased > > > * Fixed a bug in p6. > > > > > > Numan Siddique (8): > > > northd I-P: Sync SB load balancers in a separate engine node. > > > northd: Add a new engine node - northd_lb_data. > > > northd: Add initial I-P for load balancer and load balancer groups > > > northd: Refactor the 'northd' node code which handles logical switch > > > changes. > > > northd: Handle load balancer changes for a logical switch. > > > northd: Handle load balancer group changes for a logical switch. > > > northd: Sync SB Port bindings NAT column in a separate engine node. > > > northd: Handle load balancer changes for a logical router. > > > > > > lib/lb.c | 356 ++++++- > > > lib/lb.h | 113 ++- > > > northd/automake.mk | 2 + > > > northd/en-lflow.c | 8 +- > > > northd/en-northd-lb-data.c | 308 ++++++ > > > northd/en-northd-lb-data.h | 56 ++ > > > northd/en-northd.c | 57 +- > > > northd/en-northd.h | 2 + > > > northd/en-sync-sb.c | 60 ++ > > > northd/en-sync-sb.h | 9 + > > > northd/inc-proc-northd.c | 27 +- > > > northd/northd.c | 1858 +++++++++++++++++++++++++----------- > > > northd/northd.h | 44 +- > > > tests/ovn-northd.at | 131 +++ > > > 14 files changed, 2354 insertions(+), 677 deletions(-) > > > create mode 100644 northd/en-northd-lb-data.c > > > create mode 100644 northd/en-northd-lb-data.h > > > > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
