Re: kernel >= 4.0: crashes when using traceroute6 with isatap

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Cong Wang]

(Cc'ing netdev.)

On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
> Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
>> Hello,
>>
>> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with an
>> isatap-tunnel.
>
> I did some further tests. To trigger the crash you need
>
> * isatap-tunnel (probably any sit-tunnel will do it)
> * raw-socket
> * udp
>
> Using icmpv6 or tcp i.e. does not trigger it.
>

Do you have a script to reproduce it?


Thanks for the bug report!

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Eric Dumazet]

On Wed, 2015-05-06 at 11:15 -0700, Cong Wang wrote:
> (Cc'ing netdev.)
> 
> On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
> > Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
> >> Hello,
> >>
> >> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with an
> >> isatap-tunnel.
> >
> > I did some further tests. To trigger the crash you need
> >
> > * isatap-tunnel (probably any sit-tunnel will do it)
> > * raw-socket
> > * udp
> >
> > Using icmpv6 or tcp i.e. does not trigger it.
> >
> 
> Do you have a script to reproduce it?
> 
> 
> Thanks for the bug report!
> --

Please Wolfgang try to revert 32dce968dd987adfb0c00946d78dad9154f64759
("ipv6: Allow for partial checksums on non-ufo packets")

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Wolfgang Walter]

Am Mittwoch, 6. Mai 2015, 12:10:00 schrieb Eric Dumazet:
> On Wed, 2015-05-06 at 11:15 -0700, Cong Wang wrote:
> > (Cc'ing netdev.)
> > 
> > On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
> > > Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
> > >> Hello,
> > >> 
> > >> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with
> > >> an
> > >> isatap-tunnel.
> > > 
> > > I did some further tests. To trigger the crash you need
> > > 
> > > * isatap-tunnel (probably any sit-tunnel will do it)
> > > * raw-socket
> > > * udp
> > > 
> > > Using icmpv6 or tcp i.e. does not trigger it.
> > 
> > Do you have a script to reproduce it?
> > 
> > 
> > Thanks for the bug report!
> > --
> 
> Please Wolfgang try to revert 32dce968dd987adfb0c00946d78dad9154f64759
> ("ipv6: Allow for partial checksums on non-ufo packets")

Indeed, that fixes the problem.

Regards
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Eric Dumazet]

