On Fri, Nov 9, 2018 at 5:39 PM Yunsheng Lin <linyunsh...@huawei.com> 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 <xiyou.wangc...@gmail.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?
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.
>
> Also, dev->features was dumped before this patch, why remove it?
Seriously? Where do I remove it? Please be specific. :)
