From: Geliang Tang <tanggeliang@kylinos.cn>

v15:
 - patch 7, add the declaration of bpf_mptcp_subflow_tcp_sock

Depends on:
 - Squash to "Add mptcp_subflow bpf_iter support", v3

Based-on: <cover.1739787744.git.tanggeliang@kylinos.cn>

v14:
 - patch 1, keep mptcp_sched_data_set_contexts() helper for future use.
 - patch 2, use "sk = sk__ign" in bpf_mptcp_subflow_ctx() and
   bpf_sk_stream_memory_free().
 - patch 8, drop "subflows" from struct mptcp_sched_data too.

v13:
 - use '__ign' suffix to ignore the argument type checks of
   bpf_mptcp_subflow_ctx() and bpf_sk_stream_memory_free(),
   instead of adding a new helper bpf_mptcp_send_info_to_ssk().
 - use 'bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk)' instead
   of using 'bpf_for_each(mptcp_subflow, subflow, msk)'.
 - keep struct mptcp_sched_data for future use.

v12:
 - drop struct mptcp_sched_data.
 - rebased on "split get_subflow interface into two" v2.

v11:
If another squash-to patchset (Squash to "Add mptcp_subflow bpf_iter
support") under review is merged before this set, v10 will fail to run.
v11 fixes this issue and can run regardless of whether it is merged
before or after the squash-to patchset.

Compared with v10, only patches 3, 5, and 8 have been modified:
 - use mptcp_subflow_tcp_sock instead of bpf_mptcp_subflow_tcp_sock in
   patch 3 and patch 5.
 - drop bpf_mptcp_sched_kfunc_set, use bpf_mptcp_common_kfunc_set instead
   in patch 8.

v10:
 - drop mptcp_subflow_set_scheduled() helper and WRITE_ONCE() in BPF.
 - add new bpf helper bpf_mptcp_send_info_to_ssk() for burst scheduler.

v9:
 - merge 'Fixes for "use bpf_iter in bpf schedulers" v8' into this set.
 - rebased on "add netns helpers" v4

v8:
 - address Mat's comments in v7.
 - move sk_stream_memory_free check inside bpf_for_each() loop.
 - implement mptcp_subflow_set_scheduled helper in BPF.
 - add cleanup patches into this set again.

v7:
 - move cleanup patches out of this set.
 - rebased.

v6:
 - rebased to "add mptcp_subflow bpf_iter" v10

v5:
 - patch 2, drop mptcp_sock_type and mptcp_subflow_type.
 - patch 3, revert "bpf: Export more bpf_burst related functions"
 - patch 4, merge "bpf: Export more bpf_burst related functions" into it.

v4:
 - patch 2, a new cleanup for "bpf: Add bpf_mptcp_sched_ops".
 - patch 3 should be reverted.
 - patch 8, register kfunc_set.

v3:
 - rebased.
 - put the "drop has_bytes_sent" squash-to patch into this set.

v2:
 - update bpf_rr and bpf_burst

With the newly added mptcp_subflow bpf_iter, we can get rid of the
subflows array "contexts" in struct mptcp_sched_data. This set
uses bpf_for_each(mptcp_subflow) helper to update all the bpf
schedules:

        bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk) {
                ... ...
                mptcp_subflow_set_scheduled(subflow, true);
        }

Geliang Tang (8):
  Squash to "mptcp: add sched_data helpers"
  Squash to "bpf: Export mptcp packet scheduler helpers"
  Squash to "selftests/bpf: Add bpf_first scheduler & test"
  Squash to "selftests/bpf: Add bpf_bkup scheduler & test"
  Squash to "selftests/bpf: Add bpf_rr scheduler & test"
  Squash to "selftests/bpf: Add bpf_red scheduler & test"
  Squash to "selftests/bpf: Add bpf_burst scheduler & test"
  mptcp: drop subflow contexts in mptcp_sched_data

 include/net/mptcp.h                           |  4 -
 net/mptcp/bpf.c                               | 50 ++++++------
 net/mptcp/protocol.h                          |  2 -
 net/mptcp/sched.c                             | 15 ----
 tools/testing/selftests/bpf/progs/mptcp_bpf.h |  5 +-
 .../selftests/bpf/progs/mptcp_bpf_bkup.c      | 16 +---
 .../selftests/bpf/progs/mptcp_bpf_burst.c     | 78 +++++++------------
 .../selftests/bpf/progs/mptcp_bpf_first.c     |  8 +-
 .../selftests/bpf/progs/mptcp_bpf_red.c       |  8 +-
 .../selftests/bpf/progs/mptcp_bpf_rr.c        | 31 ++++----
 10 files changed, 83 insertions(+), 134 deletions(-)

