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 <b...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
