net/mptcp/mib.c | 4 + net/mptcp/mib.h | 4 + net/mptcp/subflow.c | 21 +- tools/testing/selftests/net/mptcp/mptcp_join.sh | 319 +++++++++++++++--------- 4 files changed, 234 insertions(+), 114 deletions(-)
Recently, a few issues have been discovered around the creation of
additional subflows. Without these counters, it was difficult to point
out the reason why some subflows were not created as expected.
In patch 2, all error paths from __mptcp_subflow_connect() are covered,
except the one related to the 'fully established mode', because it
should only happen with the userspace PM, which will propagate the error
in this case (ENOTCONN).
These new counters are also verified in the MPTCP Join selftest in patch
5.
While at it, a few other patches are improving the MPJoin selftests:
- Patch 1: a fix when using an old kernel
- Patch 3: reduce the number of positional parameters
- Patch 4: only one line for the 'join' checks, instead of 3
- Patch 6: more explicit check names, instead of sometimes too cryptic
ones: rtx, ptx, ftx, ctx, fclzrx, sum
- Patch 7: specify client/server instead of 'invert' for some checks
not suggesting one direction
- Patch 8: mute errors of mptcp_connect when ran in the background
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v4:
- Applied Geliang's comments, see patches 2, 4, 5, 6
- Link to v3: https://lore.kernel.org/r/20240806-mptcp-join-tx-mib-v3-0-c3b54d2099e9@kernel.org
Changes in v3:
- Patch 2: remove fully estab error
- Patch 5: force bind error + always validate join tx
- New patches: 1, 3, 4, 6, 7, 8.
- Link to v2: https://lore.kernel.org/r/20240729-mptcp-join-tx-mib-v2-0-8ace70fd407a@kernel.org
Changes in v2:
- Patch 1/2: Add "ERR" suffix in variable names. (Geliang)
- Link to v1: https://lore.kernel.org/r/20240726-mptcp-join-tx-mib-v1-0-7f2149ba0dcf@kernel.org
---
Matthieu Baerts (NGI0) (8):
selftests: mptcp: join: no extra msg if no counter
mptcp: MIB counters for sent MP_JOIN
selftests: mptcp: join: reduce join_nr params
selftests: mptcp: join: one line for join check
selftests: mptcp: join: validate MPJ SYN TX MIB counters
selftests: mptcp: join: more explicit check name
selftests: mptcp: join: specify host being checked
selftests: mptcp: join: mute errors when ran in the background
net/mptcp/mib.c | 4 +
net/mptcp/mib.h | 4 +
net/mptcp/subflow.c | 21 +-
tools/testing/selftests/net/mptcp/mptcp_join.sh | 319 +++++++++++++++---------
4 files changed, 234 insertions(+), 114 deletions(-)
---
base-commit: e4105976662ef3faa6520c2c5da3d40505430761
change-id: 20240724-mptcp-join-tx-mib-84e21ea4b236
Best regards,
--
Matthieu Baerts (NGI0) <matttbe@kernel.org>
Hi Matt, On Fri, 2024-08-09 at 13:51 +0200, Matthieu Baerts (NGI0) wrote: > Recently, a few issues have been discovered around the creation of > additional subflows. Without these counters, it was difficult to > point > out the reason why some subflows were not created as expected. > > In patch 2, all error paths from __mptcp_subflow_connect() are > covered, > except the one related to the 'fully established mode', because it > should only happen with the userspace PM, which will propagate the > error > in this case (ENOTCONN). > > These new counters are also verified in the MPTCP Join selftest in > patch > 5. > > While at it, a few other patches are improving the MPJoin selftests: > > - Patch 1: a fix when using an old kernel > - Patch 3: reduce the number of positional parameters > - Patch 4: only one line for the 'join' checks, instead of 3 > - Patch 6: more explicit check names, instead of sometimes too > cryptic > ones: rtx, ptx, ftx, ctx, fclzrx, sum > - Patch 7: specify client/server instead of 'invert' for some checks > not suggesting one direction > - Patch 8: mute errors of mptcp_connect when ran in the background > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> v4 looks good to me, except for a nit in patch 1. Reviewed-by: Geliang Tang <geliang@kernel.org> Thanks, -Geliang > --- > Changes in v4: > - Applied Geliang's comments, see patches 2, 4, 5, 6 > - Link to v3: > https://lore.kernel.org/r/20240806-mptcp-join-tx-mib-v3-0-c3b54d2099e9@kernel.org > > Changes in v3: > - Patch 2: remove fully estab error > - Patch 5: force bind error + always validate join tx > - New patches: 1, 3, 4, 6, 7, 8. > - Link to v2: > https://lore.kernel.org/r/20240729-mptcp-join-tx-mib-v2-0-8ace70fd407a@kernel.org > > Changes in v2: > - Patch 1/2: Add "ERR" suffix in variable names. (Geliang) > - Link to v1: > https://lore.kernel.org/r/20240726-mptcp-join-tx-mib-v1-0-7f2149ba0dcf@kernel.org > > --- > Matthieu Baerts (NGI0) (8): > selftests: mptcp: join: no extra msg if no counter > mptcp: MIB counters for sent MP_JOIN > selftests: mptcp: join: reduce join_nr params > selftests: mptcp: join: one line for join check > selftests: mptcp: join: validate MPJ SYN TX MIB counters > selftests: mptcp: join: more explicit check name > selftests: mptcp: join: specify host being checked > selftests: mptcp: join: mute errors when ran in the background > > net/mptcp/mib.c | 4 + > net/mptcp/mib.h | 4 + > net/mptcp/subflow.c | 21 +- > tools/testing/selftests/net/mptcp/mptcp_join.sh | 319 > +++++++++++++++--------- > 4 files changed, 234 insertions(+), 114 deletions(-) > --- > base-commit: e4105976662ef3faa6520c2c5da3d40505430761 > change-id: 20240724-mptcp-join-tx-mib-84e21ea4b236 > > Best regards,
Hi Geliang, On 12/08/2024 03:55, Geliang Tang wrote: > Hi Matt, > > On Fri, 2024-08-09 at 13:51 +0200, Matthieu Baerts (NGI0) wrote: >> Recently, a few issues have been discovered around the creation of >> additional subflows. Without these counters, it was difficult to >> point >> out the reason why some subflows were not created as expected. >> >> In patch 2, all error paths from __mptcp_subflow_connect() are >> covered, >> except the one related to the 'fully established mode', because it >> should only happen with the userspace PM, which will propagate the >> error >> in this case (ENOTCONN). >> >> These new counters are also verified in the MPTCP Join selftest in >> patch >> 5. >> >> While at it, a few other patches are improving the MPJoin selftests: >> >> - Patch 1: a fix when using an old kernel >> - Patch 3: reduce the number of positional parameters >> - Patch 4: only one line for the 'join' checks, instead of 3 >> - Patch 6: more explicit check names, instead of sometimes too >> cryptic >> ones: rtx, ptx, ftx, ctx, fclzrx, sum >> - Patch 7: specify client/server instead of 'invert' for some checks >> not suggesting one direction >> - Patch 8: mute errors of mptcp_connect when ran in the background >> >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > > v4 looks good to me, except for a nit in patch 1. > > Reviewed-by: Geliang Tang <geliang@kernel.org> Thank you for the review! Now in our tree (fix and feat.), with the suggested fix for patch 1: New patches for t/upstream-net and t/upstream: - b9a0a7f3e3f5: selftests: mptcp: join: no extra msg if no counter - Results: 99ab2d5a0614..b602e35ece0d (export-net) - Results: 8cddf0574b26..911ef98f476b (export) New patches for t/upstream: - 14b93b3aef89: mptcp: MIB counters for sent MP_JOIN - 733601d0fbaf: selftests: mptcp: join: reduce join_nr params - a4bf220dea45: selftests: mptcp: join: one line for join check - 6b46845acede: selftests: mptcp: join: validate MPJ SYN TX MIB counters - daf3c299b5b3: selftests: mptcp: join: more explicit check name - 861f376c89f0: selftests: mptcp: join: specify host being checked - b4d470795230: selftests: mptcp: join: mute errors when ran in the background - Results: 911ef98f476b..003fdde88eb8 (export) Tests are now in progress: - export-net: https://github.com/multipath-tcp/mptcp_net-next/commit/3a27935d710404a61eb4e61c22f1d50c2351a6d7/checks - export: https://github.com/multipath-tcp/mptcp_net-next/commit/705a289bff1023b70ce89387f4c0e6bd51d8bb35/checks Cheers, Matt -- Sponsored by the NGI0 Core fund.
Hi Matthieu,
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/10318863223
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/8cc9ef30efb1
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=878183
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)
© 2016 - 2024 Red Hat, Inc.