Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1709450 > unrolled thread

[PATCH] perf bpf: Fix endianness problem when loading parameters in prologue

Started byWang Nan <wangnan0@huawei.com>
First post2017-08-11 12:50 +0200
Last post2017-08-11 21:30 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] perf bpf: Fix endianness problem when loading parameters in prologue Wang Nan <wangnan0@huawei.com> - 2017-08-11 12:50 +0200
    Re: [PATCH] perf bpf: Fix endianness problem when loading parameters  in prologue "Wangnan (F)" <wangnan0@huawei.com> - 2017-08-11 12:50 +0200
      Re: [PATCH] perf bpf: Fix endianness problem when loading parameters  in prologue Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-08-11 21:30 +0200

#1709450 — [PATCH] perf bpf: Fix endianness problem when loading parameters in prologue

FromWang Nan <wangnan0@huawei.com>
Date2017-08-11 12:50 +0200
Subject[PATCH] perf bpf: Fix endianness problem when loading parameters in prologue
Message-ID<udfFT-2pX-5@gated-at.bofh.it>
Perf BPF prologue generator unconditionally fetches 8 bytes for function
parameters, which causes problem on big endian machine. Thomas gives a
detail analysis for this problem:

 http://lkml.kernel.org/r/968ebda5-abe4-8830-8d69-49f62529d151@linux.vnet.ibm.com

This patch parses the type of each argument and converts data from
memory to expected type.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Thomas Richter <tmricht@linux.vnet.ibm.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Cc: Li Zefan <lizefan@huawei.com>
---
 tools/perf/tests/bpf-script-test-prologue.c |  4 ++-
 tools/perf/util/bpf-prologue.c              | 49 +++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/bpf-script-test-prologue.c b/tools/perf/tests/bpf-script-test-prologue.c
index b4ebc75..43f1e16 100644
--- a/tools/perf/tests/bpf-script-test-prologue.c
+++ b/tools/perf/tests/bpf-script-test-prologue.c
@@ -26,9 +26,11 @@ static void (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
 	(void *) 6;
 
 SEC("func=null_lseek file->f_mode offset orig")
-int bpf_func__null_lseek(void *ctx, int err, unsigned long f_mode,
+int bpf_func__null_lseek(void *ctx, int err, unsigned long _f_mode,
 			 unsigned long offset, unsigned long orig)
 {
+	fmode_t f_mode = (fmode_t)_f_mode;
+
 	if (err)
 		return 0;
 	if (f_mode & FMODE_WRITE)
diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c
index 6cdbee1..ce28993 100644
--- a/tools/perf/util/bpf-prologue.c
+++ b/tools/perf/util/bpf-prologue.c
@@ -57,6 +57,46 @@ check_pos(struct bpf_insn_pos *pos)
 	return 0;
 }
 
+/*
+ * Convert type string (u8/u16/u32/u64/s8/s16/s32/s64 ..., see
+ * Documentation/trace/kprobetrace.txt) to size field of BPF_LDX_MEM
+ * instruction (BPF_{B,H,W,DW}).
+ */
+static int
+argtype_to_ldx_size(const char *type)
+{
+	int arg_size = type ? atoi(&type[1]) : 64;
+
+	switch (arg_size) {
+	case 8:
+		return BPF_B;
+	case 16:
+		return BPF_H;
+	case 32:
+		return BPF_W;
+	case 64:
+	default:
+		return BPF_DW;
+	}
+}
+
+static const char *
+insn_sz_to_str(int insn_sz)
+{
+	switch (insn_sz) {
+	case BPF_B:
+		return "BPF_B";
+	case BPF_H:
+		return "BPF_H";
+	case BPF_W:
+		return "BPF_W";
+	case BPF_DW:
+		return "BPF_DW";
+	default:
+		return "UNKNOWN";
+	}
+}
+
 /* Give it a shorter name */
 #define ins(i, p) append_insn((i), (p))
 
@@ -257,9 +297,14 @@ gen_prologue_slowpath(struct bpf_insn_pos *pos,
 	}
 
 	/* Final pass: read to registers */
-	for (i = 0; i < nargs; i++)
-		ins(BPF_LDX_MEM(BPF_DW, BPF_PROLOGUE_START_ARG_REG + i,
+	for (i = 0; i < nargs; i++) {
+		int insn_sz = argtype_to_ldx_size(args[i].type);
+
+		pr_debug("prologue: load arg %d, insn_sz is %s\n",
+			 i, insn_sz_to_str(insn_sz));
+		ins(BPF_LDX_MEM(insn_sz, BPF_PROLOGUE_START_ARG_REG + i,
 				BPF_REG_FP, -BPF_REG_SIZE * (i + 1)), pos);
+	}
 
 	ins(BPF_JMP_IMM(BPF_JA, BPF_REG_0, 0, JMP_TO_SUCCESS_CODE), pos);
 
-- 
2.10.1

[toc] | [next] | [standalone]


#1709456 — Re: [PATCH] perf bpf: Fix endianness problem when loading parameters in prologue

From"Wangnan (F)" <wangnan0@huawei.com>
Date2017-08-11 12:50 +0200
SubjectRe: [PATCH] perf bpf: Fix endianness problem when loading parameters in prologue
Message-ID<udfFU-2pX-21@gated-at.bofh.it>
In reply to#1709450
Hi Thomas,

Please try this patch on your machine and give me the result.

Thank you.

On 2017/8/13 2:49, Wang Nan wrote:
> Perf BPF prologue generator unconditionally fetches 8 bytes for function
> parameters, which causes problem on big endian machine. Thomas gives a
> detail analysis for this problem:
>
>   http://lkml.kernel.org/r/968ebda5-abe4-8830-8d69-49f62529d151@linux.vnet.ibm.com
>
> This patch parses the type of each argument and converts data from
> memory to expected type.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Thomas Richter <tmricht@linux.vnet.ibm.com>
> Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
> Cc: Li Zefan <lizefan@huawei.com>
> ---
>   tools/perf/tests/bpf-script-test-prologue.c |  4 ++-
>   tools/perf/util/bpf-prologue.c              | 49 +++++++++++++++++++++++++++--
>   2 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/tests/bpf-script-test-prologue.c b/tools/perf/tests/bpf-script-test-prologue.c
> index b4ebc75..43f1e16 100644
> --- a/tools/perf/tests/bpf-script-test-prologue.c
> +++ b/tools/perf/tests/bpf-script-test-prologue.c
> @@ -26,9 +26,11 @@ static void (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>   	(void *) 6;
>   
>   SEC("func=null_lseek file->f_mode offset orig")
> -int bpf_func__null_lseek(void *ctx, int err, unsigned long f_mode,
> +int bpf_func__null_lseek(void *ctx, int err, unsigned long _f_mode,
>   			 unsigned long offset, unsigned long orig)
>   {
> +	fmode_t f_mode = (fmode_t)_f_mode;
> +
>   	if (err)
>   		return 0;
>   	if (f_mode & FMODE_WRITE)
> diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c
> index 6cdbee1..ce28993 100644
> --- a/tools/perf/util/bpf-prologue.c
> +++ b/tools/perf/util/bpf-prologue.c
> @@ -57,6 +57,46 @@ check_pos(struct bpf_insn_pos *pos)
>   	return 0;
>   }
>   
> +/*
> + * Convert type string (u8/u16/u32/u64/s8/s16/s32/s64 ..., see
> + * Documentation/trace/kprobetrace.txt) to size field of BPF_LDX_MEM
> + * instruction (BPF_{B,H,W,DW}).
> + */
> +static int
> +argtype_to_ldx_size(const char *type)
> +{
> +	int arg_size = type ? atoi(&type[1]) : 64;
> +
> +	switch (arg_size) {
> +	case 8:
> +		return BPF_B;
> +	case 16:
> +		return BPF_H;
> +	case 32:
> +		return BPF_W;
> +	case 64:
> +	default:
> +		return BPF_DW;
> +	}
> +}
> +
> +static const char *
> +insn_sz_to_str(int insn_sz)
> +{
> +	switch (insn_sz) {
> +	case BPF_B:
> +		return "BPF_B";
> +	case BPF_H:
> +		return "BPF_H";
> +	case BPF_W:
> +		return "BPF_W";
> +	case BPF_DW:
> +		return "BPF_DW";
> +	default:
> +		return "UNKNOWN";
> +	}
> +}
> +
>   /* Give it a shorter name */
>   #define ins(i, p) append_insn((i), (p))
>   
> @@ -257,9 +297,14 @@ gen_prologue_slowpath(struct bpf_insn_pos *pos,
>   	}
>   
>   	/* Final pass: read to registers */
> -	for (i = 0; i < nargs; i++)
> -		ins(BPF_LDX_MEM(BPF_DW, BPF_PROLOGUE_START_ARG_REG + i,
> +	for (i = 0; i < nargs; i++) {
> +		int insn_sz = argtype_to_ldx_size(args[i].type);
> +
> +		pr_debug("prologue: load arg %d, insn_sz is %s\n",
> +			 i, insn_sz_to_str(insn_sz));
> +		ins(BPF_LDX_MEM(insn_sz, BPF_PROLOGUE_START_ARG_REG + i,
>   				BPF_REG_FP, -BPF_REG_SIZE * (i + 1)), pos);
> +	}
>   
>   	ins(BPF_JMP_IMM(BPF_JA, BPF_REG_0, 0, JMP_TO_SUCCESS_CODE), pos);
>   

[toc] | [prev] | [next] | [standalone]


#1709927 — Re: [PATCH] perf bpf: Fix endianness problem when loading parameters in prologue

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2017-08-11 21:30 +0200
SubjectRe: [PATCH] perf bpf: Fix endianness problem when loading parameters in prologue
Message-ID<udnN7-7vf-3@gated-at.bofh.it>
In reply to#1709456
Em Fri, Aug 11, 2017 at 06:47:56PM +0800, Wangnan (F) escreveu:
> Hi Thomas,
> 
> Please try this patch on your machine and give me the result.

Right, I'm waiting for test results for the last two patches from Wang:

      (3.0K) [PATCH] perf bpf: Fix endianness problem when loading parameters in prologue
      (1.2K) [PATCH] perf test llvm: Fix f_mode endianness problem

Thanks,

- Arnaldo
 
> Thank you.
> 
> On 2017/8/13 2:49, Wang Nan wrote:
> > Perf BPF prologue generator unconditionally fetches 8 bytes for function
> > parameters, which causes problem on big endian machine. Thomas gives a
> > detail analysis for this problem:
> > 
> >   http://lkml.kernel.org/r/968ebda5-abe4-8830-8d69-49f62529d151@linux.vnet.ibm.com
> > 
> > This patch parses the type of each argument and converts data from
> > memory to expected type.
> > 
> > Signed-off-by: Wang Nan <wangnan0@huawei.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Cc: Thomas Richter <tmricht@linux.vnet.ibm.com>
> > Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> > Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
> > Cc: Li Zefan <lizefan@huawei.com>
> > ---
> >   tools/perf/tests/bpf-script-test-prologue.c |  4 ++-
> >   tools/perf/util/bpf-prologue.c              | 49 +++++++++++++++++++++++++++--
> >   2 files changed, 50 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/tests/bpf-script-test-prologue.c b/tools/perf/tests/bpf-script-test-prologue.c
> > index b4ebc75..43f1e16 100644
> > --- a/tools/perf/tests/bpf-script-test-prologue.c
> > +++ b/tools/perf/tests/bpf-script-test-prologue.c
> > @@ -26,9 +26,11 @@ static void (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> >   	(void *) 6;
> >   SEC("func=null_lseek file->f_mode offset orig")
> > -int bpf_func__null_lseek(void *ctx, int err, unsigned long f_mode,
> > +int bpf_func__null_lseek(void *ctx, int err, unsigned long _f_mode,
> >   			 unsigned long offset, unsigned long orig)
> >   {
> > +	fmode_t f_mode = (fmode_t)_f_mode;
> > +
> >   	if (err)
> >   		return 0;
> >   	if (f_mode & FMODE_WRITE)
> > diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c
> > index 6cdbee1..ce28993 100644
> > --- a/tools/perf/util/bpf-prologue.c
> > +++ b/tools/perf/util/bpf-prologue.c
> > @@ -57,6 +57,46 @@ check_pos(struct bpf_insn_pos *pos)
> >   	return 0;
> >   }
> > +/*
> > + * Convert type string (u8/u16/u32/u64/s8/s16/s32/s64 ..., see
> > + * Documentation/trace/kprobetrace.txt) to size field of BPF_LDX_MEM
> > + * instruction (BPF_{B,H,W,DW}).
> > + */
> > +static int
> > +argtype_to_ldx_size(const char *type)
> > +{
> > +	int arg_size = type ? atoi(&type[1]) : 64;
> > +
> > +	switch (arg_size) {
> > +	case 8:
> > +		return BPF_B;
> > +	case 16:
> > +		return BPF_H;
> > +	case 32:
> > +		return BPF_W;
> > +	case 64:
> > +	default:
> > +		return BPF_DW;
> > +	}
> > +}
> > +
> > +static const char *
> > +insn_sz_to_str(int insn_sz)
> > +{
> > +	switch (insn_sz) {
> > +	case BPF_B:
> > +		return "BPF_B";
> > +	case BPF_H:
> > +		return "BPF_H";
> > +	case BPF_W:
> > +		return "BPF_W";
> > +	case BPF_DW:
> > +		return "BPF_DW";
> > +	default:
> > +		return "UNKNOWN";
> > +	}
> > +}
> > +
> >   /* Give it a shorter name */
> >   #define ins(i, p) append_insn((i), (p))
> > @@ -257,9 +297,14 @@ gen_prologue_slowpath(struct bpf_insn_pos *pos,
> >   	}
> >   	/* Final pass: read to registers */
> > -	for (i = 0; i < nargs; i++)
> > -		ins(BPF_LDX_MEM(BPF_DW, BPF_PROLOGUE_START_ARG_REG + i,
> > +	for (i = 0; i < nargs; i++) {
> > +		int insn_sz = argtype_to_ldx_size(args[i].type);
> > +
> > +		pr_debug("prologue: load arg %d, insn_sz is %s\n",
> > +			 i, insn_sz_to_str(insn_sz));
> > +		ins(BPF_LDX_MEM(insn_sz, BPF_PROLOGUE_START_ARG_REG + i,
> >   				BPF_REG_FP, -BPF_REG_SIZE * (i + 1)), pos);
> > +	}
> >   	ins(BPF_JMP_IMM(BPF_JA, BPF_REG_0, 0, JMP_TO_SUCCESS_CODE), pos);
> 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web