Skip to content

Commit

Permalink
bridge: Let ofprotos run once before reporting configuration complete.
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
blp committed Dec 17, 2013
1 parent 1ce3fa0 commit aeed787
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 14 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ Teemu Koponen [email protected]
Timothy Chen [email protected]
Torbjorn Tornkvist [email protected]
Valentin Bud [email protected]
Vasiliy Tolstov [email protected]
Vishal Swarankar [email protected]
Vjekoslav Brajkovic [email protected]
Voravit T. [email protected]
Expand Down
44 changes: 30 additions & 14 deletions vswitchd/bridge.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit aeed787

Please sign in to comment.