[PATCH] ip_gre: When TOS is inherited, use configured TOS value for non-IP packets

[PATCH] ip_gre: When TOS is inherited, use configured TOS value for non-IP packets

[David Ward]

A GRE tunnel can be configured so that outgoing tunnel packets inherit
the value of the TOS field from the inner IP header. In doing so, when
a non-IP packet is transmitted through the tunnel, the TOS field will
always be set to 0.

Instead, the user should be able to configure a different TOS value as
the fallback to use for non-IP packets. This is helpful when the non-IP
packets are all control packets and should be handled by routers outside
the tunnel as having Internet Control precedence. One example of this is
the NHRP packets that control a DMVPN-compatible mGRE tunnel; they are
encapsulated directly by GRE and do not contain an inner IP header.

Under the existing behavior, the IFLA_GRE_TOS parameter must be set to
'1' for the TOS value to be inherited. Now, only the least significant
bit of this parameter must be set to '1', and when a non-IP packet is
sent through the tunnel, the upper 6 bits of this same parameter will be
copied into the TOS field. (The ECN bits get masked off as before.)

This behavior is backwards-compatible with existing configurations and
iproute2 versions.

Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
 net/ipv4/ip_gre.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 303012a..e4c8817 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -818,8 +818,8 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 
 	ttl = tiph->ttl;
 	tos = tiph->tos;
