On 2024/7/27 03:38, Yonghong Song wrote:
>
> On 7/26/24 8:39 AM, Leon Hwang wrote:
>> Add a selftest to confirm the issue, which gets -EINVAL when update
>> attached freplace prog to prog_array map, has been fixed.
>>
>> cd tools/testing/selftests/bpf; ./test_progs -t tailcalls
>> 327/25 tailcalls/tailcall_freplace:OK
>> 327 tailcalls:OK
>> Summary: 1/25 PASSED, 0 SKIPPED, 0 FAILED
>>
>> Signed-off-by: Leon Hwang <leon.hwang@xxxxxxxxx>
>
> LGTM with some comments below.
>
> Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx>
>
>> ---
>> .../selftests/bpf/prog_tests/tailcalls.c | 65 ++++++++++++++++++-
>> .../selftests/bpf/progs/tailcall_freplace.c | 25 +++++++
>> .../testing/selftests/bpf/progs/tc_bpf2bpf.c | 21 ++++++
>> 3 files changed, 110 insertions(+), 1 deletion(-)
>> create mode 100644
>> tools/testing/selftests/bpf/progs/tailcall_freplace.c
>> create mode 100644 tools/testing/selftests/bpf/progs/tc_bpf2bpf.c
>>
[...]
>> diff --git a/tools/testing/selftests/bpf/progs/tailcall_freplace.c
>> b/tools/testing/selftests/bpf/progs/tailcall_freplace.c
>> new file mode 100644
>> index 0000000000000..2966efc06ae8f
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/tailcall_freplace.c
>> @@ -0,0 +1,25 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/bpf.h>
>> +#include <bpf/bpf_helpers.h>
>> +
>> +struct {
>> + __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
>> + __uint(max_entries, 1);
>> + __uint(key_size, sizeof(__u32));
>> + __uint(value_size, sizeof(__u32));
>> +} jmp_table SEC(".maps");
>> +
>> +int count = 0;
>> +
>> +SEC("freplace")
>> +int entry_freplace(struct __sk_buff *skb)
>> +{
>> + count++;
>> +
> remove empty line here.
>> + bpf_tail_call_static(skb, &jmp_table, 0);
>> +
> remove empty line here.
>> + return count;
>> +}
>> +
>> +char __license[] SEC("license") = "GPL";
>> diff --git a/tools/testing/selftests/bpf/progs/tc_bpf2bpf.c
>> b/tools/testing/selftests/bpf/progs/tc_bpf2bpf.c
>> new file mode 100644
>> index 0000000000000..980bb810b481c
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/tc_bpf2bpf.c
>> @@ -0,0 +1,21 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/bpf.h>
>> +#include <bpf/bpf_helpers.h>
>> +
>> +__noinline
>> +int subprog(struct __sk_buff *skb)
>> +{
>> + volatile int ret = 1;
>> +
> remove empty line here.
Should we remove this empty line?
./scripts/checkpatch.pl:
WARNING: Missing a blank line after declarations
#158: FILE: tools/testing/selftests/bpf/progs/tc_bpf2bpf.c:11:
+ int ret = 1;
+ __sink(ret);
>> + asm volatile (""::"r+"(ret));
> remove above 'volatile' key word and replace asm volatile with __sink(ret).
>> + return ret;
>> +}
>> +
>> +SEC("tc")
>> +int entry_tc(struct __sk_buff *skb)
>> +{
>> + return subprog(skb);
>> +}
>> +
>> +char __license[] SEC("license") = "GPL";
