[ovs-dev] [PATCH v2 05/26] ofproto: Use ofproto_mutex for groups and keep track of referring flows.

Jarno Rajahalme jarno at ovn.org
Fri Jul 29 20:59:23 UTC 2016


> On Jul 29, 2016, at 1:41 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> On Fri, Jul 29, 2016 at 01:19:03PM -0700, Jarno Rajahalme wrote:
>> 
>>> On Jul 29, 2016, at 12:33 PM, Ben Pfaff <blp at ovn.org> wrote:
>>> 
>>> On Thu, Jul 28, 2016 at 05:55:57PM -0700, Jarno Rajahalme wrote:
>>>> Adding groups support for bundles is simpler if also groups are
>>>> modified under ofproto_mutex.
>>>> 
>>>> Eliminate the search for rules when deleting a group so that we will
>>>> not keep the mutex for too long.
>>>> 
>>>> Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
>>> 
>>> It's confusing that rule_collection_remove() and
>>> rule_collection_remove_postponed() have unrelated effects.
>>> 
>> 
>> We want a verb that is the opposite of "add", how about
>> rule_collection_subtract(), rule_collection_retract() or
>> rule_collection_withdraw()?
> 
> I think that "remove" is OK for rule_collection_remove().
> 
>> Of should we be more careful about naming functions that change change
>> the rule collection vs. operate on the rules in the collection? E.g.,
>> rename rule_collection_add() as rule_collection_add_rule() and
>> rule_collection_remove() as rule_collection_remove_rule(). Functions
>> named like "rule_collection_VERB()" would then be reserved for
>> functions that VERB on each rule?
> 
> I'm not enthusiastic about having the name of the type twice in the
> function name.
> 
> There might be a useful distinction here between the generic object
> collection functions, which should naturally begin with
> <object>_collection_, and functions that aren't generic object
> collection functions but happen to take one as an argument.  The latter
> maybe should not have names that begin with <object>_collection_ since
> that is kind of reserved.  So, can we rename rename
> rule_collection_remove_postponed() so that it fits this pattern?  Maybe
> something like remove_rules_postponed()?

This is better. I'll change this.

> 
> Acked-by: Ben Pfaff <blp at ovn.org <mailto:blp at ovn.org>>




More information about the dev mailing list