include/trace/events/mptcp.h | 2 +- net/mptcp/mib.c | 2 + net/mptcp/mib.h | 2 + net/mptcp/options.c | 2 +- net/mptcp/pm.c | 12 ++++ net/mptcp/pm_netlink.c | 19 +++++- net/mptcp/pm_userspace.c | 18 ++++++ net/mptcp/protocol.c | 10 +-- net/mptcp/protocol.h | 4 ++ net/mptcp/subflow.c | 10 +++ tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c | 3 +- .../testing/selftests/bpf/progs/mptcp_bpf_burst.c | 12 ++-- tools/testing/selftests/net/mptcp/mptcp_join.sh | 72 +++++++++++++++++----- 13 files changed, 142 insertions(+), 26 deletions(-)
In all the backup tests we have, the backup flag is set on one side, and
the expected behaviour is to have both sides respecting this decision.
On the scheduler side, only the 'backup' field is checked, which is
supposed to be set only if the other peer flagged a subflow as backup.
But in various places, this flag is set when the local host flagged the
subflow as backup.
One last thing that has been fixed is the backup flag in 'signal'
endpoints: it was working only when setting the backup flag while the
connection was already opened. That's what the MPJ selftests were doing
to check that the signal endpoint were supporting the backup flag (on
purpose I guess, because the support was broken in MP_JOIN...).
The last two patches adapt the BPF selftests to mimic what is now done
by the in-kernel packets scheduler.
Note that the Packetdrill tests need to be updated because the behaviour
has been changed / fixed:
https://github.com/multipath-tcp/packetdrill/pull/152
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v3:
- Small fixes in patch 1 and 5: see the individual ChangeLog
- Use a new dedicated helper in patch 6: see the individual ChangeLog
- Link to v2: https://lore.kernel.org/r/20240716-mptcp-backup-mpj-v2-0-4d50247405fb@kernel.org
Changes in v2:
- new patches 4 and 5.
- split patch 6 in two (code then tests) to ease the backports.
- patch 6 addresses Mat's comments, and support the ID0 case.
- Link to v1: https://lore.kernel.org/r/20240711-mptcp-backup-mpj-v1-0-d45506182a9e@kernel.org
---
Matthieu Baerts (NGI0) (9):
mptcp: sched: check both directions for backup
mptcp: distinguish rcv vs sent backup flag in requests
mptcp: pm: only set request_bkup flag when sending MP_PRIO
mptcp: mib: count MPJ with backup flag
selftests: mptcp: join: validate backup in MPJ
mptcp: pm: fix backup support in signal endpoints
selftests: mptcp: join: check backup support in signal endp
Squash to "selftests/bpf: Add bpf_bkup scheduler & test"
Squash to "selftests/bpf: Add bpf_burst scheduler & test"
include/trace/events/mptcp.h | 2 +-
net/mptcp/mib.c | 2 +
net/mptcp/mib.h | 2 +
net/mptcp/options.c | 2 +-
net/mptcp/pm.c | 12 ++++
net/mptcp/pm_netlink.c | 19 +++++-
net/mptcp/pm_userspace.c | 18 ++++++
net/mptcp/protocol.c | 10 +--
net/mptcp/protocol.h | 4 ++
net/mptcp/subflow.c | 10 +++
tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c | 3 +-
.../testing/selftests/bpf/progs/mptcp_bpf_burst.c | 12 ++--
tools/testing/selftests/net/mptcp/mptcp_join.sh | 72 +++++++++++++++++-----
13 files changed, 142 insertions(+), 26 deletions(-)
---
base-commit: 50d1a79f5003b88b03b37d05189ad02a84d7be49
change-id: 20240711-mptcp-backup-mpj-2f8369fad366
Best regards,
--
Matthieu Baerts (NGI0) <matttbe@kernel.org>
On Thu, 18 Jul 2024, Matthieu Baerts (NGI0) wrote: > In all the backup tests we have, the backup flag is set on one side, and > the expected behaviour is to have both sides respecting this decision. > > On the scheduler side, only the 'backup' field is checked, which is > supposed to be set only if the other peer flagged a subflow as backup. > But in various places, this flag is set when the local host flagged the > subflow as backup. > > One last thing that has been fixed is the backup flag in 'signal' > endpoints: it was working only when setting the backup flag while the > connection was already opened. That's what the MPJ selftests were doing > to check that the signal endpoint were supporting the backup flag (on > purpose I guess, because the support was broken in MP_JOIN...). > > The last two patches adapt the BPF selftests to mimic what is now done > by the in-kernel packets scheduler. > > Note that the Packetdrill tests need to be updated because the behaviour > has been changed / fixed: > > https://github.com/multipath-tcp/packetdrill/pull/152 > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > Changes in v3: > - Small fixes in patch 1 and 5: see the individual ChangeLog > - Use a new dedicated helper in patch 6: see the individual ChangeLog > - Link to v2: https://lore.kernel.org/r/20240716-mptcp-backup-mpj-v2-0-4d50247405fb@kernel.org > Hi Matthieu - v3 LGTM. I'm not sure if the new MIBs are best targeted for net-next, they do help with testing but don't seem critical for -stable. Reviewed-by: Mat Martineau <martineau@kernel.org> > Changes in v2: > - new patches 4 and 5. > - split patch 6 in two (code then tests) to ease the backports. > - patch 6 addresses Mat's comments, and support the ID0 case. > - Link to v1: https://lore.kernel.org/r/20240711-mptcp-backup-mpj-v1-0-d45506182a9e@kernel.org > > --- > Matthieu Baerts (NGI0) (9): > mptcp: sched: check both directions for backup > mptcp: distinguish rcv vs sent backup flag in requests > mptcp: pm: only set request_bkup flag when sending MP_PRIO > mptcp: mib: count MPJ with backup flag > selftests: mptcp: join: validate backup in MPJ > mptcp: pm: fix backup support in signal endpoints > selftests: mptcp: join: check backup support in signal endp > Squash to "selftests/bpf: Add bpf_bkup scheduler & test" > Squash to "selftests/bpf: Add bpf_burst scheduler & test" > > include/trace/events/mptcp.h | 2 +- > net/mptcp/mib.c | 2 + > net/mptcp/mib.h | 2 + > net/mptcp/options.c | 2 +- > net/mptcp/pm.c | 12 ++++ > net/mptcp/pm_netlink.c | 19 +++++- > net/mptcp/pm_userspace.c | 18 ++++++ > net/mptcp/protocol.c | 10 +-- > net/mptcp/protocol.h | 4 ++ > net/mptcp/subflow.c | 10 +++ > tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c | 3 +- > .../testing/selftests/bpf/progs/mptcp_bpf_burst.c | 12 ++-- > tools/testing/selftests/net/mptcp/mptcp_join.sh | 72 +++++++++++++++++----- > 13 files changed, 142 insertions(+), 26 deletions(-) > --- > base-commit: 50d1a79f5003b88b03b37d05189ad02a84d7be49 > change-id: 20240711-mptcp-backup-mpj-2f8369fad366 > > Best regards, > -- > Matthieu Baerts (NGI0) <matttbe@kernel.org> > > >
Hi Mat, On 23/07/2024 02:24, Mat Martineau wrote: > On Thu, 18 Jul 2024, Matthieu Baerts (NGI0) wrote: > >> In all the backup tests we have, the backup flag is set on one side, and >> the expected behaviour is to have both sides respecting this decision. >> >> On the scheduler side, only the 'backup' field is checked, which is >> supposed to be set only if the other peer flagged a subflow as backup. >> But in various places, this flag is set when the local host flagged the >> subflow as backup. >> >> One last thing that has been fixed is the backup flag in 'signal' >> endpoints: it was working only when setting the backup flag while the >> connection was already opened. That's what the MPJ selftests were doing >> to check that the signal endpoint were supporting the backup flag (on >> purpose I guess, because the support was broken in MP_JOIN...). >> >> The last two patches adapt the BPF selftests to mimic what is now done >> by the in-kernel packets scheduler. >> >> Note that the Packetdrill tests need to be updated because the behaviour >> has been changed / fixed: >> >> https://github.com/multipath-tcp/packetdrill/pull/152 >> >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> >> --- >> Changes in v3: >> - Small fixes in patch 1 and 5: see the individual ChangeLog >> - Use a new dedicated helper in patch 6: see the individual ChangeLog >> - Link to v2: https://lore.kernel.org/r/20240716-mptcp-backup-mpj- >> v2-0-4d50247405fb@kernel.org >> > > Hi Matthieu - > > v3 LGTM. I'm not sure if the new MIBs are best targeted for net-next, > they do help with testing but don't seem critical for -stable. > > Reviewed-by: Mat Martineau <martineau@kernel.org> Thank you for the review! Now in our tree (fixes for -net): New patches for t/upstream-net and t/upstream: - 9d426c00aef4: mptcp: sched: check both directions for backup - cba82af312b3: mptcp: distinguish rcv vs sent backup flag in requests - 1ed597db4a47: mptcp: pm: only set request_bkup flag when sending MP_PRIO - 887ecb0faf1d: mptcp: mib: count MPJ with backup flag - 5e248a08f195: selftests: mptcp: join: validate backup in MPJ - 237a12154c8f: mptcp: pm: fix backup support in signal endpoints - 34b6d5cac1f0: selftests: mptcp: join: check backup support in signal endp - Results: 159097cdb0e8..001fe004e8be (export-net) - Results: 140ff27ee472..939ca15e71e2 (export) New patches for t/upstream: - b4a459e0fd62: "squashed" patch 8/9 in "selftests/bpf: Add bpf_bkup scheduler & test" - ec4acb003f8a: "squashed" patch 9/9 in "selftests/bpf: Add bpf_burst scheduler & test" - Results: 939ca15e71e2..f0f02c71ff0d (export) Tests are now in progress: - export-net: https://github.com/multipath-tcp/mptcp_net-next/commit/293546bf79b70ea127af87026d99cfb91eacb77b/checks - export: https://github.com/multipath-tcp/mptcp_net-next/commit/b601d6da52375e9a0a078ec38af104b5243c4658/checks Cheers, Matt -- Sponsored by the NGI0 Core fund.
Hi Mat, On 23/07/2024 02:24, Mat Martineau wrote: > On Thu, 18 Jul 2024, Matthieu Baerts (NGI0) wrote: > >> In all the backup tests we have, the backup flag is set on one side, and >> the expected behaviour is to have both sides respecting this decision. >> >> On the scheduler side, only the 'backup' field is checked, which is >> supposed to be set only if the other peer flagged a subflow as backup. >> But in various places, this flag is set when the local host flagged the >> subflow as backup. >> >> One last thing that has been fixed is the backup flag in 'signal' >> endpoints: it was working only when setting the backup flag while the >> connection was already opened. That's what the MPJ selftests were doing >> to check that the signal endpoint were supporting the backup flag (on >> purpose I guess, because the support was broken in MP_JOIN...). >> >> The last two patches adapt the BPF selftests to mimic what is now done >> by the in-kernel packets scheduler. >> >> Note that the Packetdrill tests need to be updated because the behaviour >> has been changed / fixed: >> >> https://github.com/multipath-tcp/packetdrill/pull/152 >> >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> >> --- >> Changes in v3: >> - Small fixes in patch 1 and 5: see the individual ChangeLog >> - Use a new dedicated helper in patch 6: see the individual ChangeLog >> - Link to v2: https://lore.kernel.org/r/20240716-mptcp-backup-mpj- >> v2-0-4d50247405fb@kernel.org >> > > Hi Matthieu - > > v3 LGTM. I'm not sure if the new MIBs are best targeted for net-next, > they do help with testing but don't seem critical for -stable. Indeed. My reasoning was that the MIB counters were not there, mainly because this behaviour has never been verified. At the end, it is only 10 new but well controlled lines :) I suggest applying them in our tree, and delay them if they got rejected. At the end, the new MIB counters and the tests are well isolated, and with a note about that. 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: Unstable: 4 failed test(s): packetdrill_mp_join packetdrill_mp_prio packetdrill_regressions packetdrill_syscalls 🔴
- KVM Validation: debug: Unstable: 4 failed test(s): packetdrill_mp_join packetdrill_mp_prio packetdrill_regressions packetdrill_syscalls 🔴
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/9994916304
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/962a6c85c567
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=872303
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)
Hello, On 18/07/2024 18:44, MPTCP CI wrote: > Our CI did some validations and here is its report: > > - KVM Validation: normal: Unstable: 4 failed test(s): packetdrill_mp_join packetdrill_mp_prio packetdrill_regressions packetdrill_syscalls 🔴 > - KVM Validation: debug: Unstable: 4 failed test(s): packetdrill_mp_join packetdrill_mp_prio packetdrill_regressions packetdrill_syscalls 🔴 Just to avoid some confusions, and as mentioned in my cover-letter, these errors were expected because these tests were looking for the backup flags in the MPJ+SYN+ACK which were not supposed to be there: https://github.com/multipath-tcp/packetdrill/pull/152 Cheers, Matt -- Sponsored by the NGI0 Core fund.
© 2016 - 2024 Red Hat, Inc.