Skip to contentSkip to conversation

gre: fix lwtunnel support

PATCH In review Jiri Benc 2 participants 4 messages Apr 27, 2016 – Apr 28, 2016

Subsystems (3)
lore
Share & tools
lore

Jiri Benc

author
#0 lore raw

v3 cover gre: fix lwtunnel support

This patchset fixes a few bugs in ipgre metadata mode implementation.

As an example, in this setup:

ip a a 192.168.1.1/24 dev eth0
ip l a gre1 type gre external
ip l s gre1 up
ip a a 192.168.99.1/24 dev gre1
ip r a 192.168.99.2/32 encap ip dst 192.168.1.2 ttl 10 dev gre1 ping 192.168.99.2

the traffic does not go through before this patchset and does as expected with it applied.

v3: Back to v1 in order not to break existing users. Dropped patch 3, will
be fixed in iproute2 instead.
v2: Rejecting invalid configuration, added patch 3, dropped patch for
ETH_P_TEB (will target net-next).

Jiri Benc (2):
gre: do not assign header_ops in collect metadata mode
gre: build header correctly for collect metadata tunnels

 net/ipv4/ip_gre.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH v3 1/2 gre: do not assign header_ops in collect metadata mode

In ipgre mode (i.e. not gretap) with collect metadata flag set, the tunnel is incorrectly assumed to be mGRE in NBMA mode (see commit 6a5f44d7a048c). This is not the case, we're controlling the encapsulation addresses by lwtunnel metadata. And anyway, assigning dev->header_ops in collect metadata mode does not make sense.

Although it would be more user firendly to reject requests that specify both the collect metadata flag and a remote/local IP address, this would break current users of gretap or introduce ugly code and differences in handling ipgre and gretap configuration. Keep the current behavior of remote/local IP address being ignored in such case.

v3: Back to v1, added explanation paragraph.
v2: Reject configuration specifying both remote/local address and collect
metadata flag.

Fixes 2e15ea390e6f4 ("ip_gre: Add support to collect tunnel metadata.")
---
 net/ipv4/ip_gre.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
net/ipv4/ip_gre.c +3 -2
index af5d1f38217f..d0abde4236af 100644
@@ -893,7 +893,7 @@ static int ipgre_tunnel_init(struct net_device *dev)
 	netif_keep_dst(dev);
 	dev->addr_len		= 4;
 
-	if (iph->daddr) {
+	if (iph->daddr && !tunnel->collect_md) {
 #ifdef CONFIG_NET_IPGRE_BROADCAST
 		if (ipv4_is_multicast(iph->daddr)) {
 			if (!iph->saddr)
@@ -902,8 +902,9 @@ static int ipgre_tunnel_init(struct net_device *dev)
 			dev->header_ops = &ipgre_header_ops;
 		}
 #endif
-	} else
+	} else if (!tunnel->collect_md) {
 		dev->header_ops = &ipgre_header_ops;
+	}
 
 	return ip_tunnel_init(dev);
 }
Signature · 1 line
1.8.3.1
author
#2 lore raw

PATCH v3 2/2 gre: build header correctly for collect metadata tunnels

In reply to Jiri Benc

In ipgre (i.e. not gretap) + collect metadata mode, the skb was assumed to contain Ethernet header and was encapsulated as ETH_P_TEB. This is not the case, the interface is ARPHRD_IPGRE and the protocol to be used for encapsulation is skb->protocol.

Fixes 2e15ea390e6f4 ("ip_gre: Add support to collect tunnel metadata.")

---
v3: unchanged
v2: unchanged

---
 net/ipv4/ip_gre.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
net/ipv4/ip_gre.c +5 -4
index d0abde4236af..f973e0a58993 100644
@@ -523,7 +523,8 @@ static struct rtable *gre_get_rt(struct sk_buff *skb,
 	return ip_route_output_key(net, fl);
 }
 
-static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev)
+static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev,
+			__be16 proto)
 {
 	struct ip_tunnel_info *tun_info;
 	const struct ip_tunnel_key *key;
@@ -575,7 +576,7 @@ static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	flags = tun_info->key.tun_flags & (TUNNEL_CSUM | TUNNEL_KEY);
-	build_header(skb, tunnel_hlen, flags, htons(ETH_P_TEB),
+	build_header(skb, tunnel_hlen, flags, proto,
 		     tunnel_id_to_key(tun_info->key.tun_id), 0);
 
 	df = key->tun_flags & TUNNEL_DONT_FRAGMENT ?  htons(IP_DF) : 0;
@@ -616,7 +617,7 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb,
 	const struct iphdr *tnl_params;
 
 	if (tunnel->collect_md) {
-		gre_fb_xmit(skb, dev);
+		gre_fb_xmit(skb, dev, skb->protocol);
 		return NETDEV_TX_OK;
 	}
 
@@ -660,7 +661,7 @@ static netdev_tx_t gre_tap_xmit(struct sk_buff *skb,
 	struct ip_tunnel *tunnel = netdev_priv(dev);
 
 	if (tunnel->collect_md) {
-		gre_fb_xmit(skb, dev);
+		gre_fb_xmit(skb, dev, htons(ETH_P_TEB));
 		return NETDEV_TX_OK;
 	}
 
Signature · 1 line
1.8.3.1
maintainer
#3 lore raw
In reply to Jiri Benc

From: Jiri Benc <[email protected]>
Date: Wed, 27 Apr 2016 11:29:05 +0200

Quoted 11 lines

This patchset fixes a few bugs in ipgre metadata mode implementation.

As an example, in this setup:

ip a a 192.168.1.1/24 dev eth0
ip l a gre1 type gre external
ip l s gre1 up
ip a a 192.168.99.1/24 dev gre1
ip r a 192.168.99.2/32 encap ip dst 192.168.1.2 ttl 10 dev gre1 ping 192.168.99.2

Quoted 7 lines

the traffic does not go through before this patchset and does as expected with it applied.

v3: Back to v1 in order not to break existing users. Dropped patch 3, will
be fixed in iproute2 instead.
v2: Rejecting invalid configuration, added patch 3, dropped patch for
ETH_P_TEB (will target net-next).

Series applied, thanks Jiri.