"add mptcp_subflow bpf_iter v3" fixes

[PATCH mptcp-next 1/2] Squash to "bpf: Add mptcp_subflow bpf_iter"

Posted by Geliang Tang 1 year, 5 months ago

From: Geliang Tang <tanggeliang@kylinos.cn>

Register mptcp_subflow bpf_iter helpers in common_btf_ids in
kernel/bpf/helpers.c, then they can be used on a larger scale,
not only in types BPF_PROG_TYPE_TRACING and BPF_PROG_TYPE_STRUCT_OPS.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 kernel/bpf/helpers.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b5f0adae8293..649ec8305b3f 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3023,6 +3023,11 @@ BTF_ID_FLAGS(func, bpf_preempt_enable)
 BTF_ID_FLAGS(func, bpf_iter_bits_new, KF_ITER_NEW)
 BTF_ID_FLAGS(func, bpf_iter_bits_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_bits_destroy, KF_ITER_DESTROY)
+#ifdef CONFIG_MPTCP
+BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_new)
+BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_next)
+BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_destroy)
+#endif
 BTF_KFUNCS_END(common_btf_ids)
 
 static const struct btf_kfunc_id_set common_kfunc_set = {
-- 
2.43.0

Re: [PATCH mptcp-next 1/2] Squash to "bpf: Add mptcp_subflow bpf_iter"

Posted by Matthieu Baerts 1 year, 5 months ago

Hi Geliang,

On 10/09/2024 10:53, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Register mptcp_subflow bpf_iter helpers in common_btf_ids in
> kernel/bpf/helpers.c, then they can be used on a larger scale,
> not only in types BPF_PROG_TYPE_TRACING and BPF_PROG_TYPE_STRUCT_OPS.

Why could they not be used in these types? I find it good to have them
defined in net/mptcp, from a maintenance point of view.

Also, the different bpf_iter_xxx helpers defined there look generic.
While here, it is specific to MPTCP, I'm not even sure that would be OK
to add them there. Is it not OK to define them only in net/mptcp/bpf.c?
(or at least for the moment)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next 1/2] Squash to "bpf: Add mptcp_subflow bpf_iter"

Posted by Geliang Tang 1 year, 5 months ago

Hi Matt,

On Tue, 2024-09-10 at 11:12 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 10/09/2024 10:53, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > Register mptcp_subflow bpf_iter helpers in common_btf_ids in
> > kernel/bpf/helpers.c, then they can be used on a larger scale,
> > not only in types BPF_PROG_TYPE_TRACING and
> > BPF_PROG_TYPE_STRUCT_OPS.
> 
> Why could they not be used in these types? I find it good to have
> them
> defined in net/mptcp, from a maintenance point of view.
> 
> Also, the different bpf_iter_xxx helpers defined there look generic.
> While here, it is specific to MPTCP, I'm not even sure that would be
> OK
> to add them there. Is it not OK to define them only in
> net/mptcp/bpf.c?

Yes, a new test for #484 (BPF: setsockopt on an MPTCP socket: check
support) will use these helpers in "cgroup/setsockopt", that is the
BPF_PROG_TYPE_CGROUP_SOCKOPT type.

register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCKOPT,
		&bpf_mptcp_sched_kfunc_set);

will fail, since BPF_PROG_TYPE_CGROUP_SOCKOPT is not allow to register
more kfuncs (see bpf_prog_type_to_kfunc_hook() in kernel/bpf/btf.c).

So I have to move them into common_btf_ids in kernel/bpf/helpers.c to
allow them can be used in almost every BPF program type.

Thanks,
-Geliang

> (or at least for the moment)
> 
> Cheers,
> Matt

Re: [PATCH mptcp-next 1/2] Squash to "bpf: Add mptcp_subflow bpf_iter"

Posted by Matthieu Baerts 1 year, 5 months ago

Hi Geliang,

