Skip to content

Commit

Permalink
nfp: bpf: record jump destination to simplify jump fixup
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jiong Wang authored and Daniel Borkmann committed Dec 1, 2017
1 parent 854dc87 commit 5b67414
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 55 deletions.
57 changes: 9 additions & 48 deletions drivers/net/ethernet/netronome/nfp/bpf/jit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -1864,23 +1858,19 @@ 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)
continue;
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);

Expand All @@ -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;
}
Expand All @@ -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);
}
}

Expand Down
13 changes: 11 additions & 2 deletions drivers/net/ethernet/netronome/nfp/bpf/main.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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
22 changes: 19 additions & 3 deletions drivers/net/ethernet/netronome/nfp/bpf/offload.c
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/netronome/nfp/bpf/verifier.c
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
{
Expand Down

0 comments on commit 5b67414

Please sign in to comment.