diff mbox series

[net-next,03/13] nfp: bpf: record jump destination to simplify jump fixup

Message ID 20171201053300.17503-4-jakub.kicinski@netronome.com
State Accepted, archived
Delegated to: BPF Maintainers
Headers show
Series nfp: bpf: jump resolution and memcpy update | expand

Commit Message

Jakub Kicinski Dec. 1, 2017, 5:32 a.m. UTC 
From: Jiong Wang <jiong.wang@netronome.com>

eBPF insns are internally organized as dual-list inside NFP offload JIT.
Random access to an insn needs to be done by either forward or backward
traversal along the list.

One place we need to do such traversal is at nfp_fixup_branches where one
traversal is needed for each jump insn to find the destination. Such
traversals could be avoided if jump destinations are collected through a
single travesal in a pre-scan pass, and such information could also be
useful in other places where jump destination info are needed.

This patch adds such jump destination collection in nfp_prog_prepare.

Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/jit.c      | 57 ++++-------------------
 drivers/net/ethernet/netronome/nfp/bpf/main.h     | 13 +++++-
 drivers/net/ethernet/netronome/nfp/bpf/offload.c  | 22 +++++++--
 drivers/net/ethernet/netronome/nfp/bpf/verifier.c |  4 +-
 4 files changed, 41 insertions(+), 55 deletions(-)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 20daf6b95601..f76659ecb654 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -65,12 +65,6 @@ 
 	     next = nfp_meta_next(pos),				\
 	     next2 = nfp_meta_next(next))
 
