From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yunsheng Lin Subject: Re: [Patch net-next] net: dump more useful information in netdev_rx_csum_fault() Date: Sat, 10 Nov 2018 10:02:06 +0800 Message-ID: <17be1af6-9aaf-593c-a68f-932d63aefbd5@huawei.com> References: <20181109194333.19949-1-xiyou.wangcong@gmail.com> <2fe5fbef-ece4-1c0a-189b-514074a7f46b@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: Linux Kernel Network Developers To: Cong Wang Return-path: Received: from szxga07-in.huawei.com ([45.249.212.35]:49535 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728595AbeKJLpa (ORCPT ); Sat, 10 Nov 2018 06:45:30 -0500 Received: from DGGEMS405-HUB.china.huawei.com (unknown [172.30.72.59]) by Forcepoint Email with ESMTP id 608702DED6021 for ; Sat, 10 Nov 2018 10:02:10 +0800 (CST) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2018/11/10 9:42, Cong Wang wrote: > On Fri, Nov 9, 2018 at 5:39 PM Yunsheng Lin wrote: >> >> On 2018/11/10 3:43, Cong Wang wrote: >>> Currently netdev_rx_csum_fault() only shows a device name, >>> we need more information about the skb for debugging. >>> >>> Sample output: >>> >>> ens3: hw csum failure >>> dev features: 0x0000000000014b89 >>> skb len=84 data_len=0 gso_size=0 gso_type=0 ip_summed=0 csum=0, csum_complete_sw=0, csum_valid=0 >>> >>> Signed-off-by: Cong Wang >>> --- >>> include/linux/netdevice.h | 5 +++-- >>> net/core/datagram.c | 6 +++--- >>> net/core/dev.c | 10 ++++++++-- >>> net/sunrpc/socklib.c | 2 +- >>> 4 files changed, 15 insertions(+), 8 deletions(-) >>> >>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >>> index 857f8abf7b91..fabcd9fa6cf7 100644 >>> --- a/include/linux/netdevice.h >>> +++ b/include/linux/netdevice.h >>> @@ -4332,9 +4332,10 @@ static inline bool can_checksum_protocol(netdev_features_t features, >>> } >>> >>> #ifdef CONFIG_BUG >>> -void netdev_rx_csum_fault(struct net_device *dev); >>> +void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb); >>> #else >>> -static inline void netdev_rx_csum_fault(struct net_device *dev) >>> +static inline void netdev_rx_csum_fault(struct net_device *dev, >>> + struct sk_buff *skb) >>> { >>> } >>> #endif >>> diff --git a/net/core/datagram.c b/net/core/datagram.c >>> index 57f3a6fcfc1e..d8f4d55cd6c5 100644 >>> --- a/net/core/datagram.c >>> +++ b/net/core/datagram.c >>> @@ -736,7 +736,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len) >>> if (likely(!sum)) { >>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) && >>> !skb->csum_complete_sw) >>> - netdev_rx_csum_fault(skb->dev); >>> + netdev_rx_csum_fault(skb->dev, skb); >>> } >>> if (!skb_shared(skb)) >>> skb->csum_valid = !sum; >>> @@ -756,7 +756,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb) >>> if (likely(!sum)) { >>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) && >>> !skb->csum_complete_sw) >>> - netdev_rx_csum_fault(skb->dev); >>> + netdev_rx_csum_fault(skb->dev, skb); >>> } >>> >>> if (!skb_shared(skb)) { >>> @@ -810,7 +810,7 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb, >>> >>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) && >>> !skb->csum_complete_sw) >>> - netdev_rx_csum_fault(NULL); >>> + netdev_rx_csum_fault(NULL, skb); >>> } >>> return 0; >>> fault: >>> diff --git a/net/core/dev.c b/net/core/dev.c >>> index 0ffcbdd55fa9..2b337df26117 100644 >>> --- a/net/core/dev.c >>> +++ b/net/core/dev.c >>> @@ -3091,10 +3091,16 @@ EXPORT_SYMBOL(__skb_gso_segment); >>> >>> /* Take action when hardware reception checksum errors are detected. */ >>> #ifdef CONFIG_BUG >>> -void netdev_rx_csum_fault(struct net_device *dev) >>> +void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb) >>> { >>> if (net_ratelimit()) { >>> pr_err("%s: hw csum failure\n", dev ? dev->name : ""); >>> + if (dev) >>> + pr_err("dev features: %pNF\n", &dev->features); >>> + pr_err("skb len=%d data_len=%d gso_size=%d gso_type=%d ip_summed=%d csum=%x, csum_complete_sw=%d, csum_valid=%d\n", >>> + skb->len, skb->data_len, skb_shinfo(skb)->gso_size, >>> + skb_shinfo(skb)->gso_type, skb->ip_summed, skb->csum, >>> + skb->csum_complete_sw, skb->csum_valid); >> >> >> This function also have the netdev available, use netdev_err to log the error? > > It is apparently not me who picked pr_err() from the beginning, > I just follow that pr_err(). If you are not happy with it, please send > a followup. Yes, but perhaps it is something to improve. When using the netdev, then maybe it does not have to check if dev is null, because netdev_err has handled the netdev being NULL case. Maybe I missed something that netdev can not be used here? If not, maybe I can send a followup. > > >> >> Also, dev->features was dumped before this patch, why remove it? > > Seriously? Where do I remove it? Please be specific. :) Sorry, I missed that, I thought it was removed when adding the new log. > > . >