Skip to content

Commit

Permalink
ofproto/bond: Fix a race condition in updating post recirculation rules
Browse files Browse the repository at this point in the history
When updating post recirc rules, rule management requires calls to
hmap APIs, which requires proper locking to ensure mutual exclsion in
accessing the hmap internal data structure. The locking currently is
missing from the output_normal() xlate path, thus causing
a race condition.

The race condition leads to segfault crash of ovs-vswitchd, with the
following stack trace:

The crash was found by adding and deleting bond interfaces repeatedly
with on-going traffic hitting the bond interfaces.  The same test was
ran over multiple days with this patch to ensure the same crash was
not seen.

The patch added the necessary lock annotation that would have caught
the bug.

Tested-by: Salvatore Cambria <[email protected]>
Reported-by: Salvatore Cambria <[email protected]>
Signed-off-by: Andy Zhou <[email protected]>
Acked-by: Ben Pfaff <[email protected]>
  • Loading branch information
azhou-nicira committed Feb 17, 2015
1 parent 737ea41 commit ca8127f
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ Spiro Kourtessis [email protected]
Sridhar Samudrala [email protected]
Srini Seetharaman [email protected]
Sabyasachi Sengupta [email protected]
Salvatore Cambria [email protected]
Stephen Hemminger [email protected]
Suganya Ramachandran [email protected]
Takayuki HAMA [email protected]
Expand Down
16 changes: 13 additions & 3 deletions ofproto/bond.c
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ add_pr_rule(struct bond *bond, const struct match *match,

static void
update_recirc_rules(struct bond *bond)
OVS_REQ_WRLOCK(rwlock)
{
struct match match;
struct bond_pr_rule_op *pr_op, *next_op;
Expand Down Expand Up @@ -923,8 +924,9 @@ bond_may_recirc(const struct bond *bond, uint32_t *recirc_id,
}
}

void
bond_update_post_recirc_rules(struct bond* bond, const bool force)
static void
bond_update_post_recirc_rules__(struct bond* bond, const bool force)
OVS_REQ_WRLOCK(rwlock)
{
struct bond_entry *e;
bool update_rules = force; /* Always update rules if caller forces it. */
Expand All @@ -945,6 +947,14 @@ bond_update_post_recirc_rules(struct bond* bond, const bool force)
update_recirc_rules(bond);
}
}

void
bond_update_post_recirc_rules(struct bond* bond, const bool force)
{
ovs_rwlock_wrlock(&rwlock);
bond_update_post_recirc_rules__(bond, force);
ovs_rwlock_unlock(&rwlock);
}

/* Rebalancing. */

Expand Down Expand Up @@ -1203,7 +1213,7 @@ bond_rebalance(struct bond *bond)
}

if (use_recirc && rebalanced) {
bond_update_post_recirc_rules(bond,true);
bond_update_post_recirc_rules__(bond,true);
}

done:
Expand Down

0 comments on commit ca8127f

Please sign in to comment.