netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next] libbpf: support CO-RE relocations for LD/LDX/ST/STX instructions
@ 2019-12-23  6:18 Andrii Nakryiko
  2019-12-23  7:48 ` Yonghong Song
  0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2019-12-23  6:18 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Clang patch [0] enables emitting relocatable generic ALU/ALU64 instructions
(i.e, shifts and arithmetic operations), as well as generic load/store
instructions. The former ones are already supported by libbpf as is. This
patch adds further support for load/store instructions. Relocatable field
offset is encoded in BPF instruction's 16-bit offset section and are adjusted
by libbpf based on target kernel BTF.

These Clang changes and corresponding libbpf changes allow for more succinct
generated BPF code by encoding relocatable field reads as a single
LD/ST/LDX/STX instruction. It also enables relocatable access to BPF context.
Previously, if context struct (e.g., __sk_buff) was accessed with CO-RE
relocations (e.g., due to preserve_access_index attribute), it would be
rejected by BPF verifier due to modified context pointer dereference. With
Clang patch, such context accesses are both relocatable and have a fixed
offset from the point of view of BPF verifier.

  [0] https://reviews.llvm.org/D71790

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9576a90c5a1c..2dbc2204a02c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -18,6 +18,7 @@
 #include <stdarg.h>
 #include <libgen.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <string.h>
 #include <unistd.h>
 #include <endian.h>
@@ -3810,11 +3811,13 @@ static int bpf_core_reloc_insn(struct bpf_program *prog,
 	insn = &prog->insns[insn_idx];
 	class = BPF_CLASS(insn->code);
 
-	if (class == BPF_ALU || class == BPF_ALU64) {
+	switch (class) {
+	case BPF_ALU:
+	case BPF_ALU64:
 		if (BPF_SRC(insn->code) != BPF_K)
 			return -EINVAL;
 		if (!failed && validate && insn->imm != orig_val) {
-			pr_warn("prog '%s': unexpected insn #%d value: got %u, exp %u -> %u\n",
+			pr_warn("prog '%s': unexpected insn #%d (ALU/ALU64) value: got %u, exp %u -> %u\n",
 				bpf_program__title(prog, false), insn_idx,
 				insn->imm, orig_val, new_val);
 			return -EINVAL;
@@ -3824,7 +3827,30 @@ static int bpf_core_reloc_insn(struct bpf_program *prog,
 		pr_debug("prog '%s': patched insn #%d (ALU/ALU64)%s imm %u -> %u\n",
 			 bpf_program__title(prog, false), insn_idx,
 			 failed ? " w/ failed reloc" : "", orig_val, new_val);
-	} else {
+		break;
+	case BPF_LD:
+	case BPF_LDX:
+	case BPF_ST:
+	case BPF_STX:
+		if (!failed && validate && insn->off != orig_val) {
+			pr_warn("prog '%s': unexpected insn #%d (LD/LDX/ST/STX) value: got %u, exp %u -> %u\n",
+				bpf_program__title(prog, false), insn_idx,
+				insn->off, orig_val, new_val);
+			return -EINVAL;
+		}
+		if (new_val > SHRT_MAX) {
+			pr_warn("prog '%s': insn #%d (LD/LDX/ST/STX) value too big: %u\n",
+				bpf_program__title(prog, false), insn_idx,
+				new_val);
+			return -ERANGE;
+		}
+		orig_val = insn->off;
+		insn->off = new_val;
+		pr_debug("prog '%s': patched insn #%d (LD/LDX/ST/STX)%s off %u -> %u\n",
+			 bpf_program__title(prog, false), insn_idx,
+			 failed ? " w/ failed reloc" : "", orig_val, new_val);
+		break;
+	default:
 		pr_warn("prog '%s': trying to relocate unrecognized insn #%d, code:%x, src:%x, dst:%x, off:%x, imm:%x\n",
 			bpf_program__title(prog, false),
 			insn_idx, insn->code, insn->src_reg, insn->dst_reg,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] libbpf: support CO-RE relocations for LD/LDX/ST/STX instructions
  2019-12-23  6:18 [PATCH bpf-next] libbpf: support CO-RE relocations for LD/LDX/ST/STX instructions Andrii Nakryiko
@ 2019-12-23  7:48 ` Yonghong Song
  2019-12-23 17:58   ` Andrii Nakryiko
  0 siblings, 1 reply; 3+ messages in thread
From: Yonghong Song @ 2019-12-23  7:48 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, Alexei Starovoitov, daniel
  Cc: andrii.nakryiko, Kernel Team



On 12/22/19 10:18 PM, Andrii Nakryiko wrote:
> Clang patch [0] enables emitting relocatable generic ALU/ALU64 instructions
> (i.e, shifts and arithmetic operations), as well as generic load/store
> instructions. The former ones are already supported by libbpf as is. This
> patch adds further support for load/store instructions. Relocatable field
> offset is encoded in BPF instruction's 16-bit offset section and are adjusted
> by libbpf based on target kernel BTF.
> 
> These Clang changes and corresponding libbpf changes allow for more succinct
> generated BPF code by encoding relocatable field reads as a single
> LD/ST/LDX/STX instruction. It also enables relocatable access to BPF context.
> Previously, if context struct (e.g., __sk_buff) was accessed with CO-RE
> relocations (e.g., due to preserve_access_index attribute), it would be
> rejected by BPF verifier due to modified context pointer dereference. With
> Clang patch, such context accesses are both relocatable and have a fixed
> offset from the point of view of BPF verifier.
> 
>    [0] https://reviews.llvm.org/D71790
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>   tools/lib/bpf/libbpf.c | 32 +++++++++++++++++++++++++++++---
>   1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 9576a90c5a1c..2dbc2204a02c 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -18,6 +18,7 @@
>   #include <stdarg.h>
>   #include <libgen.h>
>   #include <inttypes.h>
> +#include <limits.h>
>   #include <string.h>
>   #include <unistd.h>
>   #include <endian.h>
> @@ -3810,11 +3811,13 @@ static int bpf_core_reloc_insn(struct bpf_program *prog,
>   	insn = &prog->insns[insn_idx];
>   	class = BPF_CLASS(insn->code);
>   
> -	if (class == BPF_ALU || class == BPF_ALU64) {
> +	switch (class) {
> +	case BPF_ALU:
> +	case BPF_ALU64:
>   		if (BPF_SRC(insn->code) != BPF_K)
>   			return -EINVAL;
>   		if (!failed && validate && insn->imm != orig_val) {
> -			pr_warn("prog '%s': unexpected insn #%d value: got %u, exp %u -> %u\n",
> +			pr_warn("prog '%s': unexpected insn #%d (ALU/ALU64) value: got %u, exp %u -> %u\n",
>   				bpf_program__title(prog, false), insn_idx,
>   				insn->imm, orig_val, new_val);
>   			return -EINVAL;
> @@ -3824,7 +3827,30 @@ static int bpf_core_reloc_insn(struct bpf_program *prog,
>   		pr_debug("prog '%s': patched insn #%d (ALU/ALU64)%s imm %u -> %u\n",
>   			 bpf_program__title(prog, false), insn_idx,
>   			 failed ? " w/ failed reloc" : "", orig_val, new_val);
> -	} else {
> +		break;
> +	case BPF_LD:

Maybe we should remove BPF_LD here? BPF_LD is used for ld_imm64, ld_abs 
and ld_ind, where the insn->off = 0 and not really used.

> +	case BPF_LDX:
> +	case BPF_ST:
> +	case BPF_STX: > +		if (!failed && validate && insn->off != orig_val) {
> +			pr_warn("prog '%s': unexpected insn #%d (LD/LDX/ST/STX) value: got %u, exp %u -> %u\n",
> +				bpf_program__title(prog, false), insn_idx,
> +				insn->off, orig_val, new_val);
> +			return -EINVAL;
> +		}
> +		if (new_val > SHRT_MAX) {
> +			pr_warn("prog '%s': insn #%d (LD/LDX/ST/STX) value too big: %u\n",
> +				bpf_program__title(prog, false), insn_idx,
> +				new_val);
> +			return -ERANGE;
> +		}
> +		orig_val = insn->off;
> +		insn->off = new_val;
> +		pr_debug("prog '%s': patched insn #%d (LD/LDX/ST/STX)%s off %u -> %u\n",
> +			 bpf_program__title(prog, false), insn_idx,
> +			 failed ? " w/ failed reloc" : "", orig_val, new_val);
> +		break;
> +	default:
>   		pr_warn("prog '%s': trying to relocate unrecognized insn #%d, code:%x, src:%x, dst:%x, off:%x, imm:%x\n",
>   			bpf_program__title(prog, false),
>   			insn_idx, insn->code, insn->src_reg, insn->dst_reg,
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] libbpf: support CO-RE relocations for LD/LDX/ST/STX instructions
  2019-12-23  7:48 ` Yonghong Song
@ 2019-12-23 17:58   ` Andrii Nakryiko
  0 siblings, 0 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2019-12-23 17:58 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Andrii Nakryiko, bpf, netdev, Alexei Starovoitov, daniel, Kernel Team

On Sun, Dec 22, 2019 at 11:48 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 12/22/19 10:18 PM, Andrii Nakryiko wrote:
> > Clang patch [0] enables emitting relocatable generic ALU/ALU64 instructions
> > (i.e, shifts and arithmetic operations), as well as generic load/store
> > instructions. The former ones are already supported by libbpf as is. This
> > patch adds further support for load/store instructions. Relocatable field
> > offset is encoded in BPF instruction's 16-bit offset section and are adjusted
> > by libbpf based on target kernel BTF.
> >
> > These Clang changes and corresponding libbpf changes allow for more succinct
> > generated BPF code by encoding relocatable field reads as a single
> > LD/ST/LDX/STX instruction. It also enables relocatable access to BPF context.
> > Previously, if context struct (e.g., __sk_buff) was accessed with CO-RE
> > relocations (e.g., due to preserve_access_index attribute), it would be
> > rejected by BPF verifier due to modified context pointer dereference. With
> > Clang patch, such context accesses are both relocatable and have a fixed
> > offset from the point of view of BPF verifier.
> >
> >    [0] https://reviews.llvm.org/D71790
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >   tools/lib/bpf/libbpf.c | 32 +++++++++++++++++++++++++++++---
> >   1 file changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 9576a90c5a1c..2dbc2204a02c 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -18,6 +18,7 @@
> >   #include <stdarg.h>
> >   #include <libgen.h>
> >   #include <inttypes.h>
> > +#include <limits.h>
> >   #include <string.h>
> >   #include <unistd.h>
> >   #include <endian.h>
> > @@ -3810,11 +3811,13 @@ static int bpf_core_reloc_insn(struct bpf_program *prog,
> >       insn = &prog->insns[insn_idx];
> >       class = BPF_CLASS(insn->code);
> >
> > -     if (class == BPF_ALU || class == BPF_ALU64) {
> > +     switch (class) {
> > +     case BPF_ALU:
> > +     case BPF_ALU64:
> >               if (BPF_SRC(insn->code) != BPF_K)
> >                       return -EINVAL;
> >               if (!failed && validate && insn->imm != orig_val) {
> > -                     pr_warn("prog '%s': unexpected insn #%d value: got %u, exp %u -> %u\n",
> > +                     pr_warn("prog '%s': unexpected insn #%d (ALU/ALU64) value: got %u, exp %u -> %u\n",
> >                               bpf_program__title(prog, false), insn_idx,
> >                               insn->imm, orig_val, new_val);
> >                       return -EINVAL;
> > @@ -3824,7 +3827,30 @@ static int bpf_core_reloc_insn(struct bpf_program *prog,
> >               pr_debug("prog '%s': patched insn #%d (ALU/ALU64)%s imm %u -> %u\n",
> >                        bpf_program__title(prog, false), insn_idx,
> >                        failed ? " w/ failed reloc" : "", orig_val, new_val);
> > -     } else {
> > +             break;
> > +     case BPF_LD:
>
> Maybe we should remove BPF_LD here? BPF_LD is used for ld_imm64, ld_abs
> and ld_ind, where the insn->off = 0 and not really used.

Sure, I can drop BPF_LD case. Will send v2 soon.

> > +     case BPF_LDX:
> > +     case BPF_ST:
> > +     case BPF_STX: > +               if (!failed && validate && insn->off != orig_val) {
> > +                     pr_warn("prog '%s': unexpected insn #%d (LD/LDX/ST/STX) value: got %u, exp %u -> %u\n",
> > +                             bpf_program__title(prog, false), insn_idx,
> > +                             insn->off, orig_val, new_val);
> > +                     return -EINVAL;
> > +             }
> > +             if (new_val > SHRT_MAX) {
> > +                     pr_warn("prog '%s': insn #%d (LD/LDX/ST/STX) value too big: %u\n",
> > +                             bpf_program__title(prog, false), insn_idx,
> > +                             new_val);
> > +                     return -ERANGE;
> > +             }
> > +             orig_val = insn->off;
> > +             insn->off = new_val;
> > +             pr_debug("prog '%s': patched insn #%d (LD/LDX/ST/STX)%s off %u -> %u\n",
> > +                      bpf_program__title(prog, false), insn_idx,
> > +                      failed ? " w/ failed reloc" : "", orig_val, new_val);
> > +             break;
> > +     default:
> >               pr_warn("prog '%s': trying to relocate unrecognized insn #%d, code:%x, src:%x, dst:%x, off:%x, imm:%x\n",
> >                       bpf_program__title(prog, false),
> >                       insn_idx, insn->code, insn->src_reg, insn->dst_reg,
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-12-23 17:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-23  6:18 [PATCH bpf-next] libbpf: support CO-RE relocations for LD/LDX/ST/STX instructions Andrii Nakryiko
2019-12-23  7:48 ` Yonghong Song
2019-12-23 17:58   ` Andrii Nakryiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).