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

[Simon Horman]

On Thu, Apr 04, 2013 at 11:21:33AM +0900, Simon Horman wrote:
> On Wed, Apr 03, 2013 at 05:55:46PM -0700, Jesse Gross wrote:
> > On Wed, Apr 3, 2013 at 5:24 PM, Simon Horman <horms at verge.net.au> wrote:
> > > On Wed, Apr 03, 2013 at 01:29:53PM -0700, Jesse Gross wrote:
> > >> 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.
> > >
> > > I agree that an incremental approach with smaller and more readable
> > > patches is desirable. And I believe that the behaviour in rfc2 and rfc3
> > > is correct for handle flow miss without facets as it forces flow miss
> > > with facets if there is any chance of recirculation.
> > >
> > > What I am a little unclear about is how to handle packet out,
> > > which currently seems to be far away from anything relating to facets.
> > > I wonder if an approach would be to make an add-on patch or patches
> > > which adds support for ovs-vswitchd to handle recirculation.
> > >
> > > I believe much of such a change would involve re-using packet execution code
> > > that is already present for use by the user-space datapath.
> > >
> > > From there I think it would be not to much of a leap to handle
> > > recirculation in ovs-vswitchd for flow misses without facets.
> > >
> > > And then we can see where we are.
> > 
> > I agree it seems likely that is the way things will evolve.  You had
> > mentioned using a synthetic facet for this before for the time being.
> > Did that not work?
> 
> To be honest it was just an idea that I have no code for.
> And it seems complex to me.

I have posted rfc4 of this patchset which adds handling of recirculation
for cases without facets by executing the packet as described above.
It does not seem to be reasonably compact and not particularly complex.
Though perhaps I have missed some tricky cases.