From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cong Wang Subject: [Patch net] net: invert the check of detecting hardware RX checksum fault Date: Thu, 15 Nov 2018 15:16:02 -0800 Message-ID: <20181115231602.18328-1-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Cong Wang , Herbert Xu , Tom Herbert , Eric Dumazet To: netdev@vger.kernel.org Return-path: Received: from mail-pg1-f173.google.com ([209.85.215.173]:41613 "EHLO mail-pg1-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388907AbeKPJ0N (ORCPT ); Fri, 16 Nov 2018 04:26:13 -0500 Received: by mail-pg1-f173.google.com with SMTP id 70so9689807pgh.8 for ; Thu, 15 Nov 2018 15:16:20 -0800 (PST) Sender: netdev-owner@vger.kernel.org List-ID: The following evidences indicate this check is likely wrong: 1. In the assignment "skb->csum_valid = !sum", sum==0 indicates a valid checksum. 2. __skb_checksum_complete() always returns sum, and TCP packets are dropped only when it returns non-zero. So non-zero indicates a failure. 3. In __skb_checksum_validate_complete(), we have a nearly same check, where zero is considered as success. 4. csum_fold() already does the one’s complement, this indicates 0 should be considered as a successful validation. 5. We have triggered this fault for many times, but InCsumErrors field in /proc/net/snmp remains 0. Base on the above, non-zero should be used as a checksum mismatch. I tested this with mlx5 driver, no warning or InCsumErrors after 1 hour. Fixes: fb286bb2990a ("[NET]: Detect hardware rx checksum faults correctly") Cc: Herbert Xu Cc: Tom Herbert Cc: Eric Dumazet Signed-off-by: Cong Wang --- net/core/datagram.c | 4 ++-- net/core/dev.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/net/core/datagram.c b/net/core/datagram.c index 57f3a6fcfc1e..e542a9a212a7 100644 --- a/net/core/datagram.c +++ b/net/core/datagram.c @@ -733,7 +733,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len) __sum16 sum; sum = csum_fold(skb_checksum(skb, 0, len, skb->csum)); - if (likely(!sum)) { + if (unlikely(sum)) { if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) && !skb->csum_complete_sw) netdev_rx_csum_fault(skb->dev); @@ -753,7 +753,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb) /* skb->csum holds pseudo checksum */ sum = csum_fold(csum_add(skb->csum, csum)); - if (likely(!sum)) { + if (unlikely(sum)) { if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) && !skb->csum_complete_sw) netdev_rx_csum_fault(skb->dev); diff --git a/net/core/dev.c b/net/core/dev.c index 0ffcbdd55fa9..c76dee329844 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5776,7 +5776,7 @@ __sum16 __skb_gro_checksum_complete(struct sk_buff *skb) /* NAPI_GRO_CB(skb)->csum holds pseudo checksum */ sum = csum_fold(csum_add(NAPI_GRO_CB(skb)->csum, wsum)); - if (likely(!sum)) { + if (unlikely(sum)) { if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) && !skb->csum_complete_sw) netdev_rx_csum_fault(skb->dev); -- 2.19.1