> On Feb 4, 2016, at 3:00 PM, Ben Pfaff <b...@ovn.org> wrote: > > On Wed, Feb 03, 2016 at 12:33:01PM -0800, Jarno Rajahalme wrote: >> From: Ethan Jackson <et...@nicira.com> >> >> There are certain use cases (such as bond rebalancing) where a >> datapath flow's actions may change, while it's wildcard pattern >> remains the same. Before this patch, revalidators would note the >> change, delete the flow, and wait for the handlers to install an >> updated version. This is inefficient, as many packets could get >> punted to userspace before the new flow is finally installed. >> >> To improve the situation, this patch implements in place modification >> of datapath flows. If the revalidators detect the only change to a >> given ukey is its actions, instead of deleting it, it does a put with >> the MODIFY flag set. >> >> This patch is a backport of commit 43b2f13 to branch-2.3. >> >> Signed-off-by: Ethan J. Jackson <et...@nicira.com> >> Signed-off-by: Jarno Rajahalme <ja...@ovn.org> > > The buffer management in modify_op_init() really bothers me. It is > brittle and confusing. It has an unstated assumption that, if 'buf' is > nonnull, then it contains 'key' and 'mask' and that, if 'buf' is null, > then 'key' and 'mask' are elsewhere. I am not sure that this assumption > is correct, because in revalidator_sweep__() the 'key' argument to > modify_op_init() seems to come from a ukey whereas the 'mask' argument > seems to come from an ofpbuf allocated by dpif_flow_get(). > > Either way, it's really weird and hard to understand and maintain. Can > we do it another way?
This is why I asked you (offline) to pay attention to the buffer management.. Adjusting the ‘key’ is just wrong, and even if fixed (i.e., left unadjusted) it is likely better to solve the buffering here in some other way. Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
