Re: [RFC net-next 0/6] net: configurable extra TX headroom for local egress

[David Keogh]

Thanks David, Dragos and Daniel for the feedback, and for the pointers to
prior art. There are really two questions in your replies: can the kernel
derive the reservation instead of an operator setting it, and is a new value
even needed given what already exists. Let me take both, and correct one thing
in my own cover letter while I am at it.

First, one correction to my cover letter: I overstated the TCP case. TCP already
reserves MAX_TCP_HEADER of headroom, so raising needed_headroom only reallocates
a TCP segment if the push would exceed that existing room, which a modest bump
does not. The packets that actually lose are the low-headroom ones the program
does not encapsulate (forwarded and received skbs) and stacked devices that
snapshot the value.

David (Woodhouse):

> could we try really hard to have it auto-tuning?

Auto-tuning is a reasonable generalisation and I want to be honest about why I
have not gone there. It is tractable in principle: the push helper, skb_cow_head()
in the encap path, knows the exact bytes it adds and whether it reallocated, so a
per-device high-water mark learned there would be attributable, roughly the
monotonic-raise shape of ip_tunnel_adj_headroom(). But making that correct brings
in bounding, decay when a map-driven push depth shrinks, per-program attribution
when several programs are attached, and a chunk of runtime state to get wrong, and
it still eats one reallocation on the first packet of every new depth.

That is a lot of machinery for what I actually need, which is narrow: reserve a
small fixed amount of egress headroom, on the order of 128 bytes, so a UDP encap
push stops reallocating every packet. It is set once for performance and never
touched at runtime. A static reservation matches that: no learning, no per-packet
state, nothing to converge, and it is trivially observable because the value is
exactly what the operator set. I would rather keep the mechanism that simple than
build an adaptive one to avoid configuring a constant. If the consensus is that
only an auto-tuning version is acceptable upstream I will take that on board, but
I think the static reserve is the proportionate fix for this problem and I would
like to make that case before reaching for a learning loop.

Dragos:

> Maybe this tx_extra_headroom could be declared as a BTF decl tag in the
> program. If this is the only usecase there's no need for a rtnetlink uAPI.

A BTF tag is an interesting way to attach the value to the program, and for a
purely BPF deployment it may be enough. The one caveat I would flag is the one
you raise yourself: a decl tag is static per program, while a program's push
depth can depend on map state at runtime, so the tag is really an upper bound
rather than an exact figure. For my case that is fine, since I want a single
fixed reservation rather than a per-program one, but it is worth bearing in mind
if the tag is meant to be tight. Whether a device-level attribute still earns its
place for non-BPF egress transforms is really a question for you and the other
maintainers; I do not have a stake in those paths myself.

Daniel:

> there was some work in this area here in the past ...
>   - b9ed315d3c4c ("netkit: Allow for configuring needed_{head,tail}room")
>   - b73b8146d7ff ("rtnetlink: add needed_{head,tail}room attributes")

Thanks, these are the right references and I had not connected them. For traffic
generated inside a Pod on netkit, the creation-time needed_headroom already feeds
LL_RESERVED_SPACE() at the allocation sites, so Cilium setting it from the control
plane does solve that case today. The gap I am trying to close is the same value
on an ordinary egress device (a physical NIC, plain veth) without raising the
expand-on-demand threshold for traffic that is not being encapsulated. Do you see
needed_headroom as sufficient for that too, or is a separate allocation-only
reserve worth it there?

On scope: the core patches (the field and the allocation-site macros) have no
in-tree consumer without a setter, so they do not stand alone for net-next, and I
will repost them together with whatever configuration surface we land on rather
than ahead of it. What I want to avoid is turning a fixed performance reservation
into an adaptive subsystem it does not need. If a static per-device value is
acceptable, the open question is only how it is set (the rtnetlink attribute as
posted, a BPF-side declaration for the BPF case, or reusing a control-plane path
like Cilium's), and I am happy to follow the list on that.

Thanks again, this was useful.

David