> On Jul 29, 2016, at 12:54 PM, Jarno Rajahalme <ja...@ovn.org> wrote:
>
>>
>> On Jul 29, 2016, at 12:48 PM, Ben Pfaff <b...@ovn.org <mailto:b...@ovn.org>>
>> wrote:
>>
>> On Fri, Jul 29, 2016 at 11:46:49AM -0700, Jarno Rajahalme wrote:
>>>
>>>> On Jul 29, 2016, at 11:20 AM, Ben Pfaff <b...@ovn.org
>>>> <mailto:b...@ovn.org>> wrote:
>>>>
>>>> On Thu, Jul 28, 2016 at 05:55:54PM -0700, Jarno Rajahalme wrote:
>>>>> Make groups RCU protected and make group lookups lockless.
>>>>>
>>>>> Signed-off-by: Jarno Rajahalme <ja...@ovn.org <mailto:ja...@ovn.org>>
>>>>
>>>> I'd add a little motivation to the commit message. A common reason to
>>>> switch to RCU is performance, but I doubt that this is the motivation
>>>> here. Rather, I suspect that it has more to do with having groups and
>>>> flows share the same synchronization mechanism so that it's easier to
>>>> understand. Also, I guess that later in the series you're going to need
>>>> versioning so that groups can be included in bundles.
>>>>
>>>
>>> Good point, you are right and I'll address this.
>>>
I changed the commit message to:
ofproto: Lockless group lookups.
Make groups RCU protected and make group lookups lockless. While this
makes group lookups perform better, the main motivation is to have an
unified memory management model for versioned data supported in
OpenFlow bundles. Later patches will make groups versioned and add
bundle support for groups.
Do you want to review the v2 or give an Ack based on this discussion?
Jarno
>>>> I wasn't sure what to make of this new comment in ofproto-provider.h:
>>>>
>>>
>>> I was wondering if I should have changed 'buckets' to const as
>>> well. Everything else is const, which makes it easy to change to RCU
>>> semantics. I suspect that making it 'const' will require quite a lot
>>> of CONST_CAST'ing when creating the group, but I'll see.
>>
>> OK. It's a good idea if it doesn't make the code too ugly.
>>
>
> It was quickly done, and all the casts were for either creating a new group
> or destroying an old one.
>
> Jarno
>
>>>> @@ -515,20 +515,22 @@ struct ofgroup {
>>>> const enum ofp11_group_type type; /* One of OFPGT_*. */
>>>>
>>>> const long long int created; /* Creation time. */
>>>> const long long int modified; /* Time of last modification. */
>>>>
>>>> + /* const ?? */
>>>> struct ovs_list buckets; /* Contains "struct ofputil_bucket"s. */
>>>> const uint32_t n_buckets;
>>>>
>>>> const struct ofputil_group_props props;
>>>> };
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
