From: Gang Yan <yangang@kylinos.cn>
There exists a stall caused by unprocessed backlog_queue in
'move_skbs_to_msk'.
This patch adds a check for backlog_queue and move skbs to receive
queue when no skbs were moved from subflow but backlog_queue is not
empty.
Fixes: 6228efe0cc01 ("mptcp: leverage the backlog for RX packet processing")
Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
Notes:
Hi Matt, Paolo:
Here is the response to the AI review for this patch:
- Potential dead-lock problems:
I've fixed this in the next patch. Keeping it as a separate patch
should make the review easier. Should I squash it into this one?
- The question of the short-circuiting logical OR (||):
In fact, the short-circuiting is intentional: if 'move_skbs_to_msk()'
returns true, there is already data in the receive queue, which
should not cause the stall problems. In that case moving more data
from the backlog isn't necessary. Using '||' avoids redundant work
and matches the intended logic.
Thanks
Gang
---
net/mptcp/protocol.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 8f1afd90290e..326b5d4d79e7 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -49,6 +49,11 @@ static struct percpu_counter mptcp_sockets_allocated ____cacheline_aligned_in_sm
static void __mptcp_destroy_sock(struct sock *sk);
static void mptcp_check_send_data_fin(struct sock *sk);
+static bool mptcp_can_spool_backlog(struct sock *sk, struct list_head *skbs);
+static void mptcp_backlog_spooled(struct sock *sk, u32 moved,
+ struct list_head *skbs);
+static bool __mptcp_move_skbs(struct sock *sk, struct list_head *skbs,
+ u32 *delta);
DEFINE_PER_CPU(struct mptcp_delegated_action, mptcp_delegated_actions) = {
.bh_lock = INIT_LOCAL_LOCK(bh_lock),
@@ -906,6 +911,19 @@ static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
return moved;
}
+static bool move_skbs_from_backlog(struct sock *sk)
+{
+ struct list_head skbs;
+ bool enqueued = false;
+ u32 moved;
+
+ while (mptcp_can_spool_backlog(sk, &skbs)) {
+ enqueued |= __mptcp_move_skbs(sk, &skbs, &moved);
+ mptcp_backlog_spooled(sk, moved, &skbs);
+ }
+ return enqueued;
+}
+
static void mptcp_rcv_rtt_update(struct mptcp_sock *msk,
struct mptcp_subflow_context *subflow)
{
@@ -948,7 +966,9 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
mptcp_rcv_rtt_update(msk, subflow);
if (!sock_owned_by_user(sk)) {
/* Wake-up the reader only for in-sequence data */
- if (move_skbs_to_msk(msk, ssk) && mptcp_epollin_ready(sk))
+ if ((move_skbs_to_msk(msk, ssk) ||
+ move_skbs_from_backlog(sk)) &&
+ mptcp_epollin_ready(sk))
sk->sk_data_ready(sk);
} else {
__mptcp_move_skbs_from_subflow(msk, ssk, false);
--
2.43.0