netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [IPSEC] Move hardware headers for decaped packets
       [not found]   ` <20030925124102.GA18188@gondor.apana.org.au>
@ 2003-12-07  9:02     ` Herbert Xu
  2003-12-07  9:47       ` David S. Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2003-12-07  9:02 UTC (permalink / raw)
  To: kuznet; +Cc: David S. Miller, jmorris, netdev

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

On Thu, Sep 25, 2003 at 10:41:02PM +1000, herbert wrote:
> 
> A quick grep shows just below 100 occurances of mac.raw under
> drivers/net.  I'll have a go at it unless someone else does
> it first.
> 
> On the other hand, I've found a way to avoid the move without
> requiring mac_len to be set at all.  We can do this in netif_rx:

It looks like I've neglected this issue for a few months.

I have had another look at it and avoiding the move altogether
is actually not that useful.  The reason is that anything that
uses mac.raw will need to handle the case where the MAC header
is not next to the payload.

In particular, this is non-trivial in the AF_PACKET case so
we'll end up moving the MAC header anyway.  Worse yet, if there
are multiple AF_PACKET users, this means that the copy/move
will have to be repeated.

So I'd like to go with the previous approach which is to move
for tunnel SAs only by using mac_len.

Here is that patch again.
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 4784 bytes --]

Index: include/linux/skbuff.h
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/include/linux/skbuff.h,v
retrieving revision 1.1.1.8
retrieving revision 1.6
diff -u -r1.1.1.8 -r1.6
--- include/linux/skbuff.h	9 Aug 2003 08:12:03 -0000	1.1.1.8
+++ include/linux/skbuff.h	13 Sep 2003 00:11:55 -0000	1.6
@@ -159,6 +161,7 @@
  *	@cb: Control buffer. Free for use by every layer. Put private vars here
  *	@len: Length of actual data
  *	@data_len: Data length
+ *	@mac_len: Length of link layer header
  *	@csum: Checksum
  *	@__unused: Dead field, may be reused
  *	@cloned: Head may be cloned (check refcnt to be sure)
@@ -199,6 +202,7 @@
 		struct icmphdr	*icmph;
 		struct igmphdr	*igmph;
 		struct iphdr	*ipiph;
+		struct ipv6hdr	*ipv6h;
 		unsigned char	*raw;
 	} h;
 
@@ -227,6 +231,7 @@
 
 	unsigned int		len,
 				data_len,
+				mac_len,
 				csum;
 	unsigned char		local_df,
 				cloned,
Index: net/core/dev.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/core/dev.c,v
retrieving revision 1.1.1.17
retrieving revision 1.2
diff -u -r1.1.1.17 -r1.2
--- net/core/dev.c	22 Aug 2003 23:56:14 -0000	1.1.1.17
+++ net/core/dev.c	13 Sep 2003 00:11:55 -0000	1.2
@@ -1547,6 +1547,7 @@
 #endif
 
 	skb->h.raw = skb->nh.raw = skb->data;
+	skb->mac_len = skb->nh.raw - skb->mac.raw;
 
 	pt_prev = NULL;
 	rcu_read_lock();
Index: net/ipv4/xfrm4_input.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/ipv4/xfrm4_input.c,v
retrieving revision 1.1.1.6
retrieving revision 1.9
diff -u -r1.1.1.6 -r1.9
--- net/ipv4/xfrm4_input.c	22 Aug 2003 23:52:18 -0000	1.1.1.6
+++ net/ipv4/xfrm4_input.c	13 Sep 2003 00:11:55 -0000	1.9
@@ -9,6 +9,7 @@
  * 	
  */
 
+#include <linux/string.h>
 #include <net/inet_ecn.h>
 #include <net/ip.h>
 #include <net/xfrm.h>
@@ -18,9 +19,10 @@
 	return xfrm4_rcv_encap(skb, 0);
 }
 
-static inline void ipip_ecn_decapsulate(struct iphdr *outer_iph, struct sk_buff *skb)
+static inline void ipip_ecn_decapsulate(struct sk_buff *skb)
 {
-	struct iphdr *inner_iph = skb->nh.iph;
+	struct iphdr *outer_iph = skb->nh.iph;
+	struct iphdr *inner_iph = skb->h.ipiph;
 
 	if (INET_ECN_is_ce(outer_iph->tos) &&
 	    INET_ECN_is_not_ce(inner_iph->tos))
@@ -95,10 +97,16 @@
 		if (x->props.mode) {
 			if (iph->protocol != IPPROTO_IPIP)
 				goto drop;
-			skb->nh.raw = skb->data;
+			if (!pskb_may_pull(skb, sizeof(struct iphdr)))
+				goto drop;
+			if (skb_cloned(skb) &&
+			    pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
+				goto drop;
 			if (!(x->props.flags & XFRM_STATE_NOECN))
-				ipip_ecn_decapsulate(iph, skb);
-			iph = skb->nh.iph;
+				ipip_ecn_decapsulate(skb);
+			skb->mac.raw = memmove(skb->data - skb->mac_len,
+					       skb->mac.raw, skb->mac_len);
+			skb->nh.raw = skb->data;
 			memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
 			decaps = 1;
 			break;
Index: net/ipv6/xfrm6_input.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/ipv6/xfrm6_input.c,v
retrieving revision 1.1.1.7
retrieving revision 1.8
diff -u -r1.1.1.7 -r1.8
--- net/ipv6/xfrm6_input.c	9 Aug 2003 08:12:04 -0000	1.1.1.7
+++ net/ipv6/xfrm6_input.c	13 Sep 2003 00:11:55 -0000	1.8
@@ -9,17 +9,20 @@
  *		IPv6 support
  */
 
+#include <linux/string.h>
 #include <net/inet_ecn.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
 #include <net/xfrm.h>
 
-static inline void ipip6_ecn_decapsulate(struct ipv6hdr *iph,
-					 struct sk_buff *skb)
+static inline void ipip6_ecn_decapsulate(struct sk_buff *skb)
 {
-	if (INET_ECN_is_ce(ip6_get_dsfield(iph)) &&
-	    INET_ECN_is_not_ce(ip6_get_dsfield(skb->nh.ipv6h)))
-		IP6_ECN_set_ce(skb->nh.ipv6h);
+	struct ipv6hdr *outer_iph = skb->nh.ipv6h;
+	struct ipv6hdr *inner_iph = skb->h.ipv6h;
+
+	if (INET_ECN_is_ce(ip6_get_dsfield(outer_iph)) &&
+	    INET_ECN_is_not_ce(ip6_get_dsfield(inner_iph)))
+		IP6_ECN_set_ce(inner_iph);
 }
 
 int xfrm6_rcv(struct sk_buff **pskb, unsigned int *nhoffp)
@@ -77,10 +80,16 @@
 		if (x->props.mode) { /* XXX */
 			if (nexthdr != IPPROTO_IPV6)
 				goto drop;
-			skb->nh.raw = skb->data;
+			if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
+				goto drop;
+			if (skb_cloned(skb) &&
+			    pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
+				goto drop;
 			if (!(x->props.flags & XFRM_STATE_NOECN))
-				ipip6_ecn_decapsulate(iph, skb);
-			iph = skb->nh.ipv6h;
+				ipip6_ecn_decapsulate(skb);
+			skb->mac.raw = memmove(skb->data - skb->mac_len,
+					       skb->mac.raw, skb->mac_len);
+			skb->nh.raw = skb->data;
 			decaps = 1;
 			break;
 		}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [IPSEC] Move hardware headers for decaped packets
  2003-12-07  9:02     ` [IPSEC] Move hardware headers for decaped packets Herbert Xu
@ 2003-12-07  9:47       ` David S. Miller
  2003-12-07  9:55         ` Herbert Xu
  0 siblings, 1 reply; 6+ messages in thread
From: David S. Miller @ 2003-12-07  9:47 UTC (permalink / raw)
  To: Herbert Xu; +Cc: kuznet, jmorris, netdev

On Sun, 7 Dec 2003 20:02:05 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> So I'd like to go with the previous approach which is to move
> for tunnel SAs only by using mac_len.

Ok, I'll review this again.  Did you audit the tree to make sure you
updated mac_len everywhere that 'skb->mac.*' is modified?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [IPSEC] Move hardware headers for decaped packets
  2003-12-07  9:47       ` David S. Miller
@ 2003-12-07  9:55         ` Herbert Xu
  2003-12-08  0:55           ` David S. Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2003-12-07  9:55 UTC (permalink / raw)
  To: David S. Miller; +Cc: kuznet, jmorris, netdev

On Sun, Dec 07, 2003 at 01:47:14AM -0800, David S. Miller wrote:
>
> Ok, I'll review this again.  Did you audit the tree to make sure you
> updated mac_len everywhere that 'skb->mac.*' is modified?

To be honest, no.  The reason is that my use of mac_len starts from
netif_receive_skb and ends just before the reentrance into netif_rx
in xfrm[46]_input.

At the start of that path, mac_len is initialised from a value that
we know to be correct.  I have also verified that within the path,
nobody expands/contracts the MAC header.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [IPSEC] Move hardware headers for decaped packets
  2003-12-07  9:55         ` Herbert Xu
@ 2003-12-08  0:55           ` David S. Miller
  2004-02-14  3:55             ` Herbert Xu
  0 siblings, 1 reply; 6+ messages in thread
From: David S. Miller @ 2003-12-08  0:55 UTC (permalink / raw)
  To: Herbert Xu; +Cc: kuznet, jmorris, netdev

On Sun, 7 Dec 2003 20:55:27 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Sun, Dec 07, 2003 at 01:47:14AM -0800, David S. Miller wrote:
> >
> > Ok, I'll review this again.  Did you audit the tree to make sure you
> > updated mac_len everywhere that 'skb->mac.*' is modified?
> 
> To be honest, no.  The reason is that my use of mac_len starts from
> netif_receive_skb and ends just before the reentrance into netif_rx
> in xfrm[46]_input.
> 
> At the start of that path, mac_len is initialised from a value that
> we know to be correct.  I have also verified that within the path,
> nobody expands/contracts the MAC header.

Ok, we can make it a receive only thing at first, I guess.

But I think this will definitely need to be deferred to
2.6.1 at least, along with that XFRM device unload fix you
sent me the other week.

Linus really only wants the obvious one-liners at this point
for 2.6.0

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [IPSEC] Move hardware headers for decaped packets
  2003-12-08  0:55           ` David S. Miller
@ 2004-02-14  3:55             ` Herbert Xu
  2004-02-14  7:10               ` David S. Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2004-02-14  3:55 UTC (permalink / raw)
  To: David S. Miller; +Cc: kuznet, jmorris, netdev

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

On Sun, Dec 07, 2003 at 04:55:16PM -0800, David S. Miller wrote:
> 
> Ok, we can make it a receive only thing at first, I guess.
> 
> But I think this will definitely need to be deferred to
> 2.6.1 at least, along with that XFRM device unload fix you
> sent me the other week.

Looks like I've forgotten about this mac moving patch.  Here it is again.

Just to recap, this moves the hardware header so that it appears
next to the payload for AF_PACKET sockets.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 4784 bytes --]

Index: include/linux/skbuff.h
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/include/linux/skbuff.h,v
retrieving revision 1.1.1.8
retrieving revision 1.6
diff -u -r1.1.1.8 -r1.6
--- include/linux/skbuff.h	9 Aug 2003 08:12:03 -0000	1.1.1.8
+++ include/linux/skbuff.h	13 Sep 2003 00:11:55 -0000	1.6
@@ -159,6 +161,7 @@
  *	@cb: Control buffer. Free for use by every layer. Put private vars here
  *	@len: Length of actual data
  *	@data_len: Data length
+ *	@mac_len: Length of link layer header
  *	@csum: Checksum
  *	@__unused: Dead field, may be reused
  *	@cloned: Head may be cloned (check refcnt to be sure)
@@ -199,6 +202,7 @@
 		struct icmphdr	*icmph;
 		struct igmphdr	*igmph;
 		struct iphdr	*ipiph;
+		struct ipv6hdr	*ipv6h;
 		unsigned char	*raw;
 	} h;
 
@@ -227,6 +231,7 @@
 
 	unsigned int		len,
 				data_len,
+				mac_len,
 				csum;
 	unsigned char		local_df,
 				cloned,
Index: net/core/dev.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/core/dev.c,v
retrieving revision 1.1.1.17
retrieving revision 1.2
diff -u -r1.1.1.17 -r1.2
--- net/core/dev.c	22 Aug 2003 23:56:14 -0000	1.1.1.17
+++ net/core/dev.c	13 Sep 2003 00:11:55 -0000	1.2
@@ -1547,6 +1547,7 @@
 #endif
 
 	skb->h.raw = skb->nh.raw = skb->data;
+	skb->mac_len = skb->nh.raw - skb->mac.raw;
 
 	pt_prev = NULL;
 	rcu_read_lock();
Index: net/ipv4/xfrm4_input.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/ipv4/xfrm4_input.c,v
retrieving revision 1.1.1.6
retrieving revision 1.9
diff -u -r1.1.1.6 -r1.9
--- net/ipv4/xfrm4_input.c	22 Aug 2003 23:52:18 -0000	1.1.1.6
+++ net/ipv4/xfrm4_input.c	13 Sep 2003 00:11:55 -0000	1.9
@@ -9,6 +9,7 @@
  * 	
  */
 
+#include <linux/string.h>
 #include <net/inet_ecn.h>
 #include <net/ip.h>
 #include <net/xfrm.h>
@@ -18,9 +19,10 @@
 	return xfrm4_rcv_encap(skb, 0);
 }
 
-static inline void ipip_ecn_decapsulate(struct iphdr *outer_iph, struct sk_buff *skb)
+static inline void ipip_ecn_decapsulate(struct sk_buff *skb)
 {
-	struct iphdr *inner_iph = skb->nh.iph;
+	struct iphdr *outer_iph = skb->nh.iph;
+	struct iphdr *inner_iph = skb->h.ipiph;
 
 	if (INET_ECN_is_ce(outer_iph->tos) &&
 	    INET_ECN_is_not_ce(inner_iph->tos))
@@ -95,10 +97,16 @@
 		if (x->props.mode) {
 			if (iph->protocol != IPPROTO_IPIP)
 				goto drop;
-			skb->nh.raw = skb->data;
+			if (!pskb_may_pull(skb, sizeof(struct iphdr)))
+				goto drop;
+			if (skb_cloned(skb) &&
+			    pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
+				goto drop;
 			if (!(x->props.flags & XFRM_STATE_NOECN))
-				ipip_ecn_decapsulate(iph, skb);
-			iph = skb->nh.iph;
+				ipip_ecn_decapsulate(skb);
+			skb->mac.raw = memmove(skb->data - skb->mac_len,
+					       skb->mac.raw, skb->mac_len);
+			skb->nh.raw = skb->data;
 			memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
 			decaps = 1;
 			break;
Index: net/ipv6/xfrm6_input.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/ipv6/xfrm6_input.c,v
retrieving revision 1.1.1.7
retrieving revision 1.8
diff -u -r1.1.1.7 -r1.8
--- net/ipv6/xfrm6_input.c	9 Aug 2003 08:12:04 -0000	1.1.1.7
+++ net/ipv6/xfrm6_input.c	13 Sep 2003 00:11:55 -0000	1.8
@@ -9,17 +9,20 @@
  *		IPv6 support
  */
 
+#include <linux/string.h>
 #include <net/inet_ecn.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
 #include <net/xfrm.h>
 
-static inline void ipip6_ecn_decapsulate(struct ipv6hdr *iph,
-					 struct sk_buff *skb)
+static inline void ipip6_ecn_decapsulate(struct sk_buff *skb)
 {
-	if (INET_ECN_is_ce(ip6_get_dsfield(iph)) &&
-	    INET_ECN_is_not_ce(ip6_get_dsfield(skb->nh.ipv6h)))
-		IP6_ECN_set_ce(skb->nh.ipv6h);
+	struct ipv6hdr *outer_iph = skb->nh.ipv6h;
+	struct ipv6hdr *inner_iph = skb->h.ipv6h;
+
+	if (INET_ECN_is_ce(ip6_get_dsfield(outer_iph)) &&
+	    INET_ECN_is_not_ce(ip6_get_dsfield(inner_iph)))
+		IP6_ECN_set_ce(inner_iph);
 }
 
 int xfrm6_rcv(struct sk_buff **pskb, unsigned int *nhoffp)
@@ -77,10 +80,16 @@
 		if (x->props.mode) { /* XXX */
 			if (nexthdr != IPPROTO_IPV6)
 				goto drop;
-			skb->nh.raw = skb->data;
+			if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
+				goto drop;
+			if (skb_cloned(skb) &&
+			    pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
+				goto drop;
 			if (!(x->props.flags & XFRM_STATE_NOECN))
-				ipip6_ecn_decapsulate(iph, skb);
-			iph = skb->nh.ipv6h;
+				ipip6_ecn_decapsulate(skb);
+			skb->mac.raw = memmove(skb->data - skb->mac_len,
+					       skb->mac.raw, skb->mac_len);
+			skb->nh.raw = skb->data;
 			decaps = 1;
 			break;
 		}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [IPSEC] Move hardware headers for decaped packets
  2004-02-14  3:55             ` Herbert Xu
@ 2004-02-14  7:10               ` David S. Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David S. Miller @ 2004-02-14  7:10 UTC (permalink / raw)
  To: Herbert Xu; +Cc: kuznet, jmorris, netdev

On Sat, 14 Feb 2004 14:55:34 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Looks like I've forgotten about this mac moving patch.  Here it is again.
> 
> Just to recap, this moves the hardware header so that it appears
> next to the payload for AF_PACKET sockets.

Queued for 2.6.4-pre1, thanks Herbert.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2004-02-14  7:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20030925121131.GA17968@gondor.apana.org.au>
     [not found] ` <200309251228.QAA11650@yakov.inr.ac.ru>
     [not found]   ` <20030925124102.GA18188@gondor.apana.org.au>
2003-12-07  9:02     ` [IPSEC] Move hardware headers for decaped packets Herbert Xu
2003-12-07  9:47       ` David S. Miller
2003-12-07  9:55         ` Herbert Xu
2003-12-08  0:55           ` David S. Miller
2004-02-14  3:55             ` Herbert Xu
2004-02-14  7:10               ` David S. Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).