On Thu, Sep 27, 2012 at 04:45:22PM -0700, Jesse Gross wrote:
> On Thu, Sep 27, 2012 at 12:05 PM, ravi kerur <rke...@gmail.com> wrote:
> > On Thu, Sep 27, 2012 at 11:16 AM, Jesse Gross <je...@nicira.com> wrote:
> >> On Thu, Sep 27, 2012 at 9:26 AM, Ben Pfaff <b...@nicira.com> wrote:
> >> > On Wed, Sep 26, 2012 at 04:13:41PM +0900, Simon Horman wrote:
> >> >> This patch provides an implementation of the non-datapath portions
> >> >> of MPLS matches and actions.
> >> >>
> >> >> This patch is based on top of Ben Pfaff's series,
> >> >> "set-field action support"
> >> >>
> >> >> Cc: Isaku Yamahata <yamah...@valinux.co.jp>
> >> >> Cc: Ravi K <rke...@gmail.com>
> >> >
> >> > I think Ravi's full last name is "Kerur".
> >> >
> >> >> Signed-off-by: Simon Horman <ho...@verge.net.au>
> >> >
> >> > Jesse, do you have any concerns about this?  I haven't read past the
> >> > diffstat yet.  But if it seems like a reasonable intermediate approach
> >> > then I'm happy to review it.
> >>
> >> I don't think there is inherently anything wrong in starting with a
> >> userspace-only approach.  I have a couple of specific concerns based
> >> on briefly skimming the patch:
> >>  * It seems like this is really the userspace half of the code which
> >> assumes that the kernel portions will still be doing the work on the
> >> actual packet flows.  If that's the case then I don't think that
> >> userspace support can go in independently.  Otherwise, userspace
> >> should really be self-contained and setup slow-path flows to do the
> >> work itself.
> >
> >
> > mpls userspace is completely self-contained i.e. doesn't depend on OVS
> > kernel code. I am saying this based on implementation and testing. During
> > testing no OVS kernel module was loaded and testing such as ping, iperf,
> > netperf and scp executed.
> 
> At the very least, userspace-only code shouldn't add anything to
> either the userspace/kernel interface or odp-util.c.  I also don't see
> how any MPLS action will actually get processed.  I do see that MPLS
> actions send packets to the local port and then install a flow but I
> think there is a misunderstanding because that doesn't make a lot of
> sense to me.

That portion is probably my handiwork. Could you give some guidance on
a method that would work? I'm more than happy to rework things.

> >>  * If it is truly userspace only, the handling of multiple levels of  tags
> >> seems a little incomplete since we actually have the full packet.
> >
> >
> > Can you elaborate  please?
> 
> Multiple levels of tags aren't handled, for example, when you pop a tag.

Do you think that adding support for multiple levels of tags to the kernel
is appropriate. Or should such cases be bounced to user-space?

> >>  If this is supposed to be a quick stepping stone to kernel support
> >> that seems less important since we will no longer have complete packet
> >> access.
> >>
> >> So it basically comes down to what the short term plans are.  There's
> >> also Leo's patch (which I haven't looked at) that I can post if there
> >> are plans to do kernel support.
> >
> >
> > <rk> so will there be separate/different kernel interface?
> 
> Separate from what?
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
> 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to