[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