netdev - Re: [Patch net-next] net: dump more useful information in netdev_rx_csum

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2fe5fbef-ece4-1c0a-189b-514074a7f46b@huawei.com>
Date:   Sat, 10 Nov 2018 09:39:11 +0800
From:   Yunsheng Lin <linyunsheng@...wei.com>
To:     Cong Wang <xiyou.wangcong@...il.com>, <netdev@...r.kernel.org>
Subject: Re: [Patch net-next] net: dump more useful information in
 netdev_rx_csum_fault()

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 <xiyou.wangcong@...il.com>
> ---
>  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 : "<unknown>");
> +		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?

Also, dev->features was dumped before this patch, why remove it?


>  		dump_stack();
>  	}
>  }
> @@ -5779,7 +5785,7 @@ __sum16 __skb_gro_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);
>  	}
>  
>  	NAPI_GRO_CB(skb)->csum = wsum;
> diff --git a/net/sunrpc/socklib.c b/net/sunrpc/socklib.c
> index 9062967575c4..7e55cfc69697 100644
> --- a/net/sunrpc/socklib.c
> +++ b/net/sunrpc/socklib.c
> @@ -175,7 +175,7 @@ int csum_partial_copy_to_xdr(struct xdr_buf *xdr, struct sk_buff *skb)
>  		return -1;
>  	if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
>  	    !skb->csum_complete_sw)
> -		netdev_rx_csum_fault(skb->dev);
> +		netdev_rx_csum_fault(skb->dev, skb);
>  	return 0;
>  no_checksum:
>  	if (xdr_partial_copy_from_skb(xdr, 0, &desc, xdr_skb_read_bits) < 0)
>