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