[PATCH] iproute2: support xfrm upper protocol gre key

[PATCH] iproute2: support xfrm upper protocol gre key

[Timo Teräs]

The gre key handling is consistent with ip tunnel side: both
dotted-quad and number are accepted, but dotted-quad is used
for printing.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
---
This is the userland part for:
  http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commitdiff;h=cc9ff19da9bf76a2f70bcb80225a1c587c162e52

However, that commit is flawed, and for this patch to work
properly, the following patch is needed:
  http://patchwork.ozlabs.org/patch/72668/

 ip/ipxfrm.c      |   42 ++++++++++++++++++++++++++++++++++++++++++
 ip/xfrm_policy.c |    3 ++-
 man/man8/ip.8    |   25 ++++++++++++++++---------
 3 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index 99a6756..cf4b7c7 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -35,6 +35,7 @@
 #include <linux/netlink.h>
 #include <linux/rtnetlink.h>
 #include <linux/xfrm.h>
+#include <arpa/inet.h>
 
 #include "utils.h"
 #include "xfrm.h"
@@ -483,6 +484,14 @@ void xfrm_selector_print(struct xfrm_selector *sel, __u16 family,
 		if (sel->dport_mask)
 			fprintf(fp, "code %u ", ntohs(sel->dport));
 		break;
+	case IPPROTO_GRE:
+		if (sel->sport_mask || sel->dport_mask) {
+			struct in_addr key;
+			key.s_addr = htonl((ntohs(sel->sport) << 16) + ntohs(sel->dport));
+			inet_ntop(AF_INET, &key, abuf, sizeof(abuf));
+			fprintf(fp, "key %s ", abuf);
+		}
+		break;
 	case IPPROTO_MH:
 		if (sel->sport_mask)
 			fprintf(fp, "type %u ", ntohs(sel->sport));
@@ -1086,6 +1095,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 	char *dportp = NULL;
 	char *typep = NULL;
 	char *codep = NULL;
+	char *grekey = NULL;
 
 	while (1) {
 		if (strcmp(*argv, "proto") == 0) {
@@ -1162,6 +1172,29 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 
 			filter.upspec_dport_mask = XFRM_FILTER_MASK_FULL;
 
+		} else if (strcmp(*argv, "key") == 0) {
+			unsigned key;
+
+			grekey = *argv;
+
+			NEXT_ARG();
+
+			if (strchr(*argv, '.'))
+				key = htonl(get_addr32(*argv));
+			else {
+				if (get_unsigned(&key, *argv, 0)<0) {
+					fprintf(stderr, "invalid value of \"key\"\n");
+					exit(-1);
+				}
+			}
+
+			sel->sport = htons(key >> 16);
+			sel->dport = htons(key & 0xffff);
+			sel->sport_mask = ~((__u16)0);
+			sel->dport_mask = ~((__u16)0);
+
+			filter.upspec_dport_mask = XFRM_FILTER_MASK_FULL;
+
 		} else {
 			PREV_ARG(); /* back track */
 			break;
@@ -1196,6 +1229,15 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 			exit(1);
 		}
 	}
+	if (grekey) {
+		switch (sel->proto) {
+		case IPPROTO_GRE:
+			break;
+		default:
+			fprintf(stderr, "\"key\" is invalid with proto=%s\n", strxf_proto(sel->proto));
+			exit(1);
+		}
+	}
 
 	*argcp = argc;
 	*argvp = argv;
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index 121afa1..dcb3da4 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -66,7 +66,8 @@ static void usage(void)
 	fprintf(stderr, "SELECTOR := src ADDR[/PLEN] dst ADDR[/PLEN] [ UPSPEC ] [ dev DEV ]\n");
 
 	fprintf(stderr, "UPSPEC := proto PROTO [ [ sport PORT ] [ dport PORT ] |\n");
-	fprintf(stderr, "                        [ type NUMBER ] [ code NUMBER ] ]\n");
+	fprintf(stderr, "                        [ type NUMBER ] [ code NUMBER ] |\n");
+	fprintf(stderr, "                        [ key { DOTTED_QUAD | NUMBER } ] ]\n");
 
 	//fprintf(stderr, "DEV - device name(default=none)\n");
 
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 1a73efa..c1e03f3 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -547,7 +547,10 @@ throw " | " unreachable " | " prohibit " | " blackhole " | " nat " ]"
 .RB " [ " type
 .IR NUMBER " ] "
 .RB " [ " code
-.IR NUMBER " ]] "
+.IR NUMBER " ] | "
+.br
+.RB " [ " key
+.IR KEY " ]] "
 
 .ti -8
 .IR LIMIT-LIST " := [ " LIMIT-LIST " ] |"
@@ -642,7 +645,10 @@ throw " | " unreachable " | " prohibit " | " blackhole " | " nat " ]"
 .RB " [ " type
 .IR NUMBER " ] "
 .RB " [ " code
-.IR NUMBER " ] ] "
+.IR NUMBER " ] | "
+.br
+.RB " [ " key
+.IR KEY " ] ] "
 
 .ti -8
 .IR ACTION " := "
@@ -2487,9 +2493,11 @@ is defined by source port
 .BR sport ", "
 destination port
 .BR dport ", " type
-as number and
+as number,
 .B code
-also number.
+also number and
+.BR key
+as dotted-quad or number.
 
 .TP
 .BI dev " DEV "
@@ -2556,11 +2564,10 @@ and the other choice is
 .TP
 .IR UPSPEC
 is specified by
-.BR sport ", "
-.BR dport ", " type
-and
-.B code
-(NUMBER).
+.BR sport " and " dport " (for UDP/TCP), "
+.BR type " and " code " (for ICMP; as number) or "
+.BR key " (for GRE; as dotted-quad or number)."
+.
 
 .SS ip xfrm monitor - is used for listing all objects or defined group of them.
 The
-- 
1.7.1

Re: [PATCH] iproute2: support xfrm upper protocol gre key

[Stephen Hemminger]

On Tue, 23 Nov 2010 17:02:39 +0200
Timo Teräs <timo.teras@iki.fi> wrote:

> +	case IPPROTO_GRE:
> +		if (sel->sport_mask || sel->dport_mask) {
> +			struct in_addr key;
> +			key.s_addr = htonl((ntohs(sel->sport) << 16) + ntohs(sel->dport));
> +			inet_ntop(AF_INET, &key, abuf, sizeof(abuf));
> +			fprintf(fp, "key %s ", abuf);
> +		}

The GRE key is not really an IPv4 address. Why should the utilities
use IPv4 address manipulation to format/scan it.  It makes more sense
to me to just use u32 an do the necessary ntohl.


-- 

Re: [PATCH] iproute2: support xfrm upper protocol gre key

[Timo Teräs]

On 11/23/2010 06:24 PM, Stephen Hemminger wrote:
> On Tue, 23 Nov 2010 17:02:39 +0200
> Timo Teräs <timo.teras@iki.fi> wrote:
> 
>> +	case IPPROTO_GRE:
>> +		if (sel->sport_mask || sel->dport_mask) {
>> +			struct in_addr key;
>> +			key.s_addr = htonl((ntohs(sel->sport) << 16) + ntohs(sel->dport));
>> +			inet_ntop(AF_INET, &key, abuf, sizeof(abuf));
>> +			fprintf(fp, "key %s ", abuf);
>> +		}
> 
> The GRE key is not really an IPv4 address. Why should the utilities
> use IPv4 address manipulation to format/scan it.  It makes more sense
> to me to just use u32 an do the necessary ntohl.

This is pretty much how iptunnel.c does it, so I copied the code. Would
you prefer to format it as single u32 number? Or use something else for
formatting it similar to IPv4?

In either case, we should change iptunnel.c to match ipxfrm.c. It'll be
easier if both parts handling the gre key treat it equivalently.

I think Cisco does indeed treat it as u32 number in the configurations.
So I'm okay updating this patch, and fixing iptunnel.c side too. We
might still want to keep the parsing of ipv4 format to keep backwards
compatibility.

Re: [PATCH] iproute2: support xfrm upper protocol gre key

[Stephen Hemminger]

On Tue, 23 Nov 2010 18:44:44 +0200
Timo Teräs <timo.teras@iki.fi> wrote:

> On 11/23/2010 06:24 PM, Stephen Hemminger wrote:
> > On Tue, 23 Nov 2010 17:02:39 +0200
> > Timo Teräs <timo.teras@iki.fi> wrote:
> > 
> >> +	case IPPROTO_GRE:
> >> +		if (sel->sport_mask || sel->dport_mask) {
> >> +			struct in_addr key;
> >> +			key.s_addr = htonl((ntohs(sel->sport) << 16) + ntohs(sel->dport));
> >> +			inet_ntop(AF_INET, &key, abuf, sizeof(abuf));
> >> +			fprintf(fp, "key %s ", abuf);
> >> +		}
> > 
> > The GRE key is not really an IPv4 address. Why should the utilities
> > use IPv4 address manipulation to format/scan it.  It makes more sense
> > to me to just use u32 an do the necessary ntohl.
> 
> This is pretty much how iptunnel.c does it, so I copied the code. Would
> you prefer to format it as single u32 number? Or use something else for
> formatting it similar to IPv4?
> 
> In either case, we should change iptunnel.c to match ipxfrm.c. It'll be
> easier if both parts handling the gre key treat it equivalently.
> 
> I think Cisco does indeed treat it as u32 number in the configurations.
> So I'm okay updating this patch, and fixing iptunnel.c side too. We
> might still want to keep the parsing of ipv4 format to keep backwards
> compatibility.

My preference would be to take both dotted quad and a single
number.


-- 

Re: [PATCH] iproute2: support xfrm upper protocol gre key

[Timo Teräs]

On 11/23/2010 08:13 PM, Stephen Hemminger wrote:
> On Tue, 23 Nov 2010 18:44:44 +0200
> Timo Teräs <timo.teras@iki.fi> wrote:
>> On 11/23/2010 06:24 PM, Stephen Hemminger wrote:
>>> The GRE key is not really an IPv4 address. Why should the utilities
>>> use IPv4 address manipulation to format/scan it.  It makes more sense
>>> to me to just use u32 an do the necessary ntohl.
>>
>> This is pretty much how iptunnel.c does it, so I copied the code. Would
>> you prefer to format it as single u32 number? Or use something else for
>> formatting it similar to IPv4?
>>
>> In either case, we should change iptunnel.c to match ipxfrm.c. It'll be
>> easier if both parts handling the gre key treat it equivalently.
>>
>> I think Cisco does indeed treat it as u32 number in the configurations.
>> So I'm okay updating this patch, and fixing iptunnel.c side too. We
>> might still want to keep the parsing of ipv4 format to keep backwards
>> compatibility.
> 
> My preference would be to take both dotted quad and a single
> number.

And I assume that when dumping stuff, it should be output as single
number. Or make that configurable with some switch?

I'll refresh my patch and fix iptunnel.c output tomorrow.

Re: [PATCH] iproute2: support xfrm upper protocol gre key

[Stephen Hemminger]

On Tue, 23 Nov 2010 20:38:58 +0200
Timo Teräs <timo.teras@iki.fi> wrote:

> On 11/23/2010 08:13 PM, Stephen Hemminger wrote:
> > On Tue, 23 Nov 2010 18:44:44 +0200
> > Timo Teräs <timo.teras@iki.fi> wrote:
> >> On 11/23/2010 06:24 PM, Stephen Hemminger wrote:
> >>> The GRE key is not really an IPv4 address. Why should the utilities
> >>> use IPv4 address manipulation to format/scan it.  It makes more sense
> >>> to me to just use u32 an do the necessary ntohl.
> >>
> >> This is pretty much how iptunnel.c does it, so I copied the code. Would
> >> you prefer to format it as single u32 number? Or use something else for
> >> formatting it similar to IPv4?
> >>
> >> In either case, we should change iptunnel.c to match ipxfrm.c. It'll be
> >> easier if both parts handling the gre key treat it equivalently.
> >>
> >> I think Cisco does indeed treat it as u32 number in the configurations.
> >> So I'm okay updating this patch, and fixing iptunnel.c side too. We
> >> might still want to keep the parsing of ipv4 format to keep backwards
> >> compatibility.
> > 
> > My preference would be to take both dotted quad and a single
> > number.
> 
> And I assume that when dumping stuff, it should be output as single
> number. Or make that configurable with some switch?
> 
> I'll refresh my patch and fix iptunnel.c output tomorrow.

Just show it as a decimal number. That is what Cisco and Juniper do.

-- 

Re: [PATCH] iproute2: support xfrm upper protocol gre key

[Ben Hutchings]

On Tue, 2010-11-23 at 10:13 -0800, Stephen Hemminger wrote:
> On Tue, 23 Nov 2010 18:44:44 +0200
> Timo Teräs <timo.teras@iki.fi> wrote:
> 
> > On 11/23/2010 06:24 PM, Stephen Hemminger wrote:
> > > On Tue, 23 Nov 2010 17:02:39 +0200
> > > Timo Teräs <timo.teras@iki.fi> wrote:
> > > 
> > >> +	case IPPROTO_GRE:
> > >> +		if (sel->sport_mask || sel->dport_mask) {
> > >> +			struct in_addr key;
> > >> +			key.s_addr = htonl((ntohs(sel->sport) << 16) + ntohs(sel->dport));
> > >> +			inet_ntop(AF_INET, &key, abuf, sizeof(abuf));
> > >> +			fprintf(fp, "key %s ", abuf);
> > >> +		}
> > > 
> > > The GRE key is not really an IPv4 address. Why should the utilities
> > > use IPv4 address manipulation to format/scan it.  It makes more sense
> > > to me to just use u32 an do the necessary ntohl.
> > 
> > This is pretty much how iptunnel.c does it, so I copied the code. Would
> > you prefer to format it as single u32 number? Or use something else for
> > formatting it similar to IPv4?
> > 
> > In either case, we should change iptunnel.c to match ipxfrm.c. It'll be
> > easier if both parts handling the gre key treat it equivalently.
> > 
> > I think Cisco does indeed treat it as u32 number in the configurations.
> > So I'm okay updating this patch, and fixing iptunnel.c side too. We
> > might still want to keep the parsing of ipv4 format to keep backwards
> > compatibility.
> 
> My preference would be to take both dotted quad and a single
> number.

inet_aton() covers that.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH 1/2] iproute2: treat gre key as number

