From: Yonghong Song <yonghong.song@linux.dev>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Anton Protopopov <aspsk@isovalent.com>,
Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Jiri Olsa <jolsa@kernel.org>,
Martin KaFai Lau <martin.lau@linux.dev>,
Stanislav Fomichev <sdf@google.com>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 6/7] libbpf: BPF Static Keys support
Date: Fri, 8 Dec 2023 20:15:50 -0800 [thread overview]
Message-ID: <bef79c65-e89a-4219-8c8b-750c60e1f2b4@linux.dev> (raw)
In-Reply-To: <CAADnVQJtWVE9+rA2232P4g7ktUJ_+Nfwo+MYpv=6p7+Z9J20hw@mail.gmail.com>
On 12/8/23 8:05 PM, Alexei Starovoitov wrote:
> On Fri, Dec 8, 2023 at 2:04 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>> I feel like embedding some sort of ID inside the instruction is very..
>> unusual, shall we say?
> yeah. no magic numbers inside insns pls.
>
> I don't like JA_CFG name, since I read CFG as control flow graph,
> while you probably meant CFG as configurable.
> How about BPF_JA_OR_NOP ?
> Then in combination with BPF_JMP or BPF_JMP32 modifier
> the insn->off|imm will be used.
> 1st bit in src_reg can indicate the default action: nop or jmp.
> In asm it may look like asm("goto_or_nop +5")
How does the C source code looks like in order to generate
BPF_JA_OR_NOP insn? Any source examples?
>
>> 2. bpf_static_branch_{likely,unlikely}() macro accepts a reference to
>> one such special global variable and and instructs compiler to emit
>> relocation between static key variable and JMP_CFG instruction.
>>
>> Libbpf will properly update these relocations during static linking
>> and subprog rearrangement, just like we do it for map references
>> today.
> Right. libbpf has RELO_SUBPROG_ADDR.
> This new relo will be pretty much that.
> And we have proper C syntax for taking an address: &&label.
> The bpf_static_branch macro can use it.
> We wanted to add it for a long time to support proper
> switch() and jmp tables.
>
> I don't like IDs and new map type for this.
> The macro can have 'branch_name' as one of the arguments and
> it will populate addresses of insns into "name.static_branch" section.
>
> From libbpf pov it will be yet another global section which
> is represented as a traditional bpf array of one element.
> No extra handling on the libbpf side.
>
> The question is how to represent the "address" of the insn.
> I think 4 byte prog_id + 4 byte insn_idx will do.
>
> Then bpf prog can pass such "address" into bpf_static_branch_enable/disable
> kfunc.
>
> The user space can iterate over 8 byte "addresses"
> in that 1 element array map and call BPF_STATIC_BRANCH_ENABLE/DISABLE
> syscall cmds.
> We can have a helper on libbpf side for that.
>
> I see no need to introduce a new map type just to reuse map_update_elem cmd.
>
next prev parent reply other threads:[~2023-12-09 4:16 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-06 14:10 [PATCH bpf-next 0/7] BPF Static Keys Anton Protopopov
2023-12-06 14:10 ` [PATCH bpf-next 1/7] bpf: extract bpf_prog_bind_map logic into an inline helper Anton Protopopov
2023-12-06 14:10 ` [PATCH bpf-next 2/7] bpf: rename and export a struct definition Anton Protopopov
2023-12-06 14:10 ` [PATCH bpf-next 3/7] bpf: adjust functions offsets when patching progs Anton Protopopov
2023-12-06 14:10 ` [PATCH bpf-next 4/7] bpf: implement BPF Static Keys support Anton Protopopov
2023-12-06 14:10 ` [PATCH bpf-next 5/7] bpf: x86: implement static keys support Anton Protopopov
2023-12-06 14:10 ` [PATCH bpf-next 6/7] libbpf: BPF Static Keys support Anton Protopopov
2023-12-08 3:45 ` Alexei Starovoitov
2023-12-08 16:19 ` Anton Protopopov
2023-12-08 22:04 ` Andrii Nakryiko
2023-12-08 23:07 ` Eduard Zingerman
2023-12-09 4:07 ` Alexei Starovoitov
2023-12-09 4:05 ` Alexei Starovoitov
2023-12-09 4:15 ` Yonghong Song [this message]
2023-12-09 4:25 ` Alexei Starovoitov
2023-12-09 5:04 ` Yonghong Song
2023-12-09 17:18 ` Alexei Starovoitov
2023-12-10 6:32 ` Yonghong Song
2023-12-10 10:30 ` Eduard Zingerman
2023-12-11 3:33 ` Alexei Starovoitov
2023-12-11 18:49 ` Andrii Nakryiko
2023-12-12 10:25 ` Anton Protopopov
2023-12-14 2:15 ` Alexei Starovoitov
2023-12-14 3:04 ` Yonghong Song
2023-12-14 16:56 ` Eduard Zingerman
2023-12-14 16:33 ` Anton Protopopov
2023-12-11 17:31 ` Anton Protopopov
2023-12-11 19:08 ` Alexei Starovoitov
2023-12-12 9:06 ` Anton Protopopov
2023-12-11 21:51 ` Yonghong Song
2023-12-11 22:52 ` Yonghong Song
2023-12-06 14:10 ` [PATCH bpf-next 7/7] selftests/bpf: Add tests for BPF Static Keys Anton Protopopov
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=bef79c65-e89a-4219-8c8b-750c60e1f2b4@linux.dev \
--to=yonghong.song@linux.dev \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=aspsk@isovalent.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=jolsa@kernel.org \
--cc=martin.lau@linux.dev \
--cc=sdf@google.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