[PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address

Geliang Tang posted 8 patches 11 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/cover.1697175899.git.geliang.tang@suse.com
Maintainers: Matthieu Baerts <matttbe@kernel.org>, Mat Martineau <martineau@kernel.org>, "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>, Kishen Maloor <kishen.maloor@intel.com>
There is a newer version of this series
net/mptcp/pm_userspace.c                      | 45 +++++++++++-
net/mptcp/protocol.c                          | 28 ++++++--
net/mptcp/protocol.h                          |  2 +-
.../testing/selftests/net/mptcp/mptcp_join.sh | 68 ++++++++++++++++++-
4 files changed, 131 insertions(+), 12 deletions(-)
[PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address
Posted by Geliang Tang 11 months, 1 week ago
v16:
 - address Matt's comments in v15.

v15:
 - update __mptcp_has_initial_subflow
 - address Matt's comments in v14.

v14:
 - reuse MPTCP_CF_FASTCLOSE flag.
 - use tcp_shutdown instead of mptcp_subflow_shutdown.

v13:
 - invoke mptcp_subflow_shutdown() in patch 1 as Paolo suggested.

v12:
 - address Matt's comments in v11.

v11:
 - avoid sending RSTs.
 - rename 'id 0 subflow' to 'inital subflow'.

Geliang Tang (7):
  mptcp: add __mptcp_subflow_disconnect helper
  Squash to "mptcp: add mptcpi_subflows_total counter"
  Squash to "selftests: mptcp: add chk_subflows_total helper"
  selftests: mptcp: userspace pm remove initial subflow
  mptcp: userspace pm send RM_ADDR for ID 0
  mptcp: userspace pm rename remove_err to out
  selftests: mptcp: userspace pm send RM_ADDR for ID 0

Matthieu Baerts (1):
  selftests: mptcp: join: no RST when rm subflow/addr

 net/mptcp/pm_userspace.c                      | 45 +++++++++++-
 net/mptcp/protocol.c                          | 28 ++++++--
 net/mptcp/protocol.h                          |  2 +-
 .../testing/selftests/net/mptcp/mptcp_join.sh | 68 ++++++++++++++++++-
 4 files changed, 131 insertions(+), 12 deletions(-)

-- 
2.35.3
Re: [PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address
Posted by Matthieu Baerts 11 months, 1 week ago
Hi Geliang, Paolo,

On 13/10/2023 07:46, Geliang Tang wrote:
> v16:
>  - address Matt's comments in v15.

Thank you for the new version! It looks good to me!

Reviewed-by: Matthieu Baerts <matttbe@kernel.org>

I plan to apply 'mptcp: userspace pm send RM_ADDR for ID 0' to the
"-net" part of the tree. I'm not sure about "mptcp: add
__mptcp_subflow_disconnect helper".

@Paolo: are you still OK with "mptcp: add __mptcp_subflow_disconnect
helper" commit?

Do you think we should send that to "-net"? The patch looks OK to me but
on the other hand, it is not a big issue to reset the initial subflow
(but not ideal). WDYT?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: [PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address
Posted by Matthieu Baerts 11 months, 1 week ago
Hi Geliang,

On 13/10/2023 10:31, Matthieu Baerts wrote:
> Hi Geliang, Paolo,
> 
> On 13/10/2023 07:46, Geliang Tang wrote:
>> v16:
>>  - address Matt's comments in v15.
> 
> Thank you for the new version! It looks good to me!
> 
> Reviewed-by: Matthieu Baerts <matttbe@kernel.org>

Mmh, I missed the errors reported by the CI:

not ok 112 - mptcp_join: userspace pm add & remove address
not ok 113 - mptcp_join: userspace pm create destroy subflow
not ok 115 - mptcp_join: userspace pm remove initial subflow
not ok 116 - mptcp_join: userspace pm send RM_ADDR for ID 0

Do you mind looking at that, please?

I wonder if I should not revert some patches (for the moment) or disable
some checks: these errors are visible on other builds, and we are
missing real issues.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: [PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address
Posted by Geliang Tang 11 months, 1 week ago
On Fri, Oct 13, 2023 at 11:36:36AM +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 13/10/2023 10:31, Matthieu Baerts wrote:
> > Hi Geliang, Paolo,
> > 
> > On 13/10/2023 07:46, Geliang Tang wrote:
> >> v16:
> >>  - address Matt's comments in v15.
> > 
> > Thank you for the new version! It looks good to me!
> > 
> > Reviewed-by: Matthieu Baerts <matttbe@kernel.org>
> 
> Mmh, I missed the errors reported by the CI:
> 
> not ok 112 - mptcp_join: userspace pm add & remove address
> not ok 113 - mptcp_join: userspace pm create destroy subflow
> not ok 115 - mptcp_join: userspace pm remove initial subflow
> not ok 116 - mptcp_join: userspace pm send RM_ADDR for ID 0
> 
> Do you mind looking at that, please?

I can't reproduce these errors. All tests passed on my side.

I run this command on my Thinkpad laptop:

docker run \
	-e INPUT_BUILD_SKIP_PERF=1 \
	-e INPUT_BUILD_SKIP_PACKETDRILL=1 \
	-v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
	auto-debug

And this is my .virtme-exec-run:

  run_selftest_one ./mptcp_join.sh -u

Every tests passed.

Is there a difference between my test environment and CI?

Thanks,
-Geliang

> 
> I wonder if I should not revert some patches (for the moment) or disable
> some checks: these errors are visible on other builds, and we are
> missing real issues.
> 
> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
Re: [PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address
Posted by Matthieu Baerts 11 months, 1 week ago
Hi Geliang,

On 13/10/2023 13:30, Geliang Tang wrote:
> On Fri, Oct 13, 2023 at 11:36:36AM +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 13/10/2023 10:31, Matthieu Baerts wrote:
>>> Hi Geliang, Paolo,
>>>
>>> On 13/10/2023 07:46, Geliang Tang wrote:
>>>> v16:
>>>>  - address Matt's comments in v15.
>>>
>>> Thank you for the new version! It looks good to me!
>>>
>>> Reviewed-by: Matthieu Baerts <matttbe@kernel.org>
>>
>> Mmh, I missed the errors reported by the CI:
>>
>> not ok 112 - mptcp_join: userspace pm add & remove address
>> not ok 113 - mptcp_join: userspace pm create destroy subflow
>> not ok 115 - mptcp_join: userspace pm remove initial subflow
>> not ok 116 - mptcp_join: userspace pm send RM_ADDR for ID 0
>>
>> Do you mind looking at that, please?
> 
> I can't reproduce these errors. All tests passed on my side.
> 
> I run this command on my Thinkpad laptop:
> 
> docker run \
> 	-e INPUT_BUILD_SKIP_PERF=1 \
> 	-e INPUT_BUILD_SKIP_PACKETDRILL=1 \
> 	-v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
>         --pull always mptcp/mptcp-upstream-virtme-docker:latest \
> 	auto-debug
> 
> And this is my .virtme-exec-run:
> 
>   run_selftest_one ./mptcp_join.sh -u
> 
> Every tests passed.

Thank you for looking at that!

Maybe try to reproduce the issue in a loop using:

  run_loop run_selftest_one ./mptcp_join.sh -u

> Is there a difference between my test environment and CI?

The public CI is very likely a slower environment that what you have on
your side. To help to simulate such environment, when the tests start
(because not needed before), I usually run 'stress-ng' in parallel:

  nproc=$(nproc); nproc2=$((nproc * 2))
  stress-ng --cpu "${nproc2}" --iomix "${nproc2}" --vm "${nproc2}"
--vm-bytes 1G --timeout 60m

If it is not enough, I also change the priority of the VM:

  sudo renice 20 <pid of qemu>

I hope it will help!

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