-- 
2.43.0

Hi Geliang, Mat,

On 17/02/2025 11:41, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v15:
>  - patch 7, add the declaration of bpf_mptcp_subflow_tcp_sock

Thank you for the patches and the reviews!

Now in our tree:

- 07aa0978f6c6: "squashed" (with conflicts) patch 1/8 in "mptcp: add
sched_data helpers"
- fa29b1f91c26: conflict in t/bpf-Add-bpf_mptcp_sched_kfunc_set

- 311895dc3441: "squashed" patch 2/8 in "bpf: Export mptcp packet
scheduler helpers"
- 3c6796421a18: "squashed" patch 3/8 in "selftests/bpf: Add bpf_first
scheduler & test"
- 86c995604f64: "squashed" patch 4/8 in "selftests/bpf: Add bpf_bkup
scheduler & test"
- 0b70cc3bdfe5: "squashed" patch 5/8 in "selftests/bpf: Add bpf_rr
scheduler & test"
- 8213e7e2d049: "squashed" patch 6/8 in "selftests/bpf: Add bpf_red
scheduler & test"
- 3fe124ac24c8: "squashed" patch 7/8 in "selftests/bpf: Add bpf_burst
scheduler & test"
- Results: 3161a41b78f4..a37b7ba9c462 (export)

While at it, I added two patches, as a follow-up of b68b106b0f15
("mptcp: sched: reduce size for unused data") instead of patch 8/8:
- e74f811b4626: Revert: "mptcp: add sched_data helpers"
- Results: a37b7ba9c462..a44778c76bc6 (export)

- fe2716bcac00: mptcp: sched: remove mptcp_sched_data
- Results: a44778c76bc6..516dcfe47eed (export)

And the adaptation in the BPF code and selftests:
- 0228f6dcc40b: Squash to "bpf: Add bpf_mptcp_sched_ops"
- 145b54903251: Squash to "selftests/bpf: Add bpf_first scheduler & test"
- 4ebb90f81181: Squash to "selftests/bpf: Add bpf_bkup scheduler & test"
- 2af0575eda92: Squash to "selftests/bpf: Add bpf_rr scheduler & test"
- 1dd84d973730: Squash to "selftests/bpf: Add bpf_red scheduler & test"
- fff0e60b0834: Squash to "selftests/bpf: Add bpf_burst scheduler & test"
- Results: 516dcfe47eed..62dab9e8d979 (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/a5cceef4fc35169482ecfd086c3d19c15d23e7e6/checks

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

Hi Matt, Mat,

On Wed, 2025-03-05 at 17:14 +0100, Matthieu Baerts wrote:
> Hi Geliang, Mat,
> 
> On 17/02/2025 11:41, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > v15:
> >  - patch 7, add the declaration of bpf_mptcp_subflow_tcp_sock
> 
> Thank you for the patches and the reviews!
> 
> Now in our tree:
> 
> - 07aa0978f6c6: "squashed" (with conflicts) patch 1/8 in "mptcp: add
> sched_data helpers"
> - fa29b1f91c26: conflict in t/bpf-Add-bpf_mptcp_sched_kfunc_set
> 
> - 311895dc3441: "squashed" patch 2/8 in "bpf: Export mptcp packet
> scheduler helpers"
> - 3c6796421a18: "squashed" patch 3/8 in "selftests/bpf: Add bpf_first
> scheduler & test"
> - 86c995604f64: "squashed" patch 4/8 in "selftests/bpf: Add bpf_bkup
> scheduler & test"
> - 0b70cc3bdfe5: "squashed" patch 5/8 in "selftests/bpf: Add bpf_rr
> scheduler & test"
> - 8213e7e2d049: "squashed" patch 6/8 in "selftests/bpf: Add bpf_red
> scheduler & test"
> - 3fe124ac24c8: "squashed" patch 7/8 in "selftests/bpf: Add bpf_burst
> scheduler & test"
> - Results: 3161a41b78f4..a37b7ba9c462 (export)

Thanks for applying this set.

> 
> While at it, I added two patches, as a follow-up of b68b106b0f15
> ("mptcp: sched: reduce size for unused data") instead of patch 8/8:
> - e74f811b4626: Revert: "mptcp: add sched_data helpers"
> - Results: a37b7ba9c462..a44778c76bc6 (export)
> 
> - fe2716bcac00: mptcp: sched: remove mptcp_sched_data
> - Results: a44778c76bc6..516dcfe47eed (export)

I think we need to keep mptcp scheduler API unchanged. We can keep
struct mptcp_sched_data and mptcp_sched_data_set_contexts helper for
future use, as I said in patch 8.


The bpf_iter_task [1] that I am adding recently can actually use them.

I've been thinking about this recently. It's not a good idea to add
bpf_iter_task to struct mptcp_sock. It's better to add it to struct
mptcp_sched_data instead since it's more related to mptcp scheduler.

1. Add bpf_iter_task in mptcp_sched_data.

struct mptcp_sched_data {
        struct task_struct *bpf_iter_task;
};

2. Set bpf_iter_task as "current" in mptcp_sched_data_set_contexts:

void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
                                   struct mptcp_sched_data *data)
{
        WRITE_ONCE(data->bpf_iter_task, current);
}

