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

[Tom Herbert <tom@herbertland.com>]

On Wed, Oct 9, 2019 at 8:42 AM Jiri Benc <jbenc@redhat.com> wrote:
>
> On Wed, 9 Oct 2019 10:58:51 -0400, Willem de Bruijn wrote:
> > 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.
>
> Willem, how would you do that? The whole fou code including its netlink
> API is built around the assumption that it's L4 encapsulation. I see no
> way to extend it to do L3 encapsulation without major hacks - in fact,
> you'll add all that Martin's patch adds, including a new netlink API,
> but just mix that into fou.c.
>
More specifically fou allows encapsulation of anything that has an IP
protocol number. That includes an L3 protocols that have been assigned
a number (e.g. MPLS, GRE, IPv6, IPv4, EtherIP). So the only need for
an alternate method to do L3 encapsulation would be for those
protocols that don't have an IP protocol number assignment.
Presumably, those just have an EtherType. In that case, it seems
simple enough to just extend fou to processed an encapsulated
EtherType. This should be little more than modifying the "struct fou"
to hold 16 bit EtherType (union with protocol), adding
FOU_ENCAP_ETHER, corresponding attribute, and then populate
appropriate receive functions for the socket.

Tom

> > 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?
>
> The nice thing about the bareudp tunnel is that it's generic. It's just
> (outer) UDP header followed by (inner) L3 header. You can use it for
> NSH over UDP. Or for multitude of other purposes.
>
> What you're proposing is MPLS only. And it would require similar amount
> of code as the bareudp generic solution. It also doesn't fit fou
> design: MPLS is not an IP protocol. Trying to add it there is like
> forcing a round peg into a square hole. Just look at the code.
> Start with parse_nl_config.
>
> > Extending this existing code has more benefits than code reuse (and
> > thereby fewer bugs), it also allows reusing the existing GRO logic,
> > say.
>
> In this case, it's the opposite: trying to retrofit L3 encapsulation
> into fou is going to introduce bugs.
>
> Moreover, given the expected usage of bareudp, the nice benefit is it's
> lwtunnel only. No need to carry all the baggage of standalone
> configuration fou has.
>
> > At a high level, I think we should try hard to avoid duplicating
> > tunnel code for every combination of inner and outer protocol.
>
> Yet you're proposing to do exactly that with special casing MPLS in fou.
>
> I'd say bareudp is as generic as you could get it. It allows any L3
> protocol inside UDP. You can hardly make it more generic than that.
> With ETH_P_TEB, it could also encapsulate Ethernet (that is, L2).
>
> > 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.
>
> You're mixing apples with oranges. IP tunnels and FOU encapsulate L4
> payload. VXLAN and Geneve encapsulate L2 payload (or L3 payload for
> VXLAN-GPE).
>
> I agree that VXLAN, Geneve and the proposed bareudp module have
> duplicated code. The solution is to factoring it out to a common
> location.
>
> > I think it will be preferable to work the other way around and extend
> > existing ip tunnel infra to add MPLS.
>
> You see, that's the problem: this is not IP tunneling.
>
>  Jiri