[ovs-dev] [encap v2 2/3] datapath: Describe policy for extending flow key, implement needed changes.

Ben Pfaff blp at nicira.com
Tue Nov 15 00:53:26 UTC 2011


On Mon, Nov 14, 2011 at 04:50:05PM -0800, Jesse Gross wrote:
> On Mon, Nov 14, 2011 at 4:47 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Mon, Nov 14, 2011 at 04:45:28PM -0800, Jesse Gross wrote:
> >> On Mon, Nov 14, 2011 at 3:58 PM, Ben Pfaff <blp at nicira.com> wrote:
> >> > On Mon, Nov 14, 2011 at 03:33:52PM -0800, Jesse Gross wrote:
> >> >> On Mon, Nov 14, 2011 at 3:20 PM, Ben Pfaff <blp at nicira.com> wrote:
> >> >> > On Mon, Nov 14, 2011 at 02:37:37PM -0800, Jesse Gross wrote:
> >> >> >> On Sat, Nov 12, 2011 at 1:01 PM, Ben Pfaff <blp at nicira.com> wrote:
> >> >> >> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> >> >> >> > index c70ab11..0ca616b 100644
> >> >> >> > --- a/lib/odp-util.c
> >> >> >> > +++ b/lib/odp-util.c
> >> >> >> > +parse_flow_nlattrs(const struct nlattr *key, size_t key_len,
> >> >> >> > + ?? ?? ?? ?? ?? ?? ?? ?? ?? const struct nlattr *attrs[], uint64_t *present_attrsp)
> >> >> >> > ??{
> >> >> >> > ?? ?? static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> >> >> >> > - ?? ??const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1];
> >> >> >> > ?? ?? const struct nlattr *nla;
> >> >> >> > - ?? ??uint64_t expected_attrs;
> >> >> >> > ?? ?? uint64_t present_attrs;
> >> >> >> > - ?? ??uint64_t missing_attrs;
> >> >> >> > - ?? ??uint64_t extra_attrs;
> >> >> >> > ?? ?? size_t left;
> >> >> >> >
> >> >> >> > - ?? ??memset(flow, 0, sizeof *flow);
> >> >> >> > -
> >> >> >> > - ?? ??memset(attrs, 0, sizeof attrs);
> >> >> >> > + ?? ??memset(attrs, 0, (OVS_KEY_ATTR_MAX + 1) * sizeof *attrs);
> >> >> >>
> >> >> >> Is there a reason why userspace and kernel do duplicate checking
> >> >> >> differently? ??The kernel does it based on present_attrs and userspace
> >> >> >> does it based on the attribute stored in the array.
> >> >> >
> >> >> > I didn't want the overhead of memset'ing all 64*4 == 256 or 64*8 ==
> >> >> > 512 bytes of the temporary array in the kernel, so I used the bitmap
> >> >> > exclusively there to keep track of whether an attribute had been seen.
> >> >> > But I'll change it to whichever way you prefer.
> >> >>
> >> >> Yeah, the kernel version seemed a little nicer to me, so I was
> >> >> actually wondering why we didn't do the same thing in userspace
> >> >> (aren't both versions executed approximately the same number of times
> >> >> and therefore the overhead has equal impact?).
> >> >
> >> > OK, I made that change as well as the others you suggested and ended
> >> > up with the following overall incremental and full patch.
> >>
> >> Looks good other than the CFI bit in actions stuff that we talked about.
> >
> > That's in patch 3 since we only start using the CFI bit distinction
> > elsewhere in that one.
> 
> OK, great.  In that case:
> Acked-by: Jesse Gross <jesse at nicira.com>

Thanks.



More information about the dev mailing list