Message-ID: <bd29d196-5854-4a0c-a78c-e4869a59b91f@kernel.org>
Date: Tue, 13 Jan 2026 19:52:53 +0100
From: Jesper Dangaard Brouer <hawk@...nel.org>
To: Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
 Jakub Sitnicki <jakub@...udflare.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Simon Horman <horms@...nel.org>,
 Michael Chan <michael.chan@...adcom.com>,
 Pavan Chebbi <pavan.chebbi@...adcom.com>, Andrew Lunn
 <andrew+netdev@...n.ch>, Tony Nguyen <anthony.l.nguyen@...el.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Saeed Mahameed <saeedm@...dia.com>, Leon Romanovsky <leon@...nel.org>,
 Tariq Toukan <tariqt@...dia.com>, Mark Bloch <mbloch@...dia.com>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>,
 Stanislav Fomichev <sdf@...ichev.me>, intel-wired-lan@...ts.osuosl.org,
 bpf@...r.kernel.org, kernel-team@...udflare.com,
 Jesse Brandeburg <jbrandeburg@...udflare.com>,
 Willem Ferguson <wferguson@...udflare.com>,
 Arthur Fabre <arthur@...hurfabre.com>
Subject: Re: [PATCH net-next 00/10] Call skb_metadata_set when skb->data
 points past metadata



On 13/01/2026 13.09, Paolo Abeni wrote:
> On 1/13/26 4:08 AM, Jakub Kicinski wrote:
>> On Sat, 10 Jan 2026 22:05:14 +0100 Jakub Sitnicki wrote:
>>> This series is split out of [1] following discussion with Jakub.
>>>
>>> To copy XDP metadata into an skb extension when skb_metadata_set() is
>>> called, we need to locate the metadata contents.
>>
>> "When skb_metadata_set() is called"? I think that may cause perf
>> regressions unless we merge major optimizations at the same time?
>> Should we defer touching the drivers until we have a PoC and some
>> idea whether allocating the extension right away is manageable or
>> we are better off doing it via a kfunc in TC (after GRO)?
>> To be clear putting the metadata in an extension right away would
>> indeed be much cleaner, just not sure how much of the perf hit we
>> can optimize away..
> 
> I agree it would be better deferring touching the driver before we have
> proof there will not be significant regressions.

It will be a performance regression to (as cover-letter says):
  "To copy XDP metadata into an skb extension when skb_metadata_set() is 
called".
The XDP to TC-ingress code path is a fast-path IMHO.

*BUT* this patchset isn't doing that. To me it looks like a cleanup
patchset that simply makes it consistent when skb_metadata_set() called.
Selling it as a pre-requirement for doing copy later seems fishy.


> IIRC, at early MPTCP impl time, Eric suggested increasing struct sk_buff
> size as an alternative to the mptcp skb extension, leaving the added
> trailing part uninitialized when the sk_buff is allocated.
> 
> If skb extensions usage become so ubicuos they are basically allocated
> for each packet, the total skb extension is kept under strict control
> and remains reasonable (assuming it is :), perhaps we could consider
> revisiting the above mentioned approach?


I really like this idea.  As using the uninitialized tail room in the
SKB (memory area) will make SKB extensions fast.  Today SKBs are
allocated via SLUB-alloacator cache-aligned so the real size is 256
bytes.  On my system the actual SKB (sk_buff) size is 232 bytes (already
leaving us 24 bytes). The area that gets zero-initialized is only 192
bytes (3 cache-lines).  My experience with the SLUB allocator is that
increasing the object size doesn't increase the allocation cost (below
PAGE_SIZE).  So, the suggestion of simply allocating a larger sk_buff is
valid as it doesn't cost more (if we don't touch those cache-lines).  We
could even make it a CONFIG compile time option how big this area should be.

For Jakub this unfortunately challenge/breaks the design of keeping
data_meta area valid deeper into the netstack.  With all the challenges
around encapsulation/decap it seems hard/infeasible to maintain this
area in-front of the packet data pointer deeper into the netstack.

Instead of blindly copying XDP data_meta area into a single SKB
extension.  What if we make it the responsibility of the TC-ingress BPF-
hook to understand the data_meta format and via (kfunc) helpers
transfer/create the SKB extension that it deems relevant.
Would this be an acceptable approach that makes it easier to propagate
metadata deeper in netstack?

--Jesper

p.s. For compact storage of SKB extensions in the SKB tail-area, we
could revisit Arthur's "traits" (compact-KV storage).

Powered by blists - more mailing lists