There was a chance for race condition when idl_txn wasn't available and the current_br_int_name changed before that old tunnels were cleaned up. To prevent the race condition update the curent_br_int_name only after old tunnel cleanup.
Signed-off-by: Ales Musil <[email protected]> --- controller/encaps.c | 21 ++++++++++++++++----- controller/encaps.h | 5 +++-- controller/ovn-controller.c | 26 ++------------------------ 3 files changed, 21 insertions(+), 31 deletions(-) diff --git a/controller/encaps.c b/controller/encaps.c index 32cd0882c..b69d72584 100644 --- a/controller/encaps.c +++ b/controller/encaps.c @@ -36,6 +36,8 @@ VLOG_DEFINE_THIS_MODULE(encaps); */ #define OVN_MVTEP_CHASSISID_DELIM '@' +static char *current_br_int_name = NULL; + void encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl) { @@ -409,14 +411,13 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct sbrec_sb_global *sbg, const struct ovsrec_open_vswitch_table *ovs_table, const struct sset *transport_zones, - const struct ovsrec_bridge_table *bridge_table, - const char *br_int_name) + const struct ovsrec_bridge_table *bridge_table) { if (!ovs_idl_txn || !br_int) { return; } - if (!br_int_name) { + if (!current_br_int_name) { /* The controller has just started, we need to look through all * bridges for old tunnel ports. */ char *tunnel_prefix = xasprintf("ovn%s-", get_chassis_idx(ovs_table)); @@ -431,14 +432,18 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, } free(tunnel_prefix); - } else if (strcmp(br_int_name, br_int->name)) { + current_br_int_name = xstrdup(br_int->name); + } else if (strcmp(current_br_int_name, br_int->name)) { /* The integration bridge was changed, clear tunnel ports from * the old one. */ const struct ovsrec_bridge *old_br_int = - get_bridge(bridge_table, br_int_name); + get_bridge(bridge_table, current_br_int_name); if (old_br_int) { clear_old_tunnels(old_br_int, "", 0); } + + free(current_br_int_name); + current_br_int_name = xstrdup(br_int->name); } const struct sbrec_chassis *chassis_rec; @@ -553,3 +558,9 @@ encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn, return !any_changes; } + +void +encaps_destroy(void) +{ + free(current_br_int_name); +} diff --git a/controller/encaps.h b/controller/encaps.h index cf38dac1a..3e58b3c82 100644 --- a/controller/encaps.h +++ b/controller/encaps.h @@ -36,8 +36,7 @@ void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct sbrec_sb_global *, const struct ovsrec_open_vswitch_table *, const struct sset *transport_zones, - const struct ovsrec_bridge_table *bridge_table, - const char *br_int_name); + const struct ovsrec_bridge_table *bridge_table); bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn, const struct ovsrec_bridge *br_int); @@ -48,4 +47,6 @@ bool encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id, bool encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id, const char *encap_ip); +void encaps_destroy(void); + #endif /* controller/encaps.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index e170e9262..c094cb74d 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -536,21 +536,6 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn, *br_int_ = br_int; } -static void -consider_br_int_change(const struct ovsrec_bridge *br_int, char **current_name) -{ - ovs_assert(current_name); - - if (!*current_name) { - *current_name = xstrdup(br_int->name); - } - - if (strcmp(*current_name, br_int->name)) { - free(*current_name); - *current_name = xstrdup(br_int->name); - } -} - static void update_ssl_config(const struct ovsrec_ssl_table *ssl_table) { @@ -4933,8 +4918,6 @@ main(int argc, char *argv[]) char *ovn_version = ovn_get_internal_version(); VLOG_INFO("OVN internal version is : [%s]", ovn_version); - char *current_br_int_name = NULL; - /* Main loop. */ exiting = false; restart = false; @@ -5088,8 +5071,7 @@ main(int argc, char *argv[]) sbrec_sb_global_first(ovnsb_idl_loop.idl), ovs_table, &transport_zones, - bridge_table, - current_br_int_name); + bridge_table); stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME, time_msec()); @@ -5276,10 +5258,6 @@ main(int argc, char *argv[]) stopwatch_stop(IF_STATUS_MGR_RUN_STOPWATCH_NAME, time_msec()); } - /* The name needs to be reflected at the end of the block. - * This allows us to detect br-int changes and act - * accordingly. */ - consider_br_int_change(br_int, ¤t_br_int_name); } if (!engine_has_run()) { @@ -5463,7 +5441,6 @@ loop_done: } free(ovn_version); - free(current_br_int_name); unixctl_server_destroy(unixctl); lflow_destroy(); ofctrl_destroy(); @@ -5471,6 +5448,7 @@ loop_done: binding_destroy(); patch_destroy(); mirror_destroy(); + encaps_destroy(); if_status_mgr_destroy(if_mgr); shash_destroy(&vif_plug_deleted_iface_ids); shash_destroy(&vif_plug_changed_iface_ids); -- 2.39.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
