[PATCH] xfrm: use gre key as flow upper protocol info

[PATCH] xfrm: use gre key as flow upper protocol info

[Timo Teräs]

The GRE Key field is intended to be used for identifying an individual
traffic flow within a tunnel. It is useful to be able to have XFRM
policy selector matches to have different policies for different
GRE tunnels.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
---
Basic testing done, but more knowledgeable should check that I did
not miss anything essential.

 include/net/flow.h      |    2 ++
 include/net/xfrm.h      |    6 ++++++
 net/ipv4/ip_gre.c       |   12 +++++++-----
 net/ipv4/xfrm4_policy.c |   15 +++++++++++++++
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/include/net/flow.h b/include/net/flow.h
index 0ac3fb5..7196e68 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -67,6 +67,7 @@ struct flowi {
 		} dnports;
 
 		__be32		spi;
+		__be32		gre_key;
 
 		struct {
 			__u8	type;
@@ -78,6 +79,7 @@ struct flowi {
 #define fl_icmp_code	uli_u.icmpt.code
 #define fl_ipsec_spi	uli_u.spi
 #define fl_mh_type	uli_u.mht.type
+#define fl_gre_key	uli_u.gre_key
 	__u32           secid;	/* used by xfrm; see secid.txt */
 } __attribute__((__aligned__(BITS_PER_LONG/8)));
 
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index bcfb6b2..54b2832 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -805,6 +805,9 @@ __be16 xfrm_flowi_sport(struct flowi *fl)
 	case IPPROTO_MH:
 		port = htons(fl->fl_mh_type);
 		break;
+	case IPPROTO_GRE:
+		port = htonl(fl->fl_gre_key) >> 16;
+		break;
 	default:
 		port = 0;	/*XXX*/
 	}
@@ -826,6 +829,9 @@ __be16 xfrm_flowi_dport(struct flowi *fl)
 	case IPPROTO_ICMPV6:
 		port = htons(fl->fl_icmp_code);
 		break;
+	case IPPROTO_GRE:
+		port = htonl(fl->fl_gre_key) & 0xffff;
+		break;
 	default:
 		port = 0;	/*XXX*/
 	}
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 70ff77f..ffc78e8 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -779,9 +779,9 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 					.tos = RT_TOS(tos)
 				}
 			},
-			.proto = IPPROTO_GRE
-		}
-;
+			.proto = IPPROTO_GRE,
+			.fl_gre_key = tunnel->parms.o_key
+		};
 		if (ip_route_output_key(dev_net(dev), &rt, &fl)) {
 			dev->stats.tx_carrier_errors++;
 			goto tx_error;
@@ -958,7 +958,8 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
 					.tos = RT_TOS(iph->tos)
 				}
 			},
-			.proto = IPPROTO_GRE
+			.proto = IPPROTO_GRE,
+			.fl_gre_key = tunnel->parms.o_key
 		};
 		struct rtable *rt;
 
@@ -1223,7 +1224,8 @@ static int ipgre_open(struct net_device *dev)
 					.tos = RT_TOS(t->parms.iph.tos)
 				}
 			},
-			.proto = IPPROTO_GRE
+			.proto = IPPROTO_GRE,
+			.fl_gre_key = t->parms.o_key
 		};
 		struct rtable *rt;
 
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 4464f3b..57af4bd 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -11,6 +11,7 @@
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/inetdevice.h>
+#include <linux/if_tunnel.h>
 #include <net/dst.h>
 #include <net/xfrm.h>
 #include <net/ip.h>
@@ -158,6 +159,20 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
 				fl->fl_ipsec_spi = htonl(ntohs(ipcomp_hdr[1]));
 			}
 			break;
+
+		case IPPROTO_GRE:
+			if (pskb_may_pull(skb, xprth + 12 - skb->data)) {
+				__be16 *greflags = (__be16 *)xprth;
+				__be32 *gre_hdr = (__be32 *)xprth;
+
+				if (greflags[0] & GRE_KEY) {
+					if (greflags[0] & GRE_CSUM)
+						gre_hdr++;
+					fl->fl_gre_key = gre_hdr[1];
+				}
+			}
+			break;
+
 		default:
 			fl->fl_ipsec_spi = 0;
 			break;
-- 
1.7.1

Re: [PATCH] xfrm: use gre key as flow upper protocol info

[Michał Mirosław]

