NET: Proper handling of IPv6 header in tun driver when TUN_NO_PI is set
Subsystems (3)
PATCH 1/1 NET: Proper handling of IPv6 header in tun driver when TUN_NO_PI is set
By default, tun.c running in TUN_TUN_DEV mode will set the protocol of packet to IPv4 if TUN_NO_PI is set. My program failed to work when I assumed that the driver will check the first nibble of packet, determine IP version and set the appropriate protocol.
The test for this change was conducted on 2.6.24.7 kernel. But the diff is produced using git commitc3b25b32e8bef526cca748e1ba023c6bdd705a99. I couldn't test it on kernel version greater than 2.6.25 due to out of tree driver failing to compile. But tun_get_user doesn't change that much between 2.6.24 and 2.6.26, so it should work, I hope :-D. This patch is trivial because this problem can also be fixed by not using TUN_NO_PI.
---
drivers/net/tun.c +15 -0
@@ -313,6 +313,21 @@ static __inline__ ssize_t tun_get_user(struct tun_struct *tun, struct iovec *iv,switch (tun->flags &TUN_TYPE_MASK ) {case TUN_TUN_DEV : +if (tun->flags &TUN_NO_PI ) { +switch (skb->data[0 ] &0xf0 ) { +case 0x40 : + pi.proto =__constant_htons (ETH_P_IP ); +break ; +case 0x60 : + pi.proto =__constant_htons (ETH_P_IPV6 ); +break ; +default : + tun->dev->stats.rx_dropped++; +kfree_skb (skb); +return -EINVAL ; + } + } +skb_reset_mac_header (skb); skb->protocol = pi.proto; skb->dev = tun->dev;
--
Dave, can you please add this patch to your tree.
--
Steve, this is what I meant by "we do not need extra copy_from_user". We already do copy into the SKB and can parse the header right there.
Max
Quoted 38 lines · Ang Way Chuang
By default,
tun.crunning inTUN_TUN_DEVmode will set the protocol of packet to IPv4 ifTUN_NO_PIis set. My program failed to work when I assumed that the driver will check the first nibble of packet, determine IP version and set the appropriate protocol.The test for this change was conducted on 2.6.24.7 kernel. But the diff is produced using git commit
c3b25b32e8bef526cca748e1ba023c6bdd705a99. I couldn't test it on kernel version greater than 2.6.25 due to out of tree driver failing to compile. Buttun_get_userdoesn't change that much between 2.6.24 and 2.6.26, so it should work, I hope :-D. This patch is trivial because this problem can also be fixed by not usingTUN_NO_PI.Signed-off-by Ang Way Chuang <[email protected]>---
drivers/net/tun.c +15
-0@@ -313,6 +313,21 @@ static __inline__ ssize_t tun_get_user(struct tun_struct *tun, struct iovec *iv, switch (tun->flags & TUN_TYPE_MASK) { case TUN_TUN_DEV: + if (tun->flags & TUN_NO_PI) { + switch (skb->data[0] & 0xf0) { + case 0x40: + pi.proto = __constant_htons(ETH_P_IP); +break ; +case 0x60 : + pi.proto =__constant_htons (ETH_P_IPV6 ); +break ; +default : + tun->dev->stats.rx_dropped++; +kfree_skb (skb); +return -EINVAL ; + } + } +skb_reset_mac_header (skb); skb->protocol = pi.proto;skb->dev = tun->dev;
Quoted 37 lines · On Fri, 06 Jun 2008 13:21:06 -0700 Max Krasnyanskiy <[email protected]> wrote:
Acked-by Max Krasnyansky <[email protected]>Dave, can you please add this patch to your tree.
--
Steve, this is what I meant by "we do not need extracopy_from_user". We already do copy into the SKB and can parse the header right there.Max
Quoted 23 lines · Ang Way Chuang
By default,
tun.crunning inTUN_TUN_DEVmode will set the protocol of packet to IPv4 ifTUN_NO_PIis set. My program failed to work when I assumed that the driver will check the first nibble of packet, determine IP version and set the appropriate protocol.The test for this change was conducted on 2.6.24.7 kernel. But the diff is produced using git commit
c3b25b32e8bef526cca748e1ba023c6bdd705a99. I couldn't test it on kernel version greater than 2.6.25 due to out of tree driver failing to compile. Buttun_get_userdoesn't change that much between 2.6.24 and 2.6.26, so it should work, I hope :-D. This patch is trivial because this problem can also be fixed by not usingTUN_NO_PI.Signed-off-by Ang Way Chuang <[email protected]>---
drivers/net/tun.c +0
-0@@ -313,6 +313,21 @@ static __inline__ ssize_t tun_get_user(struct tun_struct *tun, struct iovec *iv,
Quoted 4 lines
drivers/net/tun.c static __inline__ ssize_t tun_get_user(struct tun_struct *tun, struct iovec *iv, · line 317
Quoted 4 lines
drivers/net/tun.c +2
-0switch (tun->flags &TUN_TYPE_MASK ) {case TUN_TUN_DEV : +if (tun->flags &TUN_NO_PI ) { +switch (skb->data[0 ] &0xf0 ) {
Why the magic constants, instead of proper casts and defines.
Quoted 2 lines
drivers/net/tun.c static __inline__ ssize_t tun_get_user(struct tun_struct *tun, struct iovec *iv, · line 319
Quoted 2 lines
drivers/net/tun.c +2
-0+case 0x40 : + pi.proto =__constant_htons (ETH_P_IP );
Use htons() instead of __constant_htons() it is easier to read, and the compiler already does the right thing.
Quoted 22 lines
Quoted 13 lines
drivers/net/tun.c +11
-0+break ; +case 0x60 : + pi.proto =__constant_htons (ETH_P_IPV6 ); +break ; +default : + tun->dev->stats.rx_dropped++; +kfree_skb (skb); +return -EINVAL ; + } + } +skb_reset_mac_header (skb); skb->protocol = pi.proto;Quoted 4 lines
skb->dev = tun->dev;--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] NET: Proper handling of IPv6 header in tun driver whenTUN_NO_PI is set
Max: Now understand your comment about additional copy_from_user -- yeah, we do not want to make extra function calls if we don't need to.
I do concur with Stephen's comments.
Also, while we are at it, I guess we could also add in the IPX case -- although I'd bet IPX is OBE at this point.
Steve
-----Original Message-----
From: Stephen Hemminger [mailto:[email protected]]
Sent: Friday, June 06, 2008 4:49 PM
To: Max Krasnyanskiy
Cc: Ang Way Chuang; David Miller; Zabele, Stephen (US SSA); [email protected]; [email protected]
Subject: Re: [PATCH] NET: Proper handling of IPv6 header in tun driver whenTUN_NO_PI is set
Quoted 6 lines · On Fri, 06 Jun 2008 13:21:06 -0700 Max Krasnyanskiy <[email protected]> wrote:
Acked-by Max Krasnyansky <[email protected]>Dave, can you please add this patch to your tree.
--
Steve, this is what I meant by "we do not need extracopy_from_user".
We
Quoted 9 lines
already do copy into the SKB and can parse the header right there.
Max
Quoted 1 line · Ang Way Chuang
By default,
tun.crunning inTUN_TUN_DEVmode will set the protocol
of
Quoted 3 lines
Quoted 3 lines
packet to IPv4 if
TUN_NO_PIis set. My program failed to work when Iassumed that the driver will check the first nibble of packet,
determine
Quoted 3 lines
Quoted 3 lines
IP version and set the appropriate protocol.
The test for this change was conducted on 2.6.24.7 kernel. But the
diff
Quoted 2 lines
Quoted 2 lines
is produced using git commit
c3b25b32e8bef526cca748e1ba023c6bdd705a99. I couldn't test it on
kernel
Quoted 2 lines
Quoted 2 lines
version greater than 2.6.25 due to out of tree driver failing to compile. But
tun_get_userdoesn't change that much between 2.6.24
and
Quoted 14 lines
Quoted 14 lines
2.6.26, so it should work, I hope :-D. This patch is trivial because
this problem can also be fixed by not using
TUN_NO_PI.Signed-off-by Ang Way Chuang <[email protected]>---
drivers/net/tun.c +0
-0@@ -313,6 +313,21 @@ static __inline__ ssize_t tun_get_user(struct tun_struct *tun, struct iovec *iv,
Quoted 4 lines
drivers/net/tun.c static __inline__ ssize_t tun_get_user(struct tun_struct *tun, struct iovec *iv, · line 317
Quoted 4 lines
drivers/net/tun.c +2
-0switch (tun->flags &TUN_TYPE_MASK ) {case TUN_TUN_DEV : +if (tun->flags &TUN_NO_PI ) { +switch (skb->data[0 ] &0xf0 ) {
Why the magic constants, instead of proper casts and defines.
Quoted 2 lines
drivers/net/tun.c static __inline__ ssize_t tun_get_user(struct · line 320
Quoted 2 lines
drivers/net/tun.c +2
-0+case 0x40 : + pi.proto =
__constant_htons(ETH_P_IP);
Use htons() instead of __constant_htons() it is easier to read, and the compiler already does the right thing.
Quoted 3 lines
drivers/net/tun.c static __inline__ ssize_t tun_get_user(struct tun_struct *tun, struct iovec *iv,
Quoted 3 lines
drivers/net/tun.c +3
-0+break ; +case 0x60 : + pi.proto =
__constant_htons(ETH_P_IPV6);
Quoted 19 lines
Quoted 10 lines
drivers/net/tun.c +8
-0+break ; +default : + tun->dev->stats.rx_dropped++; +kfree_skb (skb); +return -EINVAL ; + } + } +skb_reset_mac_header (skb); skb->protocol = pi.proto;Quoted 4 lines
skb->dev = tun->dev;--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller
maintainerFrom: Max Krasnyanskiy <[email protected]>
Date: Fri, 06 Jun 2008 13:21:06 -0700
Quoted 3 lines
Acked-by Max Krasnyansky <[email protected]>Dave, can you please add this patch to your tree.
I can't, the patch is severely whitespace damaged.
Quoted 9 lines · David Miller
From: Max Krasnyanskiy <[email protected]>
Date: Fri, 06 Jun 2008 13:21:06 -0700Quoted 3 lines
Acked-by Max Krasnyansky <[email protected]>Dave, can you please add this patch to your tree.
I can't, the patch is severely whitespace damaged.
Resend. Hopefully Thunderbird won't screw this time.
By default, tun.c running in TUN_TUN_DEV mode will set the protocol of packet to IPv4 if TUN_NO_PI is set. My program failed to work when I assumed that the driver will check the first nibble of packet, determine IP version and set the appropriate protocol.
---
drivers/net/tun.c +15 -0
@@ -313,6 +313,21 @@ static __inline__ ssize_t tun_get_user(struct tun_struct *tun, struct iovec *iv,switch (tun->flags &TUN_TYPE_MASK ) {case TUN_TUN_DEV : +if (tun->flags &TUN_NO_PI ) { +switch (skb->data[0 ] &0xf0 ) { +case 0x40 : + pi.proto =htons (ETH_P_IP ); +break ; +case 0x60 : + pi.proto =htons (ETH_P_IPV6 ); +break ; +default : + tun->dev->stats.rx_dropped++; +kfree_skb (skb); +return -EINVAL ; + } + } +skb_reset_mac_header (skb); skb->protocol = pi.proto; skb->dev = tun->dev;
--
David Miller
maintainerFrom: Ang Way Chuang <[email protected]>
Date: Wed, 18 Jun 2008 10:13:31 +0800
Quoted 11 lines
Quoted 9 lines · David Miller
From: Max Krasnyanskiy <[email protected]>
Date: Fri, 06 Jun 2008 13:21:06 -0700Quoted 3 lines
Acked-by Max Krasnyansky <[email protected]>Dave, can you please add this patch to your tree.
I can't, the patch is severely whitespace damaged.
Resend. Hopefully Thunderbird won't screw this time.
Same problem, tabs were transformed into spaces, and lines composed of only whitespace were transformed into empty lines.
This is frustrating enough, I'm just going to fix this up by hand for you, but please get your shit straight for next time because I won't do this twice.
Thanks.
Quoted 14 lines · David Miller
From: Ang Way Chuang <[email protected]>
Date: Wed, 18 Jun 2008 10:13:31 +0800Quoted 10 lines
Quoted 8 lines · David Miller
From: Max Krasnyanskiy <[email protected]>
Date: Fri, 06 Jun 2008 13:21:06 -0700Quoted 3 lines
Acked-by Max Krasnyansky <[email protected]>Dave, can you please add this patch to your tree.
I can't, the patch is severely whitespace damaged.
Resend. Hopefully Thunderbird won't screw this time.
Quoted 7 lines
Same problem, tabs were transformed into spaces, and lines composed of only whitespace were transformed into empty lines.
This is frustrating enough, I'm just going to fix this up by hand for you, but please get your shit straight for next time because I won't do this twice.
Dave, thanks a lot for fixing this up. I looked at the patch and logic wise it looked perfectly fine. I did not notice the problem with the spaces. Sorry.
Max