From: Jiri Benc <jbenc@redhat.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Martin Varghese <martinvarghesenokia@gmail.com>,
Network Development <netdev@vger.kernel.org>,
David Miller <davem@davemloft.net>,
corbet@lwn.net, scott.drennan@nokia.com,
martin.varghese@nokia.com
Subject: Re: [PATCH net-next 1/2] UDP tunnel encapsulation module for tunnelling different protocols like MPLS,IP,NSH etc.
Date: Wed, 9 Oct 2019 17:42:16 +0200 [thread overview]
Message-ID: <20191009174216.1b3dd3dc@redhat.com> (raw)
In-Reply-To: <CA+FuTSdGR2G8Wp0khT9nCD49oi2U_GZiyS5vJTBikPRm+0fGPg@mail.gmail.com>
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.
> 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
next prev parent reply other threads:[~2019-10-09 15:42 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-08 9:48 [PATCH net-next 0/2] Bareudp Tunnel Module Martin Varghese
2019-10-08 9:48 ` [PATCH net-next 1/2] UDP tunnel encapsulation module for tunnelling different protocols like MPLS,IP,NSH etc Martin Varghese
2019-10-08 14:06 ` Jonathan Corbet
2019-10-08 14:57 ` Randy Dunlap
2019-10-08 16:04 ` Stephen Hemminger
2019-10-08 16:05 ` Stephen Hemminger
2019-10-08 16:28 ` Willem de Bruijn
2019-10-09 12:48 ` Martin Varghese
2019-10-09 14:58 ` Willem de Bruijn
2019-10-09 15:21 ` Willem de Bruijn
2019-10-09 15:42 ` Jiri Benc [this message]
2019-10-09 16:15 ` Willem de Bruijn
2019-10-18 20:03 ` Tom Herbert
2019-10-21 17:18 ` Jiri Benc
2019-10-17 13:20 ` Martin Varghese
2019-10-17 20:48 ` Willem de Bruijn
2019-10-18 8:20 ` Martin Varghese
2019-10-18 14:59 ` Willem de Bruijn
2019-10-23 2:40 ` Martin Varghese
2019-11-07 13:38 ` Martin Varghese
2019-11-07 15:53 ` Willem de Bruijn
2019-11-07 16:12 ` Martin Varghese
2019-11-07 16:35 ` Willem de Bruijn
2019-11-07 17:31 ` Jiri Benc
2019-11-07 18:59 ` Willem de Bruijn
2019-11-07 19:05 ` Jiri Benc
2019-11-07 19:13 ` Willem de Bruijn
2019-11-11 16:02 ` Martin Varghese
2019-11-11 21:45 ` Willem de Bruijn
2019-11-14 13:17 ` Martin Varghese
2019-10-08 9:49 ` [PATCH net-next 2/2] Special handling for IP & MPLS Martin Varghese
2019-10-08 14:58 ` Randy Dunlap
2019-10-08 16:09 ` Willem de Bruijn
2019-10-09 13:38 ` Martin Varghese
2019-10-09 15:06 ` Willem de Bruijn
2019-10-09 15:19 ` Willem de Bruijn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191009174216.1b3dd3dc@redhat.com \
--to=jbenc@redhat.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=martin.varghese@nokia.com \
--cc=martinvarghesenokia@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=scott.drennan@nokia.com \
--cc=willemdebruijn.kernel@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.