[PATCH] [RFC] Support for wccp version 1 and 2 in ip_gre.c

Hello!

Some time ago I posted the following patch.
It indeed adds support for wccp version 1 and 2 decapsulation in
ip_gre.c. But there is an open question.
I surrounded all decapsulation stuff in if (1).
Which knob I should use instead of that 1 to make this change
acceptable into mainline kernel?

Thanks in advance.

diff -urN linux-2.6.6-1.435/net/ipv4/ip_gre.c linux-2.6.6-1.435a/net/ipv4/ip_gre.c

--- linux-2.6.6-1.435/net/ipv4/ip_gre.c	2004-05-10 06:32:54.000000000 +0400
+++ linux-2.6.6-1.435a/net/ipv4/ip_gre.c	2004-06-30 16:38:43.781838344 +0400

@@ -553,6 +553,8 @@
 	return INET_ECN_encapsulate(tos, inner);
 }
 
+#define ETH_P_WCCP 0x883E
+
 int ipgre_rcv(struct sk_buff *skb)
 {
 	struct iphdr *iph;

@@ -605,13 +607,21 @@
 	if ((tunnel = ipgre_tunnel_lookup(iph->saddr, iph->daddr, key)) != NULL) {
 		secpath_reset(skb);
 
+		skb->protocol = *(u16*)(h + 2);
+		if (1) {
+			if ((flags == 0) && (skb->protocol == __constant_htons(ETH_P_WCCP))) {
+				skb->protocol = __constant_htons(ETH_P_IP);
+				if ((*(h + offset) & 0xF0) != 0x40) 
+					offset += 4;
+			}
+		}
+
 		skb->mac.raw = skb->nh.raw;
 		skb->nh.raw = __pskb_pull(skb, offset);
 		memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
 		if (skb->ip_summed == CHECKSUM_HW)
 			skb->csum = csum_sub(skb->csum,
 					     csum_partial(skb->mac.raw, skb->nh.raw-skb->mac.raw, 0));
-		skb->protocol = *(u16*)(h + 2);
 		skb->pkt_type = PACKET_HOST;
 #ifdef CONFIG_NET_IPGRE_BROADCAST
 		if (MULTICAST(iph->daddr)) {

-- 
Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key
 This message represents the official view of the voices in my head

On Sat, 11 Sep 2004 23:41:08 +0400
Paul P Komkoff Jr [off-list ref] wrote:

Some time ago I posted the following patch.
It indeed adds support for wccp version 1 and 2 decapsulation in
ip_gre.c. But there is an open question.
I surrounded all decapsulation stuff in if (1).
Which knob I should use instead of that 1 to make this change
acceptable into mainline kernel?

What are the rules for IP_GRE in general for when
to apply this transformation?

Please do me a favor also, and redo your patch by putting
ETH_P_WCCP into include/linux/if_ether.h where it belongs.

Thanks.

Replying to David S. Miller:

What are the rules for IP_GRE in general for when
to apply this transformation?

As you can see, I am applying it unconditionally when fits. For most
cases, this will be OK.
There can be situations when this is not wanted (for example, when 
debugging something), so in general, tuning knob will be useful, but 
I just don't know where to add it, maybe tunnel->parms.i_flags ...

Please do me a favor also, and redo your patch by putting
ETH_P_WCCP into include/linux/if_ether.h where it belongs.

Done.

Thanks.

diff -urN /usr/src/linux-2.6.8-1.521/include/linux/if_ether.h linux-2.6.8-1.521wccp/include/linux/if_ether.h

--- /usr/src/linux-2.6.8-1.521/include/linux/if_ether.h	2004-08-14 09:37:15.000000000 +0400
+++ linux-2.6.8-1.521wccp/include/linux/if_ether.h	2004-09-13 08:51:02.707155288 +0400

@@ -59,6 +59,8 @@
 #define ETH_P_8021Q	0x8100          /* 802.1Q VLAN Extended Header  */
 #define ETH_P_IPX	0x8137		/* IPX over DIX			*/
 #define ETH_P_IPV6	0x86DD		/* IPv6 over bluebook		*/
+#define ETH_P_WCCP	0x883E		/* Web-cache coordination protocol
+					 * defined in draft-wilson-wrec-wccp-v2-00.txt */
 #define ETH_P_PPP_DISC	0x8863		/* PPPoE discovery messages     */
 #define ETH_P_PPP_SES	0x8864		/* PPPoE session messages	*/
 #define ETH_P_MPLS_UC	0x8847		/* MPLS Unicast traffic		*/

diff -urN /usr/src/linux-2.6.8-1.521/net/ipv4/ip_gre.c linux-2.6.8-1.521wccp/net/ipv4/ip_gre.c

--- /usr/src/linux-2.6.8-1.521/net/ipv4/ip_gre.c	2004-08-14 09:37:37.000000000 +0400
+++ linux-2.6.8-1.521wccp/net/ipv4/ip_gre.c	2004-09-13 09:02:33.293170256 +0400

@@ -605,13 +605,25 @@
 	if ((tunnel = ipgre_tunnel_lookup(iph->saddr, iph->daddr, key)) != NULL) {
 		secpath_reset(skb);
 
+		skb->protocol = *(u16*)(h + 2);
+		/* WCCP version 1 and 2 protocol decoding.
+		 * - Change protocol to IP
+		 * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
+		 */
+		if (1) {
+			if ((flags == 0) && (skb->protocol == __constant_htons(ETH_P_WCCP))) {
+				skb->protocol = __constant_htons(ETH_P_IP);
+				if ((*(h + offset) & 0xF0) != 0x40) 
+					offset += 4;
+			}
+		}
+
 		skb->mac.raw = skb->nh.raw;
 		skb->nh.raw = __pskb_pull(skb, offset);
 		memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
 		if (skb->ip_summed == CHECKSUM_HW)
 			skb->csum = csum_sub(skb->csum,
 					     csum_partial(skb->mac.raw, skb->nh.raw-skb->mac.raw, 0));
-		skb->protocol = *(u16*)(h + 2);
 		skb->pkt_type = PACKET_HOST;
 #ifdef CONFIG_NET_IPGRE_BROADCAST
 		if (MULTICAST(iph->daddr)) {

-- 
Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key
 This message represents the official view of the voices in my head

On Mon, 13 Sep 2004 09:17:06 +0400
Paul P Komkoff Jr [off-list ref] wrote:

Replying to David S. Miller:

quoted

What are the rules for IP_GRE in general for when
to apply this transformation?

As you can see, I am applying it unconditionally when fits. For most
cases, this will be OK.
There can be situations when this is not wanted (for example, when 
debugging something), so in general, tuning knob will be useful, but 
I just don't know where to add it, maybe tunnel->parms.i_flags ...

I don't think adding such a knob is necessary, but yes i_flags
would be the place to do it.

I will apply your patch with the "if(1)" simply removed.

Thanks.

At 09:19 AM 14/09/2004, David S. Miller wrote:

quoted

As you can see, I am applying it unconditionally when fits. For most
cases, this will be OK.
There can be situations when this is not wanted (for example, when
debugging something), so in general, tuning knob will be useful, but
I just don't know where to add it, maybe tunnel->parms.i_flags ...

I don't think adding such a knob is necessary, but yes i_flags
would be the place to do it.

I will apply your patch with the "if(1)" simply removed.

the logic is correct, but it may make sense to call the appropriate 
netfilter hook again with the "unwrapped" GRE packet, as otherwise 
packets-inside-GRE represent a possible security hole where one can inject 
packets externally and bypass firewall rules.


cheers,

lincoln.

Replying to Lincoln Dale:

the logic is correct, but it may make sense to call the appropriate 
netfilter hook again with the "unwrapped" GRE packet, as otherwise 
packets-inside-GRE represent a possible security hole where one can inject 
packets externally and bypass firewall rules.

From what I observe, netfilter hooks *are* called for unwrapped packets.

Either for usual IP packets  passed from GRE tunnel, or for demangled
wccp packets.

-- 
Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key
 This message represents the official view of the voices in my head

At 10:39 PM 14/09/2004, Paul P Komkoff Jr wrote:

Replying to Lincoln Dale:

quoted

the logic is correct, but it may make sense to call the appropriate
netfilter hook again with the "unwrapped" GRE packet, as otherwise
packets-inside-GRE represent a possible security hole where one can inject
packets externally and bypass firewall rules.

From what I observe, netfilter hooks *are* called for unwrapped packets.
Either for usual IP packets  passed from GRE tunnel, or for demangled
wccp packets.

you probably want to ensure that the order of netfilter events are:
   1. [packet comes in]
   2. netfilter INPUT
   3. [GRE decap]
   4. [addressed to us?]
       Yes => netfilter INPUT
       No => netfilter FORWARD

i don't think that both (2) and (4) are done.

also just a minor nit: not all WCCP needs to be GRE-encoded; on 
high-performance switch/router platforms, only a layer-2 rewrite of the dst 
MAC addr is used instead of a layer-3 GRE tunnel.  you may want the comment 
at line 609 to explicitly mention "WCCPv1 and WCCPv2 GRE Forwarding mode".


cheers,

lincoln.

Replying to Lincoln Dale:

quoted

From what I observe, netfilter hooks *are* called for unwrapped packets.
Either for usual IP packets  passed from GRE tunnel, or for demangled
wccp packets.

you probably want to ensure that the order of netfilter events are:

Basically all surroinding stuff was untouched. I just enabled further
processing (by kernel IP stack) of GRE-WCCP encapsulated packets.

  1. [packet comes in]
  2. netfilter INPUT
  3. [GRE decap]
  4. [addressed to us?]
      Yes => netfilter INPUT
      No => netfilter FORWARD

i don't think that both (2) and (4) are done.

I think (2) is done before ipgre_rcv, and (4) is done because after
decapsulation
                skb->dev = tunnel->dev;
                netif_rx(skb);
this packet reenters the building from matched gre interface.

also just a minor nit: not all WCCP needs to be GRE-encoded; on 
high-performance switch/router platforms, only a layer-2 rewrite of the dst 
MAC addr is used instead of a layer-3 GRE tunnel.  you may want the comment 
at line 609 to explicitly mention "WCCPv1 and WCCPv2 GRE Forwarding mode".

Yes I know this :) L2 case do not need any extra processing, but not
all routers support it. For example, my 4500 don't.

Here is updated patch with if(1) removed and comment changed.


diff -urN /usr/src/linux-2.6.8-1.521/include/linux/if_ether.h linux-2.6.8-1.521wccp/include/linux/if_ether.h

--- /usr/src/linux-2.6.8-1.521/include/linux/if_ether.h	2004-08-14 09:37:15.000000000 +0400
+++ linux-2.6.8-1.521wccp/include/linux/if_ether.h	2004-09-13 08:51:02.000000000 +0400

@@ -59,6 +59,8 @@
 #define ETH_P_8021Q	0x8100          /* 802.1Q VLAN Extended Header  */
 #define ETH_P_IPX	0x8137		/* IPX over DIX			*/
 #define ETH_P_IPV6	0x86DD		/* IPv6 over bluebook		*/
+#define ETH_P_WCCP	0x883E		/* Web-cache coordination protocol
+					 * defined in draft-wilson-wrec-wccp-v2-00.txt */
 #define ETH_P_PPP_DISC	0x8863		/* PPPoE discovery messages     */
 #define ETH_P_PPP_SES	0x8864		/* PPPoE session messages	*/
 #define ETH_P_MPLS_UC	0x8847		/* MPLS Unicast traffic		*/

diff -urN /usr/src/linux-2.6.8-1.521/net/ipv4/ip_gre.c linux-2.6.8-1.521wccp/net/ipv4/ip_gre.c

--- /usr/src/linux-2.6.8-1.521/net/ipv4/ip_gre.c	2004-08-14 09:37:37.000000000 +0400
+++ linux-2.6.8-1.521wccp/net/ipv4/ip_gre.c	2004-09-15 00:03:45.586135760 +0400

@@ -605,13 +605,23 @@
 	if ((tunnel = ipgre_tunnel_lookup(iph->saddr, iph->daddr, key)) != NULL) {
 		secpath_reset(skb);
 
+		skb->protocol = *(u16*)(h + 2);
+		/* WCCP version 1 and 2 (in GRE forwarding mode) protocol decoding.
+		 * - Change protocol to IP
+		 * - When dealing with WCCPv2, skip extra 4 bytes in GRE header
+		 */
+		if ((flags == 0) && (skb->protocol == __constant_htons(ETH_P_WCCP))) {
+			skb->protocol = __constant_htons(ETH_P_IP);
+			if ((*(h + offset) & 0xF0) != 0x40) 
+				offset += 4;
+		}
+
 		skb->mac.raw = skb->nh.raw;
 		skb->nh.raw = __pskb_pull(skb, offset);
 		memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
 		if (skb->ip_summed == CHECKSUM_HW)
 			skb->csum = csum_sub(skb->csum,
 					     csum_partial(skb->mac.raw, skb->nh.raw-skb->mac.raw, 0));
-		skb->protocol = *(u16*)(h + 2);
 		skb->pkt_type = PACKET_HOST;
 #ifdef CONFIG_NET_IPGRE_BROADCAST
 		if (MULTICAST(iph->daddr)) {

-- 
Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key
 This message represents the official view of the voices in my head

On Tue, 14 Sep 2004 18:48:20 +1000
Lincoln Dale [off-list ref] wrote:

the logic is correct, but it may make sense to call the appropriate 
netfilter hook again with the "unwrapped" GRE packet, as otherwise 
packets-inside-GRE represent a possible security hole where one can inject 
packets externally and bypass firewall rules.

This will occur when we push the packet back into the
RX path via the netif_rx() call.

Paul if you want to fix up the comment, that's fine.
But please send such a patch relative to what I put
into the tree already which is the following:

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/09/13 16:04:36-07:00 i@stingr.net 
#   [IPV4]: Add wccp v1/v2 support to ip_gre.c
#   
#   Signed-off-by: David S. Miller [off-list ref]
# 
# net/ipv4/ip_gre.c
#   2004/09/13 16:04:06-07:00 i@stingr.net +12 -1
#   [IPV4]: Add wccp v1/v2 support to ip_gre.c
# 
# include/linux/if_ether.h
#   2004/09/13 16:04:06-07:00 i@stingr.net +2 -0
#   [IPV4]: Add wccp v1/v2 support to ip_gre.c
# 
diff -Nru a/include/linux/if_ether.h b/include/linux/if_ether.h

--- a/include/linux/if_ether.h	2004-09-14 22:27:39 -07:00
+++ b/include/linux/if_ether.h	2004-09-14 22:27:39 -07:00

@@ -59,6 +59,8 @@
 #define ETH_P_8021Q	0x8100          /* 802.1Q VLAN Extended Header  */
 #define ETH_P_IPX	0x8137		/* IPX over DIX			*/
 #define ETH_P_IPV6	0x86DD		/* IPv6 over bluebook		*/
+#define ETH_P_WCCP	0x883E		/* Web-cache coordination protocol
+					 * defined in draft-wilson-wrec-wccp-v2-00.txt */
 #define ETH_P_PPP_DISC	0x8863		/* PPPoE discovery messages     */
 #define ETH_P_PPP_SES	0x8864		/* PPPoE session messages	*/
 #define ETH_P_MPLS_UC	0x8847		/* MPLS Unicast traffic		*/

diff -Nru a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c

--- a/net/ipv4/ip_gre.c	2004-09-14 22:27:39 -07:00
+++ b/net/ipv4/ip_gre.c	2004-09-14 22:27:39 -07:00

@@ -603,13 +603,24 @@
 	if ((tunnel = ipgre_tunnel_lookup(iph->saddr, iph->daddr, key)) != NULL) {
 		secpath_reset(skb);
 
+		skb->protocol = *(u16*)(h + 2);
+		/* WCCP version 1 and 2 protocol decoding.
+		 * - Change protocol to IP
+		 * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
+		 */
+		if (flags == 0 &&
+		    skb->protocol == __constant_htons(ETH_P_WCCP)) {
+			skb->protocol = __constant_htons(ETH_P_IP);
+			if ((*(h + offset) & 0xF0) != 0x40) 
+				offset += 4;
+		}
+
 		skb->mac.raw = skb->nh.raw;
 		skb->nh.raw = __pskb_pull(skb, offset);
 		memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
 		if (skb->ip_summed == CHECKSUM_HW)
 			skb->csum = csum_sub(skb->csum,
 					     csum_partial(skb->mac.raw, skb->nh.raw-skb->mac.raw, 0));
-		skb->protocol = *(u16*)(h + 2);
 		skb->pkt_type = PACKET_HOST;
 #ifdef CONFIG_NET_IPGRE_BROADCAST
 		if (MULTICAST(iph->daddr)) {