[ovs-dev] [PATCH V3 1/5] dpif: Change dpif API to allow multiple handler threads read upcall.

Alex Wang alexw at nicira.com
Thu Mar 13 22:18:26 UTC 2014


Thanks for the suggested comments, Ben

I'll fold that in with the V4 series.


On Thu, Mar 13, 2014 at 1:37 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Wed, Mar 12, 2014 at 09:31:31AM -0800, Alex Wang wrote:
> > > - fold in Ben's clarification.
> > > > - refine the comments.
> > > > - invoke dpif_handlers_set() in udpif_set_threads().  this is a bug.
> > > >   the previous code will cause the handlers polling from closed
> > > >   fd.
> > >
> > > I'm still trying to figure out whether I properly understand the new
> > > API.  Here's a suggested replacement for the first paragraph of the
> > > comment on 'port_get_pid'.  Is it correct?
> > >
> > >     /* Returns the Netlink PID value to supply in
> OVS_ACTION_ATTR_USERSPACE
> > >      * actions as the OVS_USERSPACE_ATTR_PID attribute's value, for
> use in
> > >      * flows whose packets arrived on port 'port_no'.  In the case
> where
> > > the
> > >      * provider allocates multiple Netlink PIDs to a single port, it
> may
> > > use
> > >      * 'hash' to spread load among them.  The caller need not use a
> > > particular
> > >      * hash function, because it is not generally necessary to avoid
> > > reordering
> > >      * between packets received via flow misses (which are spread among
> > > PIDs by
> > >      * the datapath internally) and those received via userspace
> actions
> > > (which
> > >      * are spread via the return value of this function).  A 5-tuple
> hash
> > > is
> > >      * suitable.
> > >
> > >
> >
> > Thanks for pointing out the reordering issue, the suggested comment makes
> > sense.
> >
> > I'm not sure if it is good grammar, but can I add one more 'between' like
> > below?
> >
> >     /* Returns the Netlink PID value to supply in
> OVS_ACTION_ATTR_USERSPACE
> >      * actions as the OVS_USERSPACE_ATTR_PID attribute's value, for use
> in
> >      * flows whose packets arrived on port 'port_no'.  In the case where
> the
> >      * provider allocates multiple Netlink PIDs to a single port, it may
> use
> >      * 'hash' to spread load among them.  The caller need not use a
> > particular
> >      * hash function, because it is not generally necessary to avoid
> > reordering
> >      * between packets received via flow misses (which are spread among
> > PIDs by
> >      * the datapath internally) and *between *those received via
> userspace
> > actions (which
> >      * are spread via the return value of this function), (e.g. sampling
> > actions).
> >      * A 5-tuple hash is suitable.
>
> Hmm.  Usually one wants packets received via flow misses not to be
> reordered.  Usually one wants packets received via userspace actions
> not to be ordered.  One normally doesn't care about the relative
> ordering of the packets received by the two means.  To me, adding
> "between" in both cases means the former two statements, not the
> latter.
>
> How about something like this instead:
>
>     Returns the Netlink PID value to supply in
>     OVS_ACTION_ATTR_USERSPACE actions as the OVS_USERSPACE_ATTR_PID
>     attribute's value, for use in flows whose packets arrived on port
>     'port_no'.  In the case where the provider allocates multiple
>     Netlink PIDs to a single port, it may use 'hash' to spread load
>     among them.  The caller need not use a particular hash function; a
>     5-tuple hash is suitable.
>
>     (The datapath implementation might use some different hash
>     function for distributing packets received via flow misses among
>     PIDs.  This means that packets received via flow misses might be
>     reordered relative to packets received via userspace actions.
>     This is not ordinarily a problem.)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140313/fd5ca6f8/attachment-0005.html>


More information about the dev mailing list