Re: [PATCH net-next 1/2] UDP tunnel encapsulation module for tunnelling different protocols like MPLS,IP,NSH etc.

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

On Wed, Oct 9, 2019 at 8:48 AM Martin Varghese
<martinvarghesenokia@gmail.com> wrote:
>
> On Tue, Oct 08, 2019 at 12:28:23PM -0400, Willem de Bruijn wrote:
> > On Tue, Oct 8, 2019 at 5:51 AM Martin Varghese
> > <martinvarghesenokia@gmail.com> wrote:
> > >
> > > From: Martin <martin.varghese@nokia.com>
> > >
> > > The Bareudp tunnel module provides a generic L3 encapsulation
> > > tunnelling module for tunnelling different protocols like MPLS,
> > > IP,NSH etc inside a UDP tunnel.
> > >
> > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com>
> > > +++ b/Documentation/networking/bareudp.txt
> > > @@ -0,0 +1,23 @@
> > > +Bare UDP Tunnelling Module Documentation
> > > +========================================
> > > +
> > > +There are various L3 encapsulation standards using UDP being discussed to
> > > +leverage the UDP based load balancing capability of different networks.
> > > +MPLSoUDP (https://tools.ietf.org/html/rfc7510)is one among them.
> > > +
> > > +The Bareudp tunnel module provides a generic L3 encapsulation tunnelling
> > > +support for tunnelling different L3 protocols like MPLS, IP, NSH etc. inside
> > > +a UDP tunnel.
> >
> > This patch set introduces a lot of code, much of which duplicates
> > existing tunnel devices, whether FOU using ipip tunnels and
> > fou_build_header or separate devices like vxlan and geneve. Let's try
> > to reuse what's there (and tested).
> >
> > How does this differ from foo-over-udp (FOU). In supporting MPLS
> > alongside IP? If so, can it reuse the existing code, possibly with
> > patches to that existing tunnel logic?
> >
> > I happened to have been playing around with MPLS in GRE recently. Have
> > not tried the same over UDP tunnels, but most of it should apply?
> >
> >   ip tunnel add gre1 mode gre local 192.168.1.1 remote 192.168.1.2 dev eth0
> >   ip addr add 192.168.101.1 peer 192.168.101.2 dev gre1
> >   ip link set dev gre1 up
> >
> >   sysctl net.mpls.conf.gre1.input=1
> >   sysctl net.mpls.platform_labels=17
> >   ip addr add 192.168.201.1/24 dev gre1
> >   ip route add 192.168.202.0/24 encap mpls 17 dev gre1
> >   ip -f mpls route add 16 dev lo
> >
> >
> unlike FOU which does l4 encapsulation, here we are trying to acheive l3 encapsulation.
> For Example,  What is needed for MPLSoUDP is to take packets such as:
>
> eth header | mpls header | payload
>
> and encapsulate them to:
>
> eth header | ip header | udp header | mpls header | payload
> <--------- outer ------------------> <---- inner --------->
>
> which is later decapsulated back to:
>
> eth header | mpls header | payload
>
> Note that in the decapsulated case, the ethertype in the eth header is ETH_P_MPLS_UC, while in the encapsulated case, the ethertype in the eth header is ETH_P_IP. When decapsulating, ETH_P_MPLS_UC is
> restored based on the configured tunnel parameters.
>
> This cannot be done with FOU. FOU needs an ipproto, not ethertype.
>
> During receive FOU dispatches packet to l4 handlers which makes it impossible to patch it to get it working for l3 protocols like MPLS.

Yes, this needs a call to gro_cells_receive like geneve_udp_encap_recv
and vxlan_rcv, instead of returning a negative value back to
udp_queue_rcv_one_skb. Though that's not a major change.

Transmit is easier. The example I shared already encapsulates packets
with MPLs and sends them through a tunnel device. Admittedly using
mpls routing. But the ip tunneling infrastructure supports adding
arbitrary inner headers using ip_tunnel_encap_ops.build_header.
net/ipv4/fou.c could be extended with a mpls_build_header alongside
fou_build_header and gue_build_header?

Extending this existing code has more benefits than code reuse (and
thereby fewer bugs), it also allows reusing the existing GRO logic,
say.

At a high level, I think we should try hard to avoid duplicating
tunnel code for every combination of inner and outer protocol. Geneve
and vxlan on the one hand and generic ip tunnel and FOU on the other
implement most of these features. Indeed, already implements the
IPxIPx, just not MPLS. It will be less code to add MPLS support based
on existing infrastructure.

> > The route lookup logic is very similar to vxlan_get_route and
> > vxlan6_get_route. Can be reused?
>
> Yes  we could. We can move these to a common place. include/net/ip_tunnels.h ?

I think it will be preferable to work the other way around and extend
existing ip tunnel infra to add MPLS.