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

[Andy Zhou]

Yes, this is my understanding as well.
I did not mention the implication of mega flow in the commit message
because it is not the problem. But it is important in discussing about the
solution.

Your summary provides a better and more complete picture.

thanks for the review.


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?
>
> Thanks,
>
> Ben.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130803/41d274f7/attachment-0003.html>