diff mbox

r8169: fix checksum broken

Message ID 4CDD13BD.7060109@cn.fujitsu.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Shan Wei Nov. 12, 2010, 10:15 a.m. UTC
If r8196 received packets with invalid sctp/igmp(not tcp, udp) checksum, r8196 set skb->ip_summed
wit CHECKSUM_UNNECESSARY. This cause that upper protocol don't check checksum field.

I am not family with r8196 driver. I try to guess the meaning of RxProtoIP and IPFail.
RxProtoIP stands for received IPv4 packet that upper protocol is not tcp and udp. 
!(opts1 & IPFail) is true means that driver correctly to check checksum in IPv4 header.

If it's right, I think we should not set ip_summed wit CHECKSUM_UNNECESSARY for my sctp packets
with invalid checksum. 

If it's not right, please tell me. 


Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
 drivers/net/r8169.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

Comments

David Miller Nov. 12, 2010, 8:44 p.m. UTC | #1
From: Shan Wei <shanwei@cn.fujitsu.com>
Date: Fri, 12 Nov 2010 18:15:25 +0800

> If r8196 received packets with invalid sctp/igmp(not tcp, udp) checksum, r8196 set skb->ip_summed
> wit CHECKSUM_UNNECESSARY. This cause that upper protocol don't check checksum field.
> 
> I am not family with r8196 driver. I try to guess the meaning of RxProtoIP and IPFail.
> RxProtoIP stands for received IPv4 packet that upper protocol is not tcp and udp. 
> !(opts1 & IPFail) is true means that driver correctly to check checksum in IPv4 header.
> 
> If it's right, I think we should not set ip_summed wit CHECKSUM_UNNECESSARY for my sctp packets
> with invalid checksum. 
> 
> If it's not right, please tell me. 
> 
> 
> Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>

Francois, please review, it looks correct to my eyes.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Francois Romieu Nov. 12, 2010, 10:47 p.m. UTC | #2
Shan Wei <shanwei@cn.fujitsu.com> :
> If r8196 received packets with invalid sctp/igmp(not tcp, udp) checksum, r8196 set skb->ip_summed
> wit CHECKSUM_UNNECESSARY. This cause that upper protocol don't check checksum field.

...

Which kind of device do you use : PCI-E 8168 / 810x or PCI 8169 ?

Have a nice night.
Francois Romieu Nov. 12, 2010, 11:13 p.m. UTC | #3
Francois Romieu <romieu@fr.zoreil.com> :
[...]
> Which kind of device do you use : PCI-E 8168 / 810x or PCI 8169 ?

Wrong page. Forget it.

Acked-by: Francois Romieu <romieu@fr.zoreil.com>
David Miller Nov. 17, 2010, 7:54 p.m. UTC | #4
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 13 Nov 2010 00:13:25 +0100

> Francois Romieu <romieu@fr.zoreil.com> :
> [...]
>> Which kind of device do you use : PCI-E 8168 / 810x or PCI 8169 ?
> 
> Wrong page. Forget it.
> 
> Acked-by: Francois Romieu <romieu@fr.zoreil.com>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index d88ce9f..2cf6c2e 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4440,8 +4440,7 @@  static inline void rtl8169_rx_csum(struct sk_buff *skb, u32 opts1)
 	u32 status = opts1 & RxProtoMask;
 
 	if (((status == RxProtoTCP) && !(opts1 & TCPFail)) ||
-	    ((status == RxProtoUDP) && !(opts1 & UDPFail)) ||
-	    ((status == RxProtoIP) && !(opts1 & IPFail)))
+	    ((status == RxProtoUDP) && !(opts1 & UDPFail)))
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 	else
 		skb_checksum_none_assert(skb);