From: Numan Siddique <[email protected]> Since northd tracked data has the changed lb data, northd engine handler for lflow engine now handles the lb changes incrementally. All the lflows generated for each lb is stored in the ovn_lb_datapaths->lflow_ref and this is used similar to how we handle ovn_port changes.
Signed-off-by: Numan Siddique <[email protected]> --- northd/en-lflow.c | 11 +++--- northd/lflow-mgr.c | 49 ++++++++++++++++++++++++ northd/lflow-mgr.h | 4 ++ northd/northd.c | 91 +++++++++++++++++++++++++++++++++++++++++---- northd/northd.h | 28 ++++++++++++++ tests/ovn-northd.at | 31 ++++++++++----- 6 files changed, 192 insertions(+), 22 deletions(-) diff --git a/northd/en-lflow.c b/northd/en-lflow.c index efffee6384..4400460482 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -122,11 +122,6 @@ lflow_northd_handler(struct engine_node *node, return false; } - /* Fall back to recompute if load balancers have changed. */ - if (northd_has_lbs_in_tracked_data(&northd_data->trk_northd_changes)) { - return false; - } - const struct engine_context *eng_ctx = engine_get_context(); struct lflow_data *lflow_data = data; @@ -139,6 +134,12 @@ lflow_northd_handler(struct engine_node *node, return false; } + if (!lflow_handle_northd_lb_changes(eng_ctx->ovnsb_idl_txn, + &northd_data->trk_northd_changes.trk_lbs, + &lflow_input, lflow_data->lflow_table)) { + return false; + } + engine_set_node_state(node, EN_UPDATED); return true; diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c index b435d03061..30d123a7bc 100644 --- a/northd/lflow-mgr.c +++ b/northd/lflow-mgr.c @@ -111,6 +111,10 @@ static void unlink_objres_lflows(struct resource_to_objects_node *, const struct ovn_datapath *od, struct lflow_table *, struct objdep_mgr *); +static void unlink_all_dps_objres_lflows(struct resource_to_objects_node *, + size_t n_ls_datapaths, + size_t n_lr_datapaths, + struct lflow_table *); static void sync_lflows_from_objres( struct resource_to_objects_node *, struct lflow_table *, struct objdep_mgr *, struct ovsdb_idl_txn *, @@ -387,6 +391,19 @@ lflow_ref_clear_lflows(struct lflow_ref *lflow_ref, unlink_objres_lflows(res_node, od, lflow_table, &lflow_ref->objdep_mgr); } +void +lflow_ref_clear_lflows_for_all_dps(struct lflow_ref *lflow_ref, + size_t n_ls_datapaths, + size_t n_lr_datapaths, + struct lflow_table *lflow_table) +{ + struct resource_to_objects_node *res_node = objdep_mgr_find_objs( + &lflow_ref->objdep_mgr, OBJDEP_TYPE_LFLOW, lflow_ref->res_name); + + unlink_all_dps_objres_lflows(res_node, n_ls_datapaths, n_lr_datapaths, + lflow_table); +} + void lflow_ref_clear_and_sync_lflows(struct lflow_ref *lflow_ref, const struct ovn_datapath *od, @@ -1099,6 +1116,38 @@ unlink_objres_lflows(struct resource_to_objects_node *res_node, } } +static void +unlink_all_dps_objres_lflows(struct resource_to_objects_node *res_node, + size_t n_ls_datapaths, + size_t n_lr_datapaths, + struct lflow_table *lflow_table) +{ + if (!res_node) { + return; + } + + struct object_to_resources_list_node *resource_list_node; + RESOURCE_FOR_EACH_OBJ (resource_list_node, res_node) { + const struct uuid *obj_uuid = &resource_list_node->obj_uuid; + struct ovn_lflow *lflow = ovn_lflow_uuid_find( + &lflow_table->hash_map, obj_uuid); + if (!lflow) { + continue; + } + + size_t n_datapaths; + if (ovn_stage_to_datapath_type(lflow->stage) == DP_SWITCH) { + n_datapaths = n_ls_datapaths; + } else { + n_datapaths = n_lr_datapaths; + } + size_t index; + BITMAP_FOR_EACH_1 (index, n_datapaths, lflow->dpg_bitmap) { + bitmap_set0(lflow->dpg_bitmap, index); + } + } +} + static void sync_lflows_from_objres(struct resource_to_objects_node *res_node, struct lflow_table *lflow_table, diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h index 2cbc4af157..08805435c8 100644 --- a/northd/lflow-mgr.h +++ b/northd/lflow-mgr.h @@ -55,6 +55,10 @@ void lflow_ref_set_od(struct lflow_ref *, const struct ovn_datapath *); void lflow_ref_destroy(struct lflow_ref *); void lflow_ref_clear_lflows(struct lflow_ref *, const struct ovn_datapath *, struct lflow_table *); +void lflow_ref_clear_lflows_for_all_dps(struct lflow_ref *, + size_t n_ls_datapaths, + size_t n_lr_datapaths, + struct lflow_table *); void lflow_ref_clear_and_sync_lflows(struct lflow_ref *, const struct ovn_datapath *, struct lflow_table *lflow_table, diff --git a/northd/northd.c b/northd/northd.c index 7f36b7fb71..1033bc1a8f 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -3531,6 +3531,7 @@ ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t n_ls_datapaths, lb_dps->lb = lb; lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths); lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths); + lb_dps->lflow_ref = lflow_ref_alloc(lb->nlb->name); return lb_dps; } @@ -3540,6 +3541,7 @@ ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps) { bitmap_free(lb_dps->nb_lr_map); bitmap_free(lb_dps->nb_ls_map); + lflow_ref_destroy(lb_dps->lflow_ref); free(lb_dps); } @@ -16056,11 +16058,11 @@ build_lflows_thread(void *arg) lsi->lflows, &lsi->match, &lsi->actions, - NULL); + lb_dps->lflow_ref); build_lrouter_defrag_flows_for_lb(lb_dps, lsi->lflows, lsi->lr_datapaths, &lsi->match, - NULL); + lb_dps->lflow_ref); build_lrouter_flows_for_lb(lb_dps, lsi->lflows, lsi->meter_groups, lsi->lr_datapaths, @@ -16068,14 +16070,14 @@ build_lflows_thread(void *arg) lsi->features, lsi->svc_monitor_map, &lsi->match, &lsi->actions, - NULL); + lb_dps->lflow_ref); build_lswitch_flows_for_lb(lb_dps, lsi->lflows, lsi->meter_groups, lsi->ls_datapaths, lsi->features, lsi->svc_monitor_map, &lsi->match, &lsi->actions, - NULL); + lb_dps->lflow_ref); } } for (bnum = control->id; @@ -16290,20 +16292,20 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths, build_lswitch_arp_nd_service_monitor(lb_dps->lb, lsi.ls_ports, lsi.lflows, &lsi.actions, &lsi.match, - NULL); + lb_dps->lflow_ref); build_lrouter_defrag_flows_for_lb(lb_dps, lsi.lflows, lsi.lr_datapaths, &lsi.match, - NULL); + lb_dps->lflow_ref); build_lrouter_flows_for_lb(lb_dps, lsi.lflows, lsi.meter_groups, lsi.lr_datapaths, lsi.lr_lbnats, lsi.features, lsi.svc_monitor_map, &lsi.match, &lsi.actions, - NULL); + lb_dps->lflow_ref); build_lswitch_flows_for_lb(lb_dps, lsi.lflows, lsi.meter_groups, lsi.ls_datapaths, lsi.features, lsi.svc_monitor_map, &lsi.match, &lsi.actions, - NULL); + lb_dps->lflow_ref); } stopwatch_stop(LFLOWS_LBS_STOPWATCH_NAME, time_msec()); @@ -16612,6 +16614,79 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, return true; } +bool +lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn, + struct tracked_lbs *trk_lbs, + struct lflow_input *lflow_input, + struct lflow_table *lflows) +{ + struct ovn_lb_datapaths *lb_dps; + struct hmapx_node *hmapx_node; + HMAPX_FOR_EACH (hmapx_node, &trk_lbs->deleted) { + lb_dps = hmapx_node->data; + + lflow_ref_clear_lflows_for_all_dps(lb_dps->lflow_ref, + ods_size(lflow_input->ls_datapaths), + ods_size(lflow_input->lr_datapaths), + lflows); + lflow_ref_sync_lflows_to_sb(lb_dps->lflow_ref, lflows, ovnsb_txn, + lflow_input->ls_datapaths, + lflow_input->lr_datapaths, + lflow_input->ovn_internal_version_changed, + lflow_input->sbrec_logical_flow_table, + lflow_input->sbrec_logical_dp_group_table); + } + + HMAPX_FOR_EACH (hmapx_node, &trk_lbs->crupdated) { + lb_dps = hmapx_node->data; + + /* unlink old lflows. */ + lflow_ref_clear_lflows_for_all_dps(lb_dps->lflow_ref, + ods_size(lflow_input->ls_datapaths), + ods_size(lflow_input->lr_datapaths), + lflows); + + /* Generate new lflows. */ + struct ds match = DS_EMPTY_INITIALIZER; + struct ds actions = DS_EMPTY_INITIALIZER; + + build_lswitch_arp_nd_service_monitor(lb_dps->lb, lflow_input->ls_ports, + lflows, &actions, + &match, lb_dps->lflow_ref); + build_lrouter_defrag_flows_for_lb(lb_dps, lflows, + lflow_input->lr_datapaths, &match, + lb_dps->lflow_ref); + build_lrouter_flows_for_lb(lb_dps, lflows, + lflow_input->meter_groups, + lflow_input->lr_datapaths, + lflow_input->lr_lbnats, + lflow_input->features, + lflow_input->svc_monitor_map, + &match, &actions, + lb_dps->lflow_ref); + build_lswitch_flows_for_lb(lb_dps, lflows, + lflow_input->meter_groups, + lflow_input->ls_datapaths, + lflow_input->features, + lflow_input->svc_monitor_map, + &match, &actions, + lb_dps->lflow_ref); + + ds_destroy(&match); + ds_destroy(&actions); + + /* Sync the new flows to SB. */ + lflow_ref_sync_lflows_to_sb(lb_dps->lflow_ref, lflows, ovnsb_txn, + lflow_input->ls_datapaths, + lflow_input->lr_datapaths, + lflow_input->ovn_internal_version_changed, + lflow_input->sbrec_logical_flow_table, + lflow_input->sbrec_logical_dp_group_table); + } + + return true; +} + static bool mirror_needs_update(const struct nbrec_mirror *nb_mirror, const struct sbrec_mirror *sb_mirror) diff --git a/northd/northd.h b/northd/northd.h index 18225cfd07..56613f8412 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -93,6 +93,30 @@ struct ovn_lb_datapaths { size_t n_nb_lr; unsigned long *nb_lr_map; + + /* Reference of lflows generated for this load balancer. + * + * This data is initialized and destroyed by the en_northd node, but + * populated and used only by the en_lflow node. Ideally this data should + * be maintained as part of en_lflow's data (struct lflow_data): a hash + * index from ovn_port key to lflows. However, it would be less efficient + * and more complex: + * + * 1. It would require an extra search (using the index) to find the + * lflows. + * + * 2. Building the index needs to be thread-safe, using either a global + * lock which is obviously less efficient, or hash-based lock array which + * is more complex. + * + * Maintaining the lflow_ref here is more straightforward. The drawback is + * that we need to keep in mind that this data belongs to en_lflow node, + * so never access it from any other nodes. + * + * 'lflow_ref' is used to reference logical flows generated for this + * load balancer. + */ + struct lflow_ref *lflow_ref; }; struct ovn_lb_group_datapaths { @@ -687,6 +711,10 @@ bool lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, struct tracked_ovn_ports *, struct lflow_input *, struct lflow_table *lflows); +bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn, + struct tracked_lbs *, + struct lflow_input *, + struct lflow_table *lflows); bool northd_handle_sb_port_binding_changes( const struct sbrec_port_binding_table *, struct hmap *ls_ports, struct hmap *lr_ports); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 8fc5cd1d60..b276c77221 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -10407,9 +10407,8 @@ check ovn-nbctl --wait=sb lb-add lb1 10.0.0.10:80 10.0.0.3:80 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute nocompute - CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats @@ -10417,21 +10416,26 @@ check ovn-nbctl --wait=sb set load_balancer . ip_port_mappings:10.0.0.3=sw0-p1:1 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_lb_nat_data norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb set load_balancer . options:foo=bar check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_lb_nat_data norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb -- lb-add lb2 20.0.0.10:80 20.0.0.20:80 -- lb-add lb3 30.0.0.10:80 30.0.0.20:80 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_lb_nat_data norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10441,7 +10445,7 @@ check ovn-nbctl --wait=sb -- lb-del lb2 -- lb-del lb3 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_lb_nat_data norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10652,8 +10656,9 @@ check ovn-nbctl --wait=sb add load_balancer_group . load_Balancer $lb1_uuid check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_lb_nat_data norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb clear load_balancer_group . load_Balancer @@ -10668,7 +10673,7 @@ check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb add load_balancer_group . load_Balancer $lb1_uuid check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10677,6 +10682,7 @@ check ovn-nbctl --wait=sb add logical_switch sw0 load_balancer_group $lbg1_uuid check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_lb_nat_data norecompute compute +check_engine_stats ls_lbacls norecompute compute check_engine_stats lflow recompute nocompute check_engine_stats sync_to_sb_lb recompute compute @@ -10685,6 +10691,7 @@ check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb set load_balancer . options:bar=foo check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute +check_engine_stats ls_lbacls norecompute compute check_engine_stats lflow recompute nocompute check_engine_stats sync_to_sb_lb recompute compute @@ -10694,6 +10701,7 @@ check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"10.0.0.10:80"'='"10.0.0.1 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_lb_nat_data norecompute compute +check_engine_stats ls_lbacls norecompute compute check_engine_stats lflow recompute nocompute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10791,6 +10799,7 @@ check_engine_stats northd recompute nocompute check_engine_stats lr_lb_nat_data recompute nocompute check_engine_stats lflow recompute nocompute check_engine_stats sync_to_sb_lb recompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE # Add back lb group to logical switch and then delete it. check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats @@ -10800,6 +10809,7 @@ check_engine_stats northd norecompute compute check_engine_stats lr_lb_nat_data norecompute compute check_engine_stats lflow recompute nocompute check_engine_stats sync_to_sb_lb recompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb clear logical_switch sw0 load_balancer_group -- \ @@ -10833,14 +10843,17 @@ check_engine_stats northd norecompute compute check_engine_stats lr_lb_nat_data norecompute compute check_engine_stats lflow norecompute nocompute check_engine_stats sync_to_sb_lb norecompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb set load_balancer_group . load_balancer="$lb2_uuid,$lb3_uuid,$lb4_uuid" check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute +check_engine_stats ls_lbacls norecompute compute check_engine_stats lr_lb_nat_data norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb set logical_switch sw0 load_balancer_group=$lbg1_uuid -- 2.41.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
