On Thu, Oct 26, 2023 at 11:12 AM <[email protected]> wrote: > > From: Numan Siddique <[email protected]> > > This patch series adds incremental processing in the lflow engine > node to handle changes to northd and other engine nodes. > Changed related to load balancers and NAT are mainly handled in > this patch series. > > This patch series can also be found here - https://github.com/numansiddique/ovn/tree/northd_lflow_ip/v1
Thanks Numan for the great improvement! I spent some time these days to review the series and now at patch 10. I need to take a break and so I just sent the comments I had so far. I also did scale testing for northd with https://github.com/hzhou8/ovn-test-script for a 500 chassis, 50 lsp / chassis setup, and noticed that the recompute latency has increased 20% after the series. I think in general it is expected to have some performance penalty for the recompute case because of the new indexes created for I-P. However, the patch 10 "northd: Refactor lflow management into a separate module." alone introduces a 10% latency increase, which necessitates more investigation. > > Prior to this patch series, most of the changes to northd engine > resulted in full recomputation of logical flows. This series > aims to improve the performance of ovn-northd by adding the I-P > support. I'd like to clarify "most of the changes" a little. I think we should focus on the most impactful changes that happen in a cloud environment. I don't think it is realistic to achieve "most of the changes" in I-P because it is too complex (even with this series we still handle a very small part of the DB schema incrementally), but it may be realistic to handle the most impactful changes, which are the most frequent changes in the cloud, incrementally. Before this series, we could handle regular VIF changes and related address-sets, port-groups (some of) updates incrementally. Those are the most common changes related to pod/VM changes in cloud. I believe the next goal is for LB changes related to pod/VMs (i.e. the LB backends), which I believe is the main goal of this series. Is my understanding correct? While reviewing the patches, I'd say sometimes I feel a little lost because it is hard to correlate some of the changes with the above goal. I believe there is a reason for all the changes but I am not sure if it is directly related to the goal of LB backend related I-P. I understand that there are other aspects of LB that can be incrementally processed. But I'd recommend if changes necessary for this goal can be largely reduced it would help the review and we might be able to merge them sooner and add more but less impactful I-P later step by step. I guess I will have a clearer picture when I finish the rest of the patches, but it would be helpful if you could add more guidance in this cover letter. Thanks, Han > In order to add this support, some of the northd engine > node data (from struct ovn_datapath) is split and moved over to > new engine nodes - mainly related to load balancers, NAT and ACLs. > > Below are the scale testing results done with these patches applied > using ovn-heater. The test ran the scenario - > ocp-500-density-heavy.yml [1]. > > With all the lflow I-P patches applied, the resuts are: > > ------------------------------------------------------------------------------------------------------------------------------------------------------- > Min (s) Median (s) 90%ile (s) 99%ile (s) Max (s) Mean (s) Total (s) Count Failed > ------------------------------------------------------------------------------------------------------------------------------------------------------- > Iteration Total 0.136883 1.129016 1.192001 1.204167 1.212728 0.665017 83.127099 125 0 > Namespace.add_ports 0.005216 0.005736 0.007034 0.015486 0.018978 0.006211 0.776373 125 0 > WorkerNode.bind_port 0.035030 0.046082 0.052469 0.058293 0.060311 0.045973 11.493259 250 0 > WorkerNode.ping_port 0.005057 0.006727 1.047692 1.069253 1.071336 0.266896 66.724094 250 0 > ------------------------------------------------------------------------------------------------------------------------------------------------------- > > The results with the present main are: > > ------------------------------------------------------------------------------------------------------------------------------------------------------- > Min (s) Median (s) 90%ile (s) 99%ile (s) Max (s) Mean (s) Total (s) Count Failed > ------------------------------------------------------------------------------------------------------------------------------------------------------- > Iteration Total 0.135491 2.223805 3.311270 3.339078 3.345346 1.729172 216.146495 125 0 > Namespace.add_ports 0.005380 0.005744 0.006819 0.018773 0.020800 0.006292 0.786532 125 0 > WorkerNode.bind_port 0.034179 0.046055 0.053488 0.058801 0.071043 0.046117 11.529311 250 0 > WorkerNode.ping_port 0.004956 0.006952 3.086952 3.191743 3.192807 0.791544 197.886026 250 0 > ------------------------------------------------------------------------------------------------------------------------------------------------------- > > [1] - https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml > > v1 -> v2 > -------- > * Now also maintaing array indexes for ls_lbacls, lr_nat and > lr_lb_nat_data tables (similar to ovn_datapaths->array) to > make the lookup effecient. The same ovn_datapath->index > is reused. > > * Made some signficant changes to 'struct lflow_ref' in lflow-mgr.c. > In v2 we don't use objdep_mgr to maintain the resource to lflow > references. Instead we maintain the 'struct lflow' pointer. > With this we don't need to maintain additional hmap of lflows. > > > Numan Siddique (18): > northd: Refactor the northd change tracking. > northd: Track ovn_datapaths in northd engine track data. > tests: Add a couple of tests in ovn-northd for I-P. > northd: Move router ports SB PB options sync to sync_to_sb_pb node. > northd: Add a new engine 'lr-nat' to manage lr NAT data. > northd: Add a new engine 'lr-lb-nat-data' to manage routers' lb data. > northd: Generate logical router's LB and NAT flows using > lr_lbnat_data. > northd: Don't commit dhcp response flows in the conntrack. > northd: Add a new node ls_lbacls. > northd: Refactor lflow management into a separate module. > northd: Use lflow_ref when adding all logical flows. > northd: Move ovn_lb_datapaths from lib to northd module. > northd: Handle lb changes in lflow engine. > northd: Add lr_lb_nat_data handler for lflow engine node. > northd: Add ls_lbacls handler for lflow engine node. > northd: Add I-P for NB_Global and SB_Global. > northd: Add a noop handler for northd SB mac binding. > northd: Add northd change handler for sync_to_sb_lb node. > > lib/lb.c | 96 - > lib/lb.h | 57 - > lib/ovn-util.c | 17 +- > lib/ovn-util.h | 2 +- > lib/stopwatch-names.h | 3 + > northd/aging.c | 21 +- > northd/automake.mk | 12 +- > northd/en-global-config.c | 588 ++++ > northd/en-global-config.h | 65 + > northd/en-lflow.c | 123 +- > northd/en-lflow.h | 8 + > northd/en-lr-lb-nat-data.c | 685 ++++ > northd/en-lr-lb-nat-data.h | 125 + > northd/en-lr-nat.c | 498 +++ > northd/en-lr-nat.h | 137 + > northd/en-ls-lb-acls.c | 530 +++ > northd/en-ls-lb-acls.h | 111 + > northd/en-northd.c | 67 +- > northd/en-northd.h | 2 +- > northd/en-port-group.h | 3 + > northd/en-sync-sb.c | 513 ++- > northd/inc-proc-northd.c | 79 +- > northd/lflow-mgr.c | 1078 ++++++ > northd/lflow-mgr.h | 192 ++ > northd/northd.c | 6485 ++++++++++++++++-------------------- > northd/northd.h | 551 ++- > northd/ovn-northd.c | 4 + > tests/ovn-northd.at | 858 ++++- > 28 files changed, 8827 insertions(+), 4083 deletions(-) > create mode 100644 northd/en-global-config.c > create mode 100644 northd/en-global-config.h > create mode 100644 northd/en-lr-lb-nat-data.c > create mode 100644 northd/en-lr-lb-nat-data.h > create mode 100644 northd/en-lr-nat.c > create mode 100644 northd/en-lr-nat.h > create mode 100644 northd/en-ls-lb-acls.c > create mode 100644 northd/en-ls-lb-acls.h > create mode 100644 northd/lflow-mgr.c > create mode 100644 northd/lflow-mgr.h > > -- > 2.41.0 > > _______________________________________________ > 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
