[PATCH net] bareudp: set the inner protocol to the protocol of the packet

[PATCH net] bareudp: set the inner protocol to the protocol of the packet

[Haishuang Yan]

bareudp_xmit_skb() sets the inner protocol of the skb to the configured
ethertype, and bareudp6_xmit_skb() does not set it at all. The inner
protocol is used by skb_udp_tunnel_segment() to segment the inner packet
when GSO has to be done in software, for instance when the lower device
does not offload the checksum.

With multiproto, an IPv4 device also carries IPv6, so the IPv6 header
is parsed as an IPv4 one. With IPv6 underlay, the inner protocol is
whatever the skb had before. In both cases segmentation fails and the
packets are dropped. With veth tx checksum offload disabled, iperf3 TCP
over bareudp gets about 10 Mbit/s with thousands of retransmits instead
of about 700 Mbit/s for IPv6 over IPv4 and for both families over IPv6,
while IPv4 over IPv4 works.

At this point skb->protocol is the protocol of the inner packet, which
bareudp_xmit() has already checked against the configuration, so use it
on both paths.

Fixes: 571912c69f0e ("net: UDP tunnel encapsulation module for tunnelling different protocols like MPLS, IP, NSH etc.")
Fixes: 4b5f67232d95 ("net: Special handling for IP & MPLS.")
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
 drivers/net/bareudp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
index c3b5ed52d877..b9fe54151e1a 100644
--- a/drivers/net/bareudp.c
+++ b/drivers/net/bareudp.c
@@ -356,7 +356,7 @@ static int bareudp_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 	if (err)
 		goto free_dst;
 
-	skb_set_inner_protocol(skb, bareudp->ethertype);
+	skb_set_inner_protocol(skb, skb->protocol);
 	udp_tunnel_xmit_skb(rt, sk, skb, saddr, info->key.u.ipv4.dst,
 			    tos, ttl, df, sport, bareudp->port,
 			    !net_eq(bareudp->net, dev_net(bareudp->dev)),
@@ -424,6 +424,7 @@ static int bareudp6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 	if (err)
 		goto free_dst;
 
+	skb_set_inner_protocol(skb, skb->protocol);
 	daddr = info->key.u.ipv6.dst;
 	udp_tunnel6_xmit_skb(dst, sk, skb, dev,
 			     &saddr, &daddr, prio, ttl,
-- 
2.43.0

Re: [PATCH net] bareudp: set the inner protocol to the protocol of the packet

[Willem de Bruijn]

Haishuang Yan wrote:
> bareudp_xmit_skb() sets the inner protocol of the skb to the configured
> ethertype, and bareudp6_xmit_skb() does not set it at all. The inner
> protocol is used by skb_udp_tunnel_segment() to segment the inner packet
> when GSO has to be done in software, for instance when the lower device
> does not offload the checksum.
> 
> With multiproto, an IPv4 device also carries IPv6, so the IPv6 header
> is parsed as an IPv4 one. With IPv6 underlay, the inner protocol is
> whatever the skb had before. In both cases segmentation fails and the
> packets are dropped. With veth tx checksum offload disabled, iperf3 TCP
> over bareudp gets about 10 Mbit/s with thousands of retransmits instead
> of about 700 Mbit/s for IPv6 over IPv4 and for both families over IPv6,
> while IPv4 over IPv4 works.

(not for this fix patch)

optionally tools/testing/selftests/net/bareudp.sh could run a stream
test besides/alongside the ping test, to exercise this path (with
segmentation offload disabled).

> At this point skb->protocol is the protocol of the inner packet, which
> bareudp_xmit() has already checked against the configuration, so use it
> on both paths.
> 
> Fixes: 571912c69f0e ("net: UDP tunnel encapsulation module for tunnelling different protocols like MPLS, IP, NSH etc.")
> Fixes: 4b5f67232d95 ("net: Special handling for IP & MPLS.")
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>