| Message ID | 9c5947ad3b55695cb6000900acad5471b7314195.1733486870.git.pabeni@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 972d8a5b7f20ffd4ec8d82d327b98e8b6613943a |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | mptcp: rx path refactor | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf-normal__only_bpftest_all_ | success | Success! ✅ |
| matttbe/KVM_Validation__btf-debug__only_bpftest_all_ | success | Success! ✅ |
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| matttbe/shellcheck | success | MPTCP selftests files have not been modified |
| matttbe/build | success | Build and static analysis OK |
Hi Paolo, On 06/12/2024 13:10, Paolo Abeni wrote: > Currently the skb size after coalescing is only limited by the skb > layout (the skb must not carry frag_list). A single coalesced skb > covering several MSS can potentially fill completely the receive > buffer. In such a case, the snd win will zero until the receive buffer > will be empty again, affecting tput badly. Good idea! Thank you for having looked at this! > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > No fixes tag because the problem is not very visible in practice > currently, but will be apparent after the rx refactor. > Still I hope this could affect positively the simlut flows self-tests The modification looks safe enough to me: it should not make thing worst. If it helps, a Fixes tag can be added and next to the 'Cc: stable' tag, we can ask to backport this patch after a few weeks, no? e.g. Cc: <stable@vger.kernel.org> # after -rc6 Cheers, Matt
Hi Paolo, On 10/12/2024 13:03, Matthieu Baerts wrote: > Hi Paolo, > > On 06/12/2024 13:10, Paolo Abeni wrote: >> Currently the skb size after coalescing is only limited by the skb >> layout (the skb must not carry frag_list). A single coalesced skb >> covering several MSS can potentially fill completely the receive >> buffer. In such a case, the snd win will zero until the receive buffer >> will be empty again, affecting tput badly. > > Good idea! Thank you for having looked at this! > >> Signed-off-by: Paolo Abeni <pabeni@redhat.com> >> --- >> No fixes tag because the problem is not very visible in practice >> currently, but will be apparent after the rx refactor. >> Still I hope this could affect positively the simlut flows self-tests > > The modification looks safe enough to me: it should not make thing worst. Do you still prefer not targetting -net for this patch here? Or is it OK to do so and add a Fixes tag? Cheers, Matt
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index f768aa4473fb..fd9593f85a98 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -136,6 +136,7 @@ static bool mptcp_try_coalesce(struct sock *sk, struct sk_buff *to, int delta; if (MPTCP_SKB_CB(from)->offset || + ((to->len + from->len) > (sk->sk_rcvbuf >> 3)) || !skb_try_coalesce(to, from, &fragstolen, &delta)) return false;
Currently the skb size after coalescing is only limited by the skb layout (the skb must not carry frag_list). A single coalesced skb covering several MSS can potentially fill completely the receive buffer. In such a case, the snd win will zero until the receive buffer will be empty again, affecting tput badly. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- No fixes tag because the problem is not very visible in practice currently, but will be apparent after the rx refactor. Still I hope this could affect positively the simlut flows self-tests --- net/mptcp/protocol.c | 1 + 1 file changed, 1 insertion(+)