-	if (tos == 1) {
-		tos = 0;
+	if (tos & 0x1) {
+		tos &= ~0x1;
 		if (skb->protocol == htons(ETH_P_IP))
 			tos = old_iph->tos;
 		else if (skb->protocol == htons(ETH_P_IPV6))
-- 
1.7.1

[PATCH iproute2] ip/iptunnel: Extend TOS syntax

[David Ward]

The 'inherit/STRING' or 'inherit/00..ff' syntax indicates that the
TOS field of tunneled packets should be copied from the original IP
header, but for non-IP packets the value STRING or 00..ff should be
used instead. (This syntax is already used by 'ip tunnel show'.)

Also clarify the man page and the command usage text (particularly
that the TOS is not specified as a decimal number).

Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
 ip/iptunnel.c        |   15 +++++++++++----
 man/man8/ip-tunnel.8 |   34 +++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 0cf6cf8..f8b91ba 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -41,7 +41,7 @@ static void usage(void)
 	fprintf(stderr, "\n");
 	fprintf(stderr, "Where: NAME := STRING\n");
 	fprintf(stderr, "       ADDR := { IP_ADDRESS | any }\n");
-	fprintf(stderr, "       TOS  := { NUMBER | inherit }\n");
+	fprintf(stderr, "       TOS  := { STRING | 00..ff | inherit | inherit/STRING | inherit/00..ff }\n");
 	fprintf(stderr, "       TTL  := { 1..255 | inherit }\n");
 	fprintf(stderr, "       KEY  := { DOTTED_QUAD | NUMBER }\n");
 	exit(-1);
@@ -188,14 +188,21 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 		} else if (strcmp(*argv, "tos") == 0 ||
 			   strcmp(*argv, "tclass") == 0 ||
 			   matches(*argv, "dsfield") == 0) {
+			char *dsfield;
 			__u32 uval;
 			NEXT_ARG();
+			dsfield = *argv;
+			strsep(&dsfield, "/");
 			if (strcmp(*argv, "inherit") != 0) {
-				if (rtnl_dsfield_a2n(&uval, *argv))
-					invarg("bad TOS value", *argv);
-				p->iph.tos = uval;
+				dsfield = *argv;
+				p->iph.tos = 0;
 			} else
 				p->iph.tos = 1;
+			if (dsfield) {
+				if (rtnl_dsfield_a2n(&uval, dsfield))
+					invarg("bad TOS value", *argv);
+				p->iph.tos |= uval;
+			}
 		} else {
 			if (strcmp(*argv, "name") == 0) {
 				NEXT_ARG();
diff --git a/man/man8/ip-tunnel.8 b/man/man8/ip-tunnel.8
index 37ba542..b408517 100644
--- a/man/man8/ip-tunnel.8
+++ b/man/man8/ip-tunnel.8
@@ -47,7 +47,6 @@ ip-tunnel - tunnel configuration
 .RB "[ [" no "]" pmtudisc " ]"
 .RB "[ " dev
 .IR PHYS_DEV " ]"
-.RB "[ " "dscp inherit" " ]"
 
 .ti -8
 .IR MODE " := "
@@ -58,8 +57,12 @@ ip-tunnel - tunnel configuration
 .BR any " }"
 
 .ti -8
-.IR TOS " := { " NUMBER " |"
-.BR inherit " }"
+.IR TOS " := { " STRING " | " 00 ".." ff " |"
+.BR inherit " |"
+.BI "inherit/" STRING
+.R " |"
+.BI "inherit/" 00 ".." ff
+.R " }"
 
 .ti -8
 .IR ELIM " := {"
@@ -132,11 +135,21 @@ The default value for IPv6 tunnels is:
 .BI dsfield " T"
 .TP
 .BI tclass " T"
-set a fixed TOS (or traffic class in IPv6)
-.I T
-on tunneled packets.
-The default value is:
-.BR "inherit" .
+set the type of service (IPv4) or traffic class (IPv6) field on
+tunneled packets, which can be specified as either a two-digit
+hex value (e.g. c0) or a predefined string (e.g. internet).
+The value
+.B inherit
+causes the field to be copied from the original IP header. The
+values
+.BI "inherit/" STRING
+or
+.BI "inherit/" 00 ".." ff
+will set the field to
+.I STRING
+or
+.IR 00 ".." ff
+when tunneling non-IP packets. The default value is 00.
 
 .TP
 .BI dev " NAME"
@@ -202,11 +215,6 @@ flag is equivalent to the combination
 .B It isn't work. Don't use it.
 
 .TP
-.BR "dscp inherit"
-.RB ( " only IPv6 tunnels " )
-Inherit DS field between inner and outer header.
-
-.TP
 .BI encaplim " ELIM"
 .RB ( " only IPv6 tunnels " )
 set a fixed encapsulation limit.  Default is 4.
-- 
1.7.1

Re: [PATCH] ip_gre: When TOS is inherited, use configured TOS value for non-IP packets

[David Miller]

From: David Ward <david.ward@ll.mit.edu>
Date: Sun, 27 Jan 2013 18:04:58 -0500

> A GRE tunnel can be configured so that outgoing tunnel packets inherit
> the value of the TOS field from the inner IP header. In doing so, when
> a non-IP packet is transmitted through the tunnel, the TOS field will
> always be set to 0.
> 
> Instead, the user should be able to configure a different TOS value as
> the fallback to use for non-IP packets. This is helpful when the non-IP
> packets are all control packets and should be handled by routers outside
> the tunnel as having Internet Control precedence. One example of this is
> the NHRP packets that control a DMVPN-compatible mGRE tunnel; they are
> encapsulated directly by GRE and do not contain an inner IP header.
> 
> Under the existing behavior, the IFLA_GRE_TOS parameter must be set to
> '1' for the TOS value to be inherited. Now, only the least significant
> bit of this parameter must be set to '1', and when a non-IP packet is
> sent through the tunnel, the upper 6 bits of this same parameter will be
> copied into the TOS field. (The ECN bits get masked off as before.)
> 
> This behavior is backwards-compatible with existing configurations and
> iproute2 versions.
> 
> Signed-off-by: David Ward <david.ward@ll.mit.edu>

Seems reasonable, applied.  Thanks.

I worry though about the case where tiph comes from skb->data rather
than the tunnel parameter block, can you describe why this new behavior
is OK in that situation too.

Re: [PATCH] ip_gre: When TOS is inherited, use configured TOS value for non-IP packets

[Ward, David - 0663 - MITLL]

[-- Attachment #1: Type: text/plain, Size: 2134 bytes --]

On 01/29/2013 02:06 PM, David Miller wrote:
>
> From: David Ward <david.ward@ll.mit.edu>
> Date: Sun, 27 Jan 2013 18:04:58 -0500
>
> > A GRE tunnel can be configured so that outgoing tunnel packets inherit
> > the value of the TOS field from the inner IP header. In doing so, when
> > a non-IP packet is transmitted through the tunnel, the TOS field will
> > always be set to 0.
> >
> > Instead, the user should be able to configure a different TOS value as
> > the fallback to use for non-IP packets. This is helpful when the non-IP
> > packets are all control packets and should be handled by routers outside
> > the tunnel as having Internet Control precedence. One example of this is
> > the NHRP packets that control a DMVPN-compatible mGRE tunnel; they are
> > encapsulated directly by GRE and do not contain an inner IP header.
> >
> > Under the existing behavior, the IFLA_GRE_TOS parameter must be set to
> > '1' for the TOS value to be inherited. Now, only the least significant
> > bit of this parameter must be set to '1', and when a non-IP packet is
> > sent through the tunnel, the upper 6 bits of this same parameter will be
> > copied into the TOS field. (The ECN bits get masked off as before.)
> >
> > This behavior is backwards-compatible with existing configurations and
> > iproute2 versions.
> >
> > Signed-off-by: David Ward <david.ward@ll.mit.edu>
>
> Seems reasonable, applied.  Thanks.
>
> I worry though about the case where tiph comes from skb->data rather
> than the tunnel parameter block, can you describe why this new behavior
> is OK in that situation too.
>

Sorry for the late reply, I have not been well for the past few days.

The case you mentioned will occur when dev->header_ops has been set (to 
ipgre_header_ops).  In that case, ipgre_header() is called before 
ipgre_tunnel_xmit().  It pushes the outer IP header onto the SKB ahead 
of time, copying the contents from the IP header in the tunnel parameter 
block.

So even in this case, the TOS value that we check is taken from the 
tunnel parameter block, not the inner IP header.

David



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4571 bytes --]