-static bool
-nfp_meta_has_next(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
-{
-	return meta->l.next != &nfp_prog->insns;
-}
-
 static bool
 nfp_meta_has_prev(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
 {
@@ -1864,9 +1858,8 @@  static void br_set_offset(u64 *instr, u16 offset)
 /* --- Assembler logic --- */
 static int nfp_fixup_branches(struct nfp_prog *nfp_prog)
 {
-	struct nfp_insn_meta *meta, *next;
+	struct nfp_insn_meta *meta, *jmp_dst;
 	u32 idx, br_idx;
-	int off;
 
 	list_for_each_entry(meta, &nfp_prog->insns, l) {
 		if (meta->skip)
@@ -1874,13 +1867,10 @@  static int nfp_fixup_branches(struct nfp_prog *nfp_prog)
 		if (BPF_CLASS(meta->insn.code) != BPF_JMP)
 			continue;
 
-		if (list_is_last(&meta->l, &nfp_prog->insns)) {
-			next = NULL;
+		if (list_is_last(&meta->l, &nfp_prog->insns))
 			idx = nfp_prog->last_bpf_off;
-		} else {
-			next = list_next_entry(meta, l);
-			idx = next->off - 1;
-		}
+		else
+			idx = list_next_entry(meta, l)->off - 1;
 
 		br_idx = nfp_prog_offset_to_index(nfp_prog, idx);
 
@@ -1893,43 +1883,14 @@  static int nfp_fixup_branches(struct nfp_prog *nfp_prog)
 		if (FIELD_GET(OP_BR_SPECIAL, nfp_prog->prog[br_idx]))
 			continue;
 
-		/* Find the target offset in assembler realm */
-		off = meta->insn.off;
-		if (!off) {
-			pr_err("Fixup found zero offset!!\n");
+		if (!meta->jmp_dst) {
+			pr_err("Non-exit jump doesn't have destination info recorded!!\n");
 			return -ELOOP;
 		}
 
-		if (!next) {
-			/* When "next" is NULL, "meta" is the last node in the
-			 * list. Given it is an JMP, it then must be a backward
-			 * jump.
-			 *
-			 * For eBPF, the jump offset is against pc + 1, so we
-			 * need to compensate the offset by 1 as we are pointing
-			 * "next" to the current node "meta".
-			 */
-			if (WARN_ON_ONCE(off > -2))
-				return -ELOOP;
-
-			next = meta;
-			off += 1;
-		}
-
-		while (off > 0 && nfp_meta_has_next(nfp_prog, next)) {
-			next = nfp_meta_next(next);
-			off--;
-		}
-		while (off < 0 && nfp_meta_has_prev(nfp_prog, next)) {
-			next = nfp_meta_prev(next);
-			off++;
-		}
-		if (off) {
-			pr_err("Fixup found too large jump!! %d\n", off);
-			return -ELOOP;
-		}
+		jmp_dst = meta->jmp_dst;
 
-		if (next->skip) {
+		if (jmp_dst->skip) {
 			pr_err("Branch landing on removed instruction!!\n");
 			return -ELOOP;
 		}
@@ -1938,7 +1899,7 @@  static int nfp_fixup_branches(struct nfp_prog *nfp_prog)
 		     idx <= br_idx; idx++) {
 			if (!nfp_is_br(nfp_prog->prog[idx]))
 				continue;
-			br_set_offset(&nfp_prog->prog[idx], next->off);
+			br_set_offset(&nfp_prog->prog[idx], jmp_dst->off);
 		}
 	}
 
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 0f4d218fc77a..e488656f406c 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -94,6 +94,7 @@  typedef int (*instr_cb_t)(struct nfp_prog *, struct nfp_insn_meta *);
  * @insn: BPF instruction
  * @ptr: pointer type for memory operations
  * @ptr_not_const: pointer is not always constant
+ * @jmp_dst: destination info for jump instructions
  * @off: index of first generated machine instruction (in nfp_prog.prog)
  * @n: eBPF instruction number
  * @skip: skip this instruction (optimized out)
@@ -102,8 +103,13 @@  typedef int (*instr_cb_t)(struct nfp_prog *, struct nfp_insn_meta *);
  */
 struct nfp_insn_meta {
 	struct bpf_insn insn;
-	struct bpf_reg_state ptr;
-	bool ptr_not_const;
+	union {
+		struct {
+			struct bpf_reg_state ptr;
+			bool ptr_not_const;
+		};
+		struct nfp_insn_meta *jmp_dst;
+	};
 	unsigned int off;
 	unsigned short n;
 	bool skip;
@@ -191,4 +197,7 @@  int nfp_bpf_translate(struct nfp_app *app, struct nfp_net *nn,
 		      struct bpf_prog *prog);
 int nfp_bpf_destroy(struct nfp_app *app, struct nfp_net *nn,
 		    struct bpf_prog *prog);
+struct nfp_insn_meta *
+nfp_bpf_goto_meta(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
+		  unsigned int insn_idx, unsigned int n_insns);
 #endif
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index bc879aeb62d4..240db663d83f 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (C) 2016 Netronome Systems, Inc.
+ * Copyright (C) 2016-2017 Netronome Systems, Inc.
  *
  * This software is dual licensed under the GNU General License Version 2,
  * June 1991 as shown in the file COPYING in the top-level directory of this
@@ -55,11 +55,10 @@  static int
 nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,
 		 unsigned int cnt)
 {
+	struct nfp_insn_meta *meta;
 	unsigned int i;
 
 	for (i = 0; i < cnt; i++) {
-		struct nfp_insn_meta *meta;
-
 		meta = kzalloc(sizeof(*meta), GFP_KERNEL);
 		if (!meta)
 			return -ENOMEM;
@@ -70,6 +69,23 @@  nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,
 		list_add_tail(&meta->l, &nfp_prog->insns);
 	}
 
+	/* Another pass to record jump information. */
+	list_for_each_entry(meta, &nfp_prog->insns, l) {
+		u64 code = meta->insn.code;
+
+		if (BPF_CLASS(code) == BPF_JMP && BPF_OP(code) != BPF_EXIT &&
+		    BPF_OP(code) != BPF_CALL) {
+			struct nfp_insn_meta *dst_meta;
+			unsigned short dst_indx;
+
+			dst_indx = meta->n + 1 + meta->insn.off;
+			dst_meta = nfp_bpf_goto_meta(nfp_prog, meta, dst_indx,
+						     cnt);
+
+			meta->jmp_dst = dst_meta;
+		}
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
index 8d43491ddd6b..cca67730b91f 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (C) 2016 Netronome Systems, Inc.
+ * Copyright (C) 2016-2017 Netronome Systems, Inc.
  *
  * This software is dual licensed under the GNU General License Version 2,
  * June 1991 as shown in the file COPYING in the top-level directory of this
@@ -40,7 +40,7 @@ 
 
 #include "main.h"
 
-static struct nfp_insn_meta *
+struct nfp_insn_meta *
 nfp_bpf_goto_meta(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
 		  unsigned int insn_idx, unsigned int n_insns)
 {