On Thu, Sep 27, 2012 at 5:44 PM, Simon Horman <ho...@verge.net.au> wrote:
> 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? > <rk> multiple tags do work in both userspace and kernel i.e. any combination of push/pop actions. I am not sure what the confusion is? Simon: I have not looked into your patch, has anything changed here? > > > >> 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? > <rk> Our earlier disagreement was on packet recirculation for multiple actions. You mention about Leo's patch and I assumed it contains packet recirculation support and you want mpls changes for that new piece of kernel datapath and wanted clarification on that. > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > > >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