3. Add a struct mptcp_sched_data parameter for
bpf_iter_mptcp_subflow_new:

__bpf_kfunc static int
bpf_iter_mptcp_subflow_new(struct bpf_iter_mptcp_subflow1 *it,
                           struct sock *sk,
                           struct mptcp_sched_data *data)

4. Check data->bpf_iter_task in bpf_iter_mptcp_subflow_new:

        msk = mptcp_sk(sk);
        task = READ_ONCE(data->bpf_iter_task);
        if (!task || task != current)
                return -EINVAL;

        WRITE_ONCE(data->bpf_iter_task, NULL);

        msk_owned_by_me(msk);

        kit->msk = msk;
        kit->pos = &msk->conn_list;

5. Use bpf_for_each() with "data" parameter in bpf:

SEC("struct_ops")
int BPF_PROG(bpf_red_get_send, struct mptcp_sock *msk,
             struct mptcp_sched_data *data)
{
        struct mptcp_subflow_context *subflow;

        bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk, data)
                mptcp_subflow_set_scheduled(subflow, true);

        return 0;
}

WDYT?

Anyway, we can put the patch "mptcp: sched: remove mptcp_sched_data" on
the export branch and wait for a while, don't send it upstream
recently, and wait for me to send some squash-to patches to modify
them.

Thanks,
-Geliang

[1]
https://patchwork.kernel.org/project/mptcp/patch/7ff2760dcee1532f624e2ccf4fd67e83ec09a01b.1740997925.git.tanggeliang@kylinos.cn/

> 
> And the adaptation in the BPF code and selftests:
> - 0228f6dcc40b: Squash to "bpf: Add bpf_mptcp_sched_ops"
> - 145b54903251: Squash to "selftests/bpf: Add bpf_first scheduler &
> test"
> - 4ebb90f81181: Squash to "selftests/bpf: Add bpf_bkup scheduler &
> test"
> - 2af0575eda92: Squash to "selftests/bpf: Add bpf_rr scheduler &
> test"
> - 1dd84d973730: Squash to "selftests/bpf: Add bpf_red scheduler &
> test"
> - fff0e60b0834: Squash to "selftests/bpf: Add bpf_burst scheduler &
> test"
> - Results: 516dcfe47eed..62dab9e8d979 (export)
> 
> Tests are now in progress:
> 
> - export:
> https://github.com/multipath-tcp/mptcp_net-next/commit/a5cceef4fc35169482ecfd086c3d19c15d23e7e6/checks
> 
> Cheers,
> Matt

Hi Geliang,

On 07/03/2025 04:08, Geliang Tang wrote:
> On Wed, 2025-03-05 at 17:14 +0100, Matthieu Baerts wrote:

(...)

