Hi Thomas,

Your patch looks good to me. I've tested in my environment and it works.
Please resend it to lkml and let Arnaldo to collect it.

Thank you.

On 2017/8/14 17:47, Thomas Richter 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/[email protected]

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

Now the test runs successfully on 4.13.0-rc5:
[root@s8360046 perf]# ./perf test  bpf
38: BPF filter                                 :
38.1: Basic BPF filtering                      : Ok
38.2: BPF pinning                              : Ok
38.3: BPF prologue generation                  : Ok
38.4: BPF relocation checker                   : Ok
[root@s8360046 perf]#

Signed-off-by: Thomas Richter <[email protected]>
Signed-off-by: Wang Nan <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Thomas Richter <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Hendrik Brueckner <[email protected]>
Cc: Li Zefan <[email protected]>
---
  tools/perf/util/bpf-prologue.c | 49 ++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c
index 1356220..827f914 100644
--- a/tools/perf/util/bpf-prologue.c
+++ b/tools/perf/util/bpf-prologue.c
@@ -58,6 +58,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))
@@ -258,9 +298,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 = (args[i].ref) ? argtype_to_ldx_size(args[i].type) 
: BPF_DW;
+
+               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);


Reply via email to