On Fri, Nov 7, 2014 at 3:55 PM, Joe Stringer <joestrin...@nicira.com> wrote:
> On 7 November 2014 14:15, Pravin Shelar <pshe...@nicira.com> wrote:
>>
>> On Fri, Oct 31, 2014 at 4:55 PM, Joe Stringer <joestrin...@nicira.com>
>> wrote:
>> > If a datapath is created with the flag OVS_DP_F_INDEX_BY_UFID, then an
>> > additional table_instance is added to the flow_table, which is indexed
>> > by unique identifiers ("UFID"). Userspace implementations can specify a
>> > UFID 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_UFID and a UFID is not
>> > specified at flow setup time, then that operation will fail. If
>> > OVS_UFID_F_* flags are specified for an operation, then they will modify
>> > what is returned through the operation. For instance,
>> > OVS_UFID_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>
>> > ---
>> > v9: No change.
>> > v8: Rename UID -> UFID "unique flow identifier".
>> > Fix null dereference when adding flow without uid or mask.
>> > If UFID and not match are specified, and lookup fails, return
>> > ENOENT.
>> > Rebase.
>> > v7: Remove OVS_DP_F_INDEX_BY_UID.
>> > Rework UID serialisation for variable-length UID.
>> > Log error if uid not specified and OVS_UID_F_SKIP_KEY is set.
>> > Rebase against "probe" logging changes.
>> > 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.
>> > ---
>>
>> Patch looks good. I have few comments:
>
>
> Thanks for taking a look,
>
>>
>> - Can you make union of unmasked_key and (fid, ufid_hash), since they
>> are mutually exclusive.
>
>
> Unmasked key may be used when UFID is part of the flow, eg if the full flow
> is requested in ovs_flow_cmd_fill_match(). It should still be populated for
> every flow.
>
Why userspace need to know unmasked key if ufids are used for the flow?
>> - There is no need to periodically rehash ufid table since the lookup
>> are not triggered by incoming packets.
>
>
> OK.
>
>>
>> - ovs_flow_tbl_destroy() can iterate flow table and delete both hash
>> entries. that way there is no need to iterate both tables.
>
>
> OK.
>
>>
>> - I suggested kernel_only flags, but we have to keep unmasked_key for
>> compat code. So I think we can remove the kernel only key generation
>> and build ufid hash-table only if ufid is given by userspace.
>
>
> If we're happy to have the UFID table not necessarily in sync with the flow
> table, I can rework it this way.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