>> While at it, I added two patches, as a follow-up of b68b106b0f15
>> ("mptcp: sched: reduce size for unused data") instead of patch 8/8:
>> - e74f811b4626: Revert: "mptcp: add sched_data helpers"
>> - Results: a37b7ba9c462..a44778c76bc6 (export)
>>
>> - fe2716bcac00: mptcp: sched: remove mptcp_sched_data
>> - Results: a44778c76bc6..516dcfe47eed (export)
> 
> I think we need to keep mptcp scheduler API unchanged. We can keep
> struct mptcp_sched_data and mptcp_sched_data_set_contexts helper for
> future use, as I said in patch 8.

While this code has not been upstreamed, I think it is important to
clean what is not being used. If we need to add an extra parameter, we
will still be able to do so.

> The bpf_iter_task [1] that I am adding recently can actually use them.
> 
> I've been thinking about this recently. It's not a good idea to add
> bpf_iter_task to struct mptcp_sock. It's better to add it to struct
> mptcp_sched_data instead since it's more related to mptcp scheduler.

Why is it not a good idea to add it to the msk? Will you not check it
from bpf_iter kfunc where you have access to the msk, but not the data?
Also, will you not need such check also for the BPF PM, so it will not
be only dedicated to the scheduler?

> 
> 1. Add bpf_iter_task in mptcp_sched_data.
> 
> struct mptcp_sched_data {
>         struct task_struct *bpf_iter_task;
> };
> 
> 2. Set bpf_iter_task as "current" in mptcp_sched_data_set_contexts:
> 
> void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
>                                    struct mptcp_sched_data *data)
> {
>         WRITE_ONCE(data->bpf_iter_task, current);
> }
> 
> 3. Add a struct mptcp_sched_data parameter for
> bpf_iter_mptcp_subflow_new:
> 
> __bpf_kfunc static int
> bpf_iter_mptcp_subflow_new(struct bpf_iter_mptcp_subflow1 *it,
>                            struct sock *sk,
>                            struct mptcp_sched_data *data)

That means that this kfunc will only be used for the scheduler API, then
no longer for the [gs]etsockopt and BPF PM, no?

> 4. Check data->bpf_iter_task in bpf_iter_mptcp_subflow_new:
> 
>         msk = mptcp_sk(sk);
>         task = READ_ONCE(data->bpf_iter_task);
>         if (!task || task != current)
>                 return -EINVAL;
> 
>         WRITE_ONCE(data->bpf_iter_task, NULL);

I think it would make more sense to do that from sched.c, similar to a lock.

  WRITE_ONCE(data->bpf_iter_task, current);
  ret = msk->sched->get_send(msk, data);
  WRITE_ONCE(data->bpf_iter_task, NULL);

  return ret;

> 
>         msk_owned_by_me(msk);
> 
>         kit->msk = msk;
>         kit->pos = &msk->conn_list;
> 
> 5. Use bpf_for_each() with "data" parameter in bpf:
> 
> SEC("struct_ops")
> int BPF_PROG(bpf_red_get_send, struct mptcp_sock *msk,
>              struct mptcp_sched_data *data)
> {
>         struct mptcp_subflow_context *subflow;
> 
>         bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk, data)
>                 mptcp_subflow_set_scheduled(subflow, true);
> 
>         return 0;
> }
> 
> WDYT?
> 
> Anyway, we can put the patch "mptcp: sched: remove mptcp_sched_data" on
> the export branch and wait for a while, don't send it upstream
> recently, and wait for me to send some squash-to patches to modify
> them.

Sure. There are some other patches to send first anyway.

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

On Mon, 17 Feb 2025, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> v15:
> - patch 7, add the declaration of bpf_mptcp_subflow_tcp_sock
>
> Depends on:
> - Squash to "Add mptcp_subflow bpf_iter support", v3
>
> Based-on: <cover.1739787744.git.tanggeliang@kylinos.cn>

Hi Geliang -

I don't have any changes to suggest to this series, so I'll add my 
reviewed tag even though this can't be applied to the export branch yet 
(until the bpf_iter series is updated):

Reviewed-by: Mat Martineau <martineau@kernel.org>

