nfp: bpf: record jump destination to simplify jump fixup
authorJiong Wang <jiong.wang@netronome.com>
Fri, 1 Dec 2017 05:32:50 +0000 (21:32 -0800)
committerDaniel Borkmann <daniel@iogearbox.net>
Fri, 1 Dec 2017 19:59:19 +0000 (20:59 +0100)
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>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
drivers/net/ethernet/netronome/nfp/bpf/jit.c
drivers/net/ethernet/netronome/nfp/bpf/main.h
drivers/net/ethernet/netronome/nfp/bpf/offload.c
drivers/net/ethernet/netronome/nfp/bpf/verifier.c

index 20daf6b95601b127b4df492c365e430e7d0939ec..f76659ecb6544529c36eab61068bb26047171ad3 100644 (file)
             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);
                }
        }
 
index 0f4d218fc77ae8f344cb3d273910a7e304dcdc87..e488656f406cd1c95ac69c494c104f16d16ec351 100644 (file)
@@ -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
index bc879aeb62d4ee95a42654a78ae9329145d22b3f..240db663d83fac34f5d989f00ae28e950c412ff7 100644 (file)
@@ -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;
 }
 
index 8d43491ddd6b8f2f6597debb32a476f7a9b5a896..cca67730b91f4bd715dd72c010eca5e8dda5c6f1 100644 (file)
@@ -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)
 {