seq_read should be used before processing any changes. Before this patch, if pinctrl thread tried to notify main thread after ovn-controller run pinctrl_run but before pinctrl_read, the wake up was lost.
Signed-off-by: Xavier Simonart <[email protected]> --- -v2: - Based on Ales's feedback: -- Move back seq_wait within pinctrl_wait. -- Use uint64_t instead of int64_t for value read by seq_read. -- Fix similar potential seq_read issue in statctrl. - While at it: -- Test ovnsb_idl_txn only once for all wait_ function which require it. --- controller/pinctrl.c | 48 +++++++++++++++++++------------------------ controller/statctrl.c | 6 ++++-- 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/controller/pinctrl.c b/controller/pinctrl.c index c2fe75100..f2751cca6 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -161,6 +161,7 @@ VLOG_DEFINE_THIS_MODULE(pinctrl); static struct ovs_mutex pinctrl_mutex = OVS_MUTEX_INITIALIZER; static struct seq *pinctrl_handler_seq; static struct seq *pinctrl_main_seq; +static uint64_t main_seq; #define ARP_ND_DEF_MAX_TIMEOUT 16000 @@ -207,7 +208,7 @@ static void run_put_mac_bindings( struct ovsdb_idl_index *sbrec_port_binding_by_key, struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip) OVS_REQUIRES(pinctrl_mutex); -static void wait_put_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn); +static void wait_put_mac_bindings(void); static void send_mac_binding_buffered_pkts(struct rconn *swconn) OVS_REQUIRES(pinctrl_mutex); @@ -260,7 +261,7 @@ static void pinctrl_handle_nd_ns(struct rconn *swconn, static void pinctrl_handle_event(struct ofpbuf *userdata) OVS_REQUIRES(pinctrl_mutex); -static void wait_controller_event(struct ovsdb_idl_txn *ovnsb_idl_txn); +static void wait_controller_event(void); static void init_ipv6_ras(void); static void destroy_ipv6_ras(void); static void ipv6_ra_wait(long long int send_ipv6_ra_time); @@ -307,7 +308,7 @@ static void run_put_vport_bindings( struct ovsdb_idl_index *sbrec_port_binding_by_key, const struct sbrec_chassis *chassis, int64_t cur_cfg) OVS_REQUIRES(pinctrl_mutex); -static void wait_put_vport_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn); +static void wait_put_vport_bindings(void); static void pinctrl_handle_bind_vport(const struct flow *md, struct ofpbuf *userdata); static void pinctrl_handle_svc_check(struct rconn *swconn, @@ -375,7 +376,7 @@ static void run_put_fdbs(struct ovsdb_idl_txn *ovnsb_idl_txn, struct ovsdb_idl_index *sbrec_fdb_by_dp_key_mac, uint64_t cur_cfg) OVS_REQUIRES(pinctrl_mutex); -static void wait_put_fdbs(struct ovsdb_idl_txn *ovnsb_idl_txn); +static void wait_put_fdbs(void); static void pinctrl_handle_put_fdb(const struct flow *md, const struct flow *headers) OVS_REQUIRES(pinctrl_mutex); @@ -556,6 +557,7 @@ pinctrl_init(void) pinctrl.mac_binding_can_timestamp = false; pinctrl_handler_seq = seq_create(); pinctrl_main_seq = seq_create(); + main_seq = seq_read(pinctrl_main_seq); latch_init(&pinctrl.pinctrl_thread_exit); pinctrl.pinctrl_thread = ovs_thread_create("ovn_pinctrl", pinctrl_handler, @@ -4074,6 +4076,9 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, int64_t cur_cfg) { ovs_mutex_lock(&pinctrl_mutex); + + main_seq = seq_read(pinctrl_main_seq); + run_put_mac_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key, sbrec_port_binding_by_key, sbrec_mac_binding_by_lport_ip); @@ -4591,12 +4596,13 @@ void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn) { ovs_mutex_lock(&pinctrl_mutex); - wait_put_mac_bindings(ovnsb_idl_txn); - wait_controller_event(ovnsb_idl_txn); - wait_put_vport_bindings(ovnsb_idl_txn); - int64_t new_seq = seq_read(pinctrl_main_seq); - seq_wait(pinctrl_main_seq, new_seq); - wait_put_fdbs(ovnsb_idl_txn); + if (ovnsb_idl_txn) { + wait_put_mac_bindings(); + wait_controller_event(); + wait_put_vport_bindings(); + wait_put_fdbs(); + seq_wait(pinctrl_main_seq, main_seq); + } wait_activated_ports(); ovs_mutex_unlock(&pinctrl_mutex); } @@ -4861,13 +4867,9 @@ run_buffered_binding(const struct sbrec_mac_binding_table *mac_binding_table, } static void -wait_put_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn) +wait_put_mac_bindings(void) OVS_REQUIRES(pinctrl_mutex) { - if (!ovnsb_idl_txn) { - return; - } - struct mac_binding *mb; HMAP_FOR_EACH (mb, hmap_node, &put_mac_bindings) { poll_timer_wait_until(mb->timestamp); @@ -6524,12 +6526,8 @@ exit: } static void -wait_controller_event(struct ovsdb_idl_txn *ovnsb_idl_txn) +wait_controller_event(void) { - if (!ovnsb_idl_txn) { - return; - } - for (size_t i = 0; i < OVN_EVENT_MAX; i++) { if (!hmap_is_empty(&event_table[i])) { poll_immediate_wake(); @@ -6674,9 +6672,9 @@ destroy_put_vport_bindings(void) } static void -wait_put_vport_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn) +wait_put_vport_bindings(void) { - if (ovnsb_idl_txn && !hmap_is_empty(&put_vport_bindings)) { + if (!hmap_is_empty(&put_vport_bindings)) { poll_immediate_wake(); } } @@ -8814,13 +8812,9 @@ run_put_fdbs(struct ovsdb_idl_txn *ovnsb_idl_txn, static void -wait_put_fdbs(struct ovsdb_idl_txn *ovnsb_idl_txn) +wait_put_fdbs(void) OVS_REQUIRES(pinctrl_mutex) { - if (!ovnsb_idl_txn) { - return; - } - struct fdb *fdb; HMAP_FOR_EACH (fdb, hmap_node, &put_fdbs) { poll_timer_wait_until(fdb->timestamp); diff --git a/controller/statctrl.c b/controller/statctrl.c index 454314dda..a76bac056 100644 --- a/controller/statctrl.c +++ b/controller/statctrl.c @@ -96,6 +96,7 @@ struct statctrl_ctx { struct seq *thread_seq; struct seq *main_seq; + uint64_t new_main_seq; struct stats_node nodes[STATS_MAX]; }; @@ -131,6 +132,7 @@ statctrl_init(void) ovs_mutex_init(&mutex); statctrl_ctx.thread_seq = seq_create(); statctrl_ctx.main_seq = seq_create(); + statctrl_ctx.new_main_seq = seq_read(statctrl_ctx.main_seq); /* Definition of all stat nodes. */ struct ofputil_flow_stats_request mac_binding_request = { @@ -189,6 +191,7 @@ statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, long long now = time_msec(); ovs_mutex_lock(&mutex); + statctrl_ctx.new_main_seq = seq_read(statctrl_ctx.main_seq); for (size_t i = 0; i < STATS_MAX; i++) { struct stats_node *node = &statctrl_ctx.nodes[i]; uint64_t prev_delay = node->request_delay; @@ -241,8 +244,7 @@ statctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn) poll_immediate_wake(); } } - int64_t new_seq = seq_read(statctrl_ctx.main_seq); - seq_wait(statctrl_ctx.main_seq, new_seq); + seq_wait(statctrl_ctx.main_seq, statctrl_ctx.new_main_seq); ovs_mutex_unlock(&mutex); } -- 2.47.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
