All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yhs@fb.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
	Martin KaFai Lau <kafai@fb.com>,
	Network Development <netdev@vger.kernel.org>,
	Alexei Starovoitov <ast@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next v4 12/21] bpf: add PTR_TO_BTF_ID_OR_NULL support
Date: Sun, 10 May 2020 10:05:17 -0700	[thread overview]
Message-ID: <6541b3ff-98ea-f004-0c14-54688c65c902@fb.com> (raw)
In-Reply-To: <CAADnVQKzhbFe4MQ0G4ZuPnjXbbzEfQMjvTwba4MkhyXQAuNP+Q@mail.gmail.com>



On 5/10/20 9:11 AM, Alexei Starovoitov wrote:
> On Sat, May 9, 2020 at 10:19 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 5/9/20 5:50 PM, Alexei Starovoitov wrote:
>>> On Sat, May 09, 2020 at 10:59:12AM -0700, Yonghong Song wrote:
>>>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>>>> index a2cfba89a8e1..c490fbde22d4 100644
>>>> --- a/kernel/bpf/btf.c
>>>> +++ b/kernel/bpf/btf.c
>>>> @@ -3790,7 +3790,10 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>>>>               return true;
>>>>
>>>>       /* this is a pointer to another type */
>>>> -    info->reg_type = PTR_TO_BTF_ID;
>>>> +    if (off != 0 && prog->aux->btf_id_or_null_non0_off)
>>>> +            info->reg_type = PTR_TO_BTF_ID_OR_NULL;
>>>> +    else
>>>> +            info->reg_type = PTR_TO_BTF_ID;
>>>
>>> I think the verifier should be smarter than this.
>>> It's too specific and inflexible. All ctx fields of bpf_iter execpt first
>>> will be such ? let's figure out a different way to tell verifier about this.
>>> How about using typedef with specific suffix? Like:
>>> typedef struct bpf_map *bpf_map_or_null;
>>>    struct bpf_iter__bpf_map {
>>>      struct bpf_iter_meta *meta;
>>>      bpf_map_or_null map;
>>>    };
>>> or use a union with specific second member? Like:
>>>    struct bpf_iter__bpf_map {
>>>      struct bpf_iter_meta *meta;
>>>      union {
>>>        struct bpf_map *map;
>>>        long null;
>>>      };
>>>    };
>>
>> I have an alternative approach to refactor this for future
>> support for map elements as well.
>>
>> For example, for bpf_map_elements iterator the prog context type
>> can be
>>       struct bpf_iter_bpf_map_elem {
>>          struct bpf_iter_meta *meta;
>>          strruct bpf_map *map;
>>          <key type>  *key;
>>          <value type> *val;
>>      };
>>
>> target will pass the following information to bpf_iter registration:
>>      arg 1: PTR_TO_BTF_ID
>>      arg 2: PTR_TO_BTF_ID_OR_NULL
>>      arg 3: PTR_TO_BUFFER
>>      arg 4: PTR_TO_BUFFER
>>
>> verifier will retrieve the reg_type from target.
> 
> you mean to introduce something like 'struct bpf_func_proto'
> that describes types of helpers, but instead something similar
> to clarify the types in ctx ? That should work. Thanks

Yes, this is what I think will be extensible.

  reply	other threads:[~2020-05-10 17:05 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-09 17:58 [PATCH bpf-next v4 00/21] bpf: implement bpf iterator for kernel data Yonghong Song
2020-05-09 17:58 ` [PATCH bpf-next v4 01/21] bpf: implement an interface to register bpf_iter targets Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 02/21] bpf: allow loading of a bpf_iter program Yonghong Song
2020-05-10  0:41   ` Alexei Starovoitov
2020-05-10  5:07     ` Yonghong Song
2020-05-12 15:41     ` Yonghong Song
2020-05-12 16:25       ` Alexei Starovoitov
2020-05-12 16:29         ` Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 03/21] bpf: support bpf tracing/iter programs for BPF_LINK_CREATE Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 04/21] bpf: support bpf tracing/iter programs for BPF_LINK_UPDATE Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 05/21] bpf: implement bpf_seq_read() for bpf iterator Yonghong Song
2020-05-10  0:30   ` Alexei Starovoitov
2020-05-10  4:51     ` Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 06/21] bpf: create anonymous " Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 07/21] bpf: create file " Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 08/21] bpf: implement common macros/helpers for target iterators Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 09/21] bpf: add bpf_map iterator Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 10/21] net: bpf: add netlink and ipv6_route bpf_iter targets Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 11/21] bpf: add task and task/file iterator targets Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 12/21] bpf: add PTR_TO_BTF_ID_OR_NULL support Yonghong Song
2020-05-10  0:50   ` Alexei Starovoitov
2020-05-10  5:18     ` Yonghong Song
2020-05-10 16:11       ` Alexei Starovoitov
2020-05-10 17:05         ` Yonghong Song [this message]
2020-05-09 17:59 ` [PATCH bpf-next v4 13/21] bpf: add bpf_seq_printf and bpf_seq_write helpers Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 14/21] bpf: handle spilled PTR_TO_BTF_ID properly when checking stack_boundary Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 15/21] bpf: support variable length array in tracing programs Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 16/21] tools/libbpf: add bpf_iter support Yonghong Song
2020-05-10  0:35   ` Alexei Starovoitov
2020-05-10  5:06     ` Yonghong Song
2020-05-10 16:09       ` Alexei Starovoitov
2020-05-10 17:04         ` Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 17/21] tools/libpf: add offsetof/container_of macro in bpf_helpers.h Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 18/21] tools/bpftool: add bpf_iter support for bptool Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 19/21] tools/bpf: selftests: add iterator programs for ipv6_route and netlink Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 20/21] tools/bpf: selftests: add iter progs for bpf_map/task/task_file Yonghong Song
2020-05-09 17:59 ` [PATCH bpf-next v4 21/21] tools/bpf: selftests: add bpf_iter selftests Yonghong Song
2020-05-10  0:34   ` Alexei Starovoitov
2020-05-10  5:01     ` Yonghong Song
2020-05-10 16:14       ` Alexei Starovoitov
2020-05-10  0:27 ` [PATCH bpf-next v4 00/21] bpf: implement bpf iterator for kernel data Alexei Starovoitov
2020-05-10  4:47   ` Yonghong Song

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=6541b3ff-98ea-f004-0c14-54688c65c902@fb.com \
    --to=yhs@fb.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kafai@fb.com \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.