On Thu, 2015-05-07 at 00:04 +0200, Wolfgang Walter wrote:
> Am Mittwoch, 6. Mai 2015, 12:10:00 schrieb Eric Dumazet:
> > On Wed, 2015-05-06 at 11:15 -0700, Cong Wang wrote:
> > > (Cc'ing netdev.)
> > > 
> > > On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
> > > > Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
> > > >> Hello,
> > > >> 
> > > >> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with
> > > >> an
> > > >> isatap-tunnel.
> > > > 
> > > > I did some further tests. To trigger the crash you need
> > > > 
> > > > * isatap-tunnel (probably any sit-tunnel will do it)
> > > > * raw-socket
> > > > * udp
> > > > 
> > > > Using icmpv6 or tcp i.e. does not trigger it.
> > > 
> > > Do you have a script to reproduce it?
> > > 
> > > 
> > > Thanks for the bug report!
> > > --
> > 
> > Please Wolfgang try to revert 32dce968dd987adfb0c00946d78dad9154f64759
> > ("ipv6: Allow for partial checksums on non-ufo packets")
> 
> Indeed, that fixes the problem.

Yes, setting skb->csum to 0 is clearly wrong for CHECKSUM_PARTIAL

Would you try :

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 7fde1f265c90e90f16291e6c861b6e242111c25b..694ae630e1ca67e25ab1e5f6dd0b3597db3669b0 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1416,6 +1416,10 @@ alloc_new_skb:
 			data += fragheaderlen;
 			skb->transport_header = (skb->network_header +
 						 fragheaderlen);
+			if (csummode == CHECKSUM_PARTIAL) {
+				skb->csum_start = skb_transport_header(skb) - skb->head;
+				skb->csum_offset = offsetof(struct udphdr, check);
+			}
 			if (fraggap) {
 				skb->csum = skb_copy_and_csum_bits(
 					skb_prev, maxfraglen,

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Wolfgang Walter]

Am Mittwoch, 6. Mai 2015, 15:42:38 schrieben Sie:
> On Thu, 2015-05-07 at 00:04 +0200, Wolfgang Walter wrote:
> > Am Mittwoch, 6. Mai 2015, 12:10:00 schrieb Eric Dumazet:
> > > On Wed, 2015-05-06 at 11:15 -0700, Cong Wang wrote:
> > > > (Cc'ing netdev.)
> > > > 
> > > > On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
> > > > > Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
> > > > >> Hello,
> > > > >> 
> > > > >> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6
> > > > >> with
> > > > >> an
> > > > >> isatap-tunnel.
> > > > > 
> > > > > I did some further tests. To trigger the crash you need
> > > > > 
> > > > > * isatap-tunnel (probably any sit-tunnel will do it)
> > > > > * raw-socket
> > > > > * udp
> > > > > 
> > > > > Using icmpv6 or tcp i.e. does not trigger it.
> > > > 
> > > > Do you have a script to reproduce it?
> > > > 
> > > > 
> > > > Thanks for the bug report!
> > > > --
> > > 
> > > Please Wolfgang try to revert 32dce968dd987adfb0c00946d78dad9154f64759
> > > ("ipv6: Allow for partial checksums on non-ufo packets")
> > 
> > Indeed, that fixes the problem.
> 
> Yes, setting skb->csum to 0 is clearly wrong for CHECKSUM_PARTIAL
> 
> Would you try :
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index
> 7fde1f265c90e90f16291e6c861b6e242111c25b..694ae630e1ca67e25ab1e5f6dd0b3597d
> b3669b0 100644 --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1416,6 +1416,10 @@ alloc_new_skb:
>  			data += fragheaderlen;
>  			skb->transport_header = (skb->network_header +
>  						 fragheaderlen);
> +			if (csummode == CHECKSUM_PARTIAL) {
> +				skb->csum_start = skb_transport_header(skb) - skb->head;
> +				skb->csum_offset = offsetof(struct udphdr, check);
> +			}
>  			if (fraggap) {
>  				skb->csum = skb_copy_and_csum_bits(
>  					skb_prev, maxfraglen,

This patch fixes 4.0.1 (and 4.0.2).

Thanks Eric,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Vlad Yasevich]

On 05/06/2015 06:42 PM, Eric Dumazet wrote:
> On Thu, 2015-05-07 at 00:04 +0200, Wolfgang Walter wrote:
>> Am Mittwoch, 6. Mai 2015, 12:10:00 schrieb Eric Dumazet:
>>> On Wed, 2015-05-06 at 11:15 -0700, Cong Wang wrote:
>>>> (Cc'ing netdev.)
>>>>
>>>> On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
>>>>> Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
>>>>>> Hello,
>>>>>>
>>>>>> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with
>>>>>> an
>>>>>> isatap-tunnel.
>>>>>
>>>>> I did some further tests. To trigger the crash you need
>>>>>
>>>>> * isatap-tunnel (probably any sit-tunnel will do it)
>>>>> * raw-socket
>>>>> * udp
>>>>>
>>>>> Using icmpv6 or tcp i.e. does not trigger it.
>>>>
>>>> Do you have a script to reproduce it?
>>>>
>>>>
>>>> Thanks for the bug report!
>>>> --
>>>
>>> Please Wolfgang try to revert 32dce968dd987adfb0c00946d78dad9154f64759
>>> ("ipv6: Allow for partial checksums on non-ufo packets")
>>
>> Indeed, that fixes the problem.
> 
> Yes, setting skb->csum to 0 is clearly wrong for CHECKSUM_PARTIAL
> 
> Would you try :
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 7fde1f265c90e90f16291e6c861b6e242111c25b..694ae630e1ca67e25ab1e5f6dd0b3597db3669b0 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1416,6 +1416,10 @@ alloc_new_skb:
>  			data += fragheaderlen;
>  			skb->transport_header = (skb->network_header +
>  						 fragheaderlen);
> +			if (csummode == CHECKSUM_PARTIAL) {
> +				skb->csum_start = skb_transport_header(skb) - skb->head;
> +				skb->csum_offset = offsetof(struct udphdr, check);
> +			}
>  			if (fraggap) {
>  				skb->csum = skb_copy_and_csum_bits(
>  					skb_prev, maxfraglen,
> 
> 
> 

So why is this not an issue in __ip_append_data()?

-vlad

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Wolfgang Walter]

Am Mittwoch, 6. Mai 2015, 11:15:18 schrieben Sie:
> (Cc'ing netdev.)
> 
> On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
> > Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
> >> Hello,
> >> 
> >> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with an
> >> isatap-tunnel.
> > 
> > I did some further tests. To trigger the crash you need
> > 
> > * isatap-tunnel (probably any sit-tunnel will do it)
> > * raw-socket
> > * udp
> > 
> > Using icmpv6 or tcp i.e. does not trigger it.
> 
> Do you have a script to reproduce it?
> 
> 
> Thanks for the bug report!

You need a isatap-server with say ipv4-address $X

Then, on host with 4.0, start isatapd: isatapd --mtu 1280 $X 

then do

	traceroute6 www.google.de

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Vlad Yasevich]

On 05/06/2015 06:11 PM, Wolfgang Walter wrote:
> Am Mittwoch, 6. Mai 2015, 11:15:18 schrieben Sie:
>> (Cc'ing netdev.)
>>
>> On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
>>> Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
>>>> Hello,
>>>>
>>>> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with an
>>>> isatap-tunnel.
>>>
>>> I did some further tests. To trigger the crash you need
>>>
>>> * isatap-tunnel (probably any sit-tunnel will do it)
>>> * raw-socket
>>> * udp
>>>
>>> Using icmpv6 or tcp i.e. does not trigger it.
>>
>> Do you have a script to reproduce it?
>>
>>
>> Thanks for the bug report!
>
> You need a isatap-server with say ipv4-address $X
>
> Then, on host with 4.0, start isatapd: isatapd --mtu 1280 $X
>
> then do
>
> 	traceroute6 www.google.de
>
> Regards,
>

Hi Walter

Could you try this patch.  Looks like raw passes transhdrlen
of 0 on the first packet and that makes IPv4 behave correctly,
but not IPv6.


diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 7fde1f2..fd9c079 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1278,7 +1278,7 @@ emsgsize:
 	/* If this is the first and only packet and device
 	 * supports checksum offloading, let's use it.
 	 */
-	if (!skb && sk->sk_protocol == IPPROTO_UDP &&
+	if (transhdrlen && sk->sk_protocol == IPPROTO_UDP &&
 	    length + fragheaderlen < mtu &&
 	    rt->dst.dev->features & NETIF_F_V6_CSUM &&
 	    !exthdrlen)

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Eric Dumazet]

On Tue, 2015-05-12 at 16:18 -0400, Vlad Yasevich wrote:
> On 05/06/2015 06:11 PM, Wolfgang Walter wrote:
> > Am Mittwoch, 6. Mai 2015, 11:15:18 schrieben Sie:
> >> (Cc'ing netdev.)
> >>
> >> On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
> >>> Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
> >>>> Hello,
> >>>>
> >>>> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with an
> >>>> isatap-tunnel.
> >>>
> >>> I did some further tests. To trigger the crash you need
> >>>
> >>> * isatap-tunnel (probably any sit-tunnel will do it)
> >>> * raw-socket
> >>> * udp
> >>>
> >>> Using icmpv6 or tcp i.e. does not trigger it.
> >>
> >> Do you have a script to reproduce it?
> >>
> >>
> >> Thanks for the bug report!
> >
> > You need a isatap-server with say ipv4-address $X
> >
> > Then, on host with 4.0, start isatapd: isatapd --mtu 1280 $X
> >
> > then do
> >
> > 	traceroute6 www.google.de
> >
> > Regards,
> >
> 
> Hi Walter
> 
> Could you try this patch.  Looks like raw passes transhdrlen
> of 0 on the first packet and that makes IPv4 behave correctly,
> but not IPv6.
> 
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 7fde1f2..fd9c079 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1278,7 +1278,7 @@ emsgsize:
>  	/* If this is the first and only packet and device
>  	 * supports checksum offloading, let's use it.
>  	 */
> -	if (!skb && sk->sk_protocol == IPPROTO_UDP &&
> +	if (transhdrlen && sk->sk_protocol == IPPROTO_UDP &&
>  	    length + fragheaderlen < mtu &&
>  	    rt->dst.dev->features & NETIF_F_V6_CSUM &&
>  	    !exthdrlen)

And make sure the checksum is correct ;)

Vlad, can you tell where skb->cum_start and skb->csum_offset are set ?

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Vlad Yasevich]

On 05/12/2015 04:36 PM, Eric Dumazet wrote:
> On Tue, 2015-05-12 at 16:18 -0400, Vlad Yasevich wrote:
>> On 05/06/2015 06:11 PM, Wolfgang Walter wrote:
>>> Am Mittwoch, 6. Mai 2015, 11:15:18 schrieben Sie:
>>>> (Cc'ing netdev.)
>>>>
>>>> On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
>>>>> Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
>>>>>> Hello,
>>>>>>
>>>>>> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with an
>>>>>> isatap-tunnel.
>>>>>
>>>>> I did some further tests. To trigger the crash you need
>>>>>
>>>>> * isatap-tunnel (probably any sit-tunnel will do it)
>>>>> * raw-socket
>>>>> * udp
>>>>>
>>>>> Using icmpv6 or tcp i.e. does not trigger it.
>>>>
>>>> Do you have a script to reproduce it?
>>>>
>>>>
>>>> Thanks for the bug report!
>>>
>>> You need a isatap-server with say ipv4-address $X
>>>
>>> Then, on host with 4.0, start isatapd: isatapd --mtu 1280 $X
>>>
>>> then do
>>>
>>> 	traceroute6 www.google.de
>>>
>>> Regards,
>>>
>>
>> Hi Walter
>>
>> Could you try this patch.  Looks like raw passes transhdrlen
>> of 0 on the first packet and that makes IPv4 behave correctly,
>> but not IPv6.
>>
>>
>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>> index 7fde1f2..fd9c079 100644
>> --- a/net/ipv6/ip6_output.c
>> +++ b/net/ipv6/ip6_output.c
>> @@ -1278,7 +1278,7 @@ emsgsize:
>>  	/* If this is the first and only packet and device
>>  	 * supports checksum offloading, let's use it.
>>  	 */
>> -	if (!skb && sk->sk_protocol == IPPROTO_UDP &&
>> +	if (transhdrlen && sk->sk_protocol == IPPROTO_UDP &&
>>  	    length + fragheaderlen < mtu &&
>>  	    rt->dst.dev->features & NETIF_F_V6_CSUM &&
>>  	    !exthdrlen)
> 
> And make sure the checksum is correct ;)
> 
> Vlad, can you tell where skb->cum_start and skb->csum_offset are set ?
> 
> 

For udp, udp6_hwcsum_outgoing(), but with the above patch, the check above will return
false, and we'll fallback to using CHECKSUM_NONE.

Before, the !skb was true since there there was no skb on the queue.  Now, that
we are checking transhdr and raw passing in 0, the check will be false.  This
is what's making IPv4 work correctly in this case.

-vlad

Re: kernel >= 4.0: crashes when using traceroute6 with isatap

[Wolfgang Walter]

Am Dienstag, 12. Mai 2015, 16:18:54 schrieb Vlad Yasevich:
> On 05/06/2015 06:11 PM, Wolfgang Walter wrote:
> > Am Mittwoch, 6. Mai 2015, 11:15:18 schrieben Sie:
> >> (Cc'ing netdev.)
> >> 
> >> On Sat, May 2, 2015 at 5:29 AM, Wolfgang Walter <linux@stwm.de> wrote:
> >>> Am Samstag, 2. Mai 2015, 02:16:36 schrieb Wolfgang Walter:
> >>>> Hello,
> >>>> 
> >>>> kernel 4.0 (and 4.0.1) crashes immediately when I use traceroute6 with
> >>>> an
> >>>> isatap-tunnel.
> >>> 
> >>> I did some further tests. To trigger the crash you need
> >>> 
> >>> * isatap-tunnel (probably any sit-tunnel will do it)
> >>> * raw-socket
> >>> * udp
> >>> 
> >>> Using icmpv6 or tcp i.e. does not trigger it.
> >> 
> >> Do you have a script to reproduce it?
> >> 
> >> 
> >> Thanks for the bug report!
> > 
> > You need a isatap-server with say ipv4-address $X
> > 
> > Then, on host with 4.0, start isatapd: isatapd --mtu 1280 $X
> > 
> > then do
> > 
> > 	traceroute6 www.google.de
> > 
> > Regards,
> 
> Hi Walter
> 
> Could you try this patch.  Looks like raw passes transhdrlen
> of 0 on the first packet and that makes IPv4 behave correctly,
> but not IPv6.
> 
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 7fde1f2..fd9c079 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1278,7 +1278,7 @@ emsgsize:
>  	/* If this is the first and only packet and device
>  	 * supports checksum offloading, let's use it.
>  	 */
> -	if (!skb && sk->sk_protocol == IPPROTO_UDP &&
> +	if (transhdrlen && sk->sk_protocol == IPPROTO_UDP &&
>  	    length + fragheaderlen < mtu &&
>  	    rt->dst.dev->features & NETIF_F_V6_CSUM &&
>  	    !exthdrlen)

Ok, that works as fix. No crash and checksums are ok.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

[PATCH net] Revert "ipv6: Allow for partial checksums on non-ufo packets"

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Wolfgang Walter reported crashes using traceroute6, root caused to this
commit.

UDP packets can be sent from RAW sockets.

Note that the faulty commit addressed a non problem, as normal
UDP v6 packets use udp_v6_send_skb() which correctly setup
CHECKSUM_PARTIAL properly if device has the correct feature.

It seems not worth trying to 'fix' the raw path.

Fixes: 32dce968dd98 ("ipv6: Allow for partial checksums on non-ufo packets")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Wolfgang Walter <linux@stwm.de>
Cc: Vlad Yasevich <vyasevich@gmail.com>
---
 net/ipv6/ip6_output.c |   11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 7fde1f265c90..1fe5c687328e 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1218,7 +1218,6 @@ static int __ip6_append_data(struct sock *sk,
 	u32 tskey = 0;
 	struct rt6_info *rt = (struct rt6_info *)cork->dst;
 	struct ipv6_txoptions *opt = v6_cork->opt;
-	int csummode = CHECKSUM_NONE;
 
 	skb = skb_peek_tail(queue);
 	if (!skb) {
@@ -1275,14 +1274,6 @@ emsgsize:
 			tskey = sk->sk_tskey++;
 	}
 
-	/* If this is the first and only packet and device
-	 * supports checksum offloading, let's use it.
-	 */
-	if (!skb && sk->sk_protocol == IPPROTO_UDP &&
-	    length + fragheaderlen < mtu &&
-	    rt->dst.dev->features & NETIF_F_V6_CSUM &&
-	    !exthdrlen)
-		csummode = CHECKSUM_PARTIAL;
 	/*
 	 * Let's try using as much space as possible.
 	 * Use MTU if total length of the message fits into the MTU.
@@ -1396,7 +1387,7 @@ alloc_new_skb:
 			 *	Fill in the control structures
 			 */
 			skb->protocol = htons(ETH_P_IPV6);
-			skb->ip_summed = csummode;
+			skb->ip_summed = CHECKSUM_NONE;
 			skb->csum = 0;
 			/* reserve for fragmentation and ipsec header */
 			skb_reserve(skb, hh_len + sizeof(struct frag_hdr) +

Re: [PATCH net] Revert "ipv6: Allow for partial checksums on non-ufo packets"

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 08 May 2015 15:22:45 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Wolfgang Walter reported crashes using traceroute6, root caused to this
> commit.
> 
> UDP packets can be sent from RAW sockets.
> 
> Note that the faulty commit addressed a non problem, as normal
> UDP v6 packets use udp_v6_send_skb() which correctly setup
> CHECKSUM_PARTIAL properly if device has the correct feature.
> 
> It seems not worth trying to 'fix' the raw path.
> 
> Fixes: 32dce968dd98 ("ipv6: Allow for partial checksums on non-ufo packets")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Wolfgang Walter <linux@stwm.de>

Vlad, please review this.

Thanks.

Re: [PATCH net] Revert "ipv6: Allow for partial checksums on non-ufo packets"

[Vlad Yasevich]

On 05/08/2015 06:22 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Wolfgang Walter reported crashes using traceroute6, root caused to this
> commit.
> 
> UDP packets can be sent from RAW sockets.
> 
> Note that the faulty commit addressed a non problem, as normal
> UDP v6 packets use udp_v6_send_skb() which correctly setup
> CHECKSUM_PARTIAL properly if device has the correct feature.

Actually, they didn't.  udp_v6_send_skb() uses ip_summed from
the skb, which is populated by __ip6_append_data.  That in turn
always set it CHECKSUM_NONE.

The code in question looks identically to that of __ip_append_data().
The problem is similar in both places.

-vlad
> 
> It seems not worth trying to 'fix' the raw path.
> 
> Fixes: 32dce968dd98 ("ipv6: Allow for partial checksums on non-ufo packets")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Wolfgang Walter <linux@stwm.de>
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> ---
>  net/ipv6/ip6_output.c |   11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 7fde1f265c90..1fe5c687328e 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1218,7 +1218,6 @@ static int __ip6_append_data(struct sock *sk,
>  	u32 tskey = 0;
>  	struct rt6_info *rt = (struct rt6_info *)cork->dst;
>  	struct ipv6_txoptions *opt = v6_cork->opt;
> -	int csummode = CHECKSUM_NONE;
>  
>  	skb = skb_peek_tail(queue);
>  	if (!skb) {
> @@ -1275,14 +1274,6 @@ emsgsize:
>  			tskey = sk->sk_tskey++;
>  	}
>  
> -	/* If this is the first and only packet and device
> -	 * supports checksum offloading, let's use it.
> -	 */
> -	if (!skb && sk->sk_protocol == IPPROTO_UDP &&
> -	    length + fragheaderlen < mtu &&
> -	    rt->dst.dev->features & NETIF_F_V6_CSUM &&
> -	    !exthdrlen)
> -		csummode = CHECKSUM_PARTIAL;
>  	/*
>  	 * Let's try using as much space as possible.
>  	 * Use MTU if total length of the message fits into the MTU.
> @@ -1396,7 +1387,7 @@ alloc_new_skb:
>  			 *	Fill in the control structures
>  			 */
>  			skb->protocol = htons(ETH_P_IPV6);
> -			skb->ip_summed = csummode;
> +			skb->ip_summed = CHECKSUM_NONE;
>  			skb->csum = 0;
>  			/* reserve for fragmentation and ipsec header */
>  			skb_reserve(skb, hh_len + sizeof(struct frag_hdr) +
> 
> 

Re: [PATCH net] Revert "ipv6: Allow for partial checksums on non-ufo packets"

[Eric Dumazet]

On Tue, 2015-05-12 at 14:05 -0400, Vlad Yasevich wrote:
> On 05/08/2015 06:22 PM, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > Wolfgang Walter reported crashes using traceroute6, root caused to this
> > commit.
> > 
> > UDP packets can be sent from RAW sockets.
> > 
> > Note that the faulty commit addressed a non problem, as normal
> > UDP v6 packets use udp_v6_send_skb() which correctly setup
> > CHECKSUM_PARTIAL properly if device has the correct feature.
> 
> Actually, they didn't.  udp_v6_send_skb() uses ip_summed from
> the skb, which is populated by __ip6_append_data.  That in turn
> always set it CHECKSUM_NONE.
> 
> The code in question looks identically to that of __ip_append_data().
> The problem is similar in both places.

Well, one is crashing the host. IPv4 raw must have been OK, not IPv6.

As I explained in the changelog, you can try to fix
rawv6_push_pending_frames()

My opinion is that a reasonable strategy is to revert for stable
kernels.

Then if you can fix rawv6_push_pending_frames() for net-next, I will be
happy to review your patch.