[Timo Teräs]

Print GRE key as a regular number. It is not really an IPv4 address
and this is also how Cisco and Juniper treats GRE keys. Do keep the
parsing of dotted-quad format for backwards compatibility.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
---
 ip/iptunnel.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 3525fbb..48faf69 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -306,12 +306,8 @@ static void print_tunnel(struct ip_tunnel_parm *p)
 	struct ip_tunnel_6rd ip6rd;
 	char s1[1024];
 	char s2[1024];
-	char s3[64];
-	char s4[64];
 
 	memset(&ip6rd, 0, sizeof(ip6rd));
-	inet_ntop(AF_INET, &p->i_key, s3, sizeof(s3));
-	inet_ntop(AF_INET, &p->o_key, s4, sizeof(s4));
 
 	/* Do not use format_host() for local addr,
 	 * symbolic name will not be useful.
@@ -377,12 +373,12 @@ static void print_tunnel(struct ip_tunnel_parm *p)
 	}
 
 	if ((p->i_flags&GRE_KEY) && (p->o_flags&GRE_KEY) && p->o_key == p->i_key)
-		printf(" key %s", s3);
+		printf(" key %u", ntohl(p->i_key));
 	else if ((p->i_flags|p->o_flags)&GRE_KEY) {
 		if (p->i_flags&GRE_KEY)
-			printf(" ikey %s ", s3);
+			printf(" ikey %u ", ntohl(p->i_key));
 		if (p->o_flags&GRE_KEY)
-			printf(" okey %s ", s4);
+			printf(" okey %u ", ntohl(p->o_key));
 	}
 
 	if (p->i_flags&GRE_SEQ)
-- 
1.7.1

[PATCH 2/2] iproute2: support xfrm upper protocol gre key

[Timo Teräs]

Similar to tunnel side: accept dotted-quad and number formats.
Use regular number for printing the key.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
---
I decided to keep using get_addr32() and get_unsigned() since uclibc
inet_aton() does not accept all formats.

 ip/ipxfrm.c      |   39 +++++++++++++++++++++++++++++++++++++++
 ip/xfrm_policy.c |    3 ++-
 man/man8/ip.8    |   25 ++++++++++++++++---------
 3 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index 99a6756..9753822 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -483,6 +483,12 @@ void xfrm_selector_print(struct xfrm_selector *sel, __u16 family,
 		if (sel->dport_mask)
 			fprintf(fp, "code %u ", ntohs(sel->dport));
 		break;
+	case IPPROTO_GRE:
+		if (sel->sport_mask || sel->dport_mask)
+			fprintf(fp, "key %u ",
+				(((__u32)ntohs(sel->sport)) << 16) +
+				ntohs(sel->dport));
+		break;
 	case IPPROTO_MH:
 		if (sel->sport_mask)
 			fprintf(fp, "type %u ", ntohs(sel->sport));
@@ -1086,6 +1092,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 	char *dportp = NULL;
 	char *typep = NULL;
 	char *codep = NULL;
+	char *grekey = NULL;
 
 	while (1) {
 		if (strcmp(*argv, "proto") == 0) {
@@ -1162,6 +1169,29 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 
 			filter.upspec_dport_mask = XFRM_FILTER_MASK_FULL;
 
+		} else if (strcmp(*argv, "key") == 0) {
+			unsigned uval;
+
+			grekey = *argv;
+
+			NEXT_ARG();
+
+			if (strchr(*argv, '.'))
+				uval = htonl(get_addr32(*argv));
+			else {
+				if (get_unsigned(&uval, *argv, 0)<0) {
+					fprintf(stderr, "invalid value of \"key\"\n");
+					exit(-1);
+				}
+			}
+
+			sel->sport = htons(uval >> 16);
+			sel->dport = htons(uval & 0xffff);
+			sel->sport_mask = ~((__u16)0);
+			sel->dport_mask = ~((__u16)0);
+
+			filter.upspec_dport_mask = XFRM_FILTER_MASK_FULL;
+
 		} else {
 			PREV_ARG(); /* back track */
 			break;
@@ -1196,6 +1226,15 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 			exit(1);
 		}
 	}
+	if (grekey) {
+		switch (sel->proto) {
+		case IPPROTO_GRE:
+			break;
+		default:
+			fprintf(stderr, "\"key\" is invalid with proto=%s\n", strxf_proto(sel->proto));
+			exit(1);
+		}
+	}
 
 	*argcp = argc;
 	*argvp = argv;
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index 121afa1..dcb3da4 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -66,7 +66,8 @@ static void usage(void)
 	fprintf(stderr, "SELECTOR := src ADDR[/PLEN] dst ADDR[/PLEN] [ UPSPEC ] [ dev DEV ]\n");
 
 	fprintf(stderr, "UPSPEC := proto PROTO [ [ sport PORT ] [ dport PORT ] |\n");
-	fprintf(stderr, "                        [ type NUMBER ] [ code NUMBER ] ]\n");
+	fprintf(stderr, "                        [ type NUMBER ] [ code NUMBER ] |\n");
+	fprintf(stderr, "                        [ key { DOTTED_QUAD | NUMBER } ] ]\n");
 
 	//fprintf(stderr, "DEV - device name(default=none)\n");
 
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 1a73efa..c1e03f3 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -547,7 +547,10 @@ throw " | " unreachable " | " prohibit " | " blackhole " | " nat " ]"
 .RB " [ " type
 .IR NUMBER " ] "
 .RB " [ " code
