[PATCH] ipv6: Don't unset flowi6_proto in ipxip6_tnl_xmit()

Commit 8eb30be0352d0916 ("ipv6: Create ip6_tnl_xmit") unsets
flowi6_proto in ip4ip6_tnl_xmit() and ip6ip6_tnl_xmit().
Since xfrm_selector_match() relies on this info, IPv6 packets
sent by an ip6tunnel cannot be properly selected by their
protocols after removing it. This patch puts flowi6_proto back.

Cc: stable@vger.kernel.org
Fixes: 8eb30be0352d ("ipv6: Create ip6_tnl_xmit")
Signed-off-by: Eli Cooper <redacted>
---
 net/ipv6/ip6_tunnel.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 2050217..5c57797 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c

@@ -1174,6 +1174,7 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 		encap_limit = t->parms.encap_limit;
 
 	memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
+	fl6.flowi6_proto = IPPROTO_IPIP;
 
 	dsfield = ipv4_get_dsfield(iph);
 

@@ -1233,6 +1234,7 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 		encap_limit = t->parms.encap_limit;
 
 	memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
+	fl6.flowi6_proto = IPPROTO_IPV6;
 
 	dsfield = ipv6_get_dsfield(ipv6h);
 	if (t->parms.flags & IP6_TNL_F_USE_ORIG_TCLASS)

-- 
2.9.3

Hi,

On Fri, 26 Aug 2016 23:52:29 +0800 Eli Cooper [off-list ref] wrote:

I wonder if this is the right way to go.

This DOES fix the regression for ip4ip6_tnl_xmit and ip6ip6_tnl_xmit
users.

However new users of 'ip6_tnl_xmit' (e.g. __gre6_xmit callers) may still
have their 'fl6->flowi6_proto' not armed.

Maybe the right fix is assigning 'fl6->flowi6_proto = proto', either at
start of 'ip6_tnl_xmit' or just prior the ip6_route_output() call.

Tom, can you acknowledge?

Thanks,
Shmulik

Hi,

On 2016/8/27 3:49, Shmulik Ladkani wrote:

However new users of 'ip6_tnl_xmit' (e.g. __gre6_xmit callers) may still
have their 'fl6->flowi6_proto' not armed.

Maybe the right fix is assigning 'fl6->flowi6_proto = proto', either at
start of 'ip6_tnl_xmit' or just prior the ip6_route_output() call.

The only other user of ip6_tnl_xmit() is ip6gre, and it has its
fl6->flowi6_proto set in ip6gre_tnl_link_config() as in commit
252f3f5a1189 ("ip6_gre: Set flowi6_proto as IPPROTO_GRE in xmit path.")

Assigning flowi6_proto at the start of ip6_tnl_xmit() was also my first
idea, but I was unsure which fix would be better.

Thanks,
Eli

From: Eli Cooper <redacted>
Date: Fri, 26 Aug 2016 23:52:29 +0800

Let's just simply have t->fl have the proto setup properly, just like
in GRE.

Assigning it explicitly every packet transmit doesn't make much sense.

Hello,

On 2016/9/1 4:56, David Miller wrote:

From: Eli Cooper <redacted>
Date: Fri, 26 Aug 2016 23:52:29 +0800

quoted

@@ -1174,6 +1174,7 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 		encap_limit = t->parms.encap_limit;
 
 	memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
+	fl6.flowi6_proto = IPPROTO_IPIP;

Let's just simply have t->fl have the proto setup properly, just like
in GRE.

Assigning it explicitly every packet transmit doesn't make much sense.

I doubt that. Unlike GRE, where the proto must be IPPROTO_GRE, the proto
here can be either IPPROTO_IPV6 or IPPROTO_IPIP for a single tunnel, and
t->fl is shared by them. Thus it has to be assigned for every packet.

Thanks,
Eli

From: Eli Cooper <redacted>
Date: Thu, 1 Sep 2016 09:18:04 +0800

Hello,

On 2016/9/1 4:56, David Miller wrote:

quoted

From: Eli Cooper <redacted>
Date: Fri, 26 Aug 2016 23:52:29 +0800

quoted

@@ -1174,6 +1174,7 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 		encap_limit = t->parms.encap_limit;
 
 	memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
+	fl6.flowi6_proto = IPPROTO_IPIP;

Let's just simply have t->fl have the proto setup properly, just like
in GRE.

Assigning it explicitly every packet transmit doesn't make much sense.

I doubt that. Unlike GRE, where the proto must be IPPROTO_GRE, the proto
here can be either IPPROTO_IPV6 or IPPROTO_IPIP for a single tunnel, and
t->fl is shared by them. Thus it has to be assigned for every packet.

You're right.  Patch applied, thanks for explaining.