In some cases, the subflow-level's copied_seq counter was incorrectly increased, leading to an unexpected subflow reset.
Patch 1/2 fixes the RCVPRUNED MIB counter that was attached to the wrong event since its introduction in v5.14, backported to v5.11.
Patch 2/2 fixes the copied_seq counter issues, is present since v5.10.
Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- Paolo Abeni (2): mptcp: fix bad RCVPRUNED mib accounting mptcp: fix duplicate data handling
net/mptcp/protocol.c | 8 ++++---- net/mptcp/subflow.c | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-) --- base-commit: 0bf50cead4c4710d9f704778c32ab8af47ddf070 change-id: 20240731-upstream-net-20240731-mptcp-dup-data-f922353130af
Best regards,
From: Paolo Abeni pabeni@redhat.com
Since its introduction, the mentioned MIB accounted for the wrong event: wake-up being skipped as not-needed on some edge condition instead of incoming skb being dropped after landing in the (subflow) receive queue.
Move the increment in the correct location.
Fixes: ce599c516386 ("mptcp: properly account bulk freed memory") Cc: stable@vger.kernel.org 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 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index a2fc54ed68c0..0d536b183a6c 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -350,8 +350,10 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk, skb_orphan(skb);
/* try to fetch required memory from subflow */ - if (!mptcp_rmem_schedule(sk, ssk, skb->truesize)) + if (!mptcp_rmem_schedule(sk, ssk, skb->truesize)) { + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED); goto drop; + }
has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
@@ -844,10 +846,8 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk) sk_rbuf = ssk_rbuf;
/* over limit? can't append more skbs to msk, Also, no need to wake-up*/ - if (__mptcp_rmem(sk) > sk_rbuf) { - MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED); + if (__mptcp_rmem(sk) > sk_rbuf) return; - }
/* Wake-up the reader only for in-sequence data */ mptcp_data_lock(sk);
From: Paolo Abeni pabeni@redhat.com
When a subflow receives and discards duplicate data, the mptcp stack assumes that the consumed offset inside the current skb is zero.
With multiple subflows receiving data simultaneously such assertion does not held true. As a result the subflow-level copied_seq will be incorrectly increased and later on the same subflow will observe a bad mapping, leading to subflow reset.
Address the issue taking into account the skb consumed offset in mptcp_subflow_discard_data().
Fixes: 04e4cd4f7ca4 ("mptcp: cleanup mptcp_subflow_discard_data()") Cc: stable@vger.kernel.org Link: https://github.com/multipath-tcp/mptcp_net-next/issues/501 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/subflow.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 0e4b5bfbeaa1..a21c712350c3 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1230,14 +1230,22 @@ static void mptcp_subflow_discard_data(struct sock *ssk, struct sk_buff *skb, { struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk); bool fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN; - u32 incr; + struct tcp_sock *tp = tcp_sk(ssk); + u32 offset, incr, avail_len;
- incr = limit >= skb->len ? skb->len + fin : limit; + offset = tp->copied_seq - TCP_SKB_CB(skb)->seq; + if (WARN_ON_ONCE(offset > skb->len)) + goto out;
- pr_debug("discarding=%d len=%d seq=%d", incr, skb->len, - subflow->map_subflow_seq); + avail_len = skb->len - offset; + incr = limit >= avail_len ? avail_len + fin : limit; + + pr_debug("discarding=%d len=%d offset=%d seq=%d", incr, skb->len, + offset, subflow->map_subflow_seq); MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DUPDATA); tcp_sk(ssk)->copied_seq += incr; + +out: if (!before(tcp_sk(ssk)->copied_seq, TCP_SKB_CB(skb)->end_seq)) sk_eat_skb(ssk, skb); if (mptcp_subflow_get_map_offset(subflow) >= subflow->map_data_len)
Hello:
This series was applied to netdev/net.git (main) by Paolo Abeni pabeni@redhat.com:
On Wed, 31 Jul 2024 12:10:13 +0200 you wrote:
In some cases, the subflow-level's copied_seq counter was incorrectly increased, leading to an unexpected subflow reset.
Patch 1/2 fixes the RCVPRUNED MIB counter that was attached to the wrong event since its introduction in v5.14, backported to v5.11.
Patch 2/2 fixes the copied_seq counter issues, is present since v5.10.
[...]
Here is the summary with links: - [net,1/2] mptcp: fix bad RCVPRUNED mib accounting https://git.kernel.org/netdev/net/c/0a567c2a1003 - [net,2/2] mptcp: fix duplicate data handling https://git.kernel.org/netdev/net/c/68cc924729ff
You are awesome, thank you!
linux-stable-mirror@lists.linaro.org
-
Matthieu Baerts (NGI0) -
patchwork-bot+netdevbpf＠kernel.org