2010/11/3 Timo Teräs <timo.teras@iki.fi>:
> The GRE Key field is intended to be used for identifying an individual
> traffic flow within a tunnel. It is useful to be able to have XFRM
> policy selector matches to have different policies for different
> GRE tunnels.
[...]
> diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
> index 4464f3b..57af4bd 100644
> --- a/net/ipv4/xfrm4_policy.c
> +++ b/net/ipv4/xfrm4_policy.c
> @@ -158,6 +159,20 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
>                                fl->fl_ipsec_spi = htonl(ntohs(ipcomp_hdr[1]));
>                        }
>                        break;
> +
> +               case IPPROTO_GRE:
> +                       if (pskb_may_pull(skb, xprth + 12 - skb->data)) {
> +                               __be16 *greflags = (__be16 *)xprth;
> +                               __be32 *gre_hdr = (__be32 *)xprth;
> +
> +                               if (greflags[0] & GRE_KEY) {
> +                                       if (greflags[0] & GRE_CSUM)
> +                                               gre_hdr++;
> +                                       fl->fl_gre_key = gre_hdr[1];
> +                               }
> +                       }
> +                       break;
> +
>                default:
>                        fl->fl_ipsec_spi = 0;
>                        break;

I would expect that keyless tunnel would be separate from key 0 tunnel.

Best Regards,
Michał Mirosław

Re: [PATCH] xfrm: use gre key as flow upper protocol info

[Jesse Gross]

2010/11/3 Michał Mirosław <mirqus@gmail.com>:
> 2010/11/3 Timo Teräs <timo.teras@iki.fi>:
>> The GRE Key field is intended to be used for identifying an individual
>> traffic flow within a tunnel. It is useful to be able to have XFRM
>> policy selector matches to have different policies for different
>> GRE tunnels.
> [...]
>> diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
>> index 4464f3b..57af4bd 100644
>> --- a/net/ipv4/xfrm4_policy.c
>> +++ b/net/ipv4/xfrm4_policy.c
>> @@ -158,6 +159,20 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
>>                                fl->fl_ipsec_spi = htonl(ntohs(ipcomp_hdr[1]));
>>                        }
>>                        break;
>> +
>> +               case IPPROTO_GRE:
>> +                       if (pskb_may_pull(skb, xprth + 12 - skb->data)) {
>> +                               __be16 *greflags = (__be16 *)xprth;
>> +                               __be32 *gre_hdr = (__be32 *)xprth;
>> +
>> +                               if (greflags[0] & GRE_KEY) {
>> +                                       if (greflags[0] & GRE_CSUM)
>> +                                               gre_hdr++;
>> +                                       fl->fl_gre_key = gre_hdr[1];
>> +                               }
>> +                       }
>> +                       break;
>> +
>>                default:
>>                        fl->fl_ipsec_spi = 0;
>>                        break;
>
> I would expect that keyless tunnel would be separate from key 0 tunnel.

No key and key 0 are generally treated the same.  Both will match the
same tunnel when doing the lookup in the GRE receive path.

Re: [PATCH] xfrm: use gre key as flow upper protocol info

[Michał Mirosław]

W dniu 3 listopada 2010 22:35 użytkownik Jesse Gross <jesse@nicira.com> napisał:
> 2010/11/3 Michał Mirosław <mirqus@gmail.com>:
>> 2010/11/3 Timo Teräs <timo.teras@iki.fi>:
>>> The GRE Key field is intended to be used for identifying an individual
>>> traffic flow within a tunnel. It is useful to be able to have XFRM
>>> policy selector matches to have different policies for different
>>> GRE tunnels.
>> [...]
>>> diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
>>> index 4464f3b..57af4bd 100644
>>> --- a/net/ipv4/xfrm4_policy.c
>>> +++ b/net/ipv4/xfrm4_policy.c
>>> @@ -158,6 +159,20 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
>>>                                fl->fl_ipsec_spi = htonl(ntohs(ipcomp_hdr[1]));
>>>                        }
>>>                        break;
>>> +
>>> +               case IPPROTO_GRE:
>>> +                       if (pskb_may_pull(skb, xprth + 12 - skb->data)) {
>>> +                               __be16 *greflags = (__be16 *)xprth;
>>> +                               __be32 *gre_hdr = (__be32 *)xprth;
>>> +
>>> +                               if (greflags[0] & GRE_KEY) {
>>> +                                       if (greflags[0] & GRE_CSUM)
>>> +                                               gre_hdr++;
>>> +                                       fl->fl_gre_key = gre_hdr[1];
>>> +                               }
>>> +                       }
>>> +                       break;
>>> +
>>>                default:
>>>                        fl->fl_ipsec_spi = 0;
>>>                        break;
>> I would expect that keyless tunnel would be separate from key 0 tunnel.
> No key and key 0 are generally treated the same.  Both will match the
> same tunnel when doing the lookup in the GRE receive path.

