On Fri, Aug 2, 2013 at 11:36 PM, Ben Pfaff <b...@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 <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?
That sounds right to me. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
