Thanks for the reviews, pushed to master.
Jarno
On Jul 17, 2014, at 10:40 AM, Ben Pfaff <b...@nicira.com> wrote:
> On Thu, Jul 17, 2014 at 07:09:15AM -0700, Jarno Rajahalme wrote:
>>
>> On Jul 15, 2014, at 10:18 PM, YAMAMOTO Takashi <yamam...@valinux.co.jp>
>> wrote:
>>
>>>> /* A flow classifier. */
>>>> struct classifier {
>>>> - struct cls_classifier *cls;
>>>> + struct ovs_mutex mutex;
>>>> + int n_rules OVS_GUARDED; /* Total number of rules. */
>>>> + uint8_t n_flow_segments;
>>>> + uint8_t flow_segments[CLS_MAX_INDICES]; /* Flow segment boundaries to
>>>> use
>>>> + * for staged lookup. */
>>>> + struct cmap subtables_map; /* Contains "struct cls_subtable"s.
>>>> */
>>>> + struct pvector subtables;
>>>> + struct cmap partitions; /* Contains "struct cls_partition"s.
>>>> */
>>>> + struct cls_trie tries[CLS_MAX_TRIES]; /* Prefix tries. */
>>>> + unsigned int n_tries;
>>>> };
>>>
>>> i think a forward decl of the structure is enough for classifier.h.
>>> i always prefer hiding details from consumers where reasonable.
>>>
>>
>> That was the motivation for the split in the first place. Users of struct
>> classifier include it as a struct, not a pointer:
>>
>> struct oftable {
>> enum oftable_flags flags;
>> struct classifier cls; /* Contains "struct rule"s. */
>> ?
>>
>> Now that almost all of the members of the struct classifier are defined in
>> separate modules, I thought that exposing the struct classifier on the API
>> is worth it for the benefit of avoiding one level of indirection.
>>
>> Hiding the details is exactly what the current master does, but the
>> indirection is hidden from the users as they use the struct classifier,
>> which contains the pointer to the struct cls_classifier.
>>
>> I?m fine either way, though.
>
> So more than a forward decl is needed. Great, leave the definition.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
