In some cases, when a port was going to be postponed, the same port (lsp) was claimed four times: - within runtime_data handling pb changes: - lsp is claimed as handling pb changes, and added to postponed list. - it's claimed again as handling postponed list. - within runtime_data handling postponed list, the same two operations: - lsp is claimed as handling pb changes, and added to postponed list. - it's claimed again as handling postponed list.
With this patch, the lsp1 port would only only be claimed twice. - within runtime_data handling pb changes: - lsp is claimed as handling pb changes, and added to postponed list. - within runtime_data handling postponed list, the same two operations: - it's claimed again as handling postponed list. This patch is only about CPU utilization; it should not result in any flow changes. Signed-off-by: Xavier Simonart <[email protected]> --- v2: rebase on origin/main for scapy fix. v3: - Updated based on Ales' question: remove deleted pb from postponed_ports. - Rebase on origin/main. Signed-off-by: Xavier Simonart <[email protected]> --- controller/binding.c | 46 ++++++++++++++++++++++--------------- controller/binding.h | 2 ++ controller/ovn-controller.c | 37 +++++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 21 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 16302046e..a6e02bb5e 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -3102,6 +3102,33 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in, return handled; } +bool +binding_handle_postponed_ports(struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + /* Also handle any postponed (throttled) ports. */ + const char *port_name; + const struct sbrec_port_binding *pb; + bool handled = true; + struct sset postponed_ports = SSET_INITIALIZER(&postponed_ports); + sset_clone(&postponed_ports, b_ctx_out->postponed_ports); + SSET_FOR_EACH (port_name, &postponed_ports) { + pb = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, + port_name); + if (!pb) { + sset_find_and_delete(b_ctx_out->postponed_ports, port_name); + continue; + } + handled = handle_updated_port(b_ctx_in, b_ctx_out, pb); + if (!handled) { + break; + } + } + sset_destroy(&postponed_ports); + cleanup_claimed_port_timestamps(); + return handled; +} + /* Returns true if the port binding changes resulted in local binding * updates, false otherwise. */ @@ -3248,25 +3275,6 @@ delete_done: } } - /* Also handle any postponed (throttled) ports. */ - const char *port_name; - struct sset postponed_ports = SSET_INITIALIZER(&postponed_ports); - sset_clone(&postponed_ports, b_ctx_out->postponed_ports); - SSET_FOR_EACH (port_name, &postponed_ports) { - pb = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, - port_name); - if (!pb) { - sset_find_and_delete(b_ctx_out->postponed_ports, port_name); - continue; - } - handled = handle_updated_port(b_ctx_in, b_ctx_out, pb); - if (!handled) { - break; - } - } - sset_destroy(&postponed_ports); - cleanup_claimed_port_timestamps(); - if (handled) { /* There may be new local datapaths added by the above handling, so go * through each port_binding of newly added local datapaths to update diff --git a/controller/binding.h b/controller/binding.h index d13ae36c7..5303d4f58 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -196,6 +196,8 @@ bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, struct binding_ctx_out *); +bool binding_handle_postponed_ports(struct binding_ctx_in *, + struct binding_ctx_out *); bool binding_handle_port_binding_changes(struct binding_ctx_in *, struct binding_ctx_out *); void binding_tracked_dp_destroy(struct hmap *tracked_datapaths); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 157def2a3..584b4b579 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1540,6 +1540,38 @@ runtime_data_ovs_interface_shadow_handler(struct engine_node *node, void *data) return true; } +static bool +runtime_data_postponed_ports_handler(struct engine_node *node, void *data) +{ + struct ed_type_runtime_data *rt_data = 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); + if (!b_ctx_in.chassis_rec) { + return false; + } + + rt_data->tracked = true; + b_ctx_out.tracked_dp_bindings = &rt_data->tracked_dp_bindings; + + if (!binding_handle_postponed_ports(&b_ctx_in, &b_ctx_out)) { + return false; + } + + rt_data->local_lports_changed = b_ctx_out.local_lports_changed; + rt_data->localnet_learn_fdb = b_ctx_out.localnet_learn_fdb; + rt_data->localnet_learn_fdb_changed = b_ctx_out.localnet_learn_fdb_changed; + if (b_ctx_out.related_lports_changed || + b_ctx_out.non_vif_ports_changed || + b_ctx_out.local_lports_changed || + b_ctx_out.localnet_learn_fdb_changed || + !hmap_is_empty(b_ctx_out.tracked_dp_bindings)) { + engine_set_node_state(node, EN_UPDATED); + } + + return true; +} + static bool runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) { @@ -5260,9 +5292,10 @@ main(int argc, char *argv[]) runtime_data_sb_datapath_binding_handler); engine_add_input(&en_runtime_data, &en_sb_port_binding, runtime_data_sb_port_binding_handler); - /* Reuse the same handler for any previously postponed ports. */ + /* Run postponed_ports_handler after port_binding_handler in case port get + * deleted */ engine_add_input(&en_runtime_data, &en_postponed_ports, - runtime_data_sb_port_binding_handler); + runtime_data_postponed_ports_handler); /* Run sb_ro_handler after port_binding_handler in case port get deleted */ engine_add_input(&en_runtime_data, &en_sb_ro, runtime_data_sb_ro_handler); -- 2.47.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
