diff options
| -rw-r--r-- | ofproto/connmgr.c | 16 | ||||
| -rw-r--r-- | ofproto/connmgr.h | 6 | ||||
| -rw-r--r-- | ofproto/fail-open.c | 8 | ||||
| -rw-r--r-- | ofproto/fail-open.h | 2 | ||||
| -rw-r--r-- | ofproto/in-band.c | 2 | ||||
| -rw-r--r-- | ofproto/ofproto-provider.h | 2 | ||||
| -rw-r--r-- | ofproto/ofproto.c | 36 | ||||
| -rw-r--r-- | ofproto/ofproto.h | 3 |
8 files changed, 55 insertions, 20 deletions
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index d2bedadd0..51e676abc 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -191,7 +191,10 @@ struct connmgr { /* OpenFlow connections. */ struct hmap controllers; /* All OFCONN_PRIMARY controllers. */ - struct ovs_list all_conns; /* All controllers. */ + struct ovs_list all_conns; /* All controllers. All modifications are + protected by ofproto_mutex, so that any + traversals from other threads can be made + safe by holding the ofproto_mutex. */ uint64_t master_election_id; /* monotonically increasing sequence number * for master election */ bool master_election_id_defined; @@ -255,6 +258,7 @@ connmgr_create(struct ofproto *ofproto, /* Frees 'mgr' and all of its resources. */ void connmgr_destroy(struct connmgr *mgr) + OVS_REQUIRES(ofproto_mutex) { struct ofservice *ofservice, *next_ofservice; struct ofconn *ofconn, *next_ofconn; @@ -264,11 +268,9 @@ connmgr_destroy(struct connmgr *mgr) return; } - ovs_mutex_lock(&ofproto_mutex); LIST_FOR_EACH_SAFE (ofconn, next_ofconn, node, &mgr->all_conns) { ofconn_destroy(ofconn); } - ovs_mutex_unlock(&ofproto_mutex); hmap_destroy(&mgr->controllers); @@ -770,7 +772,9 @@ update_fail_open(struct connmgr *mgr) mgr->fail_open = fail_open_create(mgr->ofproto, mgr); } } else { + ovs_mutex_lock(&ofproto_mutex); fail_open_destroy(mgr->fail_open); + ovs_mutex_unlock(&ofproto_mutex); mgr->fail_open = NULL; } } @@ -1203,6 +1207,7 @@ ofconn_get_target(const struct ofconn *ofconn) static struct ofconn * ofconn_create(struct connmgr *mgr, struct rconn *rconn, enum ofconn_type type, bool enable_async_msgs) + OVS_REQUIRES(ofproto_mutex) { struct ofconn *ofconn; @@ -1607,10 +1612,13 @@ connmgr_send_requestforward(struct connmgr *mgr, const struct ofconn *source, } /* Sends an OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED message based on 'fr' to - * appropriate controllers managed by 'mgr'. */ + * appropriate controllers managed by 'mgr'. + * + * This may be called from the RCU thread. */ void connmgr_send_flow_removed(struct connmgr *mgr, const struct ofputil_flow_removed *fr) + OVS_REQUIRES(ofproto_mutex) { struct ofconn *ofconn; diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h index 0768aa0bf..1a75c4c85 100644 --- a/ofproto/connmgr.h +++ b/ofproto/connmgr.h @@ -71,7 +71,8 @@ void ofproto_async_msg_free(struct ofproto_async_msg *); /* Basics. */ struct connmgr *connmgr_create(struct ofproto *ofproto, const char *dpif_name, const char *local_name); -void connmgr_destroy(struct connmgr *); +void connmgr_destroy(struct connmgr *) + OVS_REQUIRES(ofproto_mutex); void connmgr_run(struct connmgr *, void (*handle_openflow)(struct ofconn *, @@ -142,7 +143,8 @@ bool connmgr_wants_packet_in_on_miss(struct connmgr *mgr); void connmgr_send_port_status(struct connmgr *, struct ofconn *source, const struct ofputil_phy_port *, uint8_t reason); void connmgr_send_flow_removed(struct connmgr *, - const struct ofputil_flow_removed *); + const struct ofputil_flow_removed *) + OVS_REQUIRES(ofproto_mutex); void connmgr_send_async_msg(struct connmgr *, const struct ofproto_async_msg *); void ofconn_send_role_status(struct ofconn *ofconn, uint32_t role, diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c index 3c3579ef1..e038e77ff 100644 --- a/ofproto/fail-open.c +++ b/ofproto/fail-open.c @@ -79,7 +79,7 @@ struct fail_open { bool fail_open_active; }; -static void fail_open_recover(struct fail_open *); +static void fail_open_recover(struct fail_open *) OVS_REQUIRES(ofproto_mutex); /* Returns the number of seconds of disconnection after which fail-open mode * should activate. */ @@ -197,13 +197,15 @@ fail_open_maybe_recover(struct fail_open *fo) { if (fail_open_is_active(fo) && connmgr_is_any_controller_admitted(fo->connmgr)) { + ovs_mutex_lock(&ofproto_mutex); fail_open_recover(fo); + ovs_mutex_unlock(&ofproto_mutex); } } static void fail_open_recover(struct fail_open *fo) - OVS_EXCLUDED(ofproto_mutex) + OVS_REQUIRES(ofproto_mutex) { struct match match; @@ -272,7 +274,7 @@ fail_open_create(struct ofproto *ofproto, struct connmgr *mgr) /* Destroys 'fo'. */ void fail_open_destroy(struct fail_open *fo) - OVS_EXCLUDED(ofproto_mutex) + OVS_REQUIRES(ofproto_mutex) { if (fo) { if (fail_open_is_active(fo)) { diff --git a/ofproto/fail-open.h b/ofproto/fail-open.h index 4056b3e8d..71442ffd6 100644 --- a/ofproto/fail-open.h +++ b/ofproto/fail-open.h @@ -40,7 +40,7 @@ is_fail_open_rule(const struct rule *rule) } struct fail_open *fail_open_create(struct ofproto *, struct connmgr *); -void fail_open_destroy(struct fail_open *) OVS_EXCLUDED(ofproto_mutex); +void fail_open_destroy(struct fail_open *) OVS_REQUIRES(ofproto_mutex); void fail_open_wait(struct fail_open *); bool fail_open_is_active(const struct fail_open *); void fail_open_run(struct fail_open *); diff --git a/ofproto/in-band.c b/ofproto/in-band.c index f69e94f67..4bb47c0b9 100644 --- a/ofproto/in-band.c +++ b/ofproto/in-band.c @@ -395,7 +395,9 @@ in_band_run(struct in_band *ib) break; case DEL: + ovs_mutex_lock(&ofproto_mutex); ofproto_delete_flow(ib->ofproto, &rule->match, rule->priority); + ovs_mutex_unlock(&ofproto_mutex); hmap_remove(&ib->rules, &rule->hmap_node); free(rule); break; diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 7f7813e3d..b11bf12c0 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -1890,7 +1890,7 @@ void ofproto_add_flow(struct ofproto *, const struct match *, int priority, const struct ofpact *ofpacts, size_t ofpacts_len) OVS_EXCLUDED(ofproto_mutex); void ofproto_delete_flow(struct ofproto *, const struct match *, int priority) - OVS_EXCLUDED(ofproto_mutex); + OVS_REQUIRES(ofproto_mutex); void ofproto_flush_flows(struct ofproto *); enum ofperr ofproto_check_ofpacts(struct ofproto *, diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index f813c0b1a..f4f4fd684 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -461,6 +461,7 @@ ofproto_bump_tables_version(struct ofproto *ofproto) int ofproto_create(const char *datapath_name, const char *datapath_type, struct ofproto **ofprotop) + OVS_EXCLUDED(ofproto_mutex) { const struct ofproto_class *class; struct ofproto *ofproto; @@ -530,7 +531,10 @@ ofproto_create(const char *datapath_name, const char *datapath_type, if (error) { VLOG_ERR("failed to open datapath %s: %s", datapath_name, ovs_strerror(error)); + ovs_mutex_lock(&ofproto_mutex); connmgr_destroy(ofproto->connmgr); + ofproto->connmgr = NULL; + ovs_mutex_unlock(&ofproto_mutex); ofproto_destroy__(ofproto); return error; } @@ -1486,6 +1490,8 @@ ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule) if (ofproto->ofproto_class->rule_delete) { ofproto->ofproto_class->rule_delete(rule); } + + /* This may not be the last reference to the rule. */ ofproto_rule_unref(rule); } ovs_mutex_unlock(&ofproto_mutex); @@ -1605,9 +1611,13 @@ ofproto_destroy(struct ofproto *p, bool del) p->ofproto_class->destruct(p); /* We should not postpone this because it involves deleting a listening - * socket which we may want to reopen soon. 'connmgr' should not be used - * by other threads */ + * socket which we may want to reopen soon. 'connmgr' may be used by other + * threads only if they take the ofproto_mutex and read a non-NULL + * 'ofproto->connmgr'. */ + ovs_mutex_lock(&ofproto_mutex); connmgr_destroy(p->connmgr); + p->connmgr = NULL; + ovs_mutex_unlock(&ofproto_mutex); /* Destroying rules is deferred, must have 'ofproto' around for them. */ ovsrcu_postpone(ofproto_destroy_defer__, p); @@ -2183,7 +2193,7 @@ ofproto_flow_mod(struct ofproto *ofproto, const struct ofputil_flow_mod *fm) void ofproto_delete_flow(struct ofproto *ofproto, const struct match *target, int priority) - OVS_EXCLUDED(ofproto_mutex) + OVS_REQUIRES(ofproto_mutex) { struct classifier *cls = &ofproto->tables[0].cls; struct rule *rule; @@ -2196,10 +2206,12 @@ ofproto_delete_flow(struct ofproto *ofproto, return; } - /* Execute a flow mod. We can't optimize this at all because we didn't - * take enough locks above to ensure that the flow table didn't already - * change beneath us. */ - simple_flow_mod(ofproto, target, priority, NULL, 0, OFPFC_DELETE_STRICT); + struct rule_collection rules; + + rule_collection_init(&rules); + rule_collection_add(&rules, rule); + delete_flows__(&rules, OFPRR_DELETE, NULL); + rule_collection_destroy(&rules); } /* Delete all of the flows from all of ofproto's flow tables, then reintroduce @@ -5325,7 +5337,15 @@ ofproto_rule_send_removed(struct rule *rule) minimatch_expand(&rule->cr.match, &fr.match); fr.priority = rule->cr.priority; + /* Synchronize with connmgr_destroy() calls to prevent connmgr disappearing + * while we use it. */ ovs_mutex_lock(&ofproto_mutex); + struct connmgr *connmgr = rule->ofproto->connmgr; + if (!connmgr) { + ovs_mutex_unlock(&ofproto_mutex); + return; + } + fr.cookie = rule->flow_cookie; fr.reason = rule->removed_reason; fr.table_id = rule->table_id; @@ -5337,7 +5357,7 @@ ofproto_rule_send_removed(struct rule *rule) ovs_mutex_unlock(&rule->mutex); rule->ofproto->ofproto_class->rule_get_stats(rule, &fr.packet_count, &fr.byte_count, &used); - connmgr_send_flow_removed(rule->ofproto->connmgr, &fr); + connmgr_send_flow_removed(connmgr, &fr); ovs_mutex_unlock(&ofproto_mutex); } diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 22d94c7cf..d813fdaf4 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -241,7 +241,8 @@ int ofproto_type_run(const char *datapath_type); void ofproto_type_wait(const char *datapath_type); int ofproto_create(const char *datapath, const char *datapath_type, - struct ofproto **ofprotop); + struct ofproto **ofprotop) + OVS_EXCLUDED(ofproto_mutex); void ofproto_destroy(struct ofproto *, bool del); int ofproto_delete(const char *name, const char *type); |