v5:
 - re-check for TCP_CLOSE.
 - add a new helper mptcp_check_mp_fail_response().
 - add two timers cleanup patches.

v4:
 - start and stop sk_timer instead of setting and clearing msk->flags.
 - add a new patch.

v3:
 - use msk->sk_timer and a msk->flags bit.
 - use READ_ONCE/WRITE_ONCE for subflow->mp_fail_response_expect.
 - update selftest.

v2:
 - don't clear mp_fail_response_expect flag.
 - add a helper mp_fail_response_expect_subflow to get the subflow,
   instead of using msk->first.
 - add locks as Mat suggested.
 - add a selftest.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/261

Geliang Tang (6):
  mptcp: use mptcp_stop_timer
  mptcp: add data lock for sk timers
  mptcp: add MP_FAIL response support
  mptcp: reset subflow when MP_FAIL doesn't respond
  selftests: mptcp: check MP_FAIL response mibs
  selftests: mptcp: print extra msg in chk_csum_nr

 net/mptcp/pm.c                                | 18 +++++-
 net/mptcp/protocol.c                          | 64 ++++++++++++++++++-
 net/mptcp/protocol.h                          |  2 +
 net/mptcp/subflow.c                           | 13 ++++
 .../testing/selftests/net/mptcp/mptcp_join.sh | 49 ++++++++++++--
 5 files changed, 139 insertions(+), 7 deletions(-)

-- 
2.34.1

On Tue, 15 Mar 2022, Geliang Tang wrote:

> v5:
> - re-check for TCP_CLOSE.
> - add a new helper mptcp_check_mp_fail_response().
> - add two timers cleanup patches.
>

Hi Geliang -

v5 is looking good to me so far, but I still need to run some tests.

Thanks,

Mat


> v4:
> - start and stop sk_timer instead of setting and clearing msk->flags.
> - add a new patch.
>
> v3:
> - use msk->sk_timer and a msk->flags bit.
> - use READ_ONCE/WRITE_ONCE for subflow->mp_fail_response_expect.
> - update selftest.
>
> v2:
> - don't clear mp_fail_response_expect flag.
> - add a helper mp_fail_response_expect_subflow to get the subflow,
>   instead of using msk->first.
> - add locks as Mat suggested.
> - add a selftest.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/261
>
> Geliang Tang (6):
>  mptcp: use mptcp_stop_timer
>  mptcp: add data lock for sk timers
>  mptcp: add MP_FAIL response support
>  mptcp: reset subflow when MP_FAIL doesn't respond
>  selftests: mptcp: check MP_FAIL response mibs
>  selftests: mptcp: print extra msg in chk_csum_nr
>
> net/mptcp/pm.c                                | 18 +++++-
> net/mptcp/protocol.c                          | 64 ++++++++++++++++++-
> net/mptcp/protocol.h                          |  2 +
> net/mptcp/subflow.c                           | 13 ++++
> .../testing/selftests/net/mptcp/mptcp_join.sh | 49 ++++++++++++--
> 5 files changed, 139 insertions(+), 7 deletions(-)
>
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel

On Wed, 16 Mar 2022, Mat Martineau wrote:

> On Tue, 15 Mar 2022, Geliang Tang wrote:
>
>> v5:
>> - re-check for TCP_CLOSE.
>> - add a new helper mptcp_check_mp_fail_response().
>> - add two timers cleanup patches.
>> 
>
> Hi Geliang -
>
> v5 is looking good to me so far, but I still need to run some tests.
>

Ok, I got a chance to look at the packet traces from the self tests. I 
also tested with a shorter timeout and commented out the echo, and the 
subflow was reset as expected.

Looks good for the export branch:

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


It would be good to have a packetdrill test for this, even if it does take 
a couple of minutes to fail. We wouldn't have to run it in the normal CI.


Mat


>
>> v4:
>> - start and stop sk_timer instead of setting and clearing msk->flags.
>> - add a new patch.
>> 
>> v3:
>> - use msk->sk_timer and a msk->flags bit.
>> - use READ_ONCE/WRITE_ONCE for subflow->mp_fail_response_expect.
>> - update selftest.
>> 
>> v2:
>> - don't clear mp_fail_response_expect flag.
>> - add a helper mp_fail_response_expect_subflow to get the subflow,
>>   instead of using msk->first.
>> - add locks as Mat suggested.
>> - add a selftest.
>> 
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/261
>> 
>> Geliang Tang (6):
>>  mptcp: use mptcp_stop_timer
>>  mptcp: add data lock for sk timers
>>  mptcp: add MP_FAIL response support
>>  mptcp: reset subflow when MP_FAIL doesn't respond
>>  selftests: mptcp: check MP_FAIL response mibs
>>  selftests: mptcp: print extra msg in chk_csum_nr
>> 
>> net/mptcp/pm.c                                | 18 +++++-
>> net/mptcp/protocol.c                          | 64 ++++++++++++++++++-
>> net/mptcp/protocol.h                          |  2 +
>> net/mptcp/subflow.c                           | 13 ++++
>> .../testing/selftests/net/mptcp/mptcp_join.sh | 49 ++++++++++++--
>> 5 files changed, 139 insertions(+), 7 deletions(-)
>> 
>> -- 
>> 2.34.1
>> 
>> 
>> 
>
> --
> Mat Martineau
> Intel
>
>

--
Mat Martineau
Intel

Hi Geliang, Mat,

On 19/03/2022 00:19, Mat Martineau wrote:
> On Wed, 16 Mar 2022, Mat Martineau wrote:
> 
>> On Tue, 15 Mar 2022, Geliang Tang wrote:
>>
>>> v5:
>>> - re-check for TCP_CLOSE.
>>> - add a new helper mptcp_check_mp_fail_response().
>>> - add two timers cleanup patches.
>>>
>>
>> Hi Geliang -
>>
>> v5 is looking good to me so far, but I still need to run some tests.
>>
> 
> Ok, I got a chance to look at the packet traces from the self tests. I
> also tested with a shorter timeout and commented out the echo, and the
> subflow was reset as expected.
> 
> Looks good for the export branch:
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

With a bit of delay, sorry for that, these patches are now in our tree!

Thank you for the patches and the reviews!

New patches for t/upstream:
- cf0e4660d69e: mptcp: use mptcp_stop_timer
- 4afa5bc41699: mptcp: add data lock for sk timers
- d1919f72133d: mptcp: add MP_FAIL response support
- bb04e167b541: mptcp: reset subflow when MP_FAIL doesn't respond
- 735b7655b253: selftests: mptcp: check MP_FAIL response mibs
- 054951083391: selftests: mptcp: print extra msg in chk_csum_nr
- Results: b3fcb6331ff3..73b94fb27a8c (export)

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220323T170824
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export


> It would be good to have a packetdrill test for this, even if it does
> take a couple of minutes to fail. We wouldn't have to run it in the
> normal CI.

I didn't close #261 for this reason then.

https://github.com/multipath-tcp/mptcp_net-next/issues/261

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