From: Brendan Jackman <jackmanb@google.com>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>, Yonghong Song <yhs@fb.com>,
Daniel Borkmann <daniel@iogearbox.net>,
KP Singh <kpsingh@chromium.org>,
Florent Revest <revest@chromium.org>,
Brendan Jackman <jackmanb@google.com>
Subject: [PATCH 5/7] bpf: Add BPF_FETCH field / create atomic_fetch_add instruction
Date: Mon, 23 Nov 2020 17:32:00 +0000 [thread overview]
Message-ID: <20201123173202.1335708-6-jackmanb@google.com> (raw)
In-Reply-To: <20201123173202.1335708-1-jackmanb@google.com>
This value can be set in bpf_insn.imm, for BPF_ATOMIC instructions,
in order to have the previous value of the atomically-modified memory
location loaded into the src register after an atomic op is carried
out.
Suggested-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
arch/x86/net/bpf_jit_comp.c | 21 ++++++++++---------
include/linux/filter.h | 9 +++++++++
include/uapi/linux/bpf.h | 3 +++
kernel/bpf/core.c | 17 ++++++++++++++--
kernel/bpf/disasm.c | 6 ++++++
kernel/bpf/verifier.c | 37 +++++++++++++++++++++++++---------
tools/include/linux/filter.h | 10 ++++++++-
tools/include/uapi/linux/bpf.h | 3 +++
8 files changed, 84 insertions(+), 22 deletions(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 0ff2416d99b6..b475bf525424 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1255,22 +1255,25 @@ st: if (is_imm8(insn->off))
case BPF_STX | BPF_ATOMIC | BPF_W:
case BPF_STX | BPF_ATOMIC | BPF_DW:
- if (insn->imm != BPF_ADD) {
+ if (BPF_OP(insn->imm) != BPF_ADD) {
pr_err("bpf_jit: unknown opcode %02x\n", insn->imm);
return -EFAULT;
}
- /* XADD: lock *(u32/u64*)(dst_reg + off) += src_reg */
+ EMIT1(0xF0); /* lock prefix */
- if (BPF_SIZE(insn->code) == BPF_W) {
- /* Emit 'lock add dword ptr [rax + off], eax' */
- if (is_ereg(dst_reg) || is_ereg(src_reg))
- EMIT3(0xF0, add_2mod(0x40, dst_reg, src_reg), 0x01);
- else
- EMIT2(0xF0, 0x01);
+ maybe_emit_rex(&prog, dst_reg, src_reg,
+ BPF_SIZE(insn->code) == BPF_DW);
+
+ /* emit opcode */
+ if (insn->imm & BPF_FETCH) {
+ /* src_reg = sync_fetch_and_add(*(dst_reg + off), src_reg); */
+ EMIT2(0x0F, 0xC1);
} else {
- EMIT3(0xF0, add_2mod(0x48, dst_reg, src_reg), 0x01);
+ /* lock *(u32/u64*)(dst_reg + off) += src_reg */
+ EMIT1(0x01);
}
+
emit_modrm_dstoff(&prog, dst_reg, src_reg, insn->off);
break;
diff --git a/include/linux/filter.h b/include/linux/filter.h
index ce19988fb312..bf0ff3649f46 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -270,6 +270,15 @@ static inline bool insn_is_zext(const struct bpf_insn *insn)
.imm = BPF_ADD })
#define BPF_STX_XADD BPF_ATOMIC_ADD /* alias */
+/* Atomic memory add with fetch, src_reg = sync_fetch_and_add(*(dst_reg + off), src_reg); */
+
+#define BPF_ATOMIC_FETCH_ADD(SIZE, DST, SRC, OFF) \
+ ((struct bpf_insn) { \
+ .code = BPF_STX | BPF_SIZE(SIZE) | BPF_ATOMIC, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = OFF, \
+ .imm = BPF_ADD | BPF_FETCH })
/* Memory store, *(uint *) (dst_reg + off16) = imm32 */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index dcd08783647d..ec7f415f331b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -44,6 +44,9 @@
#define BPF_CALL 0x80 /* function call */
#define BPF_EXIT 0x90 /* function return */
+/* atomic op type fields (stored in immediate) */
+#define BPF_FETCH 0x01 /* fetch previous value into src reg */
+
/* Register numbers */
enum {
BPF_REG_0 = 0,
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 48b192a8edce..49a2a533db60 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1627,21 +1627,34 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
#undef LDX_PROBE
STX_ATOMIC_W:
- switch (insn->imm) {
+ switch (IMM) {
case BPF_ADD:
/* lock xadd *(u32 *)(dst_reg + off16) += src_reg */
atomic_add((u32) SRC, (atomic_t *)(unsigned long)
(DST + insn->off));
+ break;
+ case BPF_ADD | BPF_FETCH:
+ SRC = (u32) atomic_fetch_add(
+ (u32) SRC,
+ (atomic_t *)(unsigned long) (DST + insn->off));
+ break;
default:
goto default_label;
}
CONT;
+
STX_ATOMIC_DW:
- switch (insn->imm) {
+ switch (IMM) {
case BPF_ADD:
/* lock xadd *(u64 *)(dst_reg + off16) += src_reg */
atomic64_add((u64) SRC, (atomic64_t *)(unsigned long)
(DST + insn->off));
+ break;
+ case BPF_ADD | BPF_FETCH:
+ SRC = (u64) atomic64_fetch_add(
+ (u64) SRC,
+ (atomic64_t *)(s64) (DST + insn->off));
+ break;
default:
goto default_label;
}
diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c
index 37c8d6e9b4cc..669cef265493 100644
--- a/kernel/bpf/disasm.c
+++ b/kernel/bpf/disasm.c
@@ -160,6 +160,12 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,
bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
insn->dst_reg, insn->off,
insn->src_reg);
+ } else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
+ insn->imm == (BPF_ADD | BPF_FETCH)) {
+ verbose(cbs->private_data, "(%02x) r%d = atomic_fetch_add(*(%s *)(r%d %+d), r%d)\n",
+ insn->code, insn->src_reg,
+ bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
+ insn->dst_reg, insn->off, insn->src_reg);
} else {
verbose(cbs->private_data, "BUG_%02x\n", insn->code);
}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 609cc5e9571f..14f5053daf22 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3600,9 +3600,14 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
static int check_atomic(struct bpf_verifier_env *env, int insn_idx, struct bpf_insn *insn)
{
+ struct bpf_reg_state *regs = cur_regs(env);
int err;
- if (insn->imm != BPF_ADD) {
+ switch (insn->imm) {
+ case BPF_ADD:
+ case BPF_ADD | BPF_FETCH:
+ break;
+ default:
verbose(env, "BPF_ATOMIC uses invalid atomic opcode %02x\n", insn->imm);
return -EINVAL;
}
@@ -3631,7 +3636,7 @@ static int check_atomic(struct bpf_verifier_env *env, int insn_idx, struct bpf_i
is_pkt_reg(env, insn->dst_reg) ||
is_flow_key_reg(env, insn->dst_reg) ||
is_sk_reg(env, insn->dst_reg)) {
- verbose(env, "atomic stores into R%d %s is not allowed\n",
+ verbose(env, "BPF_ATOMIC stores into R%d %s is not allowed\n",
insn->dst_reg,
reg_type_str[reg_state(env, insn->dst_reg)->type]);
return -EACCES;
@@ -3644,8 +3649,21 @@ static int check_atomic(struct bpf_verifier_env *env, int insn_idx, struct bpf_i
return err;
/* check whether we can write into the same memory */
- return check_mem_access(env, insn_idx, insn->dst_reg, insn->off,
- BPF_SIZE(insn->code), BPF_WRITE, -1, true);
+ err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off,
+ BPF_SIZE(insn->code), BPF_WRITE, -1, true);
+ if (err)
+ return err;
+
+ if (!(insn->imm & BPF_FETCH))
+ return 0;
+
+ /* check and record load of old value into src reg */
+ err = check_reg_arg(env, insn->src_reg, DST_OP);
+ if (err)
+ return err;
+ regs[insn->src_reg].type = SCALAR_VALUE;
+
+ return 0;
}
static int __check_stack_boundary(struct bpf_verifier_env *env, u32 regno,
@@ -9490,12 +9508,6 @@ static int do_check(struct bpf_verifier_env *env)
} else if (class == BPF_STX) {
enum bpf_reg_type *prev_dst_type, dst_reg_type;
- if (((BPF_MODE(insn->code) != BPF_MEM &&
- BPF_MODE(insn->code) != BPF_ATOMIC) || insn->imm != 0)) {
- verbose(env, "BPF_STX uses reserved fields\n");
- return -EINVAL;
- }
-
if (BPF_MODE(insn->code) == BPF_ATOMIC) {
err = check_atomic(env, env->insn_idx, insn);
if (err)
@@ -9504,6 +9516,11 @@ static int do_check(struct bpf_verifier_env *env)
continue;
}
+ if (BPF_MODE(insn->code) != BPF_MEM && insn->imm != 0) {
+ verbose(env, "BPF_STX uses reserved fields\n");
+ return -EINVAL;
+ }
+
/* check src1 operand */
err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
diff --git a/tools/include/linux/filter.h b/tools/include/linux/filter.h
index 95ff51d97f25..8f2707ebab18 100644
--- a/tools/include/linux/filter.h
+++ b/tools/include/linux/filter.h
@@ -180,7 +180,15 @@
.imm = BPF_ADD })
#define BPF_STX_XADD BPF_ATOMIC_ADD /* alias */
-/* Memory store, *(uint *) (dst_reg + off16) = imm32 */
+/* Atomic memory add with fetch, src_reg = sync_fetch_and_add(*(dst_reg + off), src_reg); */
+
+#define BPF_ATOMIC_FETCH_ADD(SIZE, DST, SRC, OFF) \
+ ((struct bpf_insn) { \
+ .code = BPF_STX | BPF_SIZE(SIZE) | BPF_ATOMIC, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = OFF, \
+ .imm = BPF_ADD | BPF_FETCH })
#define BPF_ST_MEM(SIZE, DST, OFF, IMM) \
((struct bpf_insn) { \
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index dcd08783647d..ec7f415f331b 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -44,6 +44,9 @@
#define BPF_CALL 0x80 /* function call */
#define BPF_EXIT 0x90 /* function return */
+/* atomic op type fields (stored in immediate) */
+#define BPF_FETCH 0x01 /* fetch previous value into src reg */
+
/* Register numbers */
enum {
BPF_REG_0 = 0,
--
2.29.2.454.gaff20da3a2-goog
next prev parent reply other threads:[~2020-11-23 17:32 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-23 17:31 [PATCH 0/7] Atomics for eBPF Brendan Jackman
2020-11-23 17:31 ` [PATCH 1/7] bpf: Factor out emission of ModR/M for *(reg + off) Brendan Jackman
2020-11-23 17:31 ` [PATCH 2/7] bpf: x86: Factor out emission of REX byte Brendan Jackman
2020-11-23 17:31 ` [PATCH 3/7] bpf: Rename BPF_XADD and prepare to encode other atomics in .imm Brendan Jackman
2020-11-23 23:54 ` Yonghong Song
2020-11-24 11:02 ` Brendan Jackman
2020-11-24 16:04 ` Yonghong Song
2020-11-24 3:28 ` kernel test robot
2020-11-24 3:28 ` kernel test robot
2020-11-24 6:50 ` Alexei Starovoitov
2020-11-24 11:21 ` Brendan Jackman
2020-11-24 22:43 ` Alexei Starovoitov
2020-11-23 17:31 ` [PATCH 4/7] bpf: Move BPF_STX reserved field check into BPF_STX verifier code Brendan Jackman
2020-11-23 17:32 ` Brendan Jackman [this message]
2020-11-23 21:12 ` [PATCH 5/7] bpf: Add BPF_FETCH field / create atomic_fetch_add instruction kernel test robot
2020-11-23 21:12 ` kernel test robot
2020-11-23 21:51 ` kernel test robot
2020-11-23 21:51 ` kernel test robot
2020-11-24 6:52 ` Alexei Starovoitov
2020-11-24 10:48 ` Brendan Jackman
2020-11-23 17:32 ` [PATCH 6/7] bpf: Add instructions for atomic_cmpxchg and friends Brendan Jackman
2020-11-23 19:29 ` Brendan Jackman
2020-11-24 6:40 ` Alexei Starovoitov
2020-11-24 10:55 ` Brendan Jackman
2020-11-24 22:51 ` Alexei Starovoitov
2020-11-23 17:32 ` [PATCH 7/7] bpf: Add tests for new BPF atomic operations Brendan Jackman
2020-11-24 0:26 ` Yonghong Song
2020-11-24 13:10 ` Brendan Jackman
2020-11-23 17:36 ` [PATCH 0/7] Atomics for eBPF Brendan Jackman
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=20201123173202.1335708-6-jackmanb@google.com \
--to=jackmanb@google.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kpsingh@chromium.org \
--cc=revest@chromium.org \
--cc=yhs@fb.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 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.