Commit 6aab704fb67d ("controller: Avoid unnecessary load balancer flow
processing.") handled the case when load balancers are updated but
addition/removal of load balancers will trigger almost all logical
flows to be reprocessed by ovn-controller.The references are just used for optimizing lookups in ovn-controller but we can avoid them all together. Signed-off-by: Dumitru Ceara <[email protected]> --- On a large scale deployment (simulating ovn-kubernetes) with 120 nodes, and 16K load balancer VIPs associated to each node's logical switch, when adding a new load balancer (just one VIP) inc_proc_eng debug logs show: - without fix: 2021-07-23T08:49:34.811Z|43363|inc_proc_eng|DBG|node: logical_flow_output, handler for input SB_logical_flow took 16824ms 2021-07-23T08:49:34.813Z|43364|inc_proc_eng|DBG|node: logical_flow_output, handler for input SB_logical_dp_group took 0ms 2021-07-23T08:49:44.018Z|43365|inc_proc_eng|DBG|node: logical_flow_output, handler for input SB_load_balancer took 9205ms - with fix: 2021-07-23T16:24:12.284Z|17720|inc_proc_eng|DBG|node: logical_flow_output, handler for input SB_logical_flow took 163ms 2021-07-23T16:24:12.284Z|17721|inc_proc_eng|DBG|node: logical_flow_output, handler for input SB_logical_dp_group took 0ms 2021-07-23T16:24:12.285Z|17722|inc_proc_eng|DBG|node: logical_flow_output, handler for input SB_load_balancer took 2ms --- controller/lflow.c | 10 +++++----- controller/ovn-controller.c | 38 +++++++++++++++++++++++++++++++++++++ controller/ovn-controller.h | 5 +++++ lib/ovn-util.c | 2 +- northd/ovn-northd.c | 17 ++++------------- northd/ovn_northd.dl | 14 ++++++++------ ovn-sb.xml | 2 +- tests/ovn-northd.at | 12 ++++++------ 8 files changed, 68 insertions(+), 32 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 871d3c54d..b2976992d 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -1764,11 +1764,11 @@ lflow_processing_end: /* Add load balancer hairpin flows if the datapath has any load balancers * associated. */ - for (size_t i = 0; i < dp->n_load_balancers; i++) { - const struct sbrec_load_balancer *lb = - sbrec_load_balancer_table_get_for_uuid(l_ctx_in->lb_table, - &dp->load_balancers[i]); - consider_lb_hairpin_flows(lb, l_ctx_in->local_datapaths, + struct local_datapath *ldp = get_local_datapath(l_ctx_in->local_datapaths, + dp->tunnel_key); + for (size_t i = 0; i < ldp->n_load_balancers; i++) { + consider_lb_hairpin_flows(ldp->load_balancers[i], + l_ctx_in->local_datapaths, l_ctx_out->flow_table); } diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 3a9bdbc97..2cf467dff 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2405,6 +2405,42 @@ lflow_output_port_groups_handler(struct engine_node *node, void *data) return _lflow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP); } +static void +init_lbs_per_datapath(struct ed_type_runtime_data *rt_data, + const struct sbrec_load_balancer_table *lb_table) +{ + const struct sbrec_load_balancer *lb; + SBREC_LOAD_BALANCER_TABLE_FOR_EACH (lb, lb_table) { + for (size_t i = 0; i < lb->n_datapaths; i++) { + struct local_datapath *ldp = + get_local_datapath(&rt_data->local_datapaths, + lb->datapaths[i]->tunnel_key); + if (!ldp) { + continue; + } + if (ldp->n_load_balancers == ldp->n_allocated_load_balancers) { + ldp->load_balancers = + x2nrealloc(ldp->load_balancers, + &ldp->n_allocated_load_balancers, + sizeof *ldp->load_balancers); + } + ldp->load_balancers[ldp->n_load_balancers++] = lb; + } + } +} + +static void +cleanup_lbs_per_datapath(struct ed_type_runtime_data *rt_data) +{ + struct local_datapath *dp; + HMAP_FOR_EACH (dp, hmap_node, &rt_data->local_datapaths) { + free(dp->load_balancers); + dp->load_balancers = NULL; + dp->n_allocated_load_balancers = 0; + dp->n_load_balancers = 0; + } +} + static bool lflow_output_runtime_data_handler(struct engine_node *node, void *data OVS_UNUSED) @@ -2430,6 +2466,7 @@ lflow_output_runtime_data_handler(struct engine_node *node, struct lflow_ctx_out l_ctx_out; struct ed_type_lflow_output *fo = data; init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); + init_lbs_per_datapath(rt_data, l_ctx_in.lb_table); struct tracked_binding_datapath *tdp; HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) { @@ -2450,6 +2487,7 @@ lflow_output_runtime_data_handler(struct engine_node *node, } } + cleanup_lbs_per_datapath(rt_data); engine_set_node_state(node, EN_UPDATED); return true; } diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h index b864ed0fa..4e907e930 100644 --- a/controller/ovn-controller.h +++ b/controller/ovn-controller.h @@ -69,6 +69,11 @@ struct local_datapath { size_t n_allocated_peer_ports; struct shash external_ports; + + const struct sbrec_load_balancer **load_balancers; + + size_t n_load_balancers; + size_t n_allocated_load_balancers; }; struct local_datapath *get_local_datapath(const struct hmap *, diff --git a/lib/ovn-util.c b/lib/ovn-util.c index 494d6d42d..3805923c8 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -760,7 +760,7 @@ ip_address_and_port_from_lb_key(const char *key, char **ip_address, /* Increment this for any logical flow changes, if an existing OVN action is * modified or a stage is added to a logical pipeline. */ -#define OVN_INTERNAL_MINOR_VER 1 +#define OVN_INTERNAL_MINOR_VER 2 /* Returns the OVN version. The caller must free the returned value. */ char * diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 1058c1c26..a0b946e71 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -3628,24 +3628,15 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *datapaths, free(lb_dps); } - /* Set the list of associated load balanacers to a logical switch - * datapath binding in the SB DB. */ + /* Datapath_Binding.load_balancers is not used anymore, it's still in the + * schema for compatibility reasons. Reset it to empty, just in case. + */ HMAP_FOR_EACH (od, key_node, datapaths) { if (!od->nbs) { continue; } - struct uuid *lb_uuids = - xmalloc(od->nbs->n_load_balancer * sizeof *lb_uuids); - for (size_t i = 0; i < od->nbs->n_load_balancer; i++) { - const struct uuid *lb_uuid = - &od->nbs->load_balancer[i]->header_.uuid; - lb = ovn_northd_lb_find(lbs, lb_uuid); - lb_uuids[i] = lb->slb->header_.uuid; - } - sbrec_datapath_binding_set_load_balancers(od->sb, lb_uuids, - od->nbs->n_load_balancer); - free(lb_uuids); + sbrec_datapath_binding_set_load_balancers(od->sb, NULL, 0); } } diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index ab33a139e..64239d809 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -53,14 +53,12 @@ sb::Out_Meter(._uuid = hash128(name), * except tunnel id, which is allocated separately (see TunKeyAllocation). */ relation OutProxy_Datapath_Binding ( _uuid: uuid, - load_balancers: Set<uuid>, external_ids: Map<string,string> ) /* Datapath_Binding table */ -OutProxy_Datapath_Binding(uuid, load_balancers, external_ids) :- +OutProxy_Datapath_Binding(uuid, external_ids) :- nb::Logical_Switch(._uuid = uuid, .name = name, .external_ids = ids, - .load_balancer = load_balancers, .other_config = other_config), var uuid_str = uuid2str(uuid), var external_ids = { @@ -76,7 +74,7 @@ OutProxy_Datapath_Binding(uuid, load_balancers, external_ids) :- eids }. -OutProxy_Datapath_Binding(uuid, set_empty(), external_ids) :- +OutProxy_Datapath_Binding(uuid, external_ids) :- lr in nb::Logical_Router(._uuid = uuid, .name = name, .external_ids = ids, .options = options), lr.is_enabled(), @@ -99,8 +97,12 @@ OutProxy_Datapath_Binding(uuid, set_empty(), external_ids) :- }. sb::Out_Datapath_Binding(uuid, tunkey, load_balancers, external_ids) :- - OutProxy_Datapath_Binding(uuid, load_balancers, external_ids), - TunKeyAllocation(uuid, tunkey). + OutProxy_Datapath_Binding(uuid, external_ids), + TunKeyAllocation(uuid, tunkey), + /* Datapath_Binding.load_balancers is not used anymore, it's still in the + * schema for compatibility reasons. Reset it to empty, just in case. + */ + var load_balancers = set_empty(). /* Proxy table for Out_Datapath_Binding: contains all Datapath_Binding fields, diff --git a/ovn-sb.xml b/ovn-sb.xml index a39778ee0..e57d4f7ec 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -2649,7 +2649,7 @@ tcp.flags = RST; <column name="load_balancers"> <p> - Load balancers associated with the datapath. + Not used anymore; kept for backwards compatibility of the schema. </p> </column> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 3fb4c8bc9..689f76737 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2366,7 +2366,7 @@ echo echo "__file__:__line__: Check that SB lb0 has sw0 in datapaths column." check_column "$sw0_sb_uuid" sb:load_balancer datapaths name=lb0 -check_column "$lb0_uuid" sb:datapath_binding load_balancers external_ids:name=sw0 +check_column "" sb:datapath_binding load_balancers external_ids:name=sw0 check ovn-nbctl --wait=sb set load_balancer . vips:"10.0.0.20\:90"="20.0.0.4:8080,30.0.0.4:8080" @@ -2390,7 +2390,7 @@ sw1_sb_uuid=$(fetch_column datapath_binding _uuid external_ids:name=sw1) echo echo "__file__:__line__: Check that SB lb0 has sw0 and sw1 in datapaths column." check_column "$sw0_sb_uuid $sw1_sb_uuid" sb:load_balancer datapaths name=lb0 -check_column "$lb0_uuid" sb:datapath_binding load_balancers external_ids:name=sw1 +check_column "" sb:datapath_binding load_balancers external_ids:name=sw1 check ovn-nbctl --wait=sb lb-add lb1 10.0.0.30:80 20.0.0.50:8080 udp check_row_count sb:load_balancer 1 @@ -2416,8 +2416,8 @@ echo "__file__:__line__: Check that SB lb1 has sw1 in datapaths column." check_column "$sw1_sb_uuid" sb:load_balancer datapaths name=lb1 echo -echo "__file__:__line__: check that datapath sw1 has lb0 and lb1 set in the load_balancers column." -check_column "$lb0_uuid $lb1_uuid" sb:datapath_binding load_balancers external_ids:name=sw1 +echo "__file__:__line__: check that datapath sw1 has no entry in the load_balancers column." +check_column "" sb:datapath_binding load_balancers external_ids:name=sw1 echo @@ -2426,10 +2426,10 @@ check ovn-nbctl --wait=sb set Load_Balancer lb1 options:hairpin_snat_ip="42.42.4 check_column "$lb1_uuid" sb:load_balancer _uuid name=lb1 options='{hairpin_orig_tuple="true", hairpin_snat_ip="42.42.42.42 4242::4242"}' echo -echo "__file__:__line__: Delete load balancer lb1 an check that datapath sw1's load_balancers are updated accordingly." +echo "__file__:__line__: Delete load balancer lb1 an check that datapath sw1's load_balancers is still empty." ovn-nbctl --wait=sb lb-del lb1 -check_column "$lb0_uuid" sb:datapath_binding load_balancers external_ids:name=sw1 +check_column "" sb:datapath_binding load_balancers external_ids:name=sw1 AT_CLEANUP ]) -- 2.27.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
