[ovs-dev] [PATCH branch-2.3 2/2] ofproto: Allow in-place modifications of datapath flows.

Jarno Rajahalme jarno at ovn.org
Thu Feb 4 23:08:48 UTC 2016


> On Feb 4, 2016, at 3:00 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> On Wed, Feb 03, 2016 at 12:33:01PM -0800, Jarno Rajahalme wrote:
>> From: Ethan Jackson <ethan at 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 <ethan at nicira.com>
>> Signed-off-by: Jarno Rajahalme <jarno at 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




More information about the dev mailing list