Patchwork [Xen-devel] compound skb frag pages appearing in start_xmit

login
register
mail settings
Submitter Ian Campbell
Date Oct. 10, 2012, 12:29 p.m.
Message ID <1349872149.10070.31.camel@zakaz.uk.xensource.com>
Download mbox | patch
Permalink /patch/190631/
State RFC
Delegated to: David Miller
Headers show

Comments

Ian Campbell - Oct. 10, 2012, 12:29 p.m.
On Wed, 2012-10-10 at 13:24 +0100, Sander Eikelenboom wrote:
> Wednesday, October 10, 2012, 12:13:04 PM, you wrote:
> 
> > On Tue, 2012-10-09 at 15:40 +0100, Ian Campbell wrote:
> >> On Tue, 2012-10-09 at 15:27 +0100, Eric Dumazet wrote:
> >> > On Tue, 2012-10-09 at 15:17 +0100, Ian Campbell wrote:
> >> > 
> >> > > Does the higher order pages effectively reduce the number of frags which
> >> > > are in use? e.g if MAX_SKB_FRAGS is 16, then for order-0 pages you could
> >> > > have 64K worth of frag data.
> >> > > 
> >> > > If we switch to order-3 pages everywhere then can the skb contain 512K
> >> > > of data, or does the effective maximum number of frags in an skb reduce
> >> > > to 2?
> >> > 
> >> > effective number of frags reduce to 2 or 3
> >> > 
> >> > (We still limit GSO packets to ~63536 bytes)
> >> 
> >> Great! Then I think the fix is more/less trivial...
> 
> > The following seems to work for me.
> 
> But it doesn't seem to work for me ... dmesg attached.

> [  191.777994] ------------[ cut here ]------------
> [  191.784245] kernel BUG at drivers/net/xen-netback/netback.c:481!

Looks like that BUG_ON is a little aggressive. It'll trigger if the data
happens to end on a frame boundary. Hopefully this will fix it for you:



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sander Eikelenboom - Oct. 10, 2012, 1:31 p.m.
Wednesday, October 10, 2012, 2:29:09 PM, you wrote:

> On Wed, 2012-10-10 at 13:24 +0100, Sander Eikelenboom wrote:
>> Wednesday, October 10, 2012, 12:13:04 PM, you wrote:
>> 
>> > On Tue, 2012-10-09 at 15:40 +0100, Ian Campbell wrote:
>> >> On Tue, 2012-10-09 at 15:27 +0100, Eric Dumazet wrote:
>> >> > On Tue, 2012-10-09 at 15:17 +0100, Ian Campbell wrote:
>> >> > 
>> >> > > Does the higher order pages effectively reduce the number of frags which
>> >> > > are in use? e.g if MAX_SKB_FRAGS is 16, then for order-0 pages you could
>> >> > > have 64K worth of frag data.
>> >> > > 
>> >> > > If we switch to order-3 pages everywhere then can the skb contain 512K
>> >> > > of data, or does the effective maximum number of frags in an skb reduce
>> >> > > to 2?
>> >> > 
>> >> > effective number of frags reduce to 2 or 3
>> >> > 
>> >> > (We still limit GSO packets to ~63536 bytes)
>> >> 
>> >> Great! Then I think the fix is more/less trivial...
>> 
>> > The following seems to work for me.
>> 
>> But it doesn't seem to work for me ... dmesg attached.

>> [  191.777994] ------------[ cut here ]------------
>> [  191.784245] kernel BUG at drivers/net/xen-netback/netback.c:481!

> Looks like that BUG_ON is a little aggressive. It'll trigger if the data
> happens to end on a frame boundary. Hopefully this will fix it for you:

Yes it does !
Thanks .. will recompile and test the netfront case as well

--
Sander

> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index d747e30..f2d6b78 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -477,7 +477,7 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
>                 size -= bytes;
>  
>                 /* Next frame */
> -               if (offset == PAGE_SIZE) {
> +               if (offset == PAGE_SIZE && size) {
>                         BUG_ON(!PageCompound(page));
>                         page++;
>                         offset = 0;




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index d747e30..f2d6b78 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -477,7 +477,7 @@  static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
 		size -= bytes;
 
 		/* Next frame */
-		if (offset == PAGE_SIZE) {
+		if (offset == PAGE_SIZE && size) {
 			BUG_ON(!PageCompound(page));
 			page++;
 			offset = 0;