All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
To: mptcp@lists.linux.dev, Mat Martineau <martineau@kernel.org>,
	 Geliang Tang <geliang.tang@linux.dev>,
	 "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>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	 linux-kselftest@vger.kernel.org,
	 "Matthieu Baerts (NGI0)" <matttbe@kernel.org>,
	stable@vger.kernel.org
Subject: [PATCH net 1/9] mptcp: fix data re-injection from stale subflow
Date: Wed, 31 Jan 2024 22:49:46 +0100	[thread overview]
Message-ID: <20240131-upstream-net-20240131-mptcp-ci-issues-v1-1-4c1c11e571ff@kernel.org> (raw)
In-Reply-To: <20240131-upstream-net-20240131-mptcp-ci-issues-v1-0-4c1c11e571ff@kernel.org>

From: Paolo Abeni <pabeni@redhat.com>

When the MPTCP PM detects that a subflow is stale, all the packet
scheduler must re-inject all the mptcp-level unacked data. To avoid
acquiring unneeded locks, it first try to check if any unacked data
is present at all in the RTX queue, but such check is currently
broken, as it uses TCP-specific helper on an MPTCP socket.

Funnily enough fuzzers and static checkers are happy, as the accessed
memory still belongs to the mptcp_sock struct, and even from a
functional perspective the recovery completed successfully, as
the short-cut test always failed.

A recent unrelated TCP change - commit d5fed5addb2b ("tcp: reorganize
tcp_sock fast path variables") - exposed the issue, as the tcp field
reorganization makes the mptcp code always skip the re-inection.

Fix the issue dropping the bogus call: we are on a slow path, the early
optimization proved once again to be evil.

Fixes: 1e1d9d6f119c ("mptcp: handle pending data on closed subflow")
Cc: stable@vger.kernel.org
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/468
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 3ed4709a7509..028e8b473626 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2314,9 +2314,6 @@ bool __mptcp_retransmit_pending_data(struct sock *sk)
 	if (__mptcp_check_fallback(msk))
 		return false;
 
-	if (tcp_rtx_and_write_queues_empty(sk))
-		return false;
-
 	/* the closing socket has some data untransmitted and/or unacked:
 	 * some data in the mptcp rtx queue has not really xmitted yet.
 	 * keep it simple and re-inject the whole mptcp level rtx queue

-- 
2.43.0


  reply	other threads:[~2024-01-31 21:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-31 21:49 [PATCH net 0/9] mptcp: fixes for recent issues reported by CI's Matthieu Baerts (NGI0)
2024-01-31 21:49 ` Matthieu Baerts (NGI0) [this message]
2024-01-31 21:49 ` [PATCH net 2/9] selftests: mptcp: add missing kconfig for NF Filter Matthieu Baerts (NGI0)
2024-01-31 21:49 ` [PATCH net 3/9] selftests: mptcp: add missing kconfig for NF Filter in v6 Matthieu Baerts (NGI0)
2024-01-31 21:49 ` [PATCH net 4/9] selftests: mptcp: add missing kconfig for NF Mangle Matthieu Baerts (NGI0)
2024-01-31 21:49 ` [PATCH net 5/9] selftests: mptcp: increase timeout to 30 min Matthieu Baerts (NGI0)
2024-01-31 21:49 ` [PATCH net 6/9] selftests: mptcp: decrease BW in simult flows Matthieu Baerts (NGI0)
2024-01-31 21:49 ` [PATCH net 7/9] selftests: mptcp: allow changing subtests prefix Matthieu Baerts (NGI0)
2024-01-31 21:49 ` [PATCH net 8/9] selftests: mptcp: join: stop transfer when check is done (part 1) Matthieu Baerts (NGI0)
2024-01-31 21:49 ` [PATCH net 9/9] selftests: mptcp: join: stop transfer when check is done (part 2) Matthieu Baerts (NGI0)
2024-02-01 17:20 ` [PATCH net 0/9] mptcp: fixes for recent issues reported by CI's patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240131-upstream-net-20240131-mptcp-ci-issues-v1-1-4c1c11e571ff@kernel.org \
    --to=matttbe@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=geliang.tang@linux.dev \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martineau@kernel.org \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.