On Mon, Sep 29, 2014 at 3:59 PM, Joe Stringer <joestrin...@nicira.com> wrote:
>
>
> On 30 September 2014 10:10, Ben Pfaff <b...@nicira.com> wrote:
>>
>> On Fri, Sep 26, 2014 at 09:28:15PM +1200, Joe Stringer wrote:
>> > If a datapath is created with the flag OVS_DP_F_INDEX_BY_UID, then an
>> > additional table_instance is added to the flow_table, which is indexed
>> > by unique identifiers ("UID"). Userspace implementations can specify a
>> > UID of up to 128 bits along with a flow operation as shorthand for the
>> > key. This allows revalidation performance improvements of up to 50%.
>> >
>> > If a datapath is created using OVS_DP_F_INDEX_BY_UID and a UID is not
>> > specified at flow setup time, then that operation will fail. If
>> > OVS_UID_F_* flags are specified for an operation, then they will modify
>> > what is returned through the operation. For instance, OVS_UID_F_SKIP_KEY
>> > allows the datapath to skip returning the key (eg, during dump to reduce
>> > memory copy).
>> >
>> > Signed-off-by: Joe Stringer <joestrin...@nicira.com>
>> > ---
>> > v6: Fix documentation for supporting UIDs between 32-128 bits.
>> > Minor style fixes.
>> > Rebase.
>> > v5: No change.
>> > v4: Fix memory leaks.
>> > Log when triggering the older userspace issue above.
>> > v3: Initial post.
>>
>> This review is from an ABI standpoint only; it's not a review of the
>> kernel code itself.
>>
>> OVS_UID_ATTR_ID is marked as 32-128 bits long. For the "userdata"
>> attribute of the userspace action, we originally had it fixed at 64
>> bits, then later we decided that it was more flexible to allow it to
>> be any size. Is there an advantage to fixing it within this range?
>
>
> I'm not sure there's any advantage, that's just the way it's written right
> now. Perhaps with a bit of tweaking, we could get rid of MAX_UID_BUFSIZE and
> have no restrictions on the size of this.
>
>>
>>
>> I'm a little surprised that OVS_DP_F_INDEX_BY_UID is necessary. In
>> the past we've only added flags for features that otherwise required a
>> backward-incompatible change to the datapath interface. Is adding a
>> UID such a change?
>
>
> Pravin had some preferences on this during the original drafting, but I
> can't find a direct requirement for this. The alternative means that flows
> might not be present in both of the hastables (indexed by UID vs. exact
> flow_key), although they would always need to be in the exact flow_key
> table. Might be worth bouncing this off Pravin to see if I'm on the mark
> with how I've used it here.
I looked into the patch and I think we can get rid of OVS_DP_F_INDEX_BY_UID.
On flow insert we can use flow-id provided by userspace, if it is not
passed we can generate in kernel and use it to insert it in the UID
hash table. sw_flow can have a flag set for kernel generated flow-uid,
this can be used along with OVS_UID_F_SKIP_KEY in flow dump operation
to return key to userspace or not. On flow dump can always iterate UID
hash table where flow iteration should be relatively stable.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
