[mptcp-next,v5,0/5] Squash to "Add mptcp_subflow bpf_iter support"

Message

Geliang Tang March 3, 2025, 10:33 a.m. UTC

From: Geliang Tang <[e-mail]>

v5:
 - check bpf_iter_task in mptcp_subflow_new() as Mat suggested.

v4:
 - drop sock_owned_by_user_nocheck and spin_is_locked. According to
   comments from Mat and Martin, in this set mptcp_subflow
   bpf_iter only used from a cg sockopt bpf prog, no need to add these
   check at this moment.

v3:
 - patch 3, continue to use sock_owned_by_user_nocheck() and spin_is_locked()
checks instead of using msk_owned_by_me().
 - patch 5, drop declaration of bpf_mptcp_subflow_tcp_sock. It's no longer
used.
 - patch 5, update the comment for mptcp_subflow_tcp_sock(), which is a BPF
helper, not a kfunc.

The commit log of "bpf: Register mptcp common kfunc set" doesn't match the
code, please update it as:

'''
bpf: Register mptcp common kfunc set

MPTCP helper mptcp_subflow_ctx() is used to convert struct sock to
struct mptcp_subflow_context. It will be used in MPTCP BPF programs.

This patch defines corresponding wrapper of this helper, and put it
into the newly defined mptcp common kfunc set and register this set
with the flag BPF_PROG_TYPE_CGROUP_SOCKOPT to let it accessible to
the 'cgroup/getsockopt' type of BPF programs.
'''

v2:
 - Drop bpf_skc_to_mptcp_sock
 - Check the owner before assigning the msk as Mat suggested.
 - Use bpf_core_cast() in mptcp_subflow bpf_iter subtest instead of
using bpf_skc_to_mptcp_sock().

Address Martin's suggestions for "Add mptcp_subflow bpf_iter support" v2.

Geliang Tang (5):
  mptcp: add bpf_iter_task for mptcp_sock
  Squash to "bpf: Extend bpf_skc_to_mptcp_sock to MPTCP sock"
  Squash to "bpf: Add mptcp_subflow bpf_iter"
  Revert "bpf: Acquire and release mptcp socket"
  Squash to "selftests/bpf: Add mptcp_subflow bpf_iter subtest"

 net/mptcp/bpf.c                               | 56 +++++++++----------
 net/mptcp/protocol.c                          |  1 +
 net/mptcp/protocol.h                          | 16 ++++++
 .../testing/selftests/bpf/bpf_experimental.h  |  2 +-
 tools/testing/selftests/bpf/progs/mptcp_bpf.h |  5 --
 .../selftests/bpf/progs/mptcp_bpf_iters.c     |  8 +--
 6 files changed, 47 insertions(+), 41 deletions(-)

Comments

Matthieu Baerts March 3, 2025, 11:13 a.m. UTC | #1

Hi Geliang, Mat,

On 03/03/2025 11:33, Geliang Tang wrote:
> From: Geliang Tang <[e-mail]>
> 
> v5:
>  - check bpf_iter_task in mptcp_subflow_new() as Mat suggested.

Thank you for the modification, but would it not be easier not to do
that in this series, not to increase the complexity?

This can be done in the "use bpf_iter in bpf schedulers" instead, no?

For me, the v4 (with my squash-to patch) is enough to be used with CG
[gs]etsocktopt.

Also, I think the idea is set this marker before calling the future
struct_ops, in places where we know the msk is owned, but not in
bpf_mptcp_sock_from_sock() which doesn't check if the msk is owned, no?

Cheers,
Matt

MPTCP CI March 3, 2025, 11:42 a.m. UTC | #2

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/13629090242

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/db85e099ebc4
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=939536


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)