Commit 5b674140 authored by Jiong Wang's avatar Jiong Wang Committed by Daniel Borkmann

nfp: bpf: record jump destination to simplify jump fixup

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: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: default avatarJiong Wang <jiong.wang@netronome.com>
Reviewed-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
parent 854dc87d
......@@ -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);
}
}
......
......@@ -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
/*
* 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;
}
......
/*
* 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)
{
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment