Re: [PATCH bpf-next 4/4] bpf/selftests: add simple selftest for bpf_smc_ops

[Zhu Yanjun <yanjun.zhu@linux.dev>]

On 21.11.24 03:00, D. Wythe wrote:
>
>
> On 11/3/24 9:01 PM, Zhu Yanjun wrote:
>> 在 2024/10/24 4:42, D. Wythe 写道:
>>> From: "D. Wythe" <alibuda@linux.alibaba.com>
>>>
>>> This PATCH adds a tiny selftest for bpf_smc_ops, to verify the ability
>>> to attach and write access.
>>>
>>> Follow the steps below to run this test.
>>>
>>> make -C tools/testing/selftests/bpf
>>> cd tools/testing/selftests/bpf
>>> sudo ./test_progs -t smc
>>
>> Thanks a lot.
>>
>> # ./test_progs -t smc
>> #27/1    bpf_smc/load:OK
>> #27      bpf_smc:OK
>> Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
>>
>> The above command is based on several kernel modules. After these 
>> dependent kernel modules are loaded, then can run the above command 
>> successfully.
>>
>> Zhu Yanjun
>>
>
> Hi, Yanjun
>
> This is indeed a problem, a better way may be to create a separate 
> testing directory for SMC, and we are trying to do this.

Got it. In the latest patch series, if a test program in sample/bpf can 
verify this bpf feature, it is better than a selftest program in the 
directory tools/testing/selftests/bpf.

I delved into this selftest tool. It seems that this selftest tool only 
makes the basic checks. A test program in sample/bpf can do more.

I mean, it is very nice that a selftest tool can make selftest on smc 
bpf. But it is better that a test program in sample/bpf can make some 
parameter changes in smc.

These parameter changes are mentioned in the previous commits.

"

     As a subsequent enhancement, this patch introduces a new hook for eBPF
     programs that allows decisions on whether to use SMC or not at runtime,
     including but not limited to local/remote IP address or ports. In
     simpler words, this feature allows modifications to syn_smc through 
eBPF
     programs before the TCP three-way handshake got established.

"

Zhu Yanjun

>
> Best wishes,
> D. Wythe
>
>>>
>>> Results shows:
>>> Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
>>>
>>> Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
>>> ---
>>>   .../selftests/bpf/prog_tests/test_bpf_smc.c        | 21 +++++++++++
>>>   tools/testing/selftests/bpf/progs/bpf_smc.c        | 44 
>>> ++++++++++++++++++++++
>>>   2 files changed, 65 insertions(+)
>>>   create mode 100644 
>>> tools/testing/selftests/bpf/prog_tests/test_bpf_smc.c
>>>   create mode 100644 tools/testing/selftests/bpf/progs/bpf_smc.c
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpf_smc.c 
>>> b/tools/testing/selftests/bpf/prog_tests/test_bpf_smc.c
>>> new file mode 100644
>>> index 00000000..2299853
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/bpf/prog_tests/test_bpf_smc.c
>>> @@ -0,0 +1,21 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +#include <test_progs.h>
>>> +
>>> +#include "bpf_smc.skel.h"
>>> +
>>> +static void load(void)
>>> +{
>>> +    struct bpf_smc *skel;
>>> +
>>> +    skel = bpf_smc__open_and_load();
>>> +    if (!ASSERT_OK_PTR(skel, "bpf_smc__open_and_load"))
>>> +        return;
>>> +
>>> +    bpf_smc__destroy(skel);
>>> +}
>>> +
>>> +void test_bpf_smc(void)
>>> +{
>>> +    if (test__start_subtest("load"))
>>> +        load();
>>> +}
>>> diff --git a/tools/testing/selftests/bpf/progs/bpf_smc.c 
>>> b/tools/testing/selftests/bpf/progs/bpf_smc.c
>>> new file mode 100644
>>> index 00000000..ebff477
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/bpf/progs/bpf_smc.c
>>> @@ -0,0 +1,44 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +#include "vmlinux.h"
>>> +
>>> +#include <bpf/bpf_helpers.h>
>>> +#include <bpf/bpf_tracing.h>
>>> +
>>> +char _license[] SEC("license") = "GPL";
>>> +
>>> +struct smc_bpf_ops_ctx {
>>> +    struct {
>>> +        struct tcp_sock *tp;
>>> +    } set_option;
>>> +    struct {
>>> +        const struct tcp_sock *tp;
>>> +        struct inet_request_sock *ireq;
>>> +        int smc_ok;
>>> +    } set_option_cond;
>>> +};
>>> +
>>> +struct smc_bpf_ops {
>>> +    void (*set_option)(struct smc_bpf_ops_ctx *ctx);
>>> +    void (*set_option_cond)(struct smc_bpf_ops_ctx *ctx);
>>> +};
>>> +
>>> +SEC("struct_ops/bpf_smc_set_tcp_option_cond")
>>> +void BPF_PROG(bpf_smc_set_tcp_option_cond, struct smc_bpf_ops_ctx 
>>> *arg)
>>> +{
>>> +    arg->set_option_cond.smc_ok = 1;
>>> +}
>>> +
>>> +SEC("struct_ops/bpf_smc_set_tcp_option")
>>> +void BPF_PROG(bpf_smc_set_tcp_option, struct smc_bpf_ops_ctx *arg)
>>> +{
>>> +    struct tcp_sock *tp = arg->set_option.tp;
>>> +
>>> +    tp->syn_smc = 1;
>>> +}
>>> +
>>> +SEC(".struct_ops.link")
>>> +struct smc_bpf_ops sample_smc_bpf_ops = {
>>> +    .set_option         = (void *) bpf_smc_set_tcp_option,
>>> +    .set_option_cond    = (void *) bpf_smc_set_tcp_option_cond,
>>> +};

-- 
Best Regards,
Yanjun.Zhu