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

Simon Horman horms at verge.net.au
Thu Apr 4 02:21:33 UTC 2013


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.



More information about the dev mailing list