I read the code again, and indeed it is. I was sure that they were
treated separately some time ago, but git knows nothing about it.

Best Regards,
Michał Mirosław

Re: [PATCH] xfrm: use gre key as flow upper protocol info

[David Miller]

From: Timo Teräs <timo.teras@iki.fi>
Date: Wed,  3 Nov 2010 16:41:38 +0200

> The GRE Key field is intended to be used for identifying an individual
> traffic flow within a tunnel. It is useful to be able to have XFRM
> policy selector matches to have different policies for different
> GRE tunnels.
> 
> Signed-off-by: Timo Teräs <timo.teras@iki.fi>

I'll apply this to net-next-2.6, thanks.

Re: [PATCH] xfrm: use gre key as flow upper protocol info

[Timo Teräs]

On 11/15/2010 08:43 PM, David Miller wrote:
> From: Timo Teräs <timo.teras@iki.fi>
> Date: Wed,  3 Nov 2010 16:41:38 +0200
> 
>> The GRE Key field is intended to be used for identifying an individual
>> traffic flow within a tunnel. It is useful to be able to have XFRM
>> policy selector matches to have different policies for different
>> GRE tunnels.
>>
>> Signed-off-by: Timo Teräs <timo.teras@iki.fi>
> 
> I'll apply this to net-next-2.6, thanks.

Hmm.. I tested this with using the "ip xfrm" sport and dport manually
(without doing the actual userland support for this), and checking it in
kernel with printk's in various places that the stuff matches. In these
tests I checked the sport/dport by hand and apparently messed up the
byte order.

Now that I'm writing the GRE support for "ip xfrm" I think that missed
two htons() calls.

I was confused if xfrm_flowi_{s|d}port was supposed to return host or
net byte order for non-TCP/UDP packets.

I was under the assumption that host byte order since case IPPROTO_ICMP
swaps the byte order. But it would appear that the fl->fl_icmp_* is
actually host order and it's turned to network order; this is also
implied by using htons instead of ntohs. Since I decided to keep
fl_gre_key in network order, the return value would now be inconsistent,
and make userland abi endianess dependent.

I'll follow up with iproute2 patch soon.

So we probably would need to do:

xfrm: fix gre key endianess

fl->fl_gre_key is network byte order contrary to fl->fl_icmp_*.
Make xfrm_flowi_{s|d}port return network byte order values for gre
key too.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 1a57ff9..916ac47 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -806,7 +806,7 @@ __be16 xfrm_flowi_sport(struct flowi *fl)
 		port = htons(fl->fl_mh_type);
 		break;
 	case IPPROTO_GRE:
-		port = htonl(fl->fl_gre_key) >> 16;
+		port = htons(ntohl(fl->fl_gre_key) >> 16);
 		break;
 	default:
 		port = 0;	/*XXX*/
@@ -830,7 +830,7 @@ __be16 xfrm_flowi_dport(struct flowi *fl)
 		port = htons(fl->fl_icmp_code);
 		break;
 	case IPPROTO_GRE:
-		port = htonl(fl->fl_gre_key) & 0xffff;
+		port = htons(ntohl(fl->fl_gre_key) & 0xffff);
 		break;
 	default:
 		port = 0;	/*XXX*/

Re: [PATCH] xfrm: use gre key as flow upper protocol info

[David Miller]

From: Timo Teräs <timo.teras@iki.fi>
Date: Tue, 23 Nov 2010 16:03:45 +0200

> So we probably would need to do:
> 
> xfrm: fix gre key endianess
> 
> fl->fl_gre_key is network byte order contrary to fl->fl_icmp_*.
> Make xfrm_flowi_{s|d}port return network byte order values for gre
> key too.
> 
> Signed-off-by: Timo Teräs <timo.teras@iki.fi>

Ok, applied, thanks Timo.