BPF List
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <hawk@kernel.org>
To: Larysa Zaremba <larysa.zaremba@intel.com>
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Magnus Karlsson <magnus.karlsson@gmail.com>,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	Yunsheng Lin <linyunsheng@huawei.com>,
	Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Aleksander Lobakin <aleksander.lobakin@intel.com>
Subject: Re: [PATCH bpf-next v3 0/2] Allow data_meta size > 32
Date: Tue, 28 Nov 2023 12:30:41 +0100	[thread overview]
Message-ID: <eb87653c-8ff8-447d-a7a1-25961f60518a@kernel.org> (raw)
In-Reply-To: <ZWXB+8FQenT6717+@lzaremba-mobl.ger.corp.intel.com>



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).



>>>
>>> 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(-)
>>>

  reply	other threads:[~2023-11-28 11:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-27 18:32 [PATCH bpf-next v3 0/2] Allow data_meta size > 32 Larysa Zaremba
2023-11-27 18:32 ` [PATCH bpf-next v3 1/2] selftests/bpf: increase invalid metadata size Larysa Zaremba
2023-11-27 18:32 ` [PATCH bpf-next v3 2/2] net, xdp: allow metadata > 32 Larysa Zaremba
2023-11-28 10:26 ` [PATCH bpf-next v3 0/2] Allow data_meta size " Jesper Dangaard Brouer
2023-11-28 10:33   ` Larysa Zaremba
2023-11-28 11:30     ` Jesper Dangaard Brouer [this message]
2023-11-28 12:25       ` Alexander Lobakin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eb87653c-8ff8-447d-a7a1-25961f60518a@kernel.org \
    --to=hawk@kernel.org \
    --cc=aleksander.lobakin@intel.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=larysa.zaremba@intel.com \
    --cc=linyunsheng@huawei.com \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox