The two first patches are some small improvements for previous patches
that are already in our tree, but not sent to netdev yet.

Then we have a fix to send a RM_ADDR with the correct ID 0, instead of
using the ID of the global endpoint. The test that is linked to that
doesn't check if the ID being sent is 0, but we can see the reaction of
the other peer if it closes the connection or not.

The next fix is to correctly reset the ID corresponding to the initial
address: to avoid some confusion, or not to send the wrong ID if it is
re-used by another address later on.

That leaded to two extra fix: sending an ACK on an active subflow, and
allowing to create a new subflow if the subflow was not closed: that's
because when the RM_ADDR was not correct, the other peer was not closing
the expected subflow, and the connection ended up being half closed.
This last issue is fixed by the next and last fix of the series.

The last patch is not for -net. Something that sounds better to do.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (11):
      Squash to "mptcp: pm: re-using ID of unused removed ADD_ADDR"
      Squash to "mptcp: pm: fullmesh: select the right ID later"
      mptcp: pm: fix RM_ADDR ID for the initial subflow
      selftests: mptcp: join: check removing ID 0 endpoint
      mptcp: pm: send ACK on an active subflow
      mptcp: pm: skip connecting to already established sf
      mptcp: pm: reset MPC endp ID when re-added
      selftests: mptcp: join: check re-adding init endp with != id
      mptcp: close subflow when receiving TCP+FIN
      selftests: mptcp: join: cannot rm sf if closed
      mptcp: pm: send ACK on non stale subflows

 net/mptcp/pm_netlink.c                          | 72 ++++++++++++++-----------
 net/mptcp/protocol.c                            |  5 +-
 net/mptcp/subflow.c                             |  8 ++-
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 49 ++++++++++-------
 4 files changed, 81 insertions(+), 53 deletions(-)
---
base-commit: 2b058d91b96082b282ab2b5bd2dbc7abdd28bacc
change-id: 20240620-mptcp-pm-avail-f5e3957be441

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

On Fri, 2 Aug 2024, Matthieu Baerts (NGI0) wrote:

> The two first patches are some small improvements for previous patches
> that are already in our tree, but not sent to netdev yet.
>
> Then we have a fix to send a RM_ADDR with the correct ID 0, instead of
> using the ID of the global endpoint. The test that is linked to that
> doesn't check if the ID being sent is 0, but we can see the reaction of
> the other peer if it closes the connection or not.
>
> The next fix is to correctly reset the ID corresponding to the initial
> address: to avoid some confusion, or not to send the wrong ID if it is
> re-used by another address later on.
>
> That leaded to two extra fix: sending an ACK on an active subflow, and
> allowing to create a new subflow if the subflow was not closed: that's
> because when the RM_ADDR was not correct, the other peer was not closing
> the expected subflow, and the connection ended up being half closed.
> This last issue is fixed by the next and last fix of the series.
>
> The last patch is not for -net. Something that sounds better to do.
>

Ok, after revising my response to patch 5, and a couple of minor changes 
noted for patches 9 and 10, I think the series looks good:

Reviewed-by: Mat Martineau <martineau@kernel.org>

If you want to rework patch 10 given the reverse-compatibility question, 
then you're welcome to apply all but 9 & 10 so you only need to respin two 
patches.


- Mat


> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Matthieu Baerts (NGI0) (11):
>      Squash to "mptcp: pm: re-using ID of unused removed ADD_ADDR"
>      Squash to "mptcp: pm: fullmesh: select the right ID later"
>      mptcp: pm: fix RM_ADDR ID for the initial subflow
>      selftests: mptcp: join: check removing ID 0 endpoint
>      mptcp: pm: send ACK on an active subflow
>      mptcp: pm: skip connecting to already established sf
>      mptcp: pm: reset MPC endp ID when re-added
>      selftests: mptcp: join: check re-adding init endp with != id
>      mptcp: close subflow when receiving TCP+FIN
>      selftests: mptcp: join: cannot rm sf if closed
>      mptcp: pm: send ACK on non stale subflows
>
> net/mptcp/pm_netlink.c                          | 72 ++++++++++++++-----------
> net/mptcp/protocol.c                            |  5 +-
> net/mptcp/subflow.c                             |  8 ++-
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 49 ++++++++++-------
> 4 files changed, 81 insertions(+), 53 deletions(-)
> ---
> base-commit: 2b058d91b96082b282ab2b5bd2dbc7abdd28bacc
> change-id: 20240620-mptcp-pm-avail-f5e3957be441
>
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
>
>

Hi Mat,

On 09/08/2024 02:36, Mat Martineau wrote:
> On Fri, 2 Aug 2024, Matthieu Baerts (NGI0) wrote:
> 
>> The two first patches are some small improvements for previous patches
>> that are already in our tree, but not sent to netdev yet.
>>
>> Then we have a fix to send a RM_ADDR with the correct ID 0, instead of
>> using the ID of the global endpoint. The test that is linked to that
>> doesn't check if the ID being sent is 0, but we can see the reaction of
>> the other peer if it closes the connection or not.
>>
>> The next fix is to correctly reset the ID corresponding to the initial
>> address: to avoid some confusion, or not to send the wrong ID if it is
>> re-used by another address later on.
>>
>> That leaded to two extra fix: sending an ACK on an active subflow, and
>> allowing to create a new subflow if the subflow was not closed: that's
>> because when the RM_ADDR was not correct, the other peer was not closing
>> the expected subflow, and the connection ended up being half closed.
>> This last issue is fixed by the next and last fix of the series.
>>
>> The last patch is not for -net. Something that sounds better to do.
>>
> 
> Ok, after revising my response to patch 5, and a couple of minor changes
> noted for patches 9 and 10, I think the series looks good:
> 
> Reviewed-by: Mat Martineau <martineau@kernel.org>

Thank you for the review!

New patches for t/upstream-net and t/upstream:
- 5ebe23ff5ade: "squashed" patch 1/11 in "mptcp: pm: re-using ID of
unused removed ADD_ADDR"
- d4e53d9d534b: "squashed" (with conflicts) in "mptcp: pm: fullmesh:
select the right ID later"
- 90ebd895d917: conflict in t/mptcp-pm-reduce-entries-iterations-on-connect
- Results: dd8eea82e12f..64bb700808df (export-net)
- Results: e4105976662e..79be0d5eca50 (export)

- d8e98c0f3bf2: mptcp: pm: fix RM_ADDR ID for the initial subflow
- ebdca4f0dafe: conflict in t/mptcp-pm-rename-helpers-linked-to-flush
- 4d0382127a23: selftests: mptcp: join: check removing ID 0 endpoint
- 67433e9c8864: mptcp: pm: send ACK on an active subflow
- 0840279fb33e: mptcp: pm: skip connecting to already established sf
- a7844983a8df: mptcp: pm: reset MPC endp ID when re-added
- 1ec2de2a8c89: selftests: mptcp: join: check re-adding init endp with != id
- Results: 64bb700808df..5c1437dbc580 (export-net)
- Results: 79be0d5eca50..beede3e0f15d (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/8283dd520fc366ce169363fe1d27df40e0e5b056/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/e76eecedaea68d03d2bd1ab1632589b1e7cb568c/checks

> If you want to rework patch 10 given the reverse-compatibility question,
> then you're welcome to apply all but 9 & 10 so you only need to respin
> two patches.

Yes, I can do that, thank you for the review!

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/10220396170

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


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)