| Message ID | 1375333461.10515.133.camel@edumazet-glaptop |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Aug 1, 2013 at 1:04 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Thu, 2013-08-01 at 12:41 +0800, Ming Lei wrote: > >> From my trace result, lots of linear SKBs are cloned or header-cloned, so >> it needs skb copy too. >> >> Is it normal in xmit path to see cloned SKBs for driver? If not, I can add check >> to avoid allocation of 8 bytes header for non-cloned skb. > > Existing code is not very friendly and very complex. > > Sure TCP stack does a clone for every skb from socket write queue, > but header should be available for pushing 8 bytes. > > The !skb_cloned(skb) test should be removed if the memmove() is not > needed. > > Could you try following patch ? Tested-by: Ming Lei <ming.lei@canonical.com> This patch does work, and it will make the patch 4/4 become very simple, :-) I will rewrite this one against your patch. Thanks, -- Ming Lei -- 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
On Thu, 2013-08-01 at 16:10 +0800, Ming Lei wrote: > On Thu, Aug 1, 2013 at 1:04 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > > On Thu, 2013-08-01 at 12:41 +0800, Ming Lei wrote: > > > >> From my trace result, lots of linear SKBs are cloned or header-cloned, so > >> it needs skb copy too. > >> > >> Is it normal in xmit path to see cloned SKBs for driver? If not, I can add check > >> to avoid allocation of 8 bytes header for non-cloned skb. > > > > Existing code is not very friendly and very complex. > > > > Sure TCP stack does a clone for every skb from socket write queue, > > but header should be available for pushing 8 bytes. > > > > The !skb_cloned(skb) test should be removed if the memmove() is not > > needed. > > > > Could you try following patch ? > > Tested-by: Ming Lei <ming.lei@canonical.com> > > This patch does work, and it will make the patch 4/4 become very > simple, :-) > > I will rewrite this one against your patch. OK I'll post an official patch then. I presume you got performance increase, since we no longer do a copy of the TCP packets. -- 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
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 2bc87e3..e2120d6 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -1166,31 +1166,18 @@ ax88179_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags) int frame_size = dev->maxpacket; int mss = skb_shinfo(skb)->gso_size; int headroom; - int tailroom; tx_hdr1 = skb->len; tx_hdr2 = mss; if (((skb->len + 8) % frame_size) == 0) tx_hdr2 |= 0x80008000; /* Enable padding */ - headroom = skb_headroom(skb); - tailroom = skb_tailroom(skb); + headroom = skb_headroom(skb) - 8; - if (!skb_header_cloned(skb) && - !skb_cloned(skb) && - (headroom + tailroom) >= 8) { - if (headroom < 8) { - skb->data = memmove(skb->head + 8, skb->data, skb->len); - skb_set_tail_pointer(skb, skb->len); - } - } else { - struct sk_buff *skb2; - - skb2 = skb_copy_expand(skb, 8, 0, flags); + if ((skb_header_cloned(skb) || headroom < 0) && + pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) { dev_kfree_skb_any(skb); - skb = skb2; - if (!skb) - return NULL; + return NULL; } skb_push(skb, 4);