net: make skb_truesize_bug() call WARN()

On Tue, 25 Nov 2008 16:42:03 +0000
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Tue, 2008-11-25 at 08:32 -0800, Arjan van de Ven wrote:
> > From 97a8ab520c378568e172c555a156b469137a3828 Mon Sep 17 00:00:00
> > 2001 From: Arjan van de Ven <arjan@linux.intel.com>
> > Date: Tue, 25 Nov 2008 08:30:04 -0800
> > Subject: [PATCH] net: make skb_truesize_bug() call WARN()
> > 
> > The truesize message check is important enough to make it print
> > "BUG" to the user console... lets also make it important enough to
> > spit a backtrace/module list etc so that kerneloops.org can track
> > them.
> > 
> > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> > ---
> >  net/core/skbuff.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index 391be91..1a1f4d1 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -149,7 +149,7 @@ void skb_under_panic(struct sk_buff *skb, int
> > sz, void *here) 
> >  void skb_truesize_bug(struct sk_buff *skb)
> >  {
> > -	printk(KERN_ERR "SKB BUG: Invalid truesize (%u) "
> > +	WARN(1, KERN_ERR "SKB BUG: Invalid truesize (%u) "
> >  	       "len=%u, sizeof(sk_buff)=%Zd\n",
> >  	       skb->truesize, skb->len, sizeof(struct sk_buff));
> >  }
> 
> The bug reports I've seen (with my Debian hat on, not relating to
> sfc) show that configurations that trigger this warning are likely to
> trigger many times over and there's nothing the user can do about
> it.  So by all means do make the warning more obvious, but please
> make it WARN_ON_ONCE and then use printk qualified by net_ratelimit().
> 

if there's no value in those subsequent printk's.. why not just this?

From 97a8ab520c378568e172c555a156b469137a3828 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Tue, 25 Nov 2008 08:30:04 -0800
Subject: [PATCH] net: make skb_truesize_bug() call WARN()

The truesize message check is important enough to make it print "BUG" to the
user console... lets also make it important enough to spit a backtrace/module list
etc so that kerneloops.org can track them.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 net/core/skbuff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

On Tue, 2008-11-25 at 09:54 -0800, Arjan van de Ven wrote:
> On Tue, 25 Nov 2008 16:42:03 +0000
> Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > On Tue, 2008-11-25 at 08:32 -0800, Arjan van de Ven wrote:
> > > From 97a8ab520c378568e172c555a156b469137a3828 Mon Sep 17 00:00:00
> > > 2001 From: Arjan van de Ven <arjan@linux.intel.com>
> > > Date: Tue, 25 Nov 2008 08:30:04 -0800
> > > Subject: [PATCH] net: make skb_truesize_bug() call WARN()
> > > 
> > > The truesize message check is important enough to make it print
> > > "BUG" to the user console... lets also make it important enough to
> > > spit a backtrace/module list etc so that kerneloops.org can track
> > > them.
[...]
> > The bug reports I've seen (with my Debian hat on, not relating to
> > sfc) show that configurations that trigger this warning are likely to
> > trigger many times over and there's nothing the user can do about
> > it.  So by all means do make the warning more obvious, but please
> > make it WARN_ON_ONCE and then use printk qualified by net_ratelimit().
> > 
> 
> if there's no value in those subsequent printk's.. why not just this?
[...]

I think there may be some value in them: users who want to help debug
this can experiment to see which configurations or traffic types do or
don't result in invalid truesize.

Ben.

From: Ben Hutchings <bhutchings@solarflare.com>

Date: Tue, 25 Nov 2008 18:02:19 +0000

> On Tue, 2008-11-25 at 09:54 -0800, Arjan van de Ven wrote:

> > On Tue, 25 Nov 2008 16:42:03 +0000

> > Ben Hutchings <bhutchings@solarflare.com> wrote:

> > 

> > > On Tue, 2008-11-25 at 08:32 -0800, Arjan van de Ven wrote:

> > > > From 97a8ab520c378568e172c555a156b469137a3828 Mon Sep 17 00:00:00

> > > > 2001 From: Arjan van de Ven <arjan@linux.intel.com>

> > > > Date: Tue, 25 Nov 2008 08:30:04 -0800

> > > > Subject: [PATCH] net: make skb_truesize_bug() call WARN()

> > > > 

> > > > The truesize message check is important enough to make it print

> > > > "BUG" to the user console... lets also make it important enough to

> > > > spit a backtrace/module list etc so that kerneloops.org can track

> > > > them.

> [...]

> > > The bug reports I've seen (with my Debian hat on, not relating to

> > > sfc) show that configurations that trigger this warning are likely to

> > > trigger many times over and there's nothing the user can do about

> > > it.  So by all means do make the warning more obvious, but please

> > > make it WARN_ON_ONCE and then use printk qualified by net_ratelimit().

> > > 

> > 

> > if there's no value in those subsequent printk's.. why not just this?

> [...]

> 

> I think there may be some value in them: users who want to help debug

> this can experiment to see which configurations or traffic types do or

> don't result in invalid truesize.


Agreed, netlimit'ing this is the best way to go.