Re: [PATCH net-next 1/8] net: add tx_extra_headroom and LL_ALLOC_SPACE()

[Tianyi Gao <tianyi@cloudflare.com>]

On Sat, Nov 07, 2026 at 09:51:07AM -0500, Willem de Bruijn wrote:
> Tianyi Gao wrote:
> > From: David Keogh <dkeogh@amazon.com>
> >
> > Add a tx_extra_headroom field to struct net_device for that purpose,
>
> Main high level concern is just adding an extra field. What's next,
> do we need to add a field extra_extra_headroom?

Thanks for the review.

A user needs more room either in every packet or only in new skbs.
needed_headroom covers the first case and tx_extra_headroom the
second. I do not see a third case.

needed_headroom does not only set the headroom of new skbs. It is also
the minimum that ip_finish_output2() and ip_forward() require of
existing skbs. Raising it for new skbs also raises that minimum. With
XDP on a veth peer and needed_headroom raised to 256, throughput drops
by 2.8% to 5.8% for local TCP and by 4.0% to 20.8% for forwarded
traffic. The cover letter has the detailed analysis. veth also cannot
set needed_headroom from user space today.

pahole on x86_64 (defconfig, net/core/dev.o) shows that the field fills
the 2-byte hole after needed_headroom, so struct net_device does not
grow:

before:
	unsigned short             needed_headroom;      /*    60     2 */
	/* XXX 2 bytes hole, try to pack */
	/* size: 2688, cachelines: 42, members: 187 */

after:
	unsigned short             needed_headroom;      /*    60     2 */
	unsigned short             tx_extra_headroom;    /*    62     2 */
	/* size: 2688, cachelines: 42, members: 188 */

arm64 defconfig gives the same offsets, and the size stays 2752 bytes.

> > together with LL_ALLOC_SPACE() and LL_ALLOC_SPACE_EX(). These add the
>
> Since this already changes callers from LL_RESERVED_SPACE to
> LL_ALLOC_SPACE, could this additional use case be integrated more in
> the struct fields and helpers that are already there?

Changing only the helpers does not help. Without a new field, the
helpers can only add a fixed amount, which every allocation on every
device then pays for. Or they can derive it from needed_headroom,
which brings back the regressions above.

hard_header_len cannot carry it, because AF_PACKET and header_ops use
it as the link-layer header length.

> > +#define LL_ALLOC_SPACE_EX(dev, hlen) \
> > +	LL_RESERVED_SPACE_EX(dev, (hlen) + READ_ONCE((dev)->tx_extra_headroom))
> > +#define LL_ALLOC_SPACE(dev) \
> > +	LL_ALLOC_SPACE_EX(dev, (dev)->hard_header_len)
>
> If having to modify/add these helpers, it's probably a good time to
> switch to actual, type safe and readable, functions instead.

Agreed. v2 will make the new helpers static inline functions.