All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matteo Croce <mcroce@linux.microsoft.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	Lorenz Bauer <lmb@cloudflare.com>,
	Matteo Croce <mcroce@microsoft.com>, bpf <bpf@vger.kernel.org>,
	Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH RFC bpf-next 01/10] bpf: Prepare relo_core.c for kernel duty.
Date: Wed, 29 Sep 2021 19:38:58 +0200	[thread overview]
Message-ID: <20210929193858.57ba3cd1@linux.microsoft.com> (raw)
In-Reply-To: <CAFnufp3hx0CaF=ukCXY3UJj0omVX+5WWk0=-QuENvTPGye_sKA@mail.gmail.com>

On Wed, 29 Sep 2021 14:32:37 +0200
Matteo Croce <mcroce@linux.microsoft.com> wrote:

> On Tue, Sep 28, 2021 at 10:35 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Tue, Sep 28, 2021 at 10:11 AM Matteo Croce
> > <mcroce@linux.microsoft.com> wrote:
> > >
> > > On Tue, 28 Sep 2021 09:37:30 -0700
> > > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > >
> > > > On Tue, Sep 28, 2021 at 04:45:15PM +0200, Matteo Croce wrote:
> > > > > On Fri, 17 Sep 2021 14:57:12 -0700
> > > > > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > > From: Alexei Starovoitov <ast@kernel.org>
> > > > > >
> > > > > > Make relo_core.c to be compiled with kernel and with libbpf.
> > > > > >
> > > > > > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > > > > > ---
> > > > >
> > > > > I give it a try with a sample co-re program.
> > > >
> > > > Thanks for testing!
> > > >
> > >
> > > I just found an error during the relocations.
> > > It was hiding because of bpf_core_apply_relo() always returning
> > > success[1].
> > >
> > > I have a BPF with the following programs:
> > >
> > > #if 0
> > > SEC("tp_btf/xdp_devmap_xmit")
> > > int BPF_PROG(tp_xdp_devmap_xmit_multi, const struct net_device
> > > *from_dev, const struct net_device *to_dev, int sent, int drops,
> > > int err)
> > >
> > > {
> > >         randmap(from_dev->ifindex);
> > >         return 0;
> > > }
> > > #endif
> > >
> > > SEC("fentry/eth_type_trans")
> > > int BPF_PROG(fentry_eth_type_trans, struct sk_buff *skb,
> > >              struct net_device *dev, unsigned short protocol)
> > > {
> > >         randmap(dev->ifindex + skb->len);
> > >         return 0;
> > > }
> > >
> > > SEC("fexit/eth_type_trans")
> > > int BPF_PROG(fexit_eth_type_trans, struct sk_buff *skb,
> > >              struct net_device *dev, unsigned short protocol)
> > > {
> > >         randmap(dev->ifindex + skb->len);
> > >         return 0;
> > > }
> > >
> > >
> > > randmap() just writes the value into a random map. If I keep #if 0
> > > everything works, if I remove it so to build
> > > tp_btf/xdp_devmap_xmit too, I get this:
> > >
> > > [ 3619.229378] libbpf: prog 'prog_name': relo #0: kind <byte_off>
> > > (0), spec is [24] STRUCT net_device.ifindex (0:17 @ offset 208) [
> > > 3619.229384] libbpf: prog 'prog_name': relo #0: matching
> > > candidate #0 [2617] STRUCT net_device.ifindex (0:17 @ offset 208)
> > > [ 3619.229538] libbpf: prog 'prog_name': relo #0: patched insn #0
> > > (LDX/ST/STX) off 208 -> 208 [ 3619.230278] libbpf: prog
> > > 'prog_name': relo #0: kind <byte_off> (0), spec is [87] STRUCT
> > > sk_buff.len (0:5 @ offset 104) [ 3619.230282] libbpf: prog
> > > 'prog_name': relo #0: matching candidate #0 [2660] STRUCT
> > > sk_buff.len (0:5 @ offset 104) [ 3619.230393] libbpf: prog
> > > 'prog_name': relo #0: trying to relocate unrecognized insn #0,
> > > code:0x85, src:0x0, dst:0x0, off:0x0, imm:0x7 [ 3619.230562]
> > > libbpf: prog 'prog_name': relo #0: failed to patch insn #0: -22
> > >
> > > The program in tp_btf/xdp_devmap_xmit makes the relocations into
> > > another section fail, note that sk_buff.len is used in the fentry
> > > program.
> > >
> > > Ideas?
> >
> > I'll take a look. Could you provide the full .c file?
> 
> Sure. I put everything in this repo:
> 
> https://gist.github.com/teknoraver/2855e0f8770d1363b57d683fa32bccc3
> 
> tp_btf/xdp_devmap_xmit is the program which lets the other fail.
> 

I enabled debugging in userspace libbpf to compare the two outputs.

Userspace libbpf:

libbpf: CO-RE relocating [0] struct net_device: found target candidate [2617] struct net_device in [vmlinux]
libbpf: prog 'tp_xdp_devmap_xmit_multi': relo #0: kind <byte_off> (0), spec is [24] struct net_device.ifindex (0:17 @ offset 208)
libbpf: prog 'tp_xdp_devmap_xmit_multi': relo #0: matching candidate #0 [2617] struct net_device.ifindex (0:17 @ offset 208)
libbpf: prog 'tp_xdp_devmap_xmit_multi': relo #0: patched insn #2 (LDX/ST/STX) off 208 -> 208
libbpf: prog 'tp_xdp_devmap_xmit_multi': relo #1: kind <byte_off> (0), spec is [24] struct net_device.ifindex (0:17 @ offset 208)
libbpf: prog 'tp_xdp_devmap_xmit_multi': relo #1: matching candidate #0 [2617] struct net_device.ifindex (0:17 @ offset 208)
libbpf: prog 'tp_xdp_devmap_xmit_multi': relo #1: patched insn #3 (LDX/ST/STX) off 208 -> 208
libbpf: sec 'fentry/eth_type_trans': found 2 CO-RE relocations
libbpf: CO-RE relocating [0] struct sk_buff: found target candidate [2660] struct sk_buff in [vmlinux]
libbpf: prog 'fentry_eth_type_trans': relo #0: kind <byte_off> (0), spec is [87] struct sk_buff.len (0:5 @ offset 104)
libbpf: prog 'fentry_eth_type_trans': relo #0: matching candidate #0 [2660] struct sk_buff.len (0:5 @ offset 104)
libbpf: prog 'fentry_eth_type_trans': relo #0: patched insn #2 (LDX/ST/STX) off 104 -> 104
libbpf: prog 'fentry_eth_type_trans': relo #1: kind <byte_off> (0), spec is [24] struct net_device.ifindex (0:17 @ offset 208)
libbpf: prog 'fentry_eth_type_trans': relo #1: matching candidate #0 [2617] struct net_device.ifindex (0:17 @ offset 208)
libbpf: prog 'fentry_eth_type_trans': relo #1: patched insn #3 (LDX/ST/STX) off 208 -> 208
libbpf: sec 'fexit/eth_type_trans': found 2 CO-RE relocations
libbpf: prog 'fexit_eth_type_trans': relo #0: kind <byte_off> (0), spec is [87] struct sk_buff.len (0:5 @ offset 104)
libbpf: prog 'fexit_eth_type_trans': relo #0: matching candidate #0 [2660] struct sk_buff.len (0:5 @ offset 104)
libbpf: prog 'fexit_eth_type_trans': relo #0: patched insn #2 (LDX/ST/STX) off 104 -> 104
libbpf: prog 'fexit_eth_type_trans': relo #1: kind <byte_off> (0), spec is [24] struct net_device.ifindex (0:17 @ offset 208)
libbpf: prog 'fexit_eth_type_trans': relo #1: matching candidate #0 [2617] struct net_device.ifindex (0:17 @ offset 208)
libbpf: prog 'fexit_eth_type_trans': relo #1: patched insn #3 (LDX/ST/STX) off 208 -> 208

Kernel implementation

