Groups were not destroyed until after lots of other important bridge
data had been destroyed, including the connection manager. There was an
indirect dependency on the connection manager for bridge destruction
because destroying a group also destroys all of the flows that reference
the group, which in turn causes the ofmonitor to be invoked to report that
the flows had been destroyed. This commit fixes the problem by destroying
groups earlier.
The problem can be observed by reverting the code changes in this commit
then running "make check-valgrind" with the test that this commit
introduces.
Reported-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
ofproto_rule_delete(&ofproto->up, &rule->up);
}
}
ofproto_rule_delete(&ofproto->up, &rule->up);
}
}
+ ofproto_group_delete_all(&ofproto->up);
guarded_list_pop_all(&ofproto->pins, &pins);
LIST_FOR_EACH_POP (pin, list_node, &pins) {
guarded_list_pop_all(&ofproto->pins, &pins);
LIST_FOR_EACH_POP (pin, list_node, &pins) {
void ofproto_group_ref(struct ofgroup *);
void ofproto_group_unref(struct ofgroup *);
void ofproto_group_ref(struct ofgroup *);
void ofproto_group_unref(struct ofgroup *);
+void ofproto_group_delete_all(struct ofproto *);
+
/* ofproto class structure, to be defined by each ofproto implementation.
*
*
/* ofproto class structure, to be defined by each ofproto implementation.
*
*
* ===========
*
* ->destruct() must also destroy all remaining rules in the ofproto's
* ===========
*
* ->destruct() must also destroy all remaining rules in the ofproto's
- * tables, by passing each remaining rule to ofproto_rule_delete().
+ * tables, by passing each remaining rule to ofproto_rule_delete(), then
+ * destroy all remaining groups by calling ofproto_group_delete_all().
*
* The client will destroy the flow tables themselves after ->destruct()
* returns.
*
* The client will destroy the flow tables themselves after ->destruct()
* returns.
struct oftable *table;
destroy_rule_executes(ofproto);
struct oftable *table;
destroy_rule_executes(ofproto);
- delete_group(ofproto, OFPG_ALL);
guarded_list_destroy(&ofproto->rule_executes);
ovs_rwlock_destroy(&ofproto->groups_rwlock);
guarded_list_destroy(&ofproto->rule_executes);
ovs_rwlock_destroy(&ofproto->groups_rwlock);
ovs_rwlock_unlock(&ofproto->groups_rwlock);
}
ovs_rwlock_unlock(&ofproto->groups_rwlock);
}
+/* Delete all groups from 'ofproto'.
+ *
+ * This is intended for use within an ofproto provider's 'destruct'
+ * function. */
+void
+ofproto_group_delete_all(struct ofproto *ofproto)
+{
+ delete_group(ofproto, OFPG_ALL);
+}
+
static enum ofperr
handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh)
{
static enum ofperr
handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh)
{
OVS_VSWITCHD_STOP
AT_CLEANUP
OVS_VSWITCHD_STOP
AT_CLEANUP
+dnl This found a use-after-free error in bridge destruction in the
+dnl presence of groups.
+AT_SETUP([ofproto - group add then bridge delete (OpenFlow 1.3)])
+OVS_VSWITCHD_START
+AT_DATA([groups.txt], [dnl
+group_id=1234,type=all,bucket=output:10
+group_id=1235,type=all,bucket=output:10
+])
+AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn add-groups br0 groups.txt])
+AT_CHECK([ovs-vsctl del-br br0])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto - mod-port (OpenFlow 1.0)])
OVS_VSWITCHD_START
for command_config_state in \
AT_SETUP([ofproto - mod-port (OpenFlow 1.0)])
OVS_VSWITCHD_START
for command_config_state in \