[ovs-dev] [PATCH] ofproto-dpif: ovs-vswitchd.log showing unexpected flow

Jesse Gross jesse at nicira.com
Sat Aug 3 06:47:39 UTC 2013


On Fri, Aug 2, 2013 at 11:36 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Fri, Aug 02, 2013 at 10:27:14PM -0700, Andy Zhou wrote:
>> In the case of mega flow would this cause multiple subfacets within a facet
>> to be considered installed?
>>
>>
>> On Fri, Aug 2, 2013 at 10:21 PM, Ben Pfaff <blp at nicira.com> wrote:
>>
>> > On Fri, Aug 02, 2013 at 08:22:17PM -0700, Andy Zhou wrote:
>> > > This patch prevents the same subfacet from trying to install the kernel
>> > > flow more than once.
>> > >
>> > > This is how a kernel flow can become unexpected to the user space.
>> > > When a incoming packet did not match any flow in the kernel, it
>> > > would be sent up to the user space. A subfacet would be created and
>> > > a corresponding kenrel flow installed. So far so good.
>> > >
>> > > Just before the kernel flow was installed, another packet of the same
>> > > flow were to arrive at the kernel, it would also be queued up
>> > > for user space.  This time, user space would find the subfacet just
>> > > created (subfacet_create()) due of the first packet, and attempt
>> > > to install the same kernel flow again, but could not (kernel returns
>> > EEXIST).
>> > > User space would then mark the subfacet as not installed
>> > (SF_NOT_INSTALLED).
>> >
>> > Can we just ignore EEXIST in the error handling loop at the bottom of
>> > handle_miss_upcalls()?
>> >
>
> OK, now I'm starting to see the whole picture.
>
> There are two EEXIST error cases:
>
>        1. A subfacet was installed successfully in a previous (recent)
>           batch.  Now we've attempted to reinstall exactly the same
>           subfacet in this batch.
>
>        2. A subfacet was installed successfully in a previous (recent)
>           batch or earlier in the current batch.  We've attempted to
>           install a subfacet for an overlapping megaflow.
>
> So far we have two ideas for handling these:
>
>        a) The current way, which is to consider on any installation
>           error that a given subfacet is not installed.  This works well
>           for case #2 but causes case #1 to yield unexpected flows, as
>           described in the commit message here.
>
>        b) The way proposed in this patch, which is the same as a) with
>           the added wrinkle that we never try to reinstall exactly the
>           same subfacet that we know we installed successfully earlier
>           (and haven't deleted) unless its actions change.  This ought
>           to work just as well as a) for case #2, and avoids the problem
>           with case #1.
>
> The (minor) flaw in b) is that if something goes wrong and a flow
> disappears from the kernel (e.g. ovs-dpctl del-flows), then userspace
> won't reinstall it (until it tries to delete it).
>
> If I understand the patch properly now, then I'm willing to commit it.
> Is that right?

That sounds right to me.



More information about the dev mailing list