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 <b...@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.) >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
