From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Maxim Mikityanskiy <maximmi@nvidia.com>
Cc: bpf <bpf@vger.kernel.org>, "Alexei Starovoitov" <ast@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Andrii Nakryiko" <andrii@kernel.org>,
Networking <netdev@vger.kernel.org>,
"Tariq Toukan" <tariqt@nvidia.com>,
"Martin KaFai Lau" <kafai@fb.com>,
"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
"John Fastabend" <john.fastabend@gmail.com>,
"KP Singh" <kpsingh@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
"Jakub Kicinski" <kuba@kernel.org>,
"Eric Dumazet" <edumazet@google.com>,
"Hideaki YOSHIFUJI" <yoshfuji@linux-ipv6.org>,
"David Ahern" <dsahern@kernel.org>,
"Shuah Khan" <shuah@kernel.org>,
"Jesper Dangaard Brouer" <hawk@kernel.org>,
"Nathan Chancellor" <nathan@kernel.org>,
"Nick Desaulniers" <ndesaulniers@google.com>,
"Joe Stringer" <joe@cilium.io>,
"Florent Revest" <revest@chromium.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@vger.kernel.org>,
"Toke Høiland-Jørgensen" <toke@toke.dk>,
"Kumar Kartikeya Dwivedi" <memxor@gmail.com>,
"Florian Westphal" <fw@strlen.de>,
pabeni@redhat.com
Subject: Re: [PATCH bpf-next v9 2/5] bpf: Allow helpers to accept pointers with a fixed size
Date: Fri, 6 May 2022 14:12:03 -0700 [thread overview]
Message-ID: <CAEf4Bzb-y7-5PG2vjdy__Jqwb2n2hPPQ_8Xs9R_cnenwT-AyTg@mail.gmail.com> (raw)
In-Reply-To: <20220503171437.666326-3-maximmi@nvidia.com>
On Tue, May 3, 2022 at 10:15 AM Maxim Mikityanskiy <maximmi@nvidia.com> wrote:
>
> Before this commit, the BPF verifier required ARG_PTR_TO_MEM arguments
> to be followed by ARG_CONST_SIZE holding the size of the memory region.
> The helpers had to check that size in runtime.
>
> There are cases where the size expected by a helper is a compile-time
> constant. Checking it in runtime is an unnecessary overhead and waste of
> BPF registers.
>
> This commit allows helpers to accept ARG_PTR_TO_MEM arguments without
> the corresponding ARG_CONST_SIZE, given that they define the memory
> region size in struct bpf_func_proto.
I think it's much less confusing and cleaner to have
ARG_PTR_TO_FIXED_SIZE_MEM (or whatever similar name), instead of
adding special casing to ARG_PTR_TO_MEM.
>
> Signed-off-by: Maxim Mikityanskiy <maximmi@nvidia.com>
> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> include/linux/bpf.h | 10 ++++++++++
> kernel/bpf/verifier.c | 26 +++++++++++++++-----------
> 2 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index be94833d390a..255ae3652225 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -514,6 +514,16 @@ struct bpf_func_proto {
> };
> u32 *arg_btf_id[5];
> };
> + union {
> + struct {
> + size_t arg1_size;
> + size_t arg2_size;
> + size_t arg3_size;
> + size_t arg4_size;
> + size_t arg5_size;
> + };
> + size_t arg_size[5];
> + };
We have almost 250 instances of struct bpf_func_proto variables:
$ rg 'const struct bpf_func_proto.* = \{' | wc -l
244
You are adding 40 bytes to it just to use it for 1-2 special
prototypes. It is quite expensive, IMHO, to increase vmlinux image
size by 10KB just for this.
Should we reuse arg_btf_id union (same argument won't be PTR_TO_BTF_ID
and PTR_TO_FIXED_SIZE_MEM, right)? Or alternatively special-case those
few prototypes in verifier code directly when trying to fetch memory
size?
> int *ret_btf_id; /* return value btf_id */
> bool (*allowed)(const struct bpf_prog *prog);
> };
[...]
next prev parent reply other threads:[~2022-05-06 21:12 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-03 17:14 [PATCH bpf-next v9 0/5] New BPF helpers to accelerate synproxy Maxim Mikityanskiy
2022-05-03 17:14 ` [PATCH bpf-next v9 1/5] bpf: Fix documentation of th_len in bpf_tcp_{gen,check}_syncookie Maxim Mikityanskiy
2022-05-03 17:14 ` [PATCH bpf-next v9 2/5] bpf: Allow helpers to accept pointers with a fixed size Maxim Mikityanskiy
2022-05-06 21:12 ` Andrii Nakryiko [this message]
2022-05-03 17:14 ` [PATCH bpf-next v9 3/5] bpf: Add helpers to issue and check SYN cookies in XDP Maxim Mikityanskiy
2022-05-06 21:19 ` Andrii Nakryiko
2022-05-10 19:20 ` Maxim Mikityanskiy
2022-05-11 0:03 ` Andrii Nakryiko
2022-05-03 17:14 ` [PATCH bpf-next v9 4/5] bpf: Add selftests for raw syncookie helpers Maxim Mikityanskiy
2022-05-06 21:34 ` Andrii Nakryiko
2022-05-10 19:21 ` Maxim Mikityanskiy
2022-05-11 0:10 ` Andrii Nakryiko
2022-05-11 11:48 ` Maxim Mikityanskiy
2022-05-03 17:14 ` [PATCH bpf-next v9 5/5] bpf: Allow the new syncookie helpers to work with SKBs Maxim Mikityanskiy
2022-05-06 21:39 ` Andrii Nakryiko
2022-05-06 21:51 ` [PATCH bpf-next v9 0/5] New BPF helpers to accelerate synproxy Andrii Nakryiko
2022-05-10 19:21 ` Maxim Mikityanskiy
2022-05-10 23:59 ` Andrii Nakryiko
2022-05-11 11:48 ` Maxim Mikityanskiy
2022-05-16 17:17 ` Maxim Mikityanskiy
2022-05-18 13:43 ` Maxim Mikityanskiy
2022-05-19 4:42 ` Andrii Nakryiko
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=CAEf4Bzb-y7-5PG2vjdy__Jqwb2n2hPPQ_8Xs9R_cnenwT-AyTg@mail.gmail.com \
--to=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=fw@strlen.de \
--cc=hawk@kernel.org \
--cc=joe@cilium.io \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=maximmi@nvidia.com \
--cc=memxor@gmail.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=revest@chromium.org \
--cc=shuah@kernel.org \
--cc=songliubraving@fb.com \
--cc=tariqt@nvidia.com \
--cc=toke@toke.dk \
--cc=yhs@fb.com \
--cc=yoshfuji@linux-ipv6.org \
/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;
as well as URLs for NNTP newsgroup(s).