v3:
 - drop patch 4 in v2
 - invert patch 10 and 11 in v2
 - rebased

1-7: some small cleanups, v3
8-9: address #428, add mptcpi_subflows_total
10-16: address #379, #391, userspace pm remove id 0 subflow & address, v11
17-19: address #403, add refcont for address entry
20: add userspace fullmesh tests
21-29: seltests cleanups

Geliang Tang (29):
  mptcp: drop useless ssk in pm_subflow_check_next
  mptcp: use mptcp_check_fallback helper
  mptcp: use mptcp_get_ext helper
  mptcp: move sk assignment statement ahead
  mptcp: define more local variables sk
  selftests: mptcp: sockopt: drop mptcp_connect var
  selftests: mptcp: display simult in extra_msg
  mptcp: add mptcpi_subflows_total counter
  selftests: mptcp: add evts_get_info helper
  selftests: mptcp: add chk_subflows_total helper
  selftests: mptcp: update userspace pm test helpers
  selftests: mptcp: userspace pm remove id 0 subflow
  mptcp: userspace pm allow creating id 0 subflow
  selftests: mptcp: userspace pm create id 0 subflow
  mptcp: userspace pm remove id 0 address
  selftests: mptcp: userspace pm remove id 0 address
  mptcp: add userspace_pm_get_entry helper
  mptcp: add userspace pm addr entry refcount
  mptcp: add netlink pm addr entry refcount
  selftests: mptcp: add userspace pm fullmesh tests
  selftests: mptcp: add mptcp_lib_kill_wait
  selftests: mptcp: add mptcp_lib_evts_*
  selftests: mptcp: userspace: print colored results
  selftests: mptcp: add mptcp_lib_verify_listener_events
  selftests: mptcp: add mptcp_lib_is_v6
  selftests: mptcp: add mptcp_lib_get_counter
  selftests: mptcp: add mptcp_lib_make_file
  selftests: mptcp: add mptcp_lib_check_transfer
  selftests: mptcp: add mptcp_lib_wait_local_port_listen

 include/uapi/linux/mptcp.h                    |   1 +
 net/mptcp/pm.c                                |   2 +-
 net/mptcp/pm_netlink.c                        |  25 +-
 net/mptcp/pm_userspace.c                      | 139 ++++--
 net/mptcp/protocol.c                          |   6 +-
 net/mptcp/protocol.h                          |  14 +-
 net/mptcp/sockopt.c                           |   4 +-
 tools/testing/selftests/net/mptcp/diag.sh     |  23 +-
 .../selftests/net/mptcp/mptcp_connect.sh      | 108 +---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 460 +++++++++---------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 188 +++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      |  55 +--
 .../selftests/net/mptcp/simult_flows.sh       |  19 +-
 .../selftests/net/mptcp/userspace_pm.sh       | 199 +++-----
 14 files changed, 661 insertions(+), 582 deletions(-)

-- 
2.35.3

Hi Geliang,

On 25/09/2023 10:41, Geliang Tang wrote:
> v3:
>  - drop patch 4 in v2
>  - invert patch 10 and 11 in v2
>  - rebased

Thank you for the v3! And again, sorry for the delays!

I think patches 1-11 + 13-14 are OK to be applied (with two small
modifications for patch 7 and 11 that I can do when applying these patches):

Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Please note that patch 13 will go to "Fixes for -net". Patch 14 can stay
in "Features for net-next" due to other dependences.

I have some comments about patch 12 and 16: on the wire, sending a
remove address for ID 0 or actually removing the subflow(s) with ID 0
should not trigger any RST (+MP_TCPRST). I guess the kernel is not
behaving correctly if you observe them during the test (that's good you
added that in the verification!). It means we will need more patches for
the kernel side not to send these RST in these cases.

If you send a v4, please move patch 15 (mptcp: userspace pm remove id 0
address) to the top as this one is for -net. It is fine to keep patch 16
where it is if it depends on other features.

> 1-7: some small cleanups, v3
> 8-9: address #428, add mptcpi_subflows_total
> 10-16: address #379, #391, userspace pm remove id 0 subflow & address, v11
> 17-19: address #403, add refcont for address entry
> 20: add userspace fullmesh tests
> 21-29: seltests cleanups

Note: I didn't review patches 17-29 yet. I will try to do that ASAP (but
very likely, not before the end of next week :-/ ).

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Hi Geliang,

On 28/09/2023 22:54, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 25/09/2023 10:41, Geliang Tang wrote:
>> v3:
>>  - drop patch 4 in v2
>>  - invert patch 10 and 11 in v2
>>  - rebased
> 
> Thank you for the v3! And again, sorry for the delays!
> 
> I think patches 1-11 + 13-14 are OK to be applied (with two small
> modifications for patch 7 and 11 that I can do when applying these patches):
> 
> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> 
> Please note that patch 13 will go to "Fixes for -net". Patch 14 can stay
> in "Features for net-next" due to other dependences.

I just did that:

New patches for t/upstream-net and t/upstream:
- 505a33d76a32: mptcp: userspace pm allow creating id 0 subflow
- Results: 05758f924fe3..999b9f7e47a5 (export-net)

New patches for t/upstream:
- 1cb016e94837: mptcp: drop useless ssk in pm_subflow_check_next
- 450deaf6dd71: mptcp: use mptcp_check_fallback helper
- 6df74a0ced73: mptcp: use mptcp_get_ext helper
- 03649553fd68: mptcp: move sk assignment statement ahead
- 6bc1e5144525: mptcp: define more local variables sk
- b7e3cc69d9fe: selftests: mptcp: sockopt: drop mptcp_connect var
- 14c099dbd7a7: selftests: mptcp: display simult in extra_msg
- 7d9e508e1117: mptcp: add mptcpi_subflows_total counter
- af6ed0819599: selftests: mptcp: add evts_get_info helper
- 9177dc0b474c: selftests: mptcp: add chk_subflows_total helper
- bc1edd827cfa: selftests: mptcp: update userspace pm test helpers
- 0dd61d4e14e8: selftests: mptcp: userspace pm create id 0 subflow
- Results: 3be1c8ce544a..516baae7a220 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20230928T211416
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230928T213439

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Hi Geliang,

On 28/09/2023 23:37, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 28/09/2023 22:54, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 25/09/2023 10:41, Geliang Tang wrote:
>>> v3:
>>>  - drop patch 4 in v2
>>>  - invert patch 10 and 11 in v2
>>>  - rebased
>>
>> Thank you for the v3! And again, sorry for the delays!
>>
>> I think patches 1-11 + 13-14 are OK to be applied (with two small
>> modifications for patch 7 and 11 that I can do when applying these patches):
>>
>> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>
>> Please note that patch 13 will go to "Fixes for -net". Patch 14 can stay
>> in "Features for net-next" due to other dependences.
> 
> I just did that:
> 
> New patches for t/upstream-net and t/upstream:
> - 505a33d76a32: mptcp: userspace pm allow creating id 0 subflow
> - Results: 05758f924fe3..999b9f7e47a5 (export-net)
> 
> New patches for t/upstream:
> - 1cb016e94837: mptcp: drop useless ssk in pm_subflow_check_next
> - 450deaf6dd71: mptcp: use mptcp_check_fallback helper
> - 6df74a0ced73: mptcp: use mptcp_get_ext helper
> - 03649553fd68: mptcp: move sk assignment statement ahead
> - 6bc1e5144525: mptcp: define more local variables sk
> - b7e3cc69d9fe: selftests: mptcp: sockopt: drop mptcp_connect var
> - 14c099dbd7a7: selftests: mptcp: display simult in extra_msg
> - 7d9e508e1117: mptcp: add mptcpi_subflows_total counter
> - af6ed0819599: selftests: mptcp: add evts_get_info helper
> - 9177dc0b474c: selftests: mptcp: add chk_subflows_total helper
> - bc1edd827cfa: selftests: mptcp: update userspace pm test helpers
> - 0dd61d4e14e8: selftests: mptcp: userspace pm create id 0 subflow
> - Results: 3be1c8ce544a..516baae7a220 (export)

I think these patches are introducing regressions because these tests
are now regularly failing on the public CI:

  - 112 - mptcp_join: userspace pm add & remove address
  - 113 - mptcp_join: userspace pm create destroy subflow

I didn't check in more details but I see they are failing since
export/20230928T213439 and only when running with a debug kconfig (slow
environment), e.g.:

  - https://cirrus-ci.com/task/5444189572300800
  -
https://api.cirrus-ci.com/v1/artifact/task/5444189572300800/summary/summary.txt

Do you mind having a look?

Maybe the connection has been closed when looking at the number of subflows?
Also, it looks like the helper looking at 'subflows_total' with older
versions ss is producing a lot of debug output that can be muted.

Cheers,
Matt