On 10/09/2024 11:27, Geliang Tang wrote:
> Hi Matt,
> 
> On Tue, 2024-09-10 at 11:12 +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 10/09/2024 10:53, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> Register mptcp_subflow bpf_iter helpers in common_btf_ids in
>>> kernel/bpf/helpers.c, then they can be used on a larger scale,
>>> not only in types BPF_PROG_TYPE_TRACING and
>>> BPF_PROG_TYPE_STRUCT_OPS.
>>
>> Why could they not be used in these types? I find it good to have
>> them
>> defined in net/mptcp, from a maintenance point of view.
>>
>> Also, the different bpf_iter_xxx helpers defined there look generic.
>> While here, it is specific to MPTCP, I'm not even sure that would be
>> OK
>> to add them there. Is it not OK to define them only in
>> net/mptcp/bpf.c?
> 
> Yes, a new test for #484 (BPF: setsockopt on an MPTCP socket: check
> support) will use these helpers in "cgroup/setsockopt", that is the
> BPF_PROG_TYPE_CGROUP_SOCKOPT type.
> 
> register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCKOPT,
> 		&bpf_mptcp_sched_kfunc_set);
> 
> will fail, since BPF_PROG_TYPE_CGROUP_SOCKOPT is not allow to register
> more kfuncs (see bpf_prog_type_to_kfunc_hook() in kernel/bpf/btf.c).
> 
> So I have to move them into common_btf_ids in kernel/bpf/helpers.c to
> allow them can be used in almost every BPF program type.

OK, but then I guess these helpers can be upstreamed before the packet
schedulers, right?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next 1/2] Squash to "bpf: Add mptcp_subflow bpf_iter"

Posted by Geliang Tang 1 year, 5 months ago

Hi Matt,

On Tue, 2024-09-10 at 11:33 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 10/09/2024 11:27, Geliang Tang wrote:
> > Hi Matt,
> > 
> > On Tue, 2024-09-10 at 11:12 +0200, Matthieu Baerts wrote:
> > > Hi Geliang,
> > > 
> > > On 10/09/2024 10:53, Geliang Tang wrote:
> > > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > > 
> > > > Register mptcp_subflow bpf_iter helpers in common_btf_ids in
> > > > kernel/bpf/helpers.c, then they can be used on a larger scale,
> > > > not only in types BPF_PROG_TYPE_TRACING and
> > > > BPF_PROG_TYPE_STRUCT_OPS.
> > > 
> > > Why could they not be used in these types? I find it good to have
> > > them
> > > defined in net/mptcp, from a maintenance point of view.
> > > 
> > > Also, the different bpf_iter_xxx helpers defined there look
> > > generic.
> > > While here, it is specific to MPTCP, I'm not even sure that would
> > > be
> > > OK
> > > to add them there. Is it not OK to define them only in
> > > net/mptcp/bpf.c?
> > 
> > Yes, a new test for #484 (BPF: setsockopt on an MPTCP socket: check
> > support) will use these helpers in "cgroup/setsockopt", that is the
> > BPF_PROG_TYPE_CGROUP_SOCKOPT type.
> > 
> > register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCKOPT,
> > 		&bpf_mptcp_sched_kfunc_set);
> > 
> > will fail, since BPF_PROG_TYPE_CGROUP_SOCKOPT is not allow to
> > register
> > more kfuncs (see bpf_prog_type_to_kfunc_hook() in
> > kernel/bpf/btf.c).

I rethought it again, we can easily extend bpf_prog_type_to_kfunc_hook:

+++ b/kernel/bpf/btf.c
@@ -8310,6 +8310,7 @@ static int bpf_prog_type_to_kfunc_hook(enum
bpf_prog_type prog_type)
                return BTF_KFUNC_HOOK_SYSCALL;
        case BPF_PROG_TYPE_CGROUP_SKB:
        case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
+       case BPF_PROG_TYPE_CGROUP_SOCKOPT:
                return BTF_KFUNC_HOOK_CGROUP_SKB;
        case BPF_PROG_TYPE_SCHED_ACT:
                return BTF_KFUNC_HOOK_SCHED_ACT;

So we can still define and register mptcp_subflow bpf_iter helpers only
in net/mptcp/bpf.c. v4 is sent, this patch is superseded.

Thanks,
-Geliang

> > 
> > So I have to move them into common_btf_ids in kernel/bpf/helpers.c
> > to
> > allow them can be used in almost every BPF program type.
> 
> OK, but then I guess these helpers can be upstreamed before the
> packet
> schedulers, right?
> 
> Cheers,
> Matt

Re: [PATCH mptcp-next 1/2] Squash to "bpf: Add mptcp_subflow bpf_iter"

Posted by Geliang Tang 1 year, 5 months ago

On Tue, 2024-09-10 at 11:33 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 10/09/2024 11:27, Geliang Tang wrote:
> > Hi Matt,
> > 
> > On Tue, 2024-09-10 at 11:12 +0200, Matthieu Baerts wrote:
> > > Hi Geliang,
> > > 
> > > On 10/09/2024 10:53, Geliang Tang wrote:
> > > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > > 
> > > > Register mptcp_subflow bpf_iter helpers in common_btf_ids in
> > > > kernel/bpf/helpers.c, then they can be used on a larger scale,
> > > > not only in types BPF_PROG_TYPE_TRACING and
> > > > BPF_PROG_TYPE_STRUCT_OPS.
> > > 
> > > Why could they not be used in these types? I find it good to have
> > > them
> > > defined in net/mptcp, from a maintenance point of view.
> > > 
> > > Also, the different bpf_iter_xxx helpers defined there look
> > > generic.
> > > While here, it is specific to MPTCP, I'm not even sure that would
> > > be
> > > OK
> > > to add them there. Is it not OK to define them only in
> > > net/mptcp/bpf.c?
> > 
> > Yes, a new test for #484 (BPF: setsockopt on an MPTCP socket: check
> > support) will use these helpers in "cgroup/setsockopt", that is the
> > BPF_PROG_TYPE_CGROUP_SOCKOPT type.
> > 
> > register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCKOPT,
> > 		&bpf_mptcp_sched_kfunc_set);
> > 
> > will fail, since BPF_PROG_TYPE_CGROUP_SOCKOPT is not allow to
> > register
> > more kfuncs (see bpf_prog_type_to_kfunc_hook() in
> > kernel/bpf/btf.c).
> > 
> > So I have to move them into common_btf_ids in kernel/bpf/helpers.c
> > to
> > allow them can be used in almost every BPF program type.
> 
> OK, but then I guess these helpers can be upstreamed before the
> packet
> schedulers, right?

Yes, indeed. I hope them can be upstreamed after the "new MPTCP subflow
subtest" set under reviewing on BPF list.

So this set should be applied in this order:

selftests/bpf: Add mptcp subflow example
selftests/bpf: Add getsockopt to inspect mptcp subflow
selftests/bpf: Add mptcp subflow subtest
bpf: Add mptcp_subflow bpf_iter
bpf: Register mptcp tracing kfunc set
selftests/bpf: Add mptcp_subflow bpf_iter test prog
selftests/bpf: Add mptcp_subflow bpf_iter subtest
selftests/bpf: Add bpf scheduler test
selftests/bpf: Add bpf_first scheduler & test

Thanks,
-Geliang

> 
> Cheers,
> Matt

Re: [PATCH mptcp-next 1/2] Squash to "bpf: Add mptcp_subflow bpf_iter"

Posted by Matthieu Baerts 1 year, 5 months ago

On 10/09/2024 11:41, Geliang Tang wrote:
> On Tue, 2024-09-10 at 11:33 +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 10/09/2024 11:27, Geliang Tang wrote:
>>> Hi Matt,
>>>
>>> On Tue, 2024-09-10 at 11:12 +0200, Matthieu Baerts wrote:
>>>> Hi Geliang,
>>>>
>>>> On 10/09/2024 10:53, Geliang Tang wrote:
>>>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>>>
>>>>> Register mptcp_subflow bpf_iter helpers in common_btf_ids in
>>>>> kernel/bpf/helpers.c, then they can be used on a larger scale,
>>>>> not only in types BPF_PROG_TYPE_TRACING and
>>>>> BPF_PROG_TYPE_STRUCT_OPS.
>>>>
>>>> Why could they not be used in these types? I find it good to have
>>>> them
>>>> defined in net/mptcp, from a maintenance point of view.
>>>>
>>>> Also, the different bpf_iter_xxx helpers defined there look
>>>> generic.
>>>> While here, it is specific to MPTCP, I'm not even sure that would
>>>> be
>>>> OK
>>>> to add them there. Is it not OK to define them only in
>>>> net/mptcp/bpf.c?
>>>
>>> Yes, a new test for #484 (BPF: setsockopt on an MPTCP socket: check
>>> support) will use these helpers in "cgroup/setsockopt", that is the
>>> BPF_PROG_TYPE_CGROUP_SOCKOPT type.
>>>
>>> register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCKOPT,
>>> 		&bpf_mptcp_sched_kfunc_set);
>>>
>>> will fail, since BPF_PROG_TYPE_CGROUP_SOCKOPT is not allow to
>>> register
>>> more kfuncs (see bpf_prog_type_to_kfunc_hook() in
>>> kernel/bpf/btf.c).
>>>
>>> So I have to move them into common_btf_ids in kernel/bpf/helpers.c
>>> to
>>> allow them can be used in almost every BPF program type.
>>
>> OK, but then I guess these helpers can be upstreamed before the
>> packet
>> schedulers, right?
> 
> Yes, indeed. I hope them can be upstreamed after the "new MPTCP subflow
> subtest" set under reviewing on BPF list.

OK good!

> So this set should be applied in this order:
> 
> selftests/bpf: Add mptcp subflow example
> selftests/bpf: Add getsockopt to inspect mptcp subflow
> selftests/bpf: Add mptcp subflow subtest

I will first send these 3, before focusing on the other ones if that's OK.

> bpf: Add mptcp_subflow bpf_iter
> bpf: Register mptcp tracing kfunc set
> selftests/bpf: Add mptcp_subflow bpf_iter test prog
> selftests/bpf: Add mptcp_subflow bpf_iter subtest
> selftests/bpf: Add bpf scheduler test
> selftests/bpf: Add bpf_first scheduler & test
Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next 1/2] Squash to "bpf: Add mptcp_subflow bpf_iter"

Posted by Geliang Tang 1 year, 5 months ago

On Tue, 2024-09-10 at 11:44 +0200, Matthieu Baerts wrote:
> On 10/09/2024 11:41, Geliang Tang wrote:
> > On Tue, 2024-09-10 at 11:33 +0200, Matthieu Baerts wrote:
> > > Hi Geliang,
> > > 
> > > On 10/09/2024 11:27, Geliang Tang wrote:
> > > > Hi Matt,
> > > > 
> > > > On Tue, 2024-09-10 at 11:12 +0200, Matthieu Baerts wrote:
> > > > > Hi Geliang,
> > > > > 
> > > > > On 10/09/2024 10:53, Geliang Tang wrote:
> > > > > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > > > > 
> > > > > > Register mptcp_subflow bpf_iter helpers in common_btf_ids
> > > > > > in
> > > > > > kernel/bpf/helpers.c, then they can be used on a larger
> > > > > > scale,
> > > > > > not only in types BPF_PROG_TYPE_TRACING and
> > > > > > BPF_PROG_TYPE_STRUCT_OPS.
> > > > > 
> > > > > Why could they not be used in these types? I find it good to
> > > > > have
> > > > > them
> > > > > defined in net/mptcp, from a maintenance point of view.
> > > > > 
> > > > > Also, the different bpf_iter_xxx helpers defined there look
> > > > > generic.
> > > > > While here, it is specific to MPTCP, I'm not even sure that
> > > > > would
> > > > > be
> > > > > OK
> > > > > to add them there. Is it not OK to define them only in
> > > > > net/mptcp/bpf.c?
> > > > 
> > > > Yes, a new test for #484 (BPF: setsockopt on an MPTCP socket:
> > > > check
> > > > support) will use these helpers in "cgroup/setsockopt", that is
> > > > the
> > > > BPF_PROG_TYPE_CGROUP_SOCKOPT type.
> > > > 
> > > > register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCKOPT,
> > > > 		&bpf_mptcp_sched_kfunc_set);
> > > > 
> > > > will fail, since BPF_PROG_TYPE_CGROUP_SOCKOPT is not allow to
> > > > register
> > > > more kfuncs (see bpf_prog_type_to_kfunc_hook() in
> > > > kernel/bpf/btf.c).
> > > > 
> > > > So I have to move them into common_btf_ids in
> > > > kernel/bpf/helpers.c
> > > > to
> > > > allow them can be used in almost every BPF program type.
> > > 
> > > OK, but then I guess these helpers can be upstreamed before the
> > > packet
> > > schedulers, right?
> > 
> > Yes, indeed. I hope them can be upstreamed after the "new MPTCP
> > subflow
> > subtest" set under reviewing on BPF list.
> 
> OK good!
> 
> > So this set should be applied in this order:
> > 
> > selftests/bpf: Add mptcp subflow example
> > selftests/bpf: Add getsockopt to inspect mptcp subflow
> > selftests/bpf: Add mptcp subflow subtest
> 
> I will first send these 3, before focusing on the other ones if
> that's OK.

Thanks.

> 
> > bpf: Add mptcp_subflow bpf_iter
> > bpf: Register mptcp tracing kfunc set
> > selftests/bpf: Add mptcp_subflow bpf_iter test prog
> > selftests/bpf: Add mptcp_subflow bpf_iter subtest

The next set is to use this mptcp_subflow bpf_iter to update all the
BPF scheduler examples, which is almost ready. I will post them in the
next few days.

Regards,
-Geliang

> > selftests/bpf: Add bpf scheduler test
> > selftests/bpf: Add bpf_first scheduler & test
> Cheers,
> Matt