>
> v14:
> - patch 1, keep mptcp_sched_data_set_contexts() helper for future use.
> - patch 2, use "sk = sk__ign" in bpf_mptcp_subflow_ctx() and
>   bpf_sk_stream_memory_free().
> - patch 8, drop "subflows" from struct mptcp_sched_data too.
>
> v13:
> - use '__ign' suffix to ignore the argument type checks of
>   bpf_mptcp_subflow_ctx() and bpf_sk_stream_memory_free(),
>   instead of adding a new helper bpf_mptcp_send_info_to_ssk().
> - use 'bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk)' instead
>   of using 'bpf_for_each(mptcp_subflow, subflow, msk)'.
> - keep struct mptcp_sched_data for future use.
>
> v12:
> - drop struct mptcp_sched_data.
> - rebased on "split get_subflow interface into two" v2.
>
> v11:
> If another squash-to patchset (Squash to "Add mptcp_subflow bpf_iter
> support") under review is merged before this set, v10 will fail to run.
> v11 fixes this issue and can run regardless of whether it is merged
> before or after the squash-to patchset.
>
> Compared with v10, only patches 3, 5, and 8 have been modified:
> - use mptcp_subflow_tcp_sock instead of bpf_mptcp_subflow_tcp_sock in
>   patch 3 and patch 5.
> - drop bpf_mptcp_sched_kfunc_set, use bpf_mptcp_common_kfunc_set instead
>   in patch 8.
>
> v10:
> - drop mptcp_subflow_set_scheduled() helper and WRITE_ONCE() in BPF.
> - add new bpf helper bpf_mptcp_send_info_to_ssk() for burst scheduler.
>
> v9:
> - merge 'Fixes for "use bpf_iter in bpf schedulers" v8' into this set.
> - rebased on "add netns helpers" v4
>
> v8:
> - address Mat's comments in v7.
> - move sk_stream_memory_free check inside bpf_for_each() loop.
> - implement mptcp_subflow_set_scheduled helper in BPF.
> - add cleanup patches into this set again.
>
> v7:
> - move cleanup patches out of this set.
> - rebased.
>
> v6:
> - rebased to "add mptcp_subflow bpf_iter" v10
>
> v5:
> - patch 2, drop mptcp_sock_type and mptcp_subflow_type.
> - patch 3, revert "bpf: Export more bpf_burst related functions"
> - patch 4, merge "bpf: Export more bpf_burst related functions" into it.
>
> v4:
> - patch 2, a new cleanup for "bpf: Add bpf_mptcp_sched_ops".
> - patch 3 should be reverted.
> - patch 8, register kfunc_set.
>
> v3:
> - rebased.
> - put the "drop has_bytes_sent" squash-to patch into this set.
>
> v2:
> - update bpf_rr and bpf_burst
>
> With the newly added mptcp_subflow bpf_iter, we can get rid of the
> subflows array "contexts" in struct mptcp_sched_data. This set
> uses bpf_for_each(mptcp_subflow) helper to update all the bpf
> schedules:
>
>        bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk) {
>                ... ...
>                mptcp_subflow_set_scheduled(subflow, true);
>        }
>
> Geliang Tang (8):
>  Squash to "mptcp: add sched_data helpers"
>  Squash to "bpf: Export mptcp packet scheduler helpers"
>  Squash to "selftests/bpf: Add bpf_first scheduler & test"
>  Squash to "selftests/bpf: Add bpf_bkup scheduler & test"
>  Squash to "selftests/bpf: Add bpf_rr scheduler & test"
>  Squash to "selftests/bpf: Add bpf_red scheduler & test"
>  Squash to "selftests/bpf: Add bpf_burst scheduler & test"
>  mptcp: drop subflow contexts in mptcp_sched_data
>
> include/net/mptcp.h                           |  4 -
> net/mptcp/bpf.c                               | 50 ++++++------
> net/mptcp/protocol.h                          |  2 -
> net/mptcp/sched.c                             | 15 ----
> tools/testing/selftests/bpf/progs/mptcp_bpf.h |  5 +-
> .../selftests/bpf/progs/mptcp_bpf_bkup.c      | 16 +---
> .../selftests/bpf/progs/mptcp_bpf_burst.c     | 78 +++++++------------
> .../selftests/bpf/progs/mptcp_bpf_first.c     |  8 +-
> .../selftests/bpf/progs/mptcp_bpf_red.c       |  8 +-
> .../selftests/bpf/progs/mptcp_bpf_rr.c        | 31 ++++----
> 10 files changed, 83 insertions(+), 134 deletions(-)
>
> -- 
> 2.43.0
>
>
>