[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