On 1 October 2014 11:55, Pravin Shelar <pshe...@nicira.com> wrote:
> On Tue, Sep 30, 2014 at 3:15 PM, Joe Stringer <joestrin...@nicira.com>
> wrote:
> >
> >
> > On 1 October 2014 06:56, Pravin Shelar <pshe...@nicira.com> wrote:
> >>
> >> 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.
> >
> >
> > OK, so in this case generally when userspace doesn't support UIDs, the
> > datapath will generate them mainly to keep the flow-id and flow-key
> > hashtables in sync. (Currently, we just disable the flow-id hashtable).
> >
> > Currently, OVS_UID_F_SKIP_KEY is a request flag that means "omit the flow
> > key". Are you suggesting that it should only omit flow keys for flows
> which
> > have a userspace-specified flow-id?
> >
> I used wrong wording, What I mean is keep OVS_UID_F_SKIP_KEY semantic
> as it is and do not return kernel generated uids to userspace. Since I
> do not see any use of kernel uids to userspace.
>
OK, this sounds quite reasonable.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
