Re: [RFC v2] mac80211: assign needed_headroom/tailroom for netdevs

[Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>]

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


> Where is the patch that adds these new members to struct netdevice,
> and where is the code that uses these new values?

Inlined below this time.

> Anyways I see your basic idea and this may be the best way to handle
> the problem.  The invariants would be:
> 
> 1) LL_MAX_HEADER has to include all of these bits.

Yes. Well, it has to be long enough for it, yes, but mac80211 has
drivers that want 100 bytes more headroom for themselves and probably
won't get added to LL_MAX_HEADER. But still putting that much into
needed_headroom is beneficial for many cases even if that means that
some packets will have to be copy-expanded. This, however, means that
you cannot put an assertion into dev_queue_xmit.

Also, taking into account tailroom isn't trivial because you need to
reserve the headroom but not the tailroom etc. I'm still thinking of a
way to avoid reserving tailroom completely when the hardware will handle
the crypto-checksumming but right now we don't have hardware that does
all crypto, only some algorithms. But we can even update
needed_headroom/tailroom on the fly.

> 2) LL_RESERVED_SPACE*() has to take the new needed_headroom
>    into account.

Check. I added LL_ALLOCATED_SPACE() for the tailroom and used that in
places for the allocation. I'm fairly sure I didn't get all of them
though.

> Your patch which I can't find, which adds netdev->needed_*, probably
> does all of that.  But I'm just making sure :-)
> 
> Note that what would be really nice is if we could assert, in
> dev_queue_xmit, that the SKB has all of the necessary headroom, and
> give a WARN_ON_ONCE() backtrace if not.
> 
> If we can ensure that, things like mac80211 and others will not need
> to skb_realloc_headroom() or anything like that unless they need to
> modify packet contents after skb->data and the packet is shared
> (ie. the pskb_expand_headroom(skb, 0, 0, GFP_*) case the TSO drivers
> use).

That would be nice, but I don't think it's possible.

johannes

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