All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: Sandipan Das <sandipan@linux.vnet.ibm.com>,
	naveen.n.rao@linux.vnet.ibm.com
Cc: netdev@vger.kernel.org, jakub.kicinski@netronome.com,
	linuxppc-dev@lists.ozlabs.org, ast@fb.com
Subject: Re: [RFC][PATCH bpf] tools: bpftool: Fix tags for bpf-to-bpf calls
Date: Tue, 27 Feb 2018 15:44:55 +0100	[thread overview]
Message-ID: <4cdcc751-d830-51ce-23a0-62f773dc015e@iogearbox.net> (raw)
In-Reply-To: <20180227121346.16199-1-sandipan@linux.vnet.ibm.com>

On 02/27/2018 01:13 PM, Sandipan Das wrote:
> "Naveen N. Rao" wrote:
>> I'm wondering if we can instead encode the bpf prog id in
>> imm32. That way, we should be able to indicate the BPF
>> function being called into.  Daniel, is that something we
>> can consider?
> 
> Since each subprog does not get a separate id, we cannot fetch
> the fd and therefore the tag of a subprog. Instead we can use
> the tag of the complete program as shown below.
> 
> "Daniel Borkmann" wrote:
>> I think one limitation that would still need to be addressed later
>> with such approach would be regarding the xlated prog dump in bpftool,
>> see 'BPF calls via JIT' in 7105e828c087 ("bpf: allow for correlation
>> of maps and helpers in dump"). Any ideas for this (potentially if we
>> could use off + imm for calls, we'd get to 48 bits, but that seems
>> still not be enough as you say)?
> 
> As an alternative, this is what I am thinking of:
> 
> Currently, for bpf-to-bpf calls, if bpf_jit_kallsyms is enabled,
> bpftool looks up the name of the corresponding symbol for the
> JIT-ed subprogram and shows it in the xlated dump alongside the
> actual call instruction. However, the lookup is based on the
> target address which is calculated using the imm field of the
> instruction. So, once again, if imm is truncated, we will end
> up with the wrong address. Also, the subprog aux data (which
> has been proposed as a mitigation for this) is not accessible
> from this tool.
> 
> We can still access the tag for the complete bpf program and use
> this with the correct offset in an objdump-like notation as an
> alterative for the name of the subprog that is the target of a
> bpf-to-bpf call instruction.
> 
> Currently, an xlated dump looks like this:
>    0: (85) call pc+2#bpf_prog_5f76847930402518_F
>    1: (b7) r0 = 1
>    2: (95) exit
>    3: (b7) r0 = 2
>    4: (95) exit
> 
> With this patch, it will look like this:
>    0: (85) call pc+2#bpf_prog_8f85936f29a7790a+3

(Note the +2 is the insn->off already.)

>    1: (b7) r0 = 1
>    2: (95) exit
>    3: (b7) r0 = 2
>    4: (95) exit
> 
> where 8f85936f29a7790a is the tag of the bpf program and 3 is
> the offset to the start of the subprog from the start of the
> program.

The problem with this approach would be that right now the name is
something like bpf_prog_5f76847930402518_F where the subprog tag is
just a placeholder so in future, this may well adapt to e.g. the actual
function name from the elf file. Note that when kallsyms is enabled
then a name like bpf_prog_5f76847930402518_F will also appear in stack
traces, perf records, etc, so for correlation/debugging it would really
help to have them the same everywhere.

Worst case if there's nothing better, potentially what one could do in
bpf_prog_get_info_by_fd() is to dump an array of full addresses and
have the imm part as the index pointing to one of them, just unfortunate
that it's likely only needed in ppc64.

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Borkmann <daniel@iogearbox.net>
To: Sandipan Das <sandipan@linux.vnet.ibm.com>,
	naveen.n.rao@linux.vnet.ibm.com
Cc: ast@fb.com, mpe@ellerman.id.au, jakub.kicinski@netronome.com,
	netdev@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [RFC][PATCH bpf] tools: bpftool: Fix tags for bpf-to-bpf calls
Date: Tue, 27 Feb 2018 15:44:55 +0100	[thread overview]
Message-ID: <4cdcc751-d830-51ce-23a0-62f773dc015e@iogearbox.net> (raw)
In-Reply-To: <20180227121346.16199-1-sandipan@linux.vnet.ibm.com>

On 02/27/2018 01:13 PM, Sandipan Das wrote:
> "Naveen N. Rao" wrote:
>> I'm wondering if we can instead encode the bpf prog id in
>> imm32. That way, we should be able to indicate the BPF
>> function being called into.  Daniel, is that something we
>> can consider?
> 
> Since each subprog does not get a separate id, we cannot fetch
> the fd and therefore the tag of a subprog. Instead we can use
> the tag of the complete program as shown below.
> 
> "Daniel Borkmann" wrote:
>> I think one limitation that would still need to be addressed later
>> with such approach would be regarding the xlated prog dump in bpftool,
>> see 'BPF calls via JIT' in 7105e828c087 ("bpf: allow for correlation
>> of maps and helpers in dump"). Any ideas for this (potentially if we
>> could use off + imm for calls, we'd get to 48 bits, but that seems
>> still not be enough as you say)?
> 
> As an alternative, this is what I am thinking of:
> 
> Currently, for bpf-to-bpf calls, if bpf_jit_kallsyms is enabled,
> bpftool looks up the name of the corresponding symbol for the
> JIT-ed subprogram and shows it in the xlated dump alongside the
> actual call instruction. However, the lookup is based on the
> target address which is calculated using the imm field of the
> instruction. So, once again, if imm is truncated, we will end
> up with the wrong address. Also, the subprog aux data (which
> has been proposed as a mitigation for this) is not accessible
> from this tool.
> 
> We can still access the tag for the complete bpf program and use
> this with the correct offset in an objdump-like notation as an
> alterative for the name of the subprog that is the target of a
> bpf-to-bpf call instruction.
> 
> Currently, an xlated dump looks like this:
>    0: (85) call pc+2#bpf_prog_5f76847930402518_F
>    1: (b7) r0 = 1
>    2: (95) exit
>    3: (b7) r0 = 2
>    4: (95) exit
> 
> With this patch, it will look like this:
>    0: (85) call pc+2#bpf_prog_8f85936f29a7790a+3

(Note the +2 is the insn->off already.)

>    1: (b7) r0 = 1
>    2: (95) exit
>    3: (b7) r0 = 2
>    4: (95) exit
> 
> where 8f85936f29a7790a is the tag of the bpf program and 3 is
> the offset to the start of the subprog from the start of the
> program.

The problem with this approach would be that right now the name is
something like bpf_prog_5f76847930402518_F where the subprog tag is
just a placeholder so in future, this may well adapt to e.g. the actual
function name from the elf file. Note that when kallsyms is enabled
then a name like bpf_prog_5f76847930402518_F will also appear in stack
traces, perf records, etc, so for correlation/debugging it would really
help to have them the same everywhere.

Worst case if there's nothing better, potentially what one could do in
bpf_prog_get_info_by_fd() is to dump an array of full addresses and
have the imm part as the index pointing to one of them, just unfortunate
that it's likely only needed in ppc64.

  reply	other threads:[~2018-02-27 14:44 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-13  4:05 [RFC][PATCH bpf v2 1/2] bpf: allow 64-bit offsets for bpf function calls Sandipan Das
2018-02-13  4:06 ` [RFC][PATCH bpf v2 2/2] bpf: powerpc64: add JIT support for multi-function programs Sandipan Das
2018-02-15 16:25 ` [RFC][PATCH bpf v2 1/2] bpf: allow 64-bit offsets for bpf function calls Daniel Borkmann
2018-02-15 20:18   ` Daniel Borkmann
2018-02-16 15:50     ` Naveen N. Rao
2018-02-16 15:50       ` Naveen N. Rao
2018-02-20  9:29       ` Michael Ellerman
2018-02-20  9:29         ` Michael Ellerman
2018-02-20 19:22         ` Naveen N. Rao
2018-02-20 19:22           ` Naveen N. Rao
2018-02-27 12:13           ` [RFC][PATCH bpf] tools: bpftool: Fix tags for bpf-to-bpf calls Sandipan Das
2018-02-27 12:13             ` Sandipan Das
2018-02-27 14:44             ` Daniel Borkmann [this message]
2018-02-27 14:44               ` Daniel Borkmann
2018-03-01  8:51               ` Naveen N. Rao
2018-03-01  8:51                 ` Naveen N. Rao
2018-03-05 17:02                 ` Alexei Starovoitov
2018-03-05 17:02                   ` Alexei Starovoitov
     [not found]                 ` <415b415e-f47f-082c-1bc9-87d3e9d3aed1__9575.16645216874$1520270545$gmane$org@fb.com>
     [not found]                   ` <415b415e-f47f-082c-1bc9-87d3e9d3aed1__9575.16645216874$1520270545$gmane$org@ fb.com>
2018-05-03 15:20                     ` Naveen N. Rao
2018-05-03 15:20                       ` Naveen N. Rao
2018-02-22 12:06       ` [RFC][PATCH bpf v2 1/2] bpf: allow 64-bit offsets for bpf function calls Michael Holzheu
2018-02-22 12:06         ` Michael Holzheu
2018-02-22 12:10         ` Michael Holzheu
2018-02-22 12:10           ` Michael Holzheu

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=4cdcc751-d830-51ce-23a0-62f773dc015e@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=ast@fb.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=naveen.n.rao@linux.vnet.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=sandipan@linux.vnet.ibm.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.