When bond is removed or when its configuration changes, the post recirculation rules that are installed by current bond configuration, if any, should be also be removed.
Reported-by: Huanle Han <[email protected]> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-February/328969.html CC: Huanle Han <[email protected]> Signed-off-by: Andy Zhou <[email protected]> --- ofproto/bond.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/ofproto/bond.c b/ofproto/bond.c index 6e10c5143c0e..5bb124bda5ad 100644 --- a/ofproto/bond.c +++ b/ofproto/bond.c @@ -190,6 +190,7 @@ static struct bond_slave *choose_output_slave(const struct bond *, struct flow_wildcards *, uint16_t vlan) OVS_REQ_RDLOCK(rwlock); +static void update_recirc_rules__(struct bond *bond); /* Attempts to parse 's' as the name of a bond balancing mode. If successful, * stores the mode in '*balance' and returns true. Otherwise returns false @@ -264,7 +265,6 @@ bond_ref(const struct bond *bond_) void bond_unref(struct bond *bond) { - struct bond_pr_rule_op *pr_op; struct bond_slave *slave; if (!bond || ovs_refcount_unref_relaxed(&bond->ref_cnt) != 1) { @@ -283,18 +283,18 @@ bond_unref(struct bond *bond) hmap_destroy(&bond->slaves); ovs_mutex_destroy(&bond->mutex); - free(bond->hash); - free(bond->name); - - HMAP_FOR_EACH_POP (pr_op, hmap_node, &bond->pr_rule_ops) { - free(pr_op); - } - hmap_destroy(&bond->pr_rule_ops); + /* Free bond resources. Remove existing post recirc rules. */ if (bond->recirc_id) { recirc_free_id(bond->recirc_id); + bond->recirc_id = 0; } + free(bond->hash); + bond->hash = NULL; + update_recirc_rules__(bond); + hmap_destroy(&bond->pr_rule_ops); + free(bond->name); free(bond); } @@ -322,9 +322,17 @@ add_pr_rule(struct bond *bond, const struct match *match, hmap_insert(&bond->pr_rule_ops, &pr_op->hmap_node, hash); } +/* This function should almost never be called directly. + * 'update_recirc_rules()' should be called instead. Since + * this function modifies 'bond->pr_rule_ops', it is only + * safe when 'rwlock' is held. + * + * However, when the 'bond' is the only reference in the system, + * calling this function avoid acquiring lock only to satisfy + * lock annotation. Currently, only 'bond_unref()' calls + * this function directly. */ static void -update_recirc_rules(struct bond *bond) - OVS_REQ_WRLOCK(rwlock) +update_recirc_rules__(struct bond *bond) { struct match match; struct bond_pr_rule_op *pr_op, *next_op; @@ -394,6 +402,12 @@ update_recirc_rules(struct bond *bond) ofpbuf_uninit(&ofpacts); } +static void +update_recirc_rules(struct bond *bond) + OVS_REQ_RDLOCK(rwlock) +{ + update_recirc_rules__(bond); +} /* Updates 'bond''s overall configuration to 's'. * @@ -1640,6 +1654,8 @@ bond_entry_reset(struct bond *bond) } else { free(bond->hash); bond->hash = NULL; + /* Remove existing post recirc rules. */ + update_recirc_rules(bond); } } -- 1.9.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
