Following OpenFlow 1.4 semantics, make barriers wait for flow monitor replies. This should fix a race in "ofproto - flow monitoring pause and resume" test.
Signed-off-by: YAMAMOTO Takashi <yamam...@valinux.co.jp> --- ofproto/connmgr.c | 27 +++++++++++++++++++++++++++ ofproto/connmgr.h | 1 + ofproto/ofproto.c | 16 +++++++++++++++- 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index ea79795..aa9b54c 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -1118,6 +1118,13 @@ ofconn_pktbuf_retrieve(struct ofconn *ofconn, uint32_t id, return pktbuf_retrieve(ofconn->pktbuf, id, bufferp, in_port); } +/* Returns true if 'ofconn' has any pending flow monitor updates. */ +bool +ofconn_has_pending_monitor(const struct ofconn *ofconn) +{ + return !list_is_empty(&ofconn->updates) || ofconn->monitor_paused; +} + /* Returns true if 'ofconn' has any pending opgroups. */ bool ofconn_has_pending_opgroups(const struct ofconn *ofconn) @@ -2040,6 +2047,20 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule, } } +static bool +ofmonitor_has_pending(struct connmgr *mgr) + OVS_REQUIRES(ofproto_mutex) +{ + struct ofconn *ofconn; + + LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { + if (ofconn_has_pending_monitor(ofconn)) { + return true; + } + } + return false; +} + void ofmonitor_flush(struct connmgr *mgr) OVS_REQUIRES(ofproto_mutex) @@ -2066,6 +2087,9 @@ ofmonitor_flush(struct connmgr *mgr) } } } + if (!ofmonitor_has_pending(mgr)) { + connmgr_retry(mgr); + } } static void @@ -2091,6 +2115,9 @@ ofmonitor_resume(struct ofconn *ofconn) ofconn_send_replies(ofconn, &msgs); ofconn->monitor_paused = 0; + if (!ofmonitor_has_pending(ofconn->connmgr)) { + connmgr_retry(ofconn->connmgr); + } } static bool diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h index 170d872..40b3ff6 100644 --- a/ofproto/connmgr.h +++ b/ofproto/connmgr.h @@ -142,6 +142,7 @@ enum ofperr ofconn_pktbuf_retrieve(struct ofconn *, uint32_t id, struct ofpbuf **bufferp, ofp_port_t *in_port); bool ofconn_has_pending_opgroups(const struct ofconn *); +bool ofconn_has_pending_monitor(const struct ofconn *); void ofconn_add_opgroup(struct ofconn *, struct list *); void ofconn_remove_opgroup(struct ofconn *, struct list *, const struct ofp_header *request, int error); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 676a6cb..1b59a28 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -4678,7 +4678,21 @@ handle_barrier_request(struct ofconn *ofconn, const struct ofp_header *oh) { struct ofpbuf *buf; - if (ofconn_has_pending_opgroups(ofconn)) { + /* + * OpenFlow 1.4.0 p.110: + * A OFPMP_FLOW_MONITOR multipart reply can not cross a barrier + * handshake. The switch must always send the OFPMP_FLOW_MONITOR + * multipart reply for a given flow table change before the reply + * to a OFPT_BARRIER_REQUEST request that follows the request + * responsible for the flow table change. + * + * The spec seems unclear what to do for multiple connections, though. + * We serialize only flow monitor replies on our connection. + * We don't care which connection has made flow modifications triggering + * the flow monitor replies. + */ + if (ofconn_has_pending_opgroups(ofconn) || + ofconn_has_pending_monitor(ofconn)) { return OFPROTO_POSTPONE; } -- 1.8.3.1 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev