Re: [PATCH] drivers/net/usb/r8152 fix broken rx checksums

[Paul Bolle]

On Sun, 2016-10-30 at 17:22 -0400, David Miller wrote:
>         3) "Fix broken RX checksums."  Commit header lines and commit
>    messages are proper English, therefore sentences should
>    begin with a capitalized letter and end with a period.

Commit messages should be proper English. But commit header lines
should not end with a period. The vast majority doesn't. Yes, I've just
checked.

How many newspaper headlines end with a period?

Thanks,


Paul Bolle

Re: [PATCH] drivers/net/usb/r8152 fix broken rx checksums

[Paul Bolle]

On Sun, 2016-10-30 at 17:22 -0400, David Miller wrote:
>         3) "Fix broken RX checksums."  Commit header lines and commit
>    messages are proper English, therefore sentences should
>    begin with a capitalized letter and end with a period.

Commit messages should be proper English. But commit header lines
should not end with a period. The vast majority doesn't. Yes, I've just
checked.

How many newspaper headlines end with a period?

Thanks,


Paul Bolle

Re: [PATCH] drivers/net/usb/r8152 fix broken rx checksums

[David Miller]

From: Mark Lord 
Date: Wed, 26 Oct 2016 18:36:57 -0400

> Patch attached (to deal with buggy mailer) and also below for review.

Please make your mailer work properly so that you can submit
patches properly which work inline, just like every other developer
does for the kernel.

Also please format your Subject line properly, it must be of the
form:

[PATCH net] r8152: Fix broken RX checksums.

The important parts are:

1) "[PATCH net]"  This says that it is a patch, and that it
   is targetting the 'net' GIT tree specifically.

2) "r8152: "  This indicates the "subsystem" that the patch
   specifically targets, in this case the r8152 driver.  It
   must end with a colon character then a space.

3) "Fix broken RX checksums."  Commit header lines and commit
   messages are proper English, therefore sentences should
   begin with a capitalized letter and end with a period.

Thanks.

Re: [PATCH] drivers/net/usb/r8152 fix broken rx checksums

[David Miller]

From: Mark Lord 
Date: Wed, 26 Oct 2016 18:36:57 -0400

> Patch attached (to deal with buggy mailer) and also below for review.

Please make your mailer work properly so that you can submit
patches properly which work inline, just like every other developer
does for the kernel.

Also please format your Subject line properly, it must be of the
form:

[PATCH net] r8152: Fix broken RX checksums.

The important parts are:

1) "[PATCH net]"  This says that it is a patch, and that it
   is targetting the 'net' GIT tree specifically.

2) "r8152: "  This indicates the "subsystem" that the patch
   specifically targets, in this case the r8152 driver.  It
   must end with a colon character then a space.

3) "Fix broken RX checksums."  Commit header lines and commit
   messages are proper English, therefore sentences should
   begin with a capitalized letter and end with a period.

Thanks.

Re: [PATCH] drivers/net/usb/r8152 fix broken rx checksums

[Mark Lord]

On 16-10-26 06:36 PM, Mark Lord wrote:

The r8152 driver has been broken since (approx) 3.6.16,


Correction:  broken since 3.16.xx.


when support was added for hardware rx checksum on newer chip versions.
Symptoms include random segfaults and silent data corruption over NFS.

This does not work on the VER_02 dongle I have here
when used with a slow embedded system CPU.
Google reveals others reporting similar issues on Raspberry Pi.

So, disable hardware rx checksum for VER_02, and fix
an obvious coding error for IPV6 checksums in the same function.

Because this bug results in silent data corruption,
it is a good candidate for back-porting to -stable >= 3.16.xx.
Patch attached (to deal with buggy mailer) and also below for review.

Signed-off-by: Mark Lord 

--- old/drivers/net/usb/r8152.c2016-09-30 04:20:43.0 -0400
+++ linux/drivers/net/usb/r8152.c2016-10-26 14:15:44.932517676 -0400
@@ -1645,7 +1645,7 @@
 u8 checksum = CHECKSUM_NONE;
 u32 opts2, opts3;

-if (tp->version == RTL_VER_01)
+if (tp->version == RTL_VER_01 || tp->version == RTL_VER_02)
 goto return_result;

 opts2 = le32_to_cpu(rx_desc->opts2);
@@ -1660,7 +1660,7 @@
 checksum = CHECKSUM_NONE;
 else
 checksum = CHECKSUM_UNNECESSARY;
-} else if (RD_IPV6_CS) {
+} else if (opts2 & RD_IPV6_CS) {
 if ((opts2 & RD_UDP_CS) && !(opts3 & UDPF))
 checksum = CHECKSUM_UNNECESSARY;
 else if ((opts2 & RD_TCP_CS) && !(opts3 & TCPF))



--
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com

Re: [PATCH] drivers/net/usb/r8152 fix broken rx checksums

[Mark Lord]

On 16-10-26 06:36 PM, Mark Lord wrote:

The r8152 driver has been broken since (approx) 3.6.16,


Correction:  broken since 3.16.xx.


when support was added for hardware rx checksum on newer chip versions.
Symptoms include random segfaults and silent data corruption over NFS.

This does not work on the VER_02 dongle I have here
when used with a slow embedded system CPU.
Google reveals others reporting similar issues on Raspberry Pi.

So, disable hardware rx checksum for VER_02, and fix
an obvious coding error for IPV6 checksums in the same function.

Because this bug results in silent data corruption,
it is a good candidate for back-porting to -stable >= 3.16.xx.
Patch attached (to deal with buggy mailer) and also below for review.

Signed-off-by: Mark Lord 

--- old/drivers/net/usb/r8152.c2016-09-30 04:20:43.0 -0400
+++ linux/drivers/net/usb/r8152.c2016-10-26 14:15:44.932517676 -0400
@@ -1645,7 +1645,7 @@
 u8 checksum = CHECKSUM_NONE;
 u32 opts2, opts3;

-if (tp->version == RTL_VER_01)
+if (tp->version == RTL_VER_01 || tp->version == RTL_VER_02)
 goto return_result;

 opts2 = le32_to_cpu(rx_desc->opts2);
@@ -1660,7 +1660,7 @@
 checksum = CHECKSUM_NONE;
 else
 checksum = CHECKSUM_UNNECESSARY;
-} else if (RD_IPV6_CS) {
+} else if (opts2 & RD_IPV6_CS) {
 if ((opts2 & RD_UDP_CS) && !(opts3 & UDPF))
 checksum = CHECKSUM_UNNECESSARY;
 else if ((opts2 & RD_TCP_CS) && !(opts3 & TCPF))



--
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com