[PATCH mptcp-next v10 0/6] mptcp: add statistics for mptcp socket in use

menglong8.dong@gmail.com posted 6 patches 3 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20221219102316.627491-1-imagedong@tencent.com
Maintainers: Mat Martineau <mathew.j.martineau@linux.intel.com>, Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
net/mptcp/protocol.c                          | 25 ++++++---
net/mptcp/protocol.h                          |  2 +-
net/mptcp/token.c                             | 14 +++--
net/mptcp/token_test.c                        |  3 +
tools/testing/selftests/net/mptcp/diag.sh     | 56 +++++++++++++++++--
.../selftests/net/mptcp/mptcp_connect.c       |  4 +-
6 files changed, 85 insertions(+), 19 deletions(-)
[PATCH mptcp-next v10 0/6] mptcp: add statistics for mptcp socket in use
Posted by menglong8.dong@gmail.com 3 years, 1 month ago
From: Menglong Dong <imagedong@tencent.com>

In the 1th patch, we do some code cleanup with replease 'sock->sk'
with 'sk'. In the 2th patch, we rename 'sk' to 'ssk' in
mptcp_token_new_connect(). In the 3th patch, we add statistics for mptcp
socket in use. In the 4th patch, we init the 'sk_prot' field with
tcp_prot in build_msk. In the 5th patch, we make mptcp_connect can exit
when receive 'SIGUSR1' with '-r' flag. And in the 6th patch, we add the
testing for this commit.

With the commit e8695e504942("mptcp: don't orphan ssk in mptcp_close()"),
I belive that the testing of diag.sh can pass now. In fallback and
simultaneous close case, the msk can't release normal (sometimes?) without
that commit, and makes the testing fail. Enn...let's just see that the
CI is saying~

Changes since v9:
- rename 'sk' to 'ssk' in the document of mptcp_token_new_connect()

Changes since v8:
- fix the panic caused by mptcp token KUNIT tests by init sk->sk_prot in
  build_msk()
- check the state of msk instead of ssk for listening socket

Changes since v7:
- remove the MPTCP_INUSE flag and do the statistics according to the
  creation and destruction of the token in the 2th patch.

Changes since v6:
- check all processes exit in flush_pids() in the 4th patch

Changes since v5:
- introduce MPTCP_INUSE flag to store if msk is in use, as I find
  that it's not correct to check is a msk is in use by
  !sk_unhashed(sk) in mptcp_destroy_common(), because the token
  can be release in mptcp_check_fastclose()
- add the 3th patch
- reuse __chk_nr in 4th patch

Changes since v4:
- rebase to solve merge conflict

Changes since v3:
- rename MPTCP_DESTROIED to MPTCP_DESTROYED in the 2th patch

Changes since v2:
- add testing

Changes since v1:
- split the code cleanup into the 1th patch.
- decrease the statistics for listening mptcp socket inuse with
  mptcp_listen_inuse_dec()
- add MPTCP_DESTROIED flags to store if mptcp_destroy_common() was
  called on the msk. For fallback case, we need to decrease the
  statistics only once, and mptcp_destroy_common() can be called
  more than once.

Menglong Dong (6):
  mptcp: introduce 'sk' to replace 'sock->sk' in mptcp_listen()
  mptcp: init sk->sk_prot in build_msk()
  mptcp: rename 'sk' to 'ssk' in mptcp_token_new_connect()
  mptcp: add statistics for mptcp socket in use
  selftest: mptcp: exit from copyfd_io_poll() when receive SIGUSR1
  selftest: mptcp: add test for mptcp socket in use

 net/mptcp/protocol.c                          | 25 ++++++---
 net/mptcp/protocol.h                          |  2 +-
 net/mptcp/token.c                             | 14 +++--
 net/mptcp/token_test.c                        |  3 +
 tools/testing/selftests/net/mptcp/diag.sh     | 56 +++++++++++++++++--
 .../selftests/net/mptcp/mptcp_connect.c       |  4 +-
 6 files changed, 85 insertions(+), 19 deletions(-)

-- 
2.37.2
Re: [PATCH mptcp-next v10 0/6] mptcp: add statistics for mptcp socket in use
Posted by Matthieu Baerts 3 years, 1 month ago
Hi Menglong Dong, Paolo, Mat,

On 19/12/2022 11:23, menglong8.dong@gmail.com wrote:
> From: Menglong Dong <imagedong@tencent.com>
> 
> In the 1th patch, we do some code cleanup with replease 'sock->sk'
> with 'sk'. In the 2th patch, we rename 'sk' to 'ssk' in
> mptcp_token_new_connect(). In the 3th patch, we add statistics for mptcp
> socket in use. In the 4th patch, we init the 'sk_prot' field with
> tcp_prot in build_msk. In the 5th patch, we make mptcp_connect can exit
> when receive 'SIGUSR1' with '-r' flag. And in the 6th patch, we add the
> testing for this commit.

