Re: [PATCH bpf-next 6/7] libbpf: BPF Static Keys support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/11/23 1:51 PM, Yonghong Song wrote:

On 12/11/23 9:31 AM, Anton Protopopov wrote:
On Sat, Dec 09, 2023 at 10:32:42PM -0800, Yonghong Song wrote:
On 12/9/23 9:18 AM, Alexei Starovoitov wrote:
On Fri, Dec 8, 2023 at 9:05 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
On 12/8/23 8:25 PM, Alexei Starovoitov wrote:
On Fri, Dec 8, 2023 at 8:15 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
On 12/8/23 8:05 PM, Alexei Starovoitov wrote:
On Fri, Dec 8, 2023 at 2:04 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> 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?
It will be in inline asm only. The address of that insn will
be taken either via && or via asm (".long %l[label]").
   From llvm pov both should go through the same relo creation logic. I hope :)
A hack in llvm below with an example, could you check whether the C
syntax and object dump result
is what you want to see?
[...]
Thank you for the ultra quick llvm diff!
I was thinking of burning the new 0xE opcode for it,
but you're right. It's a flavor of existing JA insn and it's indeed
better to just use src_reg=1 bit to indicate so.
Right, using src_reg to indicate a new flavor of JA insn sounds
a good idea. My previously-used 'imm' field is a pure hack.

We probably need to use the 2nd bit of src_reg to indicate its default state
(jmp or fallthrough).
Good point.

           asm volatile goto ("r0 = 0; \
                               goto_or_nop %l[label]; \
                               r2 = 2; \
                               r3 = 3; \
Not sure how to represent the default state in assembly though.
"goto_or_nop" defaults to goto
"nop_or_goto" default to nop
?

Do we need "gotol" for imm32 or will it be automatic?
It won't be automatic.

At the end of this email, I will show the new change
to have gotol_or_nop and nop_or_gotol insn and an example
Thanks a lot Yonghong! May I ask you to send a full patch for LLVM
(with gotol) so that I can test it?

Okay, I will send a RFC patch to llvm-project so you can do 'git fetch'
to get the patch into your local llvm-project repo and build a compiler
to test.

Okay, the following is the llvm-project diff:

https://github.com/llvm/llvm-project/pull/75110

I added a label in inline asm like below:
|asm volatile goto ("r0 = 0; \ static_key_loc_1: \ gotol_or_nop %l[label]; \ r2 = 2; \ r3 = 3; \ ":: : "r0", "r2", "r3" :label); to identify the location of a gotol_or_nop/nop_or_gotol insn and the label '||||static_key_loc_1|' is in ELF symbol table
can be easily searched and validated (the location is
a gotol_or_nop/nop_or_gotol insn).



Overall, I think that JA + flags in SRC_REG is indeed better than a
new instruction, as a new code is not used.

This looks for me that two bits aren't enough, and the third is
required, as the second bit seems to be overloaded:

   * bit 1 indicates that this is a "JA_MAYBE"
   * bit 2 indicates a jump or nop (i.e., the current state)

However, we also need another bit which indicates what to do with the
instruction when we issue [an abstract] command

   flip_branch_on_or_off(branch, 0/1)

Without this information (and in the absense of external meta-data on
how to patch the branch) we can't determine what a given (BPF, not
jitted) program currently does. For example, if we issue

   flip_branch_on_or_off(branch, 0)

then we can't reflect this in the xlated program by setting the second
bit to jmp/off. Again, JITted program is fine, but it will be
desynchronized from xlated in term of logic (some instructions will be
mapped as NOP -> x86_JUMP, others as NOP -> x86_NOP).

In my original patch we kept this triplet as

   (offset to indicate a "special jump", JA+0/JA+OFF, Normal/Inverse)
[...]





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux