From: Numan Siddique <num...@ovn.org> In order to handle runtime data changes incrementally, the flow outut runtime data handle should know the changed runtime data. Runtime data now tracks the changed data for any OVS interface and SB port binding changes. The tracked data contains a hmap of tracked datapaths (which changed during runtime data processing.
The flow outout runtime_data handler in this patch doesn't do much with the tracked data. It returns false if there is tracked data available so that flow_output run is called. If no tracked data is available then there is no need for flow computation and the handler returns true. Next patch in the series processes the tracked data incrementally. Co-Authored-by: Venkata Anil <anilvenk...@redhat.com> Signed-off-by: Venkata Anil <anilvenk...@redhat.com> Signed-off-by: Numan Siddique <num...@ovn.org> --- controller/binding.c | 156 +++++++++++++++++++++++++++++++++--- controller/binding.h | 6 ++ controller/ovn-controller.c | 62 +++++++++++++- controller/ovn-controller.h | 16 +++- tests/ovn-performance.at | 24 +++--- 5 files changed, 237 insertions(+), 27 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 6eab08e5c..2223c7e66 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -69,13 +69,20 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl) ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type); } +static struct tracked_binding_datapath *tracked_binding_datapath_create( + struct hmap *tracked_dps, const struct sbrec_datapath_binding *, + bool is_new); +static struct tracked_binding_datapath *tracked_binding_datapath_find( + struct hmap *, const struct sbrec_datapath_binding *); + static void add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_datapath, struct ovsdb_idl_index *sbrec_port_binding_by_name, const struct sbrec_datapath_binding *datapath, bool has_local_l3gateway, int depth, - struct hmap *local_datapaths) + struct hmap *local_datapaths, + struct hmap *updated_dp_bindings) { uint32_t dp_key = datapath->tunnel_key; struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key); @@ -92,6 +99,11 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, ld->localnet_port = NULL; ld->has_local_l3gateway = has_local_l3gateway; + if (updated_dp_bindings && + !tracked_binding_datapath_find(updated_dp_bindings, datapath)) { + tracked_binding_datapath_create(updated_dp_bindings, datapath, true); + } + if (depth >= 100) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "datapaths nested too deep"); @@ -124,7 +136,8 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, peer->datapath, false, - depth + 1, local_datapaths); + depth + 1, local_datapaths, + updated_dp_bindings); } ld->n_peer_ports++; if (ld->n_peer_ports > ld->n_allocated_peer_ports) { @@ -147,12 +160,14 @@ add_local_datapath(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_datapath, struct ovsdb_idl_index *sbrec_port_binding_by_name, const struct sbrec_datapath_binding *datapath, - bool has_local_l3gateway, struct hmap *local_datapaths) + bool has_local_l3gateway, struct hmap *local_datapaths, + struct hmap *updated_dp_bindings) { add_local_datapath__(sbrec_datapath_binding_by_key, sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, - datapath, has_local_l3gateway, 0, local_datapaths); + datapath, has_local_l3gateway, 0, local_datapaths, + updated_dp_bindings); } static void @@ -580,6 +595,71 @@ local_binding_find_child(struct local_binding *lbinding, return NULL; } +static struct tracked_binding_datapath * +tracked_binding_datapath_create(struct hmap *tracked_datapaths, + const struct sbrec_datapath_binding *dp, + bool is_new) +{ + struct tracked_binding_datapath *t_dp = xzalloc(sizeof *t_dp); + t_dp->dp = dp; + t_dp->is_new = is_new; + ovs_list_init(&t_dp->lports_head); + hmap_insert(tracked_datapaths, &t_dp->node, uuid_hash(&dp->header_.uuid)); + return t_dp; +} + +static struct tracked_binding_datapath * +tracked_binding_datapath_find(struct hmap *tracked_datapaths, + const struct sbrec_datapath_binding *dp) +{ + struct tracked_binding_datapath *t_dp; + size_t hash = uuid_hash(&dp->header_.uuid); + HMAP_FOR_EACH_WITH_HASH (t_dp, node, hash, tracked_datapaths) { + if (uuid_equals(&t_dp->dp->header_.uuid, &dp->header_.uuid)) { + return t_dp; + } + } + + return NULL; +} + +static void +tracked_binding_datapath_lport_add(struct hmap *tracked_datapaths, + const struct sbrec_port_binding *pb, + bool deleted) +{ + if (!tracked_datapaths) { + return; + } + + struct tracked_binding_datapath *tracked_dp = + tracked_binding_datapath_find(tracked_datapaths, pb->datapath); + if (!tracked_dp) { + tracked_dp = tracked_binding_datapath_create(tracked_datapaths, + pb->datapath, false); + } + struct tracked_binding_lport *lport = xmalloc(sizeof *lport); + lport->pb = pb; + lport->deleted = deleted; + ovs_list_push_back(&tracked_dp->lports_head, &lport->list_node); +} + +void +binding_tracked_dp_destroy(struct hmap *tracked_datapaths) +{ + struct tracked_binding_datapath *t_dp; + HMAP_FOR_EACH_POP (t_dp, node, tracked_datapaths) { + struct tracked_binding_lport *lport, *next; + LIST_FOR_EACH_SAFE (lport, next, list_node, &t_dp->lports_head) { + ovs_list_remove(&lport->list_node); + free(lport); + } + free(t_dp); + } + + hmap_destroy(tracked_datapaths); +} + void binding_add_vport_to_local_bindings(struct shash *local_bindings, const struct sbrec_port_binding *parent, @@ -668,6 +748,7 @@ release_lport(const struct sbrec_port_binding *pb, bool cant_update_sb) static bool release_local_binding_children(struct local_binding *lbinding, + struct hmap *tracked_dp_bindings, bool cant_update_sb) { struct local_binding *l; @@ -675,15 +756,23 @@ release_local_binding_children(struct local_binding *lbinding, if (!release_lport(l->pb, cant_update_sb)) { return false; } + if (tracked_dp_bindings) { + tracked_binding_datapath_lport_add(tracked_dp_bindings, l->pb, + true); + } } return true; } static bool -release_local_binding(struct local_binding *lbinding, bool cant_update_sb) +release_local_binding(struct local_binding *lbinding, + struct hmap *tracked_dp_bindings, + bool cant_update_sb) { - if (!release_local_binding_children(lbinding, cant_update_sb)) { + if (!release_local_binding_children(lbinding, + tracked_dp_bindings, + cant_update_sb)) { return false; } @@ -691,6 +780,10 @@ release_local_binding(struct local_binding *lbinding, bool cant_update_sb) return false; } + if (tracked_dp_bindings) { + tracked_binding_datapath_lport_add(tracked_dp_bindings, lbinding->pb, + true); + } return true; } @@ -748,6 +841,10 @@ consider_port_binding_for_vif(const struct sbrec_port_binding *pb, child = local_binding_create(pb->logical_port, lbinding->iface, pb, binding_type); local_binding_add_child(lbinding, child); + if (b_ctx_out->tracked_dp_bindings) { + tracked_binding_datapath_lport_add( + b_ctx_out->tracked_dp_bindings, pb, false); + } } else { ovs_assert(child->type == BT_CHILD || child->type == BT_VIRTUAL); @@ -763,7 +860,8 @@ consider_port_binding_for_vif(const struct sbrec_port_binding *pb, add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, - pb->datapath, false, b_ctx_out->local_datapaths); + pb->datapath, false, b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); update_local_lport_ids(b_ctx_out->local_lport_ids, pb); if (lbinding->iface && qos_map && b_ctx_in->ovs_idl_txn) { get_qos_params(pb, qos_map); @@ -807,7 +905,8 @@ consider_port_binding(const struct sbrec_port_binding *pb, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, pb->datapath, false, - b_ctx_out->local_datapaths); + b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); } } else if (!strcmp(pb->type, "chassisredirect")) { if (ha_chassis_group_contains(pb->ha_chassis_group, @@ -816,14 +915,16 @@ consider_port_binding(const struct sbrec_port_binding *pb, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, pb->datapath, false, - b_ctx_out->local_datapaths); + b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); } } else if (!strcmp(pb->type, "l3gateway")) { if (our_chassis) { add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, - pb->datapath, true, b_ctx_out->local_datapaths); + pb->datapath, true, b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); } } else if (!strcmp(pb->type, "localnet")) { /* Add all localnet ports to local_lports so that we allocate ct zones @@ -839,7 +940,8 @@ consider_port_binding(const struct sbrec_port_binding *pb, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, pb->datapath, false, - b_ctx_out->local_datapaths); + b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); } } @@ -978,6 +1080,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) consider_port_binding_for_vif(binding_rec, b_ctx_in, binding_type, lbinding, b_ctx_out, qos_map_ptr); + if (lbinding && lbinding->pb && + lbinding->pb->chassis == b_ctx_in->chassis_rec) { + } } else { consider_port_binding(binding_rec, b_ctx_in, b_ctx_out, qos_map_ptr); @@ -1094,7 +1199,8 @@ add_local_datapath_peer_port(const struct sbrec_port_binding *pb, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, peer->datapath, false, - 1, b_ctx_out->local_datapaths); + 1, b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); return; } @@ -1217,6 +1323,7 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, struct hmap *qos_map_ptr = sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; + *b_ctx_out->local_lports_changed = false; const struct ovsrec_interface *iface_rec; OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, b_ctx_in->iface_table) { @@ -1259,6 +1366,7 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, lbinding->pb->chassis == b_ctx_in->chassis_rec) { if (!release_local_binding(lbinding, + b_ctx_out->tracked_dp_bindings, !b_ctx_in->ovnsb_idl_txn)) { handled = false; goto out; @@ -1271,12 +1379,14 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, b_ctx_in->chassis_rec, b_ctx_out, ld); } + local_binding_delete(b_ctx_out->local_bindings, lbinding); *changed = true; } sset_find_and_delete(b_ctx_out->local_lports, cleared_iface_id); smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); + *b_ctx_out->local_lports_changed = true; } if (iface_id && ofport > 0) { @@ -1284,6 +1394,7 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, sset_add(b_ctx_out->local_lports, iface_id); smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, iface_id); + *b_ctx_out->local_lports_changed = true; claim_lbinding = local_binding_find(b_ctx_out->local_bindings, iface_id); if (!claim_lbinding) { @@ -1307,6 +1418,8 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, } if (claim_lbinding->pb) { + bool claimed = + (claim_lbinding->pb->chassis == b_ctx_in->chassis_rec); if (!consider_port_binding_for_vif(claim_lbinding->pb, b_ctx_in, BT_VIF, claim_lbinding, @@ -1314,6 +1427,14 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, handled = false; goto out; } + bool now_claimed = (claim_lbinding->pb && ( + claim_lbinding->pb->chassis == b_ctx_in->chassis_rec)); + if (!claimed && now_claimed) { + /* Add this to the updated tracked datapath bindings. */ + tracked_binding_datapath_lport_add( + b_ctx_out->tracked_dp_bindings, claim_lbinding->pb, + false); + } } } } @@ -1377,9 +1498,13 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, if (is_deleted) { if (lbinding) { lbinding->pb = NULL; + /* Add this to the deleted tracked datapath bindings. */ + tracked_binding_datapath_lport_add( + b_ctx_out->tracked_dp_bindings, pb, true); if (binding_type == BT_VIF && !release_local_binding_children( - lbinding, !b_ctx_in->ovnsb_idl_txn)) { + lbinding, b_ctx_out->tracked_dp_bindings, + !b_ctx_in->ovnsb_idl_txn)) { handled = false; break; } @@ -1407,6 +1532,11 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec); if (!claimed && now_claimed) { *changed = true; + + /* Add this to the updated tracked datapath + * bindings. */ + tracked_binding_datapath_lport_add( + b_ctx_out->tracked_dp_bindings, pb, false); } } } else { diff --git a/controller/binding.h b/controller/binding.h index fda680723..2f79dd63c 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -18,6 +18,9 @@ #define OVN_BINDING_H 1 #include <stdbool.h> +#include "openvswitch/hmap.h" +#include "openvswitch/uuid.h" +#include "openvswitch/list.h" struct hmap; struct ovsdb_idl; @@ -57,6 +60,8 @@ struct binding_ctx_out { struct sset *local_lport_ids; struct sset *egress_ifaces; struct smap *local_iface_ids; + struct hmap *tracked_dp_bindings; + bool *local_lports_changed; }; void binding_register_ovs_idl(struct ovsdb_idl *); @@ -74,4 +79,5 @@ bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, bool binding_handle_port_binding_changes(struct binding_ctx_in *, struct binding_ctx_out *, bool *changed); +void binding_tracked_dp_destroy(struct hmap *tracked_datapaths); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 289a55dbd..94ad79295 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -978,6 +978,23 @@ struct ed_type_runtime_data { struct smap local_iface_ids; }; +struct ed_type_runtime_tracked_data { + struct hmap tracked_dp_bindings; + bool local_lports_changed; + bool tracked; +}; + +static void +en_runtime_clear_tracked_data(void *tracked_data) +{ + struct ed_type_runtime_tracked_data *data = tracked_data; + + binding_tracked_dp_destroy(&data->tracked_dp_bindings); + hmap_init(&data->tracked_dp_bindings); + data->local_lports_changed = false; + data->tracked = false; +} + static void * en_runtime_data_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) @@ -991,6 +1008,13 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, sset_init(&data->egress_ifaces); shash_init(&data->local_bindings); smap_init(&data->local_iface_ids); + + struct ed_type_runtime_tracked_data *tracked_data = + xzalloc(sizeof *tracked_data); + hmap_init(&tracked_data->tracked_dp_bindings); + node->tracked_data = tracked_data; + node->clear_tracked_data = en_runtime_clear_tracked_data; + return data; } @@ -1093,6 +1117,8 @@ init_binding_ctx(struct engine_node *node, b_ctx_out->egress_ifaces = &rt_data->egress_ifaces; b_ctx_out->local_bindings = &rt_data->local_bindings; b_ctx_out->local_iface_ids = &rt_data->local_iface_ids; + b_ctx_out->tracked_dp_bindings = NULL; + b_ctx_out->local_lports_changed = NULL; } static void @@ -1104,6 +1130,8 @@ en_runtime_data_run(struct engine_node *node, void *data) struct sset *local_lport_ids = &rt_data->local_lport_ids; struct sset *active_tunnels = &rt_data->active_tunnels; + en_runtime_clear_tracked_data(node->tracked_data); + static bool first_run = true; if (first_run) { /* don't cleanup since there is no data yet */ @@ -1154,9 +1182,13 @@ static bool runtime_data_ovs_interface_handler(struct engine_node *node, void *data) { struct ed_type_runtime_data *rt_data = data; + struct ed_type_runtime_tracked_data *tracked_data = node->tracked_data; struct binding_ctx_in b_ctx_in; struct binding_ctx_out b_ctx_out; init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out); + tracked_data->tracked = true; + b_ctx_out.tracked_dp_bindings = &tracked_data->tracked_dp_bindings; + b_ctx_out.local_lports_changed = &tracked_data->local_lports_changed; bool changed = false; if (!binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out, @@ -1189,6 +1221,10 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) return false; } + struct ed_type_runtime_tracked_data *tracked_data = node->tracked_data; + tracked_data->tracked = true; + b_ctx_out.tracked_dp_bindings = &tracked_data->tracked_dp_bindings; + bool changed = false; if (!binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out, &changed)) { @@ -1870,6 +1906,29 @@ physical_flow_changes_ovs_iface_handler(struct engine_node *node OVS_UNUSED, return true; } +static bool +flow_output_runtime_data_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + struct ed_type_runtime_tracked_data *tracked_data = + engine_get_input_tracked_data("runtime_data", node); + + if (!tracked_data || !tracked_data->tracked) { + return false; + } + + if (!hmap_is_empty(&tracked_data->tracked_dp_bindings)) { + /* We are not yet handling the tracked datapath binding + * changes. Return false to trigger full recompute. */ + return false; + } + + if (tracked_data->local_lports_changed) { + engine_set_node_state(node, EN_UPDATED); + } + return true; +} + struct ovn_controller_exit_args { bool *exiting; bool *restart; @@ -2021,7 +2080,8 @@ main(int argc, char *argv[]) flow_output_addr_sets_handler); engine_add_input(&en_flow_output, &en_port_groups, flow_output_port_groups_handler); - engine_add_input(&en_flow_output, &en_runtime_data, NULL); + engine_add_input(&en_flow_output, &en_runtime_data, + flow_output_runtime_data_handler); engine_add_input(&en_flow_output, &en_mff_ovn_geneve, NULL); engine_add_input(&en_flow_output, &en_physical_flow_changes, flow_output_physical_flow_changes_handler); diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h index 40c358cef..6c40a6492 100644 --- a/controller/ovn-controller.h +++ b/controller/ovn-controller.h @@ -84,7 +84,6 @@ struct local_binding { enum local_binding_type type; const struct ovsrec_interface *iface; const struct sbrec_port_binding *pb; - struct ovs_list children; }; @@ -94,6 +93,21 @@ local_binding_find(struct shash *local_bindings, const char *name) return shash_find_data(local_bindings, name); } +/* Represents a tracked binding logical port. */ +struct tracked_binding_lport { + const struct sbrec_port_binding *pb; + struct ovs_list list_node; + bool deleted; +}; + +/* Represent a tracked binding datapath. */ +struct tracked_binding_datapath { + struct hmap_node node; + const struct sbrec_datapath_binding *dp; + bool is_new; + struct ovs_list lports_head; /* List of struct tracked_binding_lport. */ +}; + const struct ovsrec_bridge *get_bridge(const struct ovsrec_bridge_table *, const char *br_name); diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index 5cc1960b6..6df42ce3a 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -274,7 +274,7 @@ for i in 1 2; do ) # Add router port to $ls - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lrp-add lr1 $lrp 02:00:00:00:0$i:01 10.0.$i.1/24] ) @@ -282,15 +282,15 @@ for i in 1 2; do [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-add $ls $lsp] ) - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-set-type $lsp router] ) - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-set-options $lsp router-port=$lrp] ) - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-set-addresses $lsp router] ) @@ -416,15 +416,15 @@ OVN_CONTROLLER_EXPECT_NO_HIT( [ovn-nbctl --wait=hv destroy Port_Group pg1] ) -for i in 1 2; do - ls=ls$i +OVN_CONTROLLER_EXPECT_HIT( + [hv1 hv2], [lflow_run], + [ovn-nbctl --wait=hv ls-del ls1] +) - # Delete switch $ls - OVN_CONTROLLER_EXPECT_HIT( - [hv1 hv2], [lflow_run], - [ovn-nbctl --wait=hv ls-del $ls] - ) -done +OVN_CONTROLLER_EXPECT_NO_HIT( + [hv1 hv2], [lflow_run], + [ovn-nbctl --wait=hv ls-del ls2] +) # Delete router lr1 OVN_CONTROLLER_EXPECT_NO_HIT( -- 2.25.4 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev