[NET] Add missing newline for skb_*_panic

[NET] Add missing newline for skb_*_panic

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

Hi Dave:

Here is a trivial patch to make the skb_under_panic/skb_over_panic
messages to look nicer.  As it is their printk is joined onto the
first line of the BUG output because of a missing newline.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 714 bytes --]

===== net/core/skbuff.c 1.44 vs edited =====
--- 1.44/net/core/skbuff.c	2005-03-12 07:32:26 +11:00
+++ edited/net/core/skbuff.c	2005-04-21 15:06:12 +10:00
@@ -86,7 +86,7 @@
  */
 void skb_over_panic(struct sk_buff *skb, int sz, void *here)
 {
-	printk(KERN_INFO "skput:over: %p:%d put:%d dev:%s",
+	printk(KERN_INFO "skput:over: %p:%d put:%d dev:%s\n",
 		here, skb->len, sz, skb->dev ? skb->dev->name : "<NULL>");
 	BUG();
 }
@@ -102,7 +102,7 @@
 
 void skb_under_panic(struct sk_buff *skb, int sz, void *here)
 {
-	printk(KERN_INFO "skput:under: %p:%d put:%d dev:%s",
+	printk(KERN_INFO "skput:under: %p:%d put:%d dev:%s\n",
                here, skb->len, sz, skb->dev ? skb->dev->name : "<NULL>");
 	BUG();
 }

Re: [NET] Add missing newline for skb_*_panic

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

Herbert Xu wrote:
> Here is a trivial patch to make the skb_under_panic/skb_over_panic
> messages to look nicer.  As it is their printk is joined onto the
> first line of the BUG output because of a missing newline.

How about this one instead? Besides the missing newlines, it adds
head/data/tail/end to the output, which can give valuable hints
about what happend before.

Signed-off-by: Patrick McHardy <kaber@trash.net>

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1092 bytes --]

===== net/core/skbuff.c 1.44 vs edited =====
--- 1.44/net/core/skbuff.c	2005-03-11 21:32:26 +01:00
+++ edited/net/core/skbuff.c	2005-04-22 00:08:09 +02:00
@@ -86,8 +86,10 @@
  */
 void skb_over_panic(struct sk_buff *skb, int sz, void *here)
 {
-	printk(KERN_INFO "skput:over: %p:%d put:%d dev:%s",
-		here, skb->len, sz, skb->dev ? skb->dev->name : "<NULL>");
+	printk(KERN_INFO "skb_over_panic: text:%p len:%d put:%d head:%p "
+	                 "data:%p tail:%p end:%p dev:%s\n",
+	       here, skb->len, sz, skb->head, skb->data, skb->tail, skb->end,
+	       skb->dev ? skb->dev->name : "<NULL>");
 	BUG();
 }
 
@@ -102,8 +104,10 @@
 
 void skb_under_panic(struct sk_buff *skb, int sz, void *here)
 {
-	printk(KERN_INFO "skput:under: %p:%d put:%d dev:%s",
-               here, skb->len, sz, skb->dev ? skb->dev->name : "<NULL>");
+	printk(KERN_INFO "skb_under_panic: text:%p len:%d put:%d head:%p "
+	                 "data:%p tail:%p end:%p dev:%s\n",
+	       here, skb->len, sz, skb->head, skb->data, skb->tail, skb->end,
+	       skb->dev ? skb->dev->name : "<NULL>");
 	BUG();
 }
 

Re: [NET] Add missing newline for skb_*_panic

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]

Patrick McHardy wrote:
> How about this one instead? Besides the missing newlines, it adds
> head/data/tail/end to the output, which can give valuable hints
> about what happend before.

While we're at it, lets also replace KERN_INFO by KERN_EMERG to
make sure the user gets to see it.

> Signed-off-by: Patrick McHardy <kaber@trash.net>



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1096 bytes --]

===== net/core/skbuff.c 1.44 vs edited =====
--- 1.44/net/core/skbuff.c	2005-03-11 21:32:26 +01:00
+++ edited/net/core/skbuff.c	2005-04-22 00:18:53 +02:00
@@ -86,8 +86,10 @@
  */
 void skb_over_panic(struct sk_buff *skb, int sz, void *here)
 {
-	printk(KERN_INFO "skput:over: %p:%d put:%d dev:%s",
-		here, skb->len, sz, skb->dev ? skb->dev->name : "<NULL>");
+	printk(KERN_EMERG "skb_over_panic: text:%p len:%d put:%d head:%p "
+	                  "data:%p tail:%p end:%p dev:%s\n",
+	       here, skb->len, sz, skb->head, skb->data, skb->tail, skb->end,
+	       skb->dev ? skb->dev->name : "<NULL>");
 	BUG();
 }
 
@@ -102,8 +104,10 @@
 
 void skb_under_panic(struct sk_buff *skb, int sz, void *here)
 {
-	printk(KERN_INFO "skput:under: %p:%d put:%d dev:%s",
-               here, skb->len, sz, skb->dev ? skb->dev->name : "<NULL>");
+	printk(KERN_EMERG "skb_under_panic: text:%p len:%d put:%d head:%p "
+	                  "data:%p tail:%p end:%p dev:%s\n",
+	       here, skb->len, sz, skb->head, skb->data, skb->tail, skb->end,
+	       skb->dev ? skb->dev->name : "<NULL>");
 	BUG();
 }
 

Re: [NET] Add missing newline for skb_*_panic

[Herbert Xu]

On Fri, Apr 22, 2005 at 12:20:08AM +0200, Patrick McHardy wrote:
> Patrick McHardy wrote:
> >How about this one instead? Besides the missing newlines, it adds
> >head/data/tail/end to the output, which can give valuable hints
> >about what happend before.
> 
> While we're at it, lets also replace KERN_INFO by KERN_EMERG to
> make sure the user gets to see it.

Thanks Patrick, this looks great.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [NET] Add missing newline for skb_*_panic

[Patrick McHardy]

Herbert Xu wrote:

> Thanks Patrick, this looks great.

BTW, Do you know if that ppp problem (Fw: [Bugme-new] [Bug 4381] New:
When i try to start a pppoe conn., crash at net/core/skbuff.c:91) was
already fixed? Otherwise I'd suggest that the reporter should capture
a crash again with this patch applied.

Regards
Patrick

Re: [NET] Add missing newline for skb_*_panic

[David S. Miller]

On Fri, 22 Apr 2005 00:20:08 +0200
Patrick McHardy <kaber@trash.net> wrote:

> Patrick McHardy wrote:
> > How about this one instead? Besides the missing newlines, it adds
> > head/data/tail/end to the output, which can give valuable hints
> > about what happend before.
> 
> While we're at it, lets also replace KERN_INFO by KERN_EMERG to
> make sure the user gets to see it.
> 
> > Signed-off-by: Patrick McHardy <kaber@trash.net>

Applied, thanks Patrick.

Re: [NET] Add missing newline for skb_*_panic

[Herbert Xu]

On Fri, Apr 22, 2005 at 01:25:22AM +0200, Patrick McHardy wrote:
> 
> BTW, Do you know if that ppp problem (Fw: [Bugme-new] [Bug 4381] New:
> When i try to start a pppoe conn., crash at net/core/skbuff.c:91) was
> already fixed? Otherwise I'd suggest that the reporter should capture
> a crash again with this patch applied.

Unfortunately the reporter says that he can't reproduce it.

However, I have the suspicion that this is really the same as the
vpnc/tun bug (4279) that prompted you to make the self-modification
fix to tun.c.

Now there is no doubt that your patch fixed a real bug in tun.c.
However, I don't think it could have caused the crash in 4279.
The reason is that the crash dump shows that the length that
was supplied to skb_put is in fact positive (0xe4 in one case
and 0xf4 in another).

As soon as I get confirmation from the submitter that he can
still reproduce this I'll get him to try your debugging patch.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [NET] Add missing newline for skb_*_panic

[Patrick McHardy]

Herbert Xu wrote:
> On Fri, Apr 22, 2005 at 01:25:22AM +0200, Patrick McHardy wrote:
> 
>>BTW, Do you know if that ppp problem (Fw: [Bugme-new] [Bug 4381] New:
>>When i try to start a pppoe conn., crash at net/core/skbuff.c:91) was
>>already fixed? Otherwise I'd suggest that the reporter should capture
>>a crash again with this patch applied.
> 
> 
> Unfortunately the reporter says that he can't reproduce it.
> 
> However, I have the suspicion that this is really the same as the
> vpnc/tun bug (4279) that prompted you to make the self-modification
> fix to tun.c.
> 
> Now there is no doubt that your patch fixed a real bug in tun.c.
> However, I don't think it could have caused the crash in 4279.
> The reason is that the crash dump shows that the length that
> was supplied to skb_put is in fact positive (0xe4 in one case
> and 0xf4 in another).

You're probably right, I only spent a few minutes looking for a
possible reason. Unfortunately the skb_over_panic() output wasn't
included in the report.

> As soon as I get confirmation from the submitter that he can
> still reproduce this I'll get him to try your debugging patch.

Great, thanks.

Regards
Patrick