The change looks fine.
On Fri, Dec 13, 2013 at 12:47 PM, Ben Pfaff <b...@nicira.com> wrote: > Occasionally in the unit tests the following race can happen: > > 1. ovs-vsctl updates database > 2. ovs-vswitchd reconfigures, notifies ovs-vsctl that it is complete > 3. ovs-appctl ofproto/trace fails to see newly added port > 4. ovs-vswitchd main loop calls ofproto's ->type_run(), making the > new port visible to translation. > > This race may be seen in the failures of tests 5 and 624 here: > > https://launchpadlibrarian.net/151884888/buildlog_ubuntu-precise-amd64.openvswitch_2.0~201309300804-1ppa1~precise_FAILEDTOBUILD.txt.gz > > Reported-by: Vasiliy Tolstov <v.tols...@selfip.ru> > Signed-off-by: Ben Pfaff <b...@nicira.com> > --- > AUTHORS | 1 + > vswitchd/bridge.c | 44 ++++++++++++++++++++++++++++++-------------- > 2 files changed, 31 insertions(+), 14 deletions(-) > > diff --git a/AUTHORS b/AUTHORS > index 1c2d9ea..485b9c1 100644 > --- a/AUTHORS > +++ b/AUTHORS > @@ -230,6 +230,7 @@ Teemu Koponen kopo...@nicira.com > Timothy Chen tc...@nicira.com > Torbjorn Tornkvist kruska...@gmail.com > Valentin Bud valen...@hackaserver.com > +Vasiliy Tolstov v.tols...@selfip.ru > Vishal Swarankar vishal.swarn...@gmail.com > Vjekoslav Brajkovic bal...@cs.washington.edu > Voravit T. vora...@kth.se > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index 1d60326..803b5aa 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -176,6 +176,7 @@ static long long int iface_stats_timer = LLONG_MIN; > #define OFP_PORT_ACTION_WINDOW 10 > > static void add_del_bridges(const struct ovsrec_open_vswitch *); > +static void bridge_run__(void); > static void bridge_create(const struct ovsrec_bridge *); > static void bridge_destroy(struct bridge *); > static struct bridge *bridge_lookup(const char *name); > @@ -601,6 +602,13 @@ bridge_reconfigure(const struct ovsrec_open_vswitch > *ovs_cfg) > } > } > free(managers); > + > + /* The ofproto-dpif provider does some final reconfiguration in its > + * ->type_run() function. We have to call it before notifying the > database > + * client that reconfiguration is complete, otherwise there is a very > + * narrow race window in which e.g. ofproto/trace will not recognize > the > + * new configuration (sometimes this causes unit test failures). */ > + bridge_run__(); > } > > /* Delete ofprotos which aren't configured or have the wrong type. Create > @@ -2252,13 +2260,32 @@ instant_stats_wait(void) > } > } > > +static void > +bridge_run__(void) > +{ > + struct bridge *br; > + struct sset types; > + const char *type; > + > + /* Let each datapath type do the work that it needs to do. */ > + sset_init(&types); > + ofproto_enumerate_types(&types); > + SSET_FOR_EACH (type, &types) { > + ofproto_type_run(type); > + } > + sset_destroy(&types); > + > + /* Let each bridge do the work that it needs to do. */ > + HMAP_FOR_EACH (br, node, &all_bridges) { > + ofproto_run(br->ofproto); > + } > +} > + > void > bridge_run(void) > { > static struct ovsrec_open_vswitch null_cfg; > const struct ovsrec_open_vswitch *cfg; > - struct sset types; > - const char *type; > > bool vlan_splinters_changed; > struct bridge *br; > @@ -2301,18 +2328,7 @@ bridge_run(void) > "flow-restore-wait", false)); > } > > - /* Let each datapath type do the work that it needs to do. */ > - sset_init(&types); > - ofproto_enumerate_types(&types); > - SSET_FOR_EACH (type, &types) { > - ofproto_type_run(type); > - } > - sset_destroy(&types); > - > - /* Let each bridge do the work that it needs to do. */ > - HMAP_FOR_EACH (br, node, &all_bridges) { > - ofproto_run(br->ofproto); > - } > + bridge_run__(); > > /* Re-configure SSL. We do this on every trip through the main loop, > * instead of just when the database changes, because the contents of > the > -- > 1.7.10.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