-.IR NUMBER " ]] "
+.IR NUMBER " ] | "
+.br
+.RB " [ " key
+.IR KEY " ]] "
 
 .ti -8
 .IR LIMIT-LIST " := [ " LIMIT-LIST " ] |"
@@ -642,7 +645,10 @@ throw " | " unreachable " | " prohibit " | " blackhole " | " nat " ]"
 .RB " [ " type
 .IR NUMBER " ] "
 .RB " [ " code
-.IR NUMBER " ] ] "
+.IR NUMBER " ] | "
+.br
+.RB " [ " key
+.IR KEY " ] ] "
 
 .ti -8
 .IR ACTION " := "
@@ -2487,9 +2493,11 @@ is defined by source port
 .BR sport ", "
 destination port
 .BR dport ", " type
-as number and
+as number,
 .B code
-also number.
+also number and
+.BR key
+as dotted-quad or number.
 
 .TP
 .BI dev " DEV "
@@ -2556,11 +2564,10 @@ and the other choice is
 .TP
 .IR UPSPEC
 is specified by
-.BR sport ", "
-.BR dport ", " type
-and
-.B code
-(NUMBER).
+.BR sport " and " dport " (for UDP/TCP), "
+.BR type " and " code " (for ICMP; as number) or "
+.BR key " (for GRE; as dotted-quad or number)."
+.
 
 .SS ip xfrm monitor - is used for listing all objects or defined group of them.
 The
-- 
1.7.1

Re: [PATCH 2/2] iproute2: support xfrm upper protocol gre key

[Stephen Hemminger]

On Wed, 24 Nov 2010 10:18:58 +0200
Timo Teräs <timo.teras@iki.fi> wrote:

> Similar to tunnel side: accept dotted-quad and number formats.
> Use regular number for printing the key.
> 
> Signed-off-by: Timo Teräs <timo.teras@iki.fi>

Both applied.

--