Re: [RFC PATCH bpf-next] bpf: Make actual max tail call count as MAX_TAIL_CALL_CNT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/08/2021 04:47 PM, Daniel Borkmann wrote:
[ You have a huge Cc list, but forgot to add Paul and Johan who recently
  looked into this. Added here. ]

On 9/8/21 10:20 AM, Tiezhu Yang wrote:
In the current code, the actual max tail call count is 33 which is greater
than MAX_TAIL_CALL_CNT, this is not consistent with the intended meaning
in the commit 04fd61ab36ec ("bpf: allow bpf programs to tail-call other
bpf programs"):

"The chain of tail calls can form unpredictable dynamic loops therefore
tail_call_cnt is used to limit the number of calls and currently is set
to 32."

Additionally, after commit 874be05f525e ("bpf, tests: Add tail call test
suite"), we can see there exists failed testcase.

On all archs when CONFIG_BPF_JIT_ALWAYS_ON is not set:
  # echo 0 > /proc/sys/net/core/bpf_jit_enable
  # modprobe test_bpf
  # dmesg | grep -w FAIL
  Tail call error path, max count reached jited:0 ret 34 != 33 FAIL

On some archs:
  # echo 1 > /proc/sys/net/core/bpf_jit_enable
  # modprobe test_bpf
  # dmesg | grep -w FAIL
  Tail call error path, max count reached jited:1 ret 34 != 33 FAIL

with this patch, make the actual max tail call count as MAX_TAIL_CALL_CNT,
at the same time, the above failed testcase can be fixed.

Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
---

Hi all,

This is a RFC patch, if I am wrong or I missed something,
please let me know, thank you!

Yes, the original commit from 04fd61ab36ec ("bpf: allow bpf programs to tail-call other bpf programs") got the counting wrong, but please also check f9dabe016b63 ("bpf: Undo off-by-one in interpreter tail call count limit") where we agreed to align everything to 33 in order to avoid changing existing behavior, and if we intend to ever change the count, then only in terms of increasing but not decreasing
since that ship has sailed.

Thank you, understood.

But I still think there is some confusion about the macro MAX_TAIL_CALL_CNT
which is 32 and the actual value 33, I spent some time to understand it
at the first glance.

Is it impossible to keep the actual max tail call count consistent with
the value 32 of MAX_TAIL_CALL_CNT now?

At least, maybe we need to modify the testcase?

Tiezhu, do you still see any arch that is not on 33
from your testing?

If the testcase "Tail call error path, max count reached" in test_bpf is right,
it seems that the tail call count limit is 32 on x86, because the testcase
passed on x86 jited.

Last time Paul fixed the remaining ones in 96bc4432f5ad ("bpf,
riscv: Limit to 33 tail calls") and e49e6f6db04e ("bpf, mips: Limit to 33 tail calls").

Thanks,
Daniel




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux