mbox series

[net,0/7] mptcp: fix endpoints with 'signal' and 'subflow' flags

Message ID 20240731-upstream-net-20240731-mptcp-endp-subflow-signal-v1-0-c8a9b036493b@kernel.org (mailing list archive)
Headers show
Series mptcp: fix endpoints with 'signal' and 'subflow' flags | expand

Message

Matthieu Baerts (NGI0) July 31, 2024, 11:05 a.m. UTC
When looking at improving the user experience around the MPTCP endpoints
setup, I noticed that setting an endpoint with both the 'signal' and the
'subflow' flags -- as it has been done in the past by users according to
bug reports we got -- was resulting on only announcing the endpoint, but
not using it to create subflows: the 'subflow' flag was then ignored.

My initial thought was to modify IPRoute2 to warn the user when the two
flags were set, but it doesn't sound normal to ignore one of them. I
then looked at modifying the kernel not to allow having the two flags
set, but when discussing about that with Mat, we thought it was maybe
not ideal to do that, as there might be use-cases, we might break some
configs. Then I saw it was working before v5.17. So instead, I fixed the
support on the kernel side (patch 5) using Paolo's suggestion. This also
includes a fix on the options side (patch 1: for v5.11+), an explicit
deny of some options combinations (patch 2: for v5.18+), and some
refactoring (patches 3 and 4) to ease the inclusion of the patch 5.

While at it, I added a new selftest (patch 7) to validate this case --
including a modification of the chk_add_nr helper to inverse the sides
were the counters are checked (patch 6) -- and allowed ADD_ADDR echo
just after the MP_JOIN 3WHS.

The selftests modification have the same Fixes tag as the previous
commit, but no 'Cc: Stable': if the backport can work, that's good --
but it still need to be verified by running the selftests -- if not, no
need to worry, many CIs will use the selftests from the last stable
version to validate previous stable releases.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (7):
      mptcp: fully established after ADD_ADDR echo on MPJ
      mptcp: pm: deny endp with signal + subflow + port
      mptcp: pm: reduce indentation blocks
      mptcp: pm: don't try to create sf if alloc failed
      mptcp: pm: do not ignore 'subflow' if 'signal' flag is also set
      selftests: mptcp: join: ability to invert ADD_ADDR check
      selftests: mptcp: join: test both signal & subflow

 net/mptcp/options.c                             |  3 +-
 net/mptcp/pm_netlink.c                          | 47 +++++++++++++--------
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 55 ++++++++++++++++++-------
 3 files changed, 73 insertions(+), 32 deletions(-)
---
base-commit: 0bf50cead4c4710d9f704778c32ab8af47ddf070
change-id: 20240731-upstream-net-20240731-mptcp-endp-subflow-signal-181d640cf5e8

Best regards,

Comments

patchwork-bot+netdevbpf@kernel.org Aug. 2, 2024, 1:40 a.m. UTC | #1
Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 31 Jul 2024 13:05:52 +0200 you wrote:
> When looking at improving the user experience around the MPTCP endpoints
> setup, I noticed that setting an endpoint with both the 'signal' and the
> 'subflow' flags -- as it has been done in the past by users according to
> bug reports we got -- was resulting on only announcing the endpoint, but
> not using it to create subflows: the 'subflow' flag was then ignored.
> 
> My initial thought was to modify IPRoute2 to warn the user when the two
> flags were set, but it doesn't sound normal to ignore one of them. I
> then looked at modifying the kernel not to allow having the two flags
> set, but when discussing about that with Mat, we thought it was maybe
> not ideal to do that, as there might be use-cases, we might break some
> configs. Then I saw it was working before v5.17. So instead, I fixed the
> support on the kernel side (patch 5) using Paolo's suggestion. This also
> includes a fix on the options side (patch 1: for v5.11+), an explicit
> deny of some options combinations (patch 2: for v5.18+), and some
> refactoring (patches 3 and 4) to ease the inclusion of the patch 5.
> 
> [...]

Here is the summary with links:
  - [net,1/7] mptcp: fully established after ADD_ADDR echo on MPJ
    https://git.kernel.org/netdev/net/c/d67c5649c154
  - [net,2/7] mptcp: pm: deny endp with signal + subflow + port
    https://git.kernel.org/netdev/net/c/8af1f11865f2
  - [net,3/7] mptcp: pm: reduce indentation blocks
    https://git.kernel.org/netdev/net/c/c95eb32ced82
  - [net,4/7] mptcp: pm: don't try to create sf if alloc failed
    https://git.kernel.org/netdev/net/c/cd7c957f936f
  - [net,5/7] mptcp: pm: do not ignore 'subflow' if 'signal' flag is also set
    https://git.kernel.org/netdev/net/c/85df533a787b
  - [net,6/7] selftests: mptcp: join: ability to invert ADD_ADDR check
    https://git.kernel.org/netdev/net/c/bec1f3b119eb
  - [net,7/7] selftests: mptcp: join: test both signal & subflow
    https://git.kernel.org/netdev/net/c/4d2868b5d191

You are awesome, thank you!