From: Andy Zhou Date: Thu, 12 Feb 2015 23:10:28 +0000 (-0800) Subject: ofproto/bond: Fix a race condition in updating post recirculation rules X-Git-Tag: v2.3.2~34 X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=a62c715408caec1b8bff1861afaa45d561acab61 ofproto/bond: Fix a race condition in updating post recirculation rules 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 Reported-by: Salvatore Cambria Signed-off-by: Andy Zhou Acked-by: Ben Pfaff --- diff --git a/AUTHORS b/AUTHORS index 4970973ac..5a0998f54 100644 --- a/AUTHORS +++ b/AUTHORS @@ -266,6 +266,7 @@ Spiro Kourtessis spiro@vmware.com Sridhar Samudrala samudrala.sridhar@gmail.com Srini Seetharaman seethara@stanford.edu Sabyasachi Sengupta Sabyasachi.Sengupta@alcatel-lucent.com +Salvatore Cambria salvatore.cambria@citrix.com Stephen Hemminger shemminger@vyatta.com Stephen Finucane stephen.finucane@intel.com Suganya Ramachandran suganyar@vmware.com diff --git a/ofproto/bond.c b/ofproto/bond.c index 44e9df148..aa34a8340 100644 --- a/ofproto/bond.c +++ b/ofproto/bond.c @@ -325,6 +325,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; @@ -946,8 +947,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. */ @@ -968,6 +970,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. */ @@ -1226,7 +1236,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: