On Fri, Jan 17, 2014 at 11:04:14AM +0900, YAMAMOTO Takashi wrote: > > On Wed, Jan 08, 2014 at 07:31:23PM +0900, YAMAMOTO Takashi wrote: > >> 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> > > > > Clang says: > > > > ../ofproto/connmgr.c:1126:56: error: reading variable 'monitor_paused' > > requires locking 'ofproto_mutex' [-Werror,-Wthread-safety-analysis] > > return !list_is_empty(&ofconn->updates) || ofconn->monitor_paused; > > ^ > > > > But I do not think that this change is actually necessary. The > > 'updates' member is only nonempty during the processing of a > > flow_mod. By the time the flow_mod is actually committed to the flow > > table, all of the updates have been flushed to the ofconns anyhow. > > > > Here is another way to look at it. I can think of two ways that the > > flow table can change. The primary way is from the main thread that > > processes all OpenFlow requests. That thread sends out the updates > > synchronously with the flow_mod, so the flow updates will always > > precede the barrier reply. The secondary way is from NXAST_LEARN > > actions. Such actions are not associated with any ofconn, so any > > updates that they send don't have to be synchronized with a barrier in > > any case. > > > > Do you agree? > > at least the ofconn->monitor_paused check is necessary > because ADDED/MODIFIED events can be generated when a monitor is > resumed. i've observed the test mentioned in the commit log > complained about missing events. > > i think you are right about ofconn->updates. > how about the following?
Sorry about the delay looking at this. What version of OVS is this against? It does not appear to apply to branch-2.0, branch-2.1, or master. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev