netdev - Re: smsc75xx & smsc95xx, setting skb->truesize correctly?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1345617286.5158.576.camel@edumazet-glaptop>
Date:	Wed, 22 Aug 2012 08:34:46 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Jussi Kivilinna <jussi.kivilinna@...et.fi>
Cc:	netdev@...r.kernel.org,
	Steve Glendinning <steve.glendinning@...well.net>
Subject: Re: smsc75xx & smsc95xx, setting skb->truesize correctly?

On Wed, 2012-08-22 at 08:35 +0300, Jussi Kivilinna wrote:
> Quoting Eric Dumazet <eric.dumazet@...il.com>:
> 
> > 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.
> 
> So all skb_clone use in drivers/net/usb/ should be removed/replaced  
> with following?

Doing a copy might be expensive on some low end hardware, so I can
understand why this skb_clone() idea was deployed years ago.

Gigabit r8169 has to perform the copy because of security issue, and so
far nobody complained of performance impact.

Best thing would be to not use large buffers from the beginning,
and switch to a frag idea.

(A large frame would needs 2 or 3 medium buffers, as done in ath9k)

Check https://gerrit.chromium.org/gerrit/#/c/18412/ 
and commit 0d95521ea74735826cb2e28bebf6a07392c75bfa (ath9k: use split rx
buffers to get rid of order-1 skb allocations)



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