2008-06-06 11:18:42

by Ang Way Chuang

[permalink] [raw]
Subject: [PATCH] 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 commit
c3b25b32e8bef526cca748e1ba023c6bdd705a99. 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.

Signed-off-by: Ang Way Chuang <[email protected]>
---

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 0ce07a3..77964bc 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -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;

--


2008-06-06 20:21:20

by Max Krasnyansky

[permalink] [raw]
Subject: Re: [PATCH] NET: Proper handling of IPv6 header in tun driver when TUN_NO_PI is set

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 extra copy_from_user". We
already do copy into the SKB and can parse the header right there.

Max




Ang Way Chuang wrote:
> 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 commit
> c3b25b32e8bef526cca748e1ba023c6bdd705a99. 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.
>
> Signed-off-by: Ang Way Chuang <[email protected]>
> ---
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 0ce07a3..77964bc 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -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;
>
> --
>

2008-06-06 20:48:53

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [PATCH] NET: Proper handling of IPv6 header in tun driver when TUN_NO_PI is set

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 extra copy_from_user". We
> already do copy into the SKB and can parse the header right there.
>
> Max
>
>
>
>
> Ang Way Chuang wrote:
> > 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 commit
> > c3b25b32e8bef526cca748e1ba023c6bdd705a99. 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.
> >
> > Signed-off-by: Ang Way Chuang <[email protected]>
> > ---
> >
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index 0ce07a3..77964bc 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -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) {

Why the magic constants, instead of proper casts and defines.
> > + 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.

> > + 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;
> >
> > --
> >
>
> --
> 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

2008-06-17 23:32:21

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] NET: Proper handling of IPv6 header in tun driver when TUN_NO_PI is set

From: Max Krasnyanskiy <[email protected]>
Date: Fri, 06 Jun 2008 13:21:06 -0700

> 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.

2008-06-18 02:14:24

by Ang Way Chuang

[permalink] [raw]
Subject: Re: [PATCH] NET: Proper handling of IPv6 header in tun driver when TUN_NO_PI is set

David Miller wrote:
> From: Max Krasnyanskiy <[email protected]>
> Date: Fri, 06 Jun 2008 13:21:06 -0700
>
>> 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.

Signed-off-by: Ang Way Chuang <[email protected]>
---

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 0ce07a3..f97bb9b 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -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;

--

2008-06-18 04:08:20

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] NET: Proper handling of IPv6 header in tun driver when TUN_NO_PI is set

From: Ang Way Chuang <[email protected]>
Date: Wed, 18 Jun 2008 10:13:31 +0800

> David Miller wrote:
> > From: Max Krasnyanskiy <[email protected]>
> > Date: Fri, 06 Jun 2008 13:21:06 -0700
> >
> >> 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.

2008-06-24 05:10:00

by Max Krasnyansky

[permalink] [raw]
Subject: Re: [PATCH] NET: Proper handling of IPv6 header in tun driver when TUN_NO_PI is set

David Miller wrote:
> From: Ang Way Chuang <[email protected]>
> Date: Wed, 18 Jun 2008 10:13:31 +0800
>
>> David Miller wrote:
>>> From: Max Krasnyanskiy <[email protected]>
>>> Date: Fri, 06 Jun 2008 13:21:06 -0700
>>>
>>>> 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.

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