Re: [PATCH bpf-next v3 0/2] Allow data_meta size > 32

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Jesper Dangaard Brouer <hawk@kernel.org>
Date: Tue, 28 Nov 2023 12:30:41 +0100

> 
> 
> On 11/28/23 11:33, Larysa Zaremba wrote:
>> On Tue, Nov 28, 2023 at 11:26:59AM +0100, Jesper Dangaard Brouer wrote:
>>>
>>>
>>> On 11/27/23 19:32, Larysa Zaremba wrote:
>>>> Currently, there is no reason for data_meta to be limited to 32 bytes.
>>>> Loosen this limitation and make maximum meta size 252.
>>>
>>> First I though you made a type here with 252 bytes, but then I
>>> remembered
>>> the 4 byte alignment.
>>> I think commit message should elaborate on why 252 bytes.
>>>
>>
>> You are right, will do.
>>   
> 
> I'm looking at the code to see if metadata can be extended into area
> used by xdp_frame, such that a BPF-prog get direct memory access to this
> (which should not be allowed).  The bpf_xdp_adjust_meta() helper does
> handle this (as you/Olek also mentioned in commit msg).  A driver could
> set data_meta such that they overlap, but I guess we will categorize
> this as a driver bug.
> 
> The XDP headroom have evolved to become dynamic (commonly 192 or 256
> bytes). Thus, we cannot statically reduce metalen with sizeof(struct
> xdp_frame).  The maximum meta size 252, could be achieved (and valid) if
> a driver chooses to have more XDP headroom e.g. 288 bytes. So, I guess
> it is correct to say maximum valid meta size is 252 bytes, but can be
> limited and reduced further by drivers chosen XDP headroom and memory
> reserved by struct xdp_frame (limited in bpf_xdp_adjust_meta).

Drivers which don't provide 256 bytes of headroom for XDP are
pathological :p

Either way, as Larysa and you mentioned, the actual available headroom
is checked with adjust_meta(), so here we just make sure its size fits
into u8 meta_len of &skb_shared_info.

> 
> 
> 
>>>>
>>>> Also, modify the selftest, so test_xdp_context_error does not complain
>>>> about the unexpected success.
>>>>
>>>> v2->v3:
>>>> * Fix main patch author
>>>> * Add selftests path
>>>>
>>>> v1->v2:
>>>> * replace 'typeof(metalen)' with the actual type
>>>>
>>>> Aleksander Lobakin (1):
>>>>     net, xdp: allow metadata > 32
>>>>
>>>> Larysa Zaremba (1):
>>>>     selftests/bpf: increase invalid metadata size
>>>>
>>>>    include/linux/skbuff.h                              | 13
>>>> ++++++++-----
>>>>    include/net/xdp.h                                   |  7 ++++++-
>>>>    .../selftests/bpf/prog_tests/xdp_context_test_run.c |  4 ++--
>>>>    3 files changed, 16 insertions(+), 8 deletions(-)
>>>>

Thanks,
Olek