[ovs-dev] [RFC PATCH 4/4] Add packet recirculation

[Jesse Gross]

On Wed, Apr 3, 2013 at 6:05 AM, Simon Horman <horms at verge.net.au> wrote:
> On Wed, Apr 03, 2013 at 10:59:11AM +0900, Simon Horman wrote:
>> On Tue, Apr 02, 2013 at 05:24:40PM -0700, Jesse Gross wrote:
>> > On Fri, Mar 22, 2013 at 6:44 AM, Simon Horman <horms at verge.net.au> wrote:
>> > > On Tue, Mar 19, 2013 at 09:01:27AM -0700, Jesse Gross wrote:
>> > >> On Mon, Mar 18, 2013 at 6:34 PM, Simon Horman <horms at verge.net.au> wrote:
>> > >> > On Mon, Mar 18, 2013 at 01:49:43PM -0700, Jesse Gross wrote:
>> > >> >> However, do we actually want to tie this directly to recirculation as
>> > >> >> opposed to as a separate action?  I see possible benefits of
>> > >> >> separating it out:
>> > >> >
>> > >> > I'm not really sure that I understand. Could you explain
>> > >> > how you see this working?
>> > >>
>> > >> Just a set action plus recirculation with no argument.  Separating out
>> > >> the issues of reusing skb mark, this could be done today with
>> > >> something like:
>> > >> pass 1: match MPLS -> pop_mpls, set(mark(X)), recirculate
>> > >> pass 2: match mark X, match MPLS -> pop_mpls, recirculate
>> > >> pass 3: match mark X, match IP -> output
>> > >
>> > > Thanks, I have a crude implementation of this working locally.
>> > >
>> > > I'm not sure what the implications are for the user-space datapath.
>> > > Though that is less of a priority for me than the kernel datapath.
>>
>> Yes, I agree. I think I have something working there too.
>>
>> > I think we could probably just add some kind of mark equivalent to the
>> > userspace datapath.  It doesn't seem like it should be too difficult.
>> >
>> > > I am also concerned, though less so, about:
>> > >
>> > > * How to handle packet_out. Perhaps some kind of synthetic facet?
>> > > * How to detect if recirculation will occur and thus force
>> > >   a miss to be handled with a facet. For now I am just checking
>> > >   if the packet MPLS or not, but it would be nice not to force facets
>> > >   unless necessary. Actually, it would be nice not to force the
>> > >   use of facets at all.
>> >
>> > Long term, the right optimization is probably to handle all
>> > recirculation in userspace for these cases.  It will certainly help
>> > performance in situations without facets since that means that we're
>> > already stressed on flow setups and it would be good to not generate
>> > more trips through the kernel.  However, for the time being the best
>> > strategy seems like some kind of delayed allocation of facets to the
>> > time that we decide to do recirculation (unless we were going to
>> > create one anyways).
>>
>> Thanks, I'll think about this some more.
>
> Yes, I agree with your statement on performance.
> That is exactly what I was thinking of too.
>
> I am wondering if by handling recirculation in userspace you mean
> that the packet should be modified, or in other words executed,
> in userspace an then a fresh actions translation should occur?

Right, we would end up doing all of the packet modifications and
recirculations in userspace and the common case would be to just
output in the kernel.

> If so, this seems similar to the code that I had in rfc1 of
> the recirculation patch. But it was rather complex in parts
> as I tried to handle recirculation of misses with facets in user-space too.
> And IIRC you asked me to remove it as you felt it duplicated datapath code.
>
> I think it should be possible to handle recirculation of just cases without
> facets in userspace in this way. But I'd like to confirm that is the
> direction you have in mind.

I think this is potentially tricky to get right in all of the corner
cases and your new version of the patch is certainly easier to read.
It seems to me that it would be better to get something in that is
correct (and doesn't affect performance for situations that don't use
this, which is why I suggested on demand allocation of facets).  We
can then come back and optimize the performance after that.