[13234.650397] libbpf: prog 'prog_name': relo #0: kind <byte_off> (0), spec is [24] STRUCT net_device.ifindex (0:17 @ offset 208)
[13234.650406] libbpf: prog 'prog_name': relo #0: matching candidate #0 [2617] STRUCT net_device.ifindex (0:17 @ offset 208)
[13234.650558] libbpf: prog 'prog_name': relo #0: patched insn #0 (LDX/ST/STX) off 208 -> 208
[13234.651251] libbpf: prog 'prog_name': relo #0: kind <byte_off> (0), spec is [24] STRUCT net_device.ifindex (0:17 @ offset 208)
[13234.651255] libbpf: prog 'prog_name': relo #0: matching candidate #0 [2617] STRUCT net_device.ifindex (0:17 @ offset 208)
[13234.651349] libbpf: prog 'prog_name': relo #0: patched insn #0 (LDX/ST/STX) off 208 -> 208
[13234.651935] libbpf: prog 'prog_name': relo #0: kind <byte_off> (0), spec is [87] STRUCT sk_buff.len (0:5 @ offset 104)
[13234.651939] libbpf: prog 'prog_name': relo #0: matching candidate #0 [2660] STRUCT sk_buff.len (0:5 @ offset 104)
[13234.652001] libbpf: prog 'prog_name': relo #0: unexpected insn #0 (LDX/ST/STX) value: got 208, exp 104 -> 104
[13234.652105] libbpf: prog 'prog_name': relo #0: failed to patch insn #0: -22

The sk_buff.len has a wrong offset, 208 instead of 104. Can it be a
leftover value from the previous relocation?

Regards,
-- 
per aspera ad upstream

  reply	other threads:[~2021-09-29 17:39 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-17 21:57 [PATCH RFC bpf-next 00/10] bpf: CO-RE support in the kernel Alexei Starovoitov
2021-09-17 21:57 ` [PATCH RFC bpf-next 01/10] bpf: Prepare relo_core.c for kernel duty Alexei Starovoitov
2021-09-21 21:25   ` Andrii Nakryiko
2021-09-28 14:45   ` Matteo Croce
2021-09-28 16:37     ` Alexei Starovoitov
2021-09-28 17:11       ` Matteo Croce
2021-09-28 20:34         ` Alexei Starovoitov
2021-09-29 12:32           ` Matteo Croce
2021-09-29 17:38             ` Matteo Croce [this message]
2021-09-29 23:00               ` Alexei Starovoitov
2021-09-29 23:49                 ` Matteo Croce
2021-10-22  0:48                   ` Matteo Croce
2021-10-22  0:51                     ` Alexei Starovoitov
2021-09-17 21:57 ` [PATCH RFC bpf-next 02/10] bpf: Define enum bpf_core_relo_kind as uapi Alexei Starovoitov
2021-09-21 21:27   ` Andrii Nakryiko
2021-09-17 21:57 ` [PATCH RFC bpf-next 03/10] bpf: Add proto of bpf_core_apply_relo() Alexei Starovoitov
2021-09-23 11:21   ` Lorenz Bauer
2021-09-23 18:48     ` Andrii Nakryiko
2021-09-17 21:57 ` [PATCH RFC bpf-next 04/10] bpf: Add bpf_core_add_cands() and wire it into bpf_core_apply_relo_insn() Alexei Starovoitov
2021-09-21 21:34   ` Andrii Nakryiko
2021-09-27 18:04   ` Matteo Croce
2021-09-17 21:57 ` [PATCH RFC bpf-next 05/10] libbpf: Use CO-RE in the kernel in light skeleton Alexei Starovoitov
2021-09-17 21:57 ` [PATCH RFC bpf-next 06/10] libbpf: Make gen_loader data aligned Alexei Starovoitov
2021-09-17 21:57 ` [PATCH RFC bpf-next 07/10] libbpf: Support init of inner maps in light skeleton Alexei Starovoitov
2021-09-17 21:57 ` [PATCH RFC bpf-next 08/10] selftests/bpf: Convert kfunc test with CO-RE to lskel Alexei Starovoitov
2021-09-17 21:57 ` [PATCH RFC bpf-next 09/10] selftests/bpf: Improve inner_map test coverage Alexei Starovoitov
2021-09-17 21:57 ` [PATCH RFC bpf-next 10/10] selftests/bpf: Convert map_ptr_kern test to use light skeleton Alexei Starovoitov
2021-09-23 11:33 ` [PATCH RFC bpf-next 00/10] bpf: CO-RE support in the kernel Lorenz Bauer
2021-09-23 18:52   ` Andrii Nakryiko
2021-09-24 23:13   ` Alexei Starovoitov
2021-09-27 16:12     ` Lorenz Bauer
2021-09-27 16:50       ` Alexei Starovoitov
2021-09-28  8:30         ` Lorenz Bauer
2021-09-28 16:35           ` Alexei Starovoitov

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=20210929193858.57ba3cd1@linux.microsoft.com \
    --to=mcroce@linux.microsoft.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=lmb@cloudflare.com \
    --cc=mcroce@microsoft.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.