Re: smsc75xx & smsc95xx, setting skb->truesize correctly?
flat view
From: Eric Dumazet <hidden>
Date: 2012-08-21 09:46:46
Subsystem:
networking drivers, the rest, usb networking drivers, usb smsc95xx ethernet driver · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds, Steve Glendinning
On Mon, 2012-08-20 at 17:57 +0300, Jussi Kivilinna wrote:
Hello, Is setting skb->truesize in smsc75xx and smsc95xx correct? In smsc75xx/smsc95xx_rx_fixup(), input skb containing multiple packets is cloned and truesize for each clone is set to packet-size + sizeof(struct sk_buff), but input skb has minimum allocation size of 9000 bytes (MAX_SINGLE_PACKET_SIZE) and maximum of 18944 bytes (DEFAULT_HS_BURST_CAP_SIZE) (+ NET_IP_ALIGN). Doesn't this cause truesize to be underestimated?
This has been discussed in a "TCP transmit performance regression" thread some weeks ago. More generally, skb_clone() is not a good idea in rx path. I dont have the hardware so cannot send a formal patch.
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index b1112e7..3d9566f 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c@@ -1080,30 +1080,17 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb) return 0; } - /* last frame in this batch */ - if (skb->len == size) { - if (dev->net->features & NETIF_F_RXCSUM) - smsc95xx_rx_csum_offload(skb); - skb_trim(skb, skb->len - 4); /* remove fcs */ - skb->truesize = size + sizeof(struct sk_buff); - - return 1; - } - - ax_skb = skb_clone(skb, GFP_ATOMIC); + ax_skb = netdev_alloc_skb_ip_align(dev->net, size); if (unlikely(!ax_skb)) { netdev_warn(dev->net, "Error allocating skb\n"); return 0; } - ax_skb->len = size; - ax_skb->data = packet; - skb_set_tail_pointer(ax_skb, size); + memcpy(skb_put(ax_skb, size), packet, size); if (dev->net->features & NETIF_F_RXCSUM) smsc95xx_rx_csum_offload(ax_skb); - skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */ - ax_skb->truesize = size + sizeof(struct sk_buff); + __skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */ usbnet_skb_return(dev, ax_skb); }