| Message ID | 20240506142835.3665037-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: usb: smsc95xx: stop lying about skb->truesize | expand |
On Mon, 6 May 2024 14:28:35 +0000 Eric Dumazet wrote: > - ax_skb->len = size; > - ax_skb->data = packet; > - skb_set_tail_pointer(ax_skb, size); > + skb_put(ax_skb, size - 4); > + memcpy(ax_skb->data, packet, size - 4); > > 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); I think this one's off: static void smsc95xx_rx_csum_offload(struct sk_buff *skb) { skb->csum = *(u16 *)(skb_tail_pointer(skb) - 2); skb->ip_summed = CHECKSUM_COMPLETE; skb_trim(skb, skb->len - 2); }
On Wed, May 8, 2024 at 2:41 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Mon, 6 May 2024 14:28:35 +0000 Eric Dumazet wrote: > > - ax_skb->len = size; > > - ax_skb->data = packet; > > - skb_set_tail_pointer(ax_skb, size); > > + skb_put(ax_skb, size - 4); > > + memcpy(ax_skb->data, packet, size - 4); > > > > 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); > > I think this one's off: > > static void smsc95xx_rx_csum_offload(struct sk_buff *skb) > { > skb->csum = *(u16 *)(skb_tail_pointer(skb) - 2); > skb->ip_summed = CHECKSUM_COMPLETE; > skb_trim(skb, skb->len - 2); > } Good catch, I will revise this patch accordingly, thanks !
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 2fa46baa589e5e87e12e145fe46268bdaf9fc219..54499c1a27f31805932cda9ed5ab5847f9c4fca1 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -1870,25 +1870,22 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb) 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); + /* Use "size - 4" to remove fcs */ + ax_skb = netdev_alloc_skb_ip_align(dev->net, size - 4); 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); + skb_put(ax_skb, size - 4); + memcpy(ax_skb->data, packet, size - 4); 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); usbnet_skb_return(dev, ax_skb); }
Some usb drivers try to set small skb->truesize and break core networking stacks. In this patch, I removed one of the skb->truesize override. I also replaced one skb_clone() by an allocation of a fresh and small skb, to get minimally sized skbs, like we did in commit 1e2c61172342 ("net: cdc_ncm: reduce skb truesize in rx path") and 4ce62d5b2f7a ("net: usb: ax88179_178a: stop lying about skb->truesize") Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Steve Glendinning <steve.glendinning@shawell.net> Cc: UNGLinuxDriver@microchip.com --- drivers/net/usb/smsc95xx.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)