Re: [PATCH v2 bpf-next 05/13] bpf: Rename BPF_XADD and prepare to encode other atomics in .imm

From: Yonghong Song
Date: Sat Nov 28 2020 - 00:02:53 EST




On 11/27/20 9:57 AM, Brendan Jackman wrote:
A subsequent patch will add additional atomic operations. These new
operations will use the same opcode field as the existing XADD, with
the immediate discriminating different operations.

In preparation, rename the instruction mode BPF_ATOMIC and start
calling the zero immediate BPF_ADD.

This is possible (doesn't break existing valid BPF progs) because the
immediate field is currently reserved MBZ and BPF_ADD is zero.

All uses are removed from the tree but the BPF_XADD definition is
kept around to avoid breaking builds for people including kernel
headers.

Signed-off-by: Brendan Jackman <jackmanb@xxxxxxxxxx>
---
Documentation/networking/filter.rst | 30 +++++++-----
arch/arm/net/bpf_jit_32.c | 7 ++-
arch/arm64/net/bpf_jit_comp.c | 16 +++++--
arch/mips/net/ebpf_jit.c | 11 +++--
arch/powerpc/net/bpf_jit_comp64.c | 25 ++++++++--
arch/riscv/net/bpf_jit_comp32.c | 20 ++++++--
arch/riscv/net/bpf_jit_comp64.c | 16 +++++--
arch/s390/net/bpf_jit_comp.c | 27 ++++++-----
arch/sparc/net/bpf_jit_comp_64.c | 17 +++++--
arch/x86/net/bpf_jit_comp.c | 46 ++++++++++++++-----
arch/x86/net/bpf_jit_comp32.c | 6 +--
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 14 ++++--
drivers/net/ethernet/netronome/nfp/bpf/main.h | 4 +-
.../net/ethernet/netronome/nfp/bpf/verifier.c | 15 ++++--
include/linux/filter.h | 8 ++--
include/uapi/linux/bpf.h | 3 +-
kernel/bpf/core.c | 31 +++++++++----
kernel/bpf/disasm.c | 6 ++-
kernel/bpf/verifier.c | 24 ++++++----
lib/test_bpf.c | 2 +-
samples/bpf/bpf_insn.h | 4 +-
samples/bpf/sock_example.c | 2 +-
samples/bpf/test_cgrp2_attach.c | 4 +-
tools/include/linux/filter.h | 7 +--
tools/include/uapi/linux/bpf.h | 3 +-
.../bpf/prog_tests/cgroup_attach_multi.c | 4 +-
tools/testing/selftests/bpf/verifier/ctx.c | 7 ++-
.../testing/selftests/bpf/verifier/leak_ptr.c | 4 +-
tools/testing/selftests/bpf/verifier/unpriv.c | 3 +-
tools/testing/selftests/bpf/verifier/xadd.c | 2 +-
30 files changed, 248 insertions(+), 120 deletions(-)

diff --git a/Documentation/networking/filter.rst b/Documentation/networking/filter.rst
[...]
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 0a721f6e8676..1c9efc74edfc 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -3109,13 +3109,19 @@ mem_xadd(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta, bool is64)
return 0;
}
-static int mem_xadd4(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
+static int mem_atomic4(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
+ if (meta->insn.off != BPF_ADD)
+ return -EOPNOTSUPP;

You probably missed this change. it should be meta->insn.imm != BPF_ADD.

+
return mem_xadd(nfp_prog, meta, false);
}
-static int mem_xadd8(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
+static int mem_atomic8(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
+ if (meta->insn.off != BPF_ADD)

same as above.

+ return -EOPNOTSUPP;
+
return mem_xadd(nfp_prog, meta, true);
}
@@ -3475,8 +3481,8 @@ static const instr_cb_t instr_cb[256] = {
[BPF_STX | BPF_MEM | BPF_H] = mem_stx2,
[BPF_STX | BPF_MEM | BPF_W] = mem_stx4,
[BPF_STX | BPF_MEM | BPF_DW] = mem_stx8,
- [BPF_STX | BPF_XADD | BPF_W] = mem_xadd4,
- [BPF_STX | BPF_XADD | BPF_DW] = mem_xadd8,
+ [BPF_STX | BPF_ATOMIC | BPF_W] = mem_atomic4,
+ [BPF_STX | BPF_ATOMIC | BPF_DW] = mem_atomic8,
[BPF_ST | BPF_MEM | BPF_B] = mem_st1,
[BPF_ST | BPF_MEM | BPF_H] = mem_st2,
[BPF_ST | BPF_MEM | BPF_W] = mem_st4,
[...]