Thank you for the patches and the reviews!

Now in our tree (feat. for net-next) with Paolo's ACK:

New patches for t/upstream:
- 3e81eef55e91: mptcp: introduce 'sk' to replace 'sock->sk' in
mptcp_listen()
- f5581c182734: mptcp: init sk->sk_prot in build_msk()
- ab505b464266: mptcp: rename 'sk' to 'ssk' in mptcp_token_new_connect()
- d0633acb8e61: mptcp: add statistics for mptcp socket in use
- b1bf2f9ebe13: selftest: mptcp: exit from copyfd_io_poll() when receive
SIGUSR1
- 1398fc057f98: selftest: mptcp: add test for mptcp socket in use
- Results: c9ea6d26632d..63450d7ce1cd (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20221220T121253

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: [PATCH mptcp-next v10 0/6] mptcp: add statistics for mptcp socket in use
Posted by Paolo Abeni 3 years, 1 month ago
On Mon, 2022-12-19 at 18:23 +0800, menglong8.dong@gmail.com wrote:
> From: Menglong Dong <imagedong@tencent.com>
> 
> In the 1th patch, we do some code cleanup with replease 'sock->sk'
> with 'sk'. In the 2th patch, we rename 'sk' to 'ssk' in
> mptcp_token_new_connect(). In the 3th patch, we add statistics for mptcp
> socket in use. In the 4th patch, we init the 'sk_prot' field with
> tcp_prot in build_msk. In the 5th patch, we make mptcp_connect can exit
> when receive 'SIGUSR1' with '-r' flag. And in the 6th patch, we add the
> testing for this commit.
> 
> With the commit e8695e504942("mptcp: don't orphan ssk in mptcp_close()"),
> I belive that the testing of diag.sh can pass now. In fallback and
> simultaneous close case, the msk can't release normal (sometimes?) without
> that commit, and makes the testing fail. Enn...let's just see that the
> CI is saying~
> 
> Changes since v9:
> - rename 'sk' to 'ssk' in the document of mptcp_token_new_connect()
> 
> Changes since v8:
> - fix the panic caused by mptcp token KUNIT tests by init sk->sk_prot in
>   build_msk()
> - check the state of msk instead of ssk for listening socket
> 
> Changes since v7:
> - remove the MPTCP_INUSE flag and do the statistics according to the
>   creation and destruction of the token in the 2th patch.
> 
> Changes since v6:
> - check all processes exit in flush_pids() in the 4th patch
> 
> Changes since v5:
> - introduce MPTCP_INUSE flag to store if msk is in use, as I find
>   that it's not correct to check is a msk is in use by
>   !sk_unhashed(sk) in mptcp_destroy_common(), because the token
>   can be release in mptcp_check_fastclose()
> - add the 3th patch
> - reuse __chk_nr in 4th patch
> 
> Changes since v4:
> - rebase to solve merge conflict
> 
> Changes since v3:
> - rename MPTCP_DESTROIED to MPTCP_DESTROYED in the 2th patch
> 
> Changes since v2:
> - add testing
> 
> Changes since v1:
> - split the code cleanup into the 1th patch.
> - decrease the statistics for listening mptcp socket inuse with
>   mptcp_listen_inuse_dec()
> - add MPTCP_DESTROIED flags to store if mptcp_destroy_common() was
>   called on the msk. For fallback case, we need to decrease the
>   statistics only once, and mptcp_destroy_common() can be called
>   more than once.
> 
> Menglong Dong (6):
>   mptcp: introduce 'sk' to replace 'sock->sk' in mptcp_listen()
>   mptcp: init sk->sk_prot in build_msk()
>   mptcp: rename 'sk' to 'ssk' in mptcp_token_new_connect()
>   mptcp: add statistics for mptcp socket in use
>   selftest: mptcp: exit from copyfd_io_poll() when receive SIGUSR1
>   selftest: mptcp: add test for mptcp socket in use
> 
>  net/mptcp/protocol.c                          | 25 ++++++---
>  net/mptcp/protocol.h                          |  2 +-
>  net/mptcp/token.c                             | 14 +++--
>  net/mptcp/token_test.c                        |  3 +
>  tools/testing/selftests/net/mptcp/diag.sh     | 56 +++++++++++++++++--
>  .../selftests/net/mptcp/mptcp_connect.c       |  4 +-
>  6 files changed, 85 insertions(+), 19 deletions(-)
> 

LGTM, thanks for all the hard work!

Acked-by: Paolo Abeni <pabeni@redhat.com>