> Am 29.08.2019 um 23:53 schrieb Song Liu <liu.song.a23@xxxxxxxxx>:
>
> On Wed, Aug 28, 2019 at 6:22 AM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote:
>>
>> A lot of test_sysctl sub-tests fail due to handling strings as a bunch
>> of immediate values in a little-endian-specific manner.
>>
>> Fix by wrapping all immediates in bpf_ntohl and the new bpf_be64_to_cpu.
>>
>> Also, sometimes tests fail because sysctl() unexpectedly succeeds with
>> an inappropriate "Unexpected failure" message and a random errno. Zero
>> out errno before calling sysctl() and replace the message with
>> "Unexpected success".
...
>> @@ -13,6 +13,7 @@
>> #include <bpf/bpf.h>
>> #include <bpf/libbpf.h>
>>
>> +#include "bpf_endian.h"
>> #include "bpf_rlimit.h"
>> #include "bpf_util.h"
>> #include "cgroup_helpers.h"
>> @@ -100,7 +101,7 @@ static struct sysctl_test tests[] = {
>> .descr = "ctx:write sysctl:write read ok",
>> .insns = {
>> /* If (write) */
>> - BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_1,
>> + BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
>
> I didn't find explanation for this change in the commit log. Is this a typo, or
> a real fix?
This is a real fix, but I forgot to explain it. We read the first byte of an
int assuming it's the least-significant one, which is not the case on BE arches.
Two fixes are possible: use a different offset on BE arches or just read the
whole int. Since we are not testing narrow accesses here (there is e.g.
"ctx:file_pos sysctl:read read ok narrow" for that), I went with the simplest
solution.
...
>> @@ -1344,20 +1379,24 @@ static size_t probe_prog_length(const struct bpf_insn *fp)
>> static int fixup_sysctl_value(const char *buf, size_t buf_len,
>> struct bpf_insn *prog, size_t insn_num)
>> {
>> - uint32_t value_num = 0;
>> + union {
>> + uint8_t raw[sizeof(uint64_t)];
>> + uint64_t num;
>> + } value = {};
>
> This change doesn't match the description in the changelog.
This one I also forgot to explain - writing an immediate into things like
BPF_LD_IMM64(BPF_REG_0, FIXUP_SYSCTL_VALUE) should be endianness-aware. We can
also do bpf_be64_to_cpu or similar here, but a better trick (suggested by
Yonghong) is to simply memcpy the raw user-provided value, since testcase
endianness and bpf program endianness match.
Let me send a v4 with an improved commit message.
