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);
>
On 08/14/2017 12:30 PM, Wangnan (F) wrote:
> 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.
>
Will do, I take this as an Acked-by and Tested-by, ok?
Thanks
--
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
OK.
On 2017/8/14 18:58, Thomas-Mich Richter wrote:
> On 08/14/2017 12:30 PM, Wangnan (F) wrote:
>> 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.
>>
> Will do, I take this as an Acked-by and Tested-by, ok?
>
> Thanks