[PATCH mptcp-next v8 0/4] fixes for "new MPTCP subflow subtest v4"

Geliang Tang posted 4 patches 2 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/cover.1725502822.git.tanggeliang@kylinos.cn
.../testing/selftests/bpf/prog_tests/mptcp.c  | 67 +++++++++++------
tools/testing/selftests/bpf/progs/mptcp_bpf.h | 27 +++++++
.../selftests/bpf/progs/mptcp_subflow.c       | 71 ++++++++++++++++++-
3 files changed, 143 insertions(+), 22 deletions(-)
[PATCH mptcp-next v8 0/4] fixes for "new MPTCP subflow subtest v4"
Posted by Geliang Tang 2 months, 2 weeks ago
From: Geliang Tang <tanggeliang@kylinos.cn>

v8:
 - drop bpf_printk, use different errnos (-1, -2) instead for debugging:

run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -1 (errno 1)
run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected error: -1 (errno 1)
run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -1 (errno 2)
run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected error: -1 (errno 2)

 - move more duplicate code into _getsockopt_subflow.

v7:
 - drop helpers mptcp_for_each_subflow_safe and
   list_for_each_entry_safe.
 - update parameters of _check_getsockopt_subflow_mark/cc.
 - print more messages in the logs as Matt suggested.

v6:
 - address Matt's comments in v5.
 - use mptcp_for_each_subflow in _check_getsockopt_subflow_mark.
 - check msk->pm.subflows in _check_getsockopt_subflow_mark/cc.
 - add wait_for_new_subflows helper.

v5:
 - rename _check_getsockopt_subflows_mark in patch 1.
 - address Matt's comments in v4.

v4:
 - address Matt's comments in v3.

v3:
 - drop "ss_search" from subflow subtest
 - fix mptcp_for_each_subflow_safe
 - add cond_break in list_for_each_entry/_safe

Two fixes address Martin's comments in "new MPTCP subflow subtest" v4.
Patch 2 will conflict with "selftests/bpf: Add bpf_first scheduler & test".

Geliang Tang (4):
  Squash to "selftests/bpf: Add mptcp subflow example"
  selftests/bpf: Add getsockopt to inspect mptcp subflow
  Squash to "selftests/bpf: Add mptcp subflow subtest"
  Squash to "selftests/bpf: Add bpf scheduler test"

 .../testing/selftests/bpf/prog_tests/mptcp.c  | 67 +++++++++++------
 tools/testing/selftests/bpf/progs/mptcp_bpf.h | 27 +++++++
 .../selftests/bpf/progs/mptcp_subflow.c       | 71 ++++++++++++++++++-
 3 files changed, 143 insertions(+), 22 deletions(-)

-- 
2.43.0
Re: [PATCH mptcp-next v8 0/4] fixes for "new MPTCP subflow subtest v4"
Posted by Matthieu Baerts 2 months, 2 weeks ago
Hi Geliang,

On 05/09/2024 04:26, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v8:
>  - drop bpf_printk, use different errnos (-1, -2) instead for debugging:
> 
> run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -1 (errno 1)
> run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected error: -1 (errno 1)
> run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -1 (errno 2)
> run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected error: -1 (errno 2)
> 
>  - move more duplicate code into _getsockopt_subflow.

Thank you for the new version! Now in our tree. Because there were a few
squashed-to patches + additional corrections + conflicts, please check
that the export branch looks OK now.

New patches for t/upstream:
- a46e44a2ac5b: "squashed" patch 1/4 in "selftests/bpf: Add mptcp
subflow example"
- df1c9da900a8: tg: remove t/selftests-bpf-Add-mptcp-pm_nl_ctl-link

- f259c76495da: selftests/bpf: Add getsockopt to inspect mptcp subflow:
  + updated commit message
  + added mptcp_bpf.h file
  + adapted MAINTAINERS file
  + added comments in mptcp_bpf.h about the origin + modifications
- 6efeaa2ddfaf: conflict in t/selftests-bpf-add-bpf_first-scheduler
- 52d77c8981d3: conflict in t/selftests-bpf-Add-bpf_bkup-scheduler

- 5daceaf2c412: "squashed" patch 3/4 in "selftests/bpf: Add mptcp
subflow subtest"
- a1f7132c9bad: Also drop _ss_search()

- d2064c588b9f: "squashed" patch 4/4 in "selftests/bpf: Add bpf
scheduler test"

- Results: df250dc71896..fb68bc3776eb (export)

Tests are now in progress:

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


Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-next v8 0/4] fixes for "new MPTCP subflow subtest v4"
Posted by Geliang Tang 2 months, 2 weeks ago
Hi Matt,

On Thu, 2024-09-05 at 12:29 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 05/09/2024 04:26, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > v8:
> >  - drop bpf_printk, use different errnos (-1, -2) instead for
> > debugging:
> > 
> > run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -
> > 1 (errno 1)
> > run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected
> > error: -1 (errno 1)
> > run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -
> > 1 (errno 2)
> > run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected
> > error: -1 (errno 2)
> > 
> >  - move more duplicate code into _getsockopt_subflow.
> 
> Thank you for the new version! Now in our tree. Because there were a
> few
> squashed-to patches + additional corrections + conflicts, please
> check
> that the export branch looks OK now.
> 
> New patches for t/upstream:
> - a46e44a2ac5b: "squashed" patch 1/4 in "selftests/bpf: Add mptcp
> subflow example"
> - df1c9da900a8: tg: remove t/selftests-bpf-Add-mptcp-pm_nl_ctl-link
> 
> - f259c76495da: selftests/bpf: Add getsockopt to inspect mptcp
> subflow:
>   + updated commit message
>   + added mptcp_bpf.h file
>   + adapted MAINTAINERS file
>   + added comments in mptcp_bpf.h about the origin + modifications
> - 6efeaa2ddfaf: conflict in t/selftests-bpf-add-bpf_first-scheduler
> - 52d77c8981d3: conflict in t/selftests-bpf-Add-bpf_bkup-scheduler
> 
> - 5daceaf2c412: "squashed" patch 3/4 in "selftests/bpf: Add mptcp
> subflow subtest"
> - a1f7132c9bad: Also drop _ss_search()
> 
> - d2064c588b9f: "squashed" patch 4/4 in "selftests/bpf: Add bpf
> scheduler test"
> 
> - Results: df250dc71896..fb68bc3776eb (export)
> 
> Tests are now in progress:
> 
> - export:
> https://github.com/multipath-tcp/mptcp_net-next/commit/51e2be27cea9c5e4dd17b25cade6fcaf055daf52/checks
> 

This error occurs:

progs/mptcp_bpf.h:53:1: error: redefinition of 'mptcp_subflow_tcp_sock'
   53 | mptcp_subflow_tcp_sock(const struct mptcp_subflow_context
*subflow)
      | ^

mptcp_subflow_tcp_sock() needs to be drop from "selftests/bpf: Add
bpf_rr scheduler & test".

Thanks,
-Geliang

> 
> Cheers,
> Matt

Re: [PATCH mptcp-next v8 0/4] fixes for "new MPTCP subflow subtest v4"
Posted by Matthieu Baerts 2 months, 2 weeks ago
Hi Geliang,

On 05/09/2024 12:59, Geliang Tang wrote:
> Hi Matt,
> 
> On Thu, 2024-09-05 at 12:29 +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 05/09/2024 04:26, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> v8:
>>>  - drop bpf_printk, use different errnos (-1, -2) instead for
>>> debugging:
>>>
>>> run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -
>>> 1 (errno 1)
>>> run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected
>>> error: -1 (errno 1)
>>> run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -
>>> 1 (errno 2)
>>> run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected
>>> error: -1 (errno 2)
>>>
>>>  - move more duplicate code into _getsockopt_subflow.
>>
>> Thank you for the new version! Now in our tree. Because there were a
>> few
>> squashed-to patches + additional corrections + conflicts, please
>> check
>> that the export branch looks OK now.
>>
>> New patches for t/upstream:
>> - a46e44a2ac5b: "squashed" patch 1/4 in "selftests/bpf: Add mptcp
>> subflow example"
>> - df1c9da900a8: tg: remove t/selftests-bpf-Add-mptcp-pm_nl_ctl-link
>>
>> - f259c76495da: selftests/bpf: Add getsockopt to inspect mptcp
>> subflow:
>>   + updated commit message
>>   + added mptcp_bpf.h file
>>   + adapted MAINTAINERS file
>>   + added comments in mptcp_bpf.h about the origin + modifications
>> - 6efeaa2ddfaf: conflict in t/selftests-bpf-add-bpf_first-scheduler
>> - 52d77c8981d3: conflict in t/selftests-bpf-Add-bpf_bkup-scheduler
>>
>> - 5daceaf2c412: "squashed" patch 3/4 in "selftests/bpf: Add mptcp
>> subflow subtest"
>> - a1f7132c9bad: Also drop _ss_search()
>>
>> - d2064c588b9f: "squashed" patch 4/4 in "selftests/bpf: Add bpf
>> scheduler test"
>>
>> - Results: df250dc71896..fb68bc3776eb (export)
>>
>> Tests are now in progress:
>>
>> - export:
>> https://github.com/multipath-tcp/mptcp_net-next/commit/51e2be27cea9c5e4dd17b25cade6fcaf055daf52/checks
>>
> 
> This error occurs:
> 
> progs/mptcp_bpf.h:53:1: error: redefinition of 'mptcp_subflow_tcp_sock'
>    53 | mptcp_subflow_tcp_sock(const struct mptcp_subflow_context
> *subflow)
>       | ^
> 
> mptcp_subflow_tcp_sock() needs to be drop from "selftests/bpf: Add
> bpf_rr scheduler & test".

Thank you for the quick validation!

Fixed now:

New patches for t/upstream:
- e4eee5701eea: selftests: bpf: fix compilation issues, "squashed" in
"selftests/bpf: Add bpf_rr scheduler & test"
- Results: fb68bc3776eb..b0b522c239d0 (export)

Tests are now in progress:

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

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

Re: [PATCH mptcp-next v8 0/4] fixes for "new MPTCP subflow subtest v4"
Posted by Matthieu Baerts 2 months, 2 weeks ago
Hi Geliang,

On 05/09/2024 04:26, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v8:

Thank you for the update!

>  - drop bpf_printk, use different errnos (-1, -2) instead for debugging:
> 
> run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -1 (errno 1)
> run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected error: -1 (errno 1)
> run_subflow:FAIL:getsockopt(client_fd, SO_MARK) unexpected error: -1 (errno 2)
> run_subflow:FAIL:getsockopt(client_fd, TCP_CONGESTION) unexpected error: -1 (errno 2)

That's a vague because we don't know what was the value of 'mark' and
'cc', but better than nothing. We can work with that.

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-next v8 0/4] fixes for "new MPTCP subflow subtest v4"
Posted by MPTCP CI 2 months, 2 weeks ago
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 (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/10712924117

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


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)