Re: mac80211 truesize bugs

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]


> > I'm confused. The area should be used say with encryption when it's
> > actually necessary. Maybe there's always enough headroom for some
> > reason now?
> 
> That's possible.
> 
> There are paths, that, although they could take advantage of the
> fact that all of the non-header packet data is going to come from
> pages, they don't and allocate a full MTU skb->data area anyways.

Ok.

> On the other hand, if we provide ways for users to subvert the socket
> buffer limits, we might as well not try to limit anything.

Well, not exactly, since we'd only do that (at least in mac80211) when
packets are about to be sent to the hardware so they wouldn't live much
longer.

> Take a look at some ethernet drivers that implement TSO in a way that
> requires munging the IP headers for whatever reason.  If they need to
> COW the packet data in order to modify it, they always do this with
> pskb_expand_head(skb, 0, 0, GFP_*) exactly so that they don't modify
> the SKB data size, and exactly so that the skb->truesize value stays
> accurate.

Right, but we might actually need more space. Say you have a device that
requires 82 bytes headroom (yes, there are such devices) for their own
transmit header. Then you need maybe up to 30 bytes of 802.11 header
plus 8 byte ICV, so minus the 14 ethernet header that we remove we now
need well over 100 bytes headroom. On the other hand, not even
accounting the actual data buffer (you proposed to skb_orphan the skb
early) seems wrong as well. Worse yet, the needed transmit header
headroom is variable depending on devices.

One of the worst devices is the Broadcom one with 82 header and nowadays
actually DMAs this header from a separate memory location, so there this
won't happen, but can we guarantee that all devices are programmable
that way? We've seen lots of rather strange devices unfortunately...

> Try to find out exactly what's going wrong here, you seem to be
> close.  Once we know the precise issue we can talk about real
> changes to make this easier to cope with and debug in the future.

I'll try. I don't really see myself being that close ;)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]