include/net/mptcp.h | 2 + net/ipv4/tcp.c | 3 + net/mptcp/options.c | 1 - net/mptcp/protocol.c | 219 +++++++++++-------------------------------- net/mptcp/protocol.h | 12 ++- 5 files changed, 68 insertions(+), 169 deletions(-)
This is a refresh and rebase of an already shared work: https://lore.kernel.org/mptcp/cover.1621963632.git.pabeni@redhat.com/ [1] The motiviation for refreshing this is: https://lore.kernel.org/mptcp/YtVhyGSsv1CWvPz4@xsang-OptiPlex-9020/ specifically it looks like that properly attaching mem_cg to the msk socket would be (much?) easier if the RX path and the fwd memory allocation would be under msk socket lock protection. The first 2 patches are proably worthy even without the refactor, but specifically the 2nd one is required to get a good mptcp-level acking behavior when we move the rx under the socket lock. The 3rd patch does the real work, and the 4th is a follow-up cleanup. Back at [1], I measured relevant performance regressions in some specific cases. I've done the same test here and I now see little to noise changes. I guess that is mostly due to the better acking strategy already introduce with commit 949dfdcf343c ("Merge branch 'mptcp-improve-mptcp-level-window-tracking'") and refined here. v2 -> v3: - dropped obsoleted comment in patch 2/4 - fixed compile warning in patch 3/4 v1 -> v2: - fix build issue in patch 3/4 due to PEBKAC - added missing commit messages(!!!) in patch 3/4 & 4/4 Paolo Abeni (4): mptcp: move RCVPRUNE event later mptcp: more accurate receive buffer updates mptcp: move msk input path under full msk socket lock mptcp: use common helper for rmem memory accounting include/net/mptcp.h | 2 + net/ipv4/tcp.c | 3 + net/mptcp/options.c | 1 - net/mptcp/protocol.c | 219 +++++++++++-------------------------------- net/mptcp/protocol.h | 12 ++- 5 files changed, 68 insertions(+), 169 deletions(-) -- 2.35.3
Hi Paolo, Mat, On 30/07/2022 10:04, Paolo Abeni wrote: > This is a refresh and rebase of an already shared work: > > https://lore.kernel.org/mptcp/cover.1621963632.git.pabeni@redhat.com/ > [1] > > The motiviation for refreshing this is: > > https://lore.kernel.org/mptcp/YtVhyGSsv1CWvPz4@xsang-OptiPlex-9020/ > > specifically it looks like that properly attaching mem_cg to the > msk socket would be (much?) easier if the RX path and the fwd memory > allocation would be under msk socket lock protection. > > The first 2 patches are proably worthy even without the refactor, > but specifically the 2nd one is required to get a good mptcp-level > acking behavior when we move the rx under the socket lock. > > The 3rd patch does the real work, and the 4th is a follow-up cleanup. > > Back at [1], I measured relevant performance regressions in some > specific cases. I've done the same test here and I now see little to > noise changes. I guess that is mostly due to the better acking > strategy already introduce with commit 949dfdcf343c ("Merge branch > 'mptcp-improve-mptcp-level-window-tracking'") and refined here. > > Paolo Abeni (4): > mptcp: move RCVPRUNE event later > mptcp: more accurate receive buffer updates > mptcp: move msk input path under full msk socket lock > mptcp: use common helper for rmem memory accounting Thank you for the patches, the review and breaking MPTCP selftests again :-D These patches have just been applied in our tree (feat. for net-next) with Mat's RvB tags and without a typo (s/acconting/accounting) detected by codespell. New patches for t/upstream: - 6ad19008277f: mptcp: move RCVPRUNE event later - 02da15069287: mptcp: more accurate receive buffer updates - 187f1240938a: mptcp: move msk input path under full msk socket lock - e81d36defc24: mptcp: use common helper for rmem memory accounting - Results: 7f8f198f2f9b..fc2b3f4211d6 (export) Tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220803T163817 Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
On Sat, 2022-07-30 at 10:04 +0200, Paolo Abeni wrote: > This is a refresh and rebase of an already shared work: > > https://lore.kernel.org/mptcp/cover.1621963632.git.pabeni@redhat.com/ > [1] > > The motiviation for refreshing this is: > > https://lore.kernel.org/mptcp/YtVhyGSsv1CWvPz4@xsang-OptiPlex-9020/ > > specifically it looks like that properly attaching mem_cg to the > msk socket would be (much?) easier if the RX path and the fwd memory > allocation would be under msk socket lock protection. > > The first 2 patches are proably worthy even without the refactor, > but specifically the 2nd one is required to get a good mptcp-level > acking behavior when we move the rx under the socket lock. > > The 3rd patch does the real work, and the 4th is a follow-up cleanup. > > Back at [1], I measured relevant performance regressions in some > specific cases. I've done the same test here and I now see little to > noise changes. I guess that is mostly due to the better acking > strategy already introduce with commit 949dfdcf343c ("Merge branch > 'mptcp-improve-mptcp-level-window-tracking'") and refined here. > > v2 -> v3: > - dropped obsoleted comment in patch 2/4 > - fixed compile warning in patch 3/4 > > v1 -> v2: > - fix build issue in patch 3/4 due to PEBKAC > - added missing commit messages(!!!) in patch 3/4 & 4/4 > > Paolo Abeni (4): > mptcp: move RCVPRUNE event later > mptcp: more accurate receive buffer updates > mptcp: move msk input path under full msk socket lock > mptcp: use common helper for rmem memory accounting > > include/net/mptcp.h | 2 + > net/ipv4/tcp.c | 3 + > net/mptcp/options.c | 1 - > net/mptcp/protocol.c | 219 +++++++++++-------------------------------- > net/mptcp/protocol.h | 12 ++- > 5 files changed, 68 insertions(+), 169 deletions(-) I *think* there is a litte more self-tests instability with series applied, specifically on simult_flows, and mptcp_join 32 && 99. Behind the CI report I also observed a few sporadid failures there. I'm wild guessing that the mpj failures are caused by the subflow socket lock being acquired/relased more often under the msk socket lock , possibly changing the timing for some events. I still have to investigate the above. I think we could still consider merging this on the export branch, and later try to hammer the issue on the branch (to get more coverage from the bots). WDYT? Thanks! Paolo
On Mon, 1 Aug 2022, Paolo Abeni wrote: > On Sat, 2022-07-30 at 10:04 +0200, Paolo Abeni wrote: >> This is a refresh and rebase of an already shared work: >> >> https://lore.kernel.org/mptcp/cover.1621963632.git.pabeni@redhat.com/ >> [1] >> >> The motiviation for refreshing this is: >> >> https://lore.kernel.org/mptcp/YtVhyGSsv1CWvPz4@xsang-OptiPlex-9020/ >> >> specifically it looks like that properly attaching mem_cg to the >> msk socket would be (much?) easier if the RX path and the fwd memory >> allocation would be under msk socket lock protection. >> >> The first 2 patches are proably worthy even without the refactor, >> but specifically the 2nd one is required to get a good mptcp-level >> acking behavior when we move the rx under the socket lock. >> >> The 3rd patch does the real work, and the 4th is a follow-up cleanup. >> >> Back at [1], I measured relevant performance regressions in some >> specific cases. I've done the same test here and I now see little to >> noise changes. I guess that is mostly due to the better acking >> strategy already introduce with commit 949dfdcf343c ("Merge branch >> 'mptcp-improve-mptcp-level-window-tracking'") and refined here. >> >> v2 -> v3: >> - dropped obsoleted comment in patch 2/4 >> - fixed compile warning in patch 3/4 >> >> v1 -> v2: >> - fix build issue in patch 3/4 due to PEBKAC >> - added missing commit messages(!!!) in patch 3/4 & 4/4 >> >> Paolo Abeni (4): >> mptcp: move RCVPRUNE event later >> mptcp: more accurate receive buffer updates >> mptcp: move msk input path under full msk socket lock >> mptcp: use common helper for rmem memory accounting >> >> include/net/mptcp.h | 2 + >> net/ipv4/tcp.c | 3 + >> net/mptcp/options.c | 1 - >> net/mptcp/protocol.c | 219 +++++++++++-------------------------------- >> net/mptcp/protocol.h | 12 ++- >> 5 files changed, 68 insertions(+), 169 deletions(-) > > I *think* there is a litte more self-tests instability with series > applied, specifically on simult_flows, and mptcp_join 32 && 99. > > Behind the CI report I also observed a few sporadid failures there. > > I'm wild guessing that the mpj failures are caused by the subflow > socket lock being acquired/relased more often under the msk socket lock > , possibly changing the timing for some events. > > I still have to investigate the above. > > I think we could still consider merging this on the export branch, and > later try to hammer the issue on the branch (to get more coverage from > the bots). > > WDYT? > I think that's a good plan. Tests are running ok on my test VM and the code looks ok, and I agree your wild guess is plausible. It's also a good time to let it bake a bit in the export branch, with net-next being newly closed. For the series: Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> -- Mat Martineau Intel
© 2016 - 2023 Red Hat, Inc.