On Thu, Oct 05, 2023 at 05:53:13PM +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 28/09/2023 23:37, Matthieu Baerts wrote:
> > Hi Geliang,
> > 
> > On 28/09/2023 22:54, Matthieu Baerts wrote:
> >> Hi Geliang,
> >>
> >> On 25/09/2023 10:41, Geliang Tang wrote:
> >>> v3:
> >>>  - drop patch 4 in v2
> >>>  - invert patch 10 and 11 in v2
> >>>  - rebased
> >>
> >> Thank you for the v3! And again, sorry for the delays!
> >>
> >> I think patches 1-11 + 13-14 are OK to be applied (with two small
> >> modifications for patch 7 and 11 that I can do when applying these patches):
> >>
> >> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> >>
> >> Please note that patch 13 will go to "Fixes for -net". Patch 14 can stay
> >> in "Features for net-next" due to other dependences.
> > 
> > I just did that:
> > 
> > New patches for t/upstream-net and t/upstream:
> > - 505a33d76a32: mptcp: userspace pm allow creating id 0 subflow
> > - Results: 05758f924fe3..999b9f7e47a5 (export-net)
> > 
> > New patches for t/upstream:
> > - 1cb016e94837: mptcp: drop useless ssk in pm_subflow_check_next
> > - 450deaf6dd71: mptcp: use mptcp_check_fallback helper
> > - 6df74a0ced73: mptcp: use mptcp_get_ext helper
> > - 03649553fd68: mptcp: move sk assignment statement ahead
> > - 6bc1e5144525: mptcp: define more local variables sk
> > - b7e3cc69d9fe: selftests: mptcp: sockopt: drop mptcp_connect var
> > - 14c099dbd7a7: selftests: mptcp: display simult in extra_msg
> > - 7d9e508e1117: mptcp: add mptcpi_subflows_total counter
> > - af6ed0819599: selftests: mptcp: add evts_get_info helper
> > - 9177dc0b474c: selftests: mptcp: add chk_subflows_total helper
> > - bc1edd827cfa: selftests: mptcp: update userspace pm test helpers
> > - 0dd61d4e14e8: selftests: mptcp: userspace pm create id 0 subflow
> > - Results: 3be1c8ce544a..516baae7a220 (export)
> 
> I think these patches are introducing regressions because these tests
> are now regularly failing on the public CI:
> 
>   - 112 - mptcp_join: userspace pm add & remove address
>   - 113 - mptcp_join: userspace pm create destroy subflow
> 
> I didn't check in more details but I see they are failing since
> export/20230928T213439 and only when running with a debug kconfig (slow
> environment), e.g.:
> 
>   - https://cirrus-ci.com/task/5444189572300800
>   -
> https://api.cirrus-ci.com/v1/artifact/task/5444189572300800/summary/summary.txt
> 
> Do you mind having a look?

Sure, I'll look at it.

-Geliang

> 
> Maybe the connection has been closed when looking at the number of subflows?
> Also, it looks like the helper looking at 'subflows_total' with older
> versions ss is producing a lot of debug output that can be muted.
> 
> Cheers,
> Matt

Hi Geliang,

On 25/09/2023 10:41, Geliang Tang wrote:

(...)

> Geliang Tang (29):
>   mptcp: drop useless ssk in pm_subflow_check_next
>   mptcp: use mptcp_check_fallback helper
>   mptcp: use mptcp_get_ext helper
>   mptcp: move sk assignment statement ahead
>   mptcp: define more local variables sk
>   selftests: mptcp: sockopt: drop mptcp_connect var
>   selftests: mptcp: display simult in extra_msg
>   mptcp: add mptcpi_subflows_total counter
>   selftests: mptcp: add evts_get_info helper
>   selftests: mptcp: add chk_subflows_total helper
>   selftests: mptcp: update userspace pm test helpers
>   selftests: mptcp: userspace pm remove id 0 subflow
>   mptcp: userspace pm allow creating id 0 subflow
>   selftests: mptcp: userspace pm create id 0 subflow
>   mptcp: userspace pm remove id 0 address
>   selftests: mptcp: userspace pm remove id 0 address
>   mptcp: add userspace_pm_get_entry helper
>   mptcp: add userspace pm addr entry refcount
>   mptcp: add netlink pm addr entry refcount
>   selftests: mptcp: add userspace pm fullmesh tests
>   selftests: mptcp: add mptcp_lib_kill_wait
>   selftests: mptcp: add mptcp_lib_evts_*
>   selftests: mptcp: userspace: print colored results
>   selftests: mptcp: add mptcp_lib_verify_listener_events
>   selftests: mptcp: add mptcp_lib_is_v6
>   selftests: mptcp: add mptcp_lib_get_counter
>   selftests: mptcp: add mptcp_lib_make_file
>   selftests: mptcp: add mptcp_lib_check_transfer
>   selftests: mptcp: add mptcp_lib_wait_local_port_listen


These patches are still listed in patchwork:

13397525: [mptcp-next,v3,17/29] mptcp: add userspace_pm_get_entry helper
13397526: [mptcp-next,v3,18/29] mptcp: add userspace pm addr entry refcount
13397527: [mptcp-next,v3,19/29] mptcp: add netlink pm addr entry refcount
13397528: [mptcp-next,v3,20/29] selftests: mptcp: add userspace pm
fullmesh tests
13397530: [mptcp-next,v3,22/29] selftests: mptcp: add mptcp_lib_evts_*
13397531: [mptcp-next,v3,23/29] selftests: mptcp: userspace: print

@Geliang: just to know what's the status, do you plan to still work on them?

No hurry at all (especially now that net-next is closed) but it is just
to know what's the current status ;-)

Cheers,
Matt