[MPTCP next 00/12] mptcp: receive path improvement

[MPTCP next 00/12] mptcp: receive path improvement

[Paolo Abeni]

This series includes several changes to the MPTCP RX path.

The main goals are improving the RX performances _and_ increase the
long term maintainability.

Some changes reflects recent (or not so) improvements introduced in the
TCP stack: patch 1, 2 and 3 are the MPTCP counter part of skb deferral
free and auto-tuning improvements.

Note that patch 3 could possibly fix issues/574, and overall such patch
should protect from similar issues to arise in the future.

All the others patches are aimed at introducing the socket backlog usage
to process the packets received by the subflows while the msk socket is
owned. That (almost completely) replace the processing currently
happening in the mptcp_release_cb().

The actual job is done in patch 9, while the others are cleanups needed
to make the change tidy and more follow-up cleanups.

Sharing earlier with known issues (at least on fallback socket) to raise
awareness about this upcoming work.

Paolo Abeni (12):
  mptcp: leverage skb deferral free
  tcp: make tcp_rcvbuf_grow() accessible to mptcp code
  mptcp: rcvbuf auto-tuning improvement
  mptcp: introduce the mptcp_init_skb helper.
  mptcp: remove unneeded mptcp_move_skb()
  mptcp: factor out a basic skb coalesce helper
  mptcp: minor move_skbs_to_msk() cleanup
  mptcp: cleanup fallback data fin reception
  mptcp: leverage the sk backlog for RX packet processing.
  mptcp: prevernt __mptcp_move_skbs() interferring with the fastpath
  mptcp: borrow forward memory from subflow
  mptcp: make fallback backlog aware

 include/net/tcp.h    |   1 +
 net/ipv4/tcp_input.c |   2 +-
 net/mptcp/ctrl.c     |   2 +
 net/mptcp/mib.c      |   2 +
 net/mptcp/mib.h      |   4 +
 net/mptcp/protocol.c | 338 ++++++++++++++++++++++++-------------------
 net/mptcp/protocol.h |   8 +-
 net/mptcp/subflow.c  |  24 +--
 8 files changed, 220 insertions(+), 161 deletions(-)

-- 
2.51.0

[MPTCP next 01/12] mptcp: leverage skb deferral free

[Paolo Abeni]

Usage of the skb deferral API is straight-forward; with multiple
subflows actives this allow moving part of the received application
load into multiple CPUs.

Also fix a typo in the related comment.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f2e7282394804..c51aede20779d 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1913,12 +1913,13 @@ static int __mptcp_recvmsg_mskq(struct sock *sk,
 		}
 
 		if (!(flags & MSG_PEEK)) {
-			/* avoid the indirect call, we know the destructor is sock_wfree */
+			/* avoid the indirect call, we know the destructor is sock_rfree */
 			skb->destructor = NULL;
+			skb->sk = NULL;
 			atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
 			sk_mem_uncharge(sk, skb->truesize);
 			__skb_unlink(skb, &sk->sk_receive_queue);
-			__kfree_skb(skb);
+			skb_attempt_defer_free(skb);
 			msk->bytes_consumed += count;
 		}
 
-- 
2.51.0

[MPTCP next 02/12] tcp: make tcp_rcvbuf_grow() accessible to mptcp code

[Paolo Abeni]

To leverage the auto-tuning improvements brought by commit 2da35e4b4df9
("Merge branch 'tcp-receive-side-improvements'"), the MPTCP stack need
to access the mentioned helper.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/net/tcp.h    | 1 +
 net/ipv4/tcp_input.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 2936b8175950f..8298029f2d0f7 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -346,6 +346,7 @@ void tcp_delack_timer_handler(struct sock *sk);
 int tcp_ioctl(struct sock *sk, int cmd, int *karg);
 enum skb_drop_reason tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb);
 void tcp_rcv_established(struct sock *sk, struct sk_buff *skb);
+void tcp_rcvbuf_grow(struct sock *sk);
 void tcp_rcv_space_adjust(struct sock *sk);
 int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp);
 void tcp_twsk_destructor(struct sock *sk);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a52a747d8a55e..f9f5705390430 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -744,7 +744,7 @@ static inline void tcp_rcv_rtt_measure_ts(struct sock *sk,
 	}
 }
 
-static void tcp_rcvbuf_grow(struct sock *sk)
+void tcp_rcvbuf_grow(struct sock *sk)
 {
 	const struct net *net = sock_net(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
-- 
2.51.0

[MPTCP next 03/12] mptcp: rcvbuf auto-tuning improvement

[Paolo Abeni]

Apply to the MPTCP auto-tuning the same improvements introduced for the
TCP protocol by the merge commit 2da35e4b4df9 ("Merge branch
'tcp-receive-side-improvements'").

The main difference is that TCP subflow and the main MPTCP socket need
to account separately for OoO: MPTCP does not care for TCP-level OoO
and vice versa.

The above additionally allow dropping the msk receive buffer update at
receive time, as the latter only intended to cope with subflow receive
buffer increase due to OoO packets.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 95 +++++++++++++++++++++-----------------------
 net/mptcp/protocol.h |  4 +-
 2 files changed, 47 insertions(+), 52 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c51aede20779d..671c51cb9539c 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -178,6 +178,33 @@ static bool mptcp_ooo_try_coalesce(struct mptcp_sock *msk, struct sk_buff *to,
 	return mptcp_try_coalesce((struct sock *)msk, to, from);
 }
 
+static bool mptcp_rcvbuf_grow(struct sock *sk)
+{
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	int rcvwin, rcvbuf;
+
+	if (!READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_moderate_rcvbuf) ||
+	    (sk->sk_userlocks & SOCK_RCVBUF_LOCK))
+		return false;
+
+	rcvwin = ((u64)msk->rcvq_space.space << 1);
+
+	if (!RB_EMPTY_ROOT(&msk->out_of_order_queue))
+		rcvwin += MPTCP_SKB_CB(msk->ooo_last_skb)->end_seq - msk->ack_seq;
+
+	rcvbuf = min_t(u64, mptcp_space_from_win(sk, rcvwin),
+		       READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_rmem[2]));
+
+	if (rcvbuf > sk->sk_rcvbuf) {
+		u32 window_clamp;
+
+		window_clamp = mptcp_win_from_space(sk, rcvbuf);
+		WRITE_ONCE(sk->sk_rcvbuf, rcvbuf);
+		return true;
+	}
+	return false;
+}
+
 /* "inspired" by tcp_data_queue_ofo(), main differences:
  * - use mptcp seqs
  * - don't cope with sacks
@@ -291,6 +318,9 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
 end:
 	skb_condense(skb);
 	skb_set_owner_r(skb, sk);
+	/* do not grow rcvbuf for not-yet-accepted or orphaned sockets. */
+	if (sk->sk_socket)
+		mptcp_rcvbuf_grow(sk);
 }
 
 static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
@@ -770,18 +800,10 @@ static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
 	return moved;
 }
 
-static void __mptcp_rcvbuf_update(struct sock *sk, struct sock *ssk)
-{
-	if (unlikely(ssk->sk_rcvbuf > sk->sk_rcvbuf))
-		WRITE_ONCE(sk->sk_rcvbuf, ssk->sk_rcvbuf);
-}
-
 static void __mptcp_data_ready(struct sock *sk, struct sock *ssk)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 
-	__mptcp_rcvbuf_update(sk, ssk);
-
 	/* Wake-up the reader only for in-sequence data */
 	if (move_skbs_to_msk(msk, ssk) && mptcp_epollin_ready(sk))
 		sk->sk_data_ready(sk);
@@ -1984,48 +2006,26 @@ static void mptcp_rcv_space_adjust(struct mptcp_sock *msk, int copied)
 	if (msk->rcvq_space.copied <= msk->rcvq_space.space)
 		goto new_measure;
 
-	if (READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_moderate_rcvbuf) &&
-	    !(sk->sk_userlocks & SOCK_RCVBUF_LOCK)) {
-		u64 rcvwin, grow;
-		int rcvbuf;
-
-		rcvwin = ((u64)msk->rcvq_space.copied << 1) + 16 * advmss;
-
-		grow = rcvwin * (msk->rcvq_space.copied - msk->rcvq_space.space);
-
-		do_div(grow, msk->rcvq_space.space);
-		rcvwin += (grow << 1);
-
-		rcvbuf = min_t(u64, mptcp_space_from_win(sk, rcvwin),
-			       READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_rmem[2]));
-
-		if (rcvbuf > sk->sk_rcvbuf) {
-			u32 window_clamp;
-
-			window_clamp = mptcp_win_from_space(sk, rcvbuf);
-			WRITE_ONCE(sk->sk_rcvbuf, rcvbuf);
+	msk->rcvq_space.space = msk->rcvq_space.copied;
+	if (mptcp_rcvbuf_grow(sk)) {
 
-			/* Make subflows follow along.  If we do not do this, we
-			 * get drops at subflow level if skbs can't be moved to
-			 * the mptcp rx queue fast enough (announced rcv_win can
-			 * exceed ssk->sk_rcvbuf).
-			 */
-			mptcp_for_each_subflow(msk, subflow) {
-				struct sock *ssk;
-				bool slow;
+		/* Make subflows follow along.  If we do not do this, we
+		 * get drops at subflow level if skbs can't be moved to
+		 * the mptcp rx queue fast enough (announced rcv_win can
+		 * exceed ssk->sk_rcvbuf).
+		 */
+		mptcp_for_each_subflow(msk, subflow) {
+			struct sock *ssk;
+			bool slow;
 
-				ssk = mptcp_subflow_tcp_sock(subflow);
-				slow = lock_sock_fast(ssk);
-				WRITE_ONCE(ssk->sk_rcvbuf, rcvbuf);
-				WRITE_ONCE(tcp_sk(ssk)->window_clamp, window_clamp);
-				if (tcp_can_send_ack(ssk))
-					tcp_cleanup_rbuf(ssk, 1);
-				unlock_sock_fast(ssk, slow);
-			}
+			ssk = mptcp_subflow_tcp_sock(subflow);
+			slow = lock_sock_fast(ssk);
+			tcp_sk(ssk)->rcvq_space.space = msk->rcvq_space.copied;
+			tcp_rcvbuf_grow(ssk);
+			unlock_sock_fast(ssk, slow);
 		}
 	}
 
-	msk->rcvq_space.space = msk->rcvq_space.copied;
 new_measure:
 	msk->rcvq_space.copied = 0;
 	msk->rcvq_space.time = mstamp;
@@ -2054,11 +2054,6 @@ static bool __mptcp_move_skbs(struct sock *sk)
 	if (list_empty(&msk->conn_list))
 		return false;
 
-	/* verify we can move any data from the subflow, eventually updating */
-	if (!(sk->sk_userlocks & SOCK_RCVBUF_LOCK))
-		mptcp_for_each_subflow(msk, subflow)
-			__mptcp_rcvbuf_update(sk, subflow->tcp_sock);
-
 	subflow = list_first_entry(&msk->conn_list,
 				   struct mptcp_subflow_context, node);
 	for (;;) {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index a1787a1344ac1..128baea5b496e 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -341,8 +341,8 @@ struct mptcp_sock {
 	struct mptcp_pm_data	pm;
 	struct mptcp_sched_ops	*sched;
 	struct {
-		u32	space;	/* bytes copied in last measurement window */
-		u32	copied; /* bytes copied in this measurement window */
+		int	space;	/* bytes copied in last measurement window */
+		int	copied; /* bytes copied in this measurement window */
 		u64	time;	/* start time of measurement window */
 		u64	rtt_us; /* last maximum rtt of subflows */
 	} rcvq_space;
-- 
2.51.0

[MPTCP next 04/12] mptcp: introduce the mptcp_init_skb helper.

[Paolo Abeni]

Factor out all the skb initialization step in a new helper and
use it. Note that this change moves the MPTCP CB initialization
earlier: we can do such step as soon as the skb leaves the
subflow socket receive queues.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 46 ++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 671c51cb9539c..879157a1f4fb1 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -323,27 +323,11 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
 		mptcp_rcvbuf_grow(sk);
 }
 
-static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
-			     struct sk_buff *skb, unsigned int offset,
-			     size_t copy_len)
+static void mptcp_init_skb(struct sock *ssk,
+			   struct mptcp_subflow_context *subflow,
+			   struct sk_buff *skb, int offset, int copy_len)
 {
-	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
-	struct sock *sk = (struct sock *)msk;
-	struct sk_buff *tail;
-	bool has_rxtstamp;
-
-	__skb_unlink(skb, &ssk->sk_receive_queue);
-
-	skb_ext_reset(skb);
-	skb_orphan(skb);
-
-	/* try to fetch required memory from subflow */
-	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
-		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
-		goto drop;
-	}
-
-	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
+	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
 
 	/* the skb map_seq accounts for the skb offset:
 	 * mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
@@ -355,6 +339,24 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
 	MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
 	MPTCP_SKB_CB(skb)->cant_coalesce = 0;
 
+	__skb_unlink(skb, &ssk->sk_receive_queue);
+
+	skb_ext_reset(skb);
+	skb_dst_drop(skb);
+}
+
+static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sk_buff *skb)
+{
+	u64 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
+	struct sock *sk = (struct sock *)msk;
+	struct sk_buff *tail;
+
+	/* try to fetch required memory from subflow */
+	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
+		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
+		goto drop;
+	}
+
 	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
 		/* in sequence */
 		msk->bytes_received += copy_len;
@@ -662,7 +664,9 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
 		if (offset < skb->len) {
 			size_t len = skb->len - offset;
 
-			ret = __mptcp_move_skb(msk, ssk, skb, offset, len) || ret;
+			mptcp_init_skb(ssk, subflow, skb, offset, len);
+			skb_orphan(skb);
+			ret = __mptcp_move_skb(msk, skb) || ret;
 			seq += len;
 
 			if (unlikely(map_remaining < len)) {
-- 
2.51.0

[MPTCP next 05/12] mptcp: remove unneeded mptcp_move_skb()

[Paolo Abeni]

Since commit b7535cfed223 ("mptcp: drop legacy code around RX EOF"),
sk_shutdown can't change during the main recvmsg loop, we can drop
the related race breaker.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 879157a1f4fb1..58744823effc8 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2173,14 +2173,8 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 				break;
 			}
 
-			if (sk->sk_shutdown & RCV_SHUTDOWN) {
-				/* race breaker: the shutdown could be after the
-				 * previous receive queue check
-				 */
-				if (__mptcp_move_skbs(sk))
-					continue;
+			if (sk->sk_shutdown & RCV_SHUTDOWN)
 				break;
-			}
 
 			if (sk->sk_state == TCP_CLOSE) {
 				copied = -ENOTCONN;
-- 
2.51.0

[MPTCP next 06/12] mptcp: factor out a basic skb coalesce helper

[Paolo Abeni]

The upcoming patch will introduced backlog processing for MPTCP
socket, and we want to leverage coalescing in such data path.

Factor out the relevant bits not touching memory accounting to
deal with such use-case.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 58744823effc8..e904571bb94e6 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -141,22 +141,34 @@ static void mptcp_drop(struct sock *sk, struct sk_buff *skb)
 	__kfree_skb(skb);
 }
 
-static bool mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
-			       struct sk_buff *from)
+static int __mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
+				struct sk_buff *from, bool *fragstolen)
 {
-	bool fragstolen;
+	int limit = READ_ONCE(sk->sk_rcvbuf);
 	int delta;
 
 	if (unlikely(MPTCP_SKB_CB(to)->cant_coalesce) ||
 	    MPTCP_SKB_CB(from)->offset ||
-	    ((to->len + from->len) > (sk->sk_rcvbuf >> 3)) ||
-	    !skb_try_coalesce(to, from, &fragstolen, &delta))
-		return false;
+	    ((to->len + from->len) > (limit >> 3)) ||
+	    !skb_try_coalesce(to, from, fragstolen, &delta))
+		return 0;
 
 	pr_debug("colesced seq %llx into %llx new len %d new end seq %llx\n",
 		 MPTCP_SKB_CB(from)->map_seq, MPTCP_SKB_CB(to)->map_seq,
 		 to->len, MPTCP_SKB_CB(from)->end_seq);
 	MPTCP_SKB_CB(to)->end_seq = MPTCP_SKB_CB(from)->end_seq;
+	return delta;
+}
+
+static bool mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
+			       struct sk_buff *from)
+{
+	bool fragstolen;
+	int delta;
+
+	delta = __mptcp_try_coalesce(sk, to, from, &fragstolen);
+	if (!delta)
+		return false;
 
 	/* note the fwd memory can reach a negative value after accounting
 	 * for the delta, but the later skb free will restore a non
-- 
2.51.0

[MPTCP next 07/12] mptcp: minor move_skbs_to_msk() cleanup

[Paolo Abeni]

Such function is called only by __mptcp_data_ready(), which in turn
is always invoked when msk is not owned by the user: we can drop the
redundant, related check.

Additionally mptcp needs to propagate the socket error only for
current subflow.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index e904571bb94e6..251760183118a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -799,12 +799,8 @@ static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
 
 	moved = __mptcp_move_skbs_from_subflow(msk, ssk);
 	__mptcp_ofo_queue(msk);
-	if (unlikely(ssk->sk_err)) {
-		if (!sock_owned_by_user(sk))
-			__mptcp_error_report(sk);
-		else
-			__set_bit(MPTCP_ERROR_REPORT,  &msk->cb_flags);
-	}
+	if (unlikely(ssk->sk_err))
+		__mptcp_subflow_error_report(sk, ssk);
 
 	/* If the moves have caught up with the DATA_FIN sequence number
 	 * it's time to ack the DATA_FIN and change socket state, but
-- 
2.51.0

[MPTCP next 08/12] mptcp: cleanup fallback data fin reception

[Paolo Abeni]

MPTCP currently generate a dummy data_fin for fallback socket
when the fallback subflow has completed data reception using
the current ack_seq.

We are going to introduce backlog usage for the msk soon, even
for fallback sockets: the above condition will be not be correct
as it will ignore data_seq sitting in the backlog.

Instead generate the dummy data_fin when the last data packet is
extracted by the fallback subflow.

The scenario with fallback socket receiving a reset while the
receive queue empty is catched via the generic 'all subflows closed'
timeout: ensure such timeout is zero for fallback sockets.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/ctrl.c    |  2 ++
 net/mptcp/subflow.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index fed40dae5583a..4f7795968abe2 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -74,6 +74,8 @@ unsigned int mptcp_stale_loss_cnt(const struct net *net)
 
 unsigned int mptcp_close_timeout(const struct sock *sk)
 {
+	if (__mptcp_check_fallback(mptcp_sk(sk)))
+		return 0;
 	if (sock_flag(sk, SOCK_DEAD))
 		return TCP_TIMEWAIT_LEN;
 	return mptcp_get_pernet(sock_net(sk))->close_timeout;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index c8a7e4b59db11..5339a00528a7a 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1293,14 +1293,6 @@ static void subflow_sched_work_if_closed(struct mptcp_sock *msk, struct sock *ss
 
 	if (!test_and_set_bit(MPTCP_WORK_CLOSE_SUBFLOW, &msk->flags))
 		mptcp_schedule_work(sk);
-
-	/* when the fallback subflow closes the rx side, trigger a 'dummy'
-	 * ingress data fin, so that the msk state will follow along
-	 */
-	if (__mptcp_check_fallback(msk) && subflow_is_done(ssk) &&
-	    msk->first == ssk &&
-	    mptcp_update_rcv_data_fin(msk, READ_ONCE(msk->ack_seq), true))
-		mptcp_schedule_work(sk);
 }
 
 static bool mptcp_subflow_fail(struct mptcp_sock *msk, struct sock *ssk)
@@ -1433,6 +1425,14 @@ static bool subflow_check_data_avail(struct sock *ssk)
 	subflow->map_data_len = skb->len;
 	subflow->map_subflow_seq = tcp_sk(ssk)->copied_seq - subflow->ssn_offset;
 	WRITE_ONCE(subflow->data_avail, true);
+
+	/* last skb in closed fallback subflow: we are at data fin */
+	if (subflow_is_done(ssk) && ssk == msk->first &&
+	    skb == skb_peek_tail(&ssk->sk_receive_queue)) {
+		mptcp_update_rcv_data_fin(msk, subflow->map_seq +
+					  subflow->map_data_len, true);
+		subflow->map_data_fin = 1;
+	}
 	return true;
 }
 
-- 
2.51.0

[MPTCP next 09/12] mptcp: leverage the sk backlog for RX packet processing.

[Paolo Abeni]

This streamline the RX path implementation and improves the RX
performances by reducing the subflow-level locking and the amount of
work done under the msk socket lock; the implementation mirror closely
the TCP backlog processing.

Note that MPTCP needs now to traverse the existing subflow looking for
data that was left there due to the msk receive buffer full, only after
that recvmsg completely empties the receive queue.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 107 ++++++++++++++++++++++++++++++-------------
 net/mptcp/protocol.h |   2 +-
 2 files changed, 75 insertions(+), 34 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 251760183118a..9c3baed948d1d 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -357,10 +357,31 @@ static void mptcp_init_skb(struct sock *ssk,
 	skb_dst_drop(skb);
 }
 
-static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sk_buff *skb)
+static void __mptcp_add_backlog(struct sock *sk, struct sock *ssk,
+				struct sk_buff *skb)
+{
+	struct sk_buff *tail = sk->sk_backlog.tail;
+	bool fragstolen;
+	int delta;
+
+	if (tail && MPTCP_SKB_CB(skb)->map_seq == MPTCP_SKB_CB(tail)->end_seq) {
+		delta = __mptcp_try_coalesce(sk, tail, skb, &fragstolen);
+		if (delta) {
+			sk->sk_backlog.len += delta;
+			kfree_skb_partial(skb, fragstolen);
+			return;
+		}
+	}
+
+	/* mptcp checks the limit before adding the skb to the backlog */
+	__sk_add_backlog(sk, skb);
+	sk->sk_backlog.len += skb->truesize;
+}
+
+static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
 {
 	u64 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
-	struct sock *sk = (struct sock *)msk;
+	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct sk_buff *tail;
 
 	/* try to fetch required memory from subflow */
@@ -632,7 +653,7 @@ static void mptcp_dss_corruption(struct mptcp_sock *msk, struct sock *ssk)
 }
 
 static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
-					   struct sock *ssk)
+					   struct sock *ssk, bool own_msk)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
 	struct sock *sk = (struct sock *)msk;
@@ -643,12 +664,13 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
 	pr_debug("msk=%p ssk=%p\n", msk, ssk);
 	tp = tcp_sk(ssk);
 	do {
+		int mem = own_msk ? sk_rmem_alloc_get(sk) : sk->sk_backlog.len;
 		u32 map_remaining, offset;
 		u32 seq = tp->copied_seq;
 		struct sk_buff *skb;
 		bool fin;
 
-		if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
+		if (mem > READ_ONCE(sk->sk_rcvbuf))
 			break;
 
 		/* try to move as much data as available */
@@ -678,7 +700,11 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
 
 			mptcp_init_skb(ssk, subflow, skb, offset, len);
 			skb_orphan(skb);
-			ret = __mptcp_move_skb(msk, skb) || ret;
+
+			if (own_msk)
+				ret |= __mptcp_move_skb(sk, skb);
+			else
+				__mptcp_add_backlog(sk, ssk, skb);
 			seq += len;
 
 			if (unlikely(map_remaining < len)) {
@@ -699,7 +725,7 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
 
 	} while (more_data_avail);
 
-	if (ret)
+	if (ret && own_msk)
 		msk->last_data_recv = tcp_jiffies32;
 	return ret;
 }
@@ -797,7 +823,7 @@ static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
 	struct sock *sk = (struct sock *)msk;
 	bool moved;
 
-	moved = __mptcp_move_skbs_from_subflow(msk, ssk);
+	moved = __mptcp_move_skbs_from_subflow(msk, ssk, true);
 	__mptcp_ofo_queue(msk);
 	if (unlikely(ssk->sk_err))
 		__mptcp_subflow_error_report(sk, ssk);
@@ -812,18 +838,10 @@ static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
 	return moved;
 }
 
-static void __mptcp_data_ready(struct sock *sk, struct sock *ssk)
-{
-	struct mptcp_sock *msk = mptcp_sk(sk);
-
-	/* Wake-up the reader only for in-sequence data */
-	if (move_skbs_to_msk(msk, ssk) && mptcp_epollin_ready(sk))
-		sk->sk_data_ready(sk);
-}
-
 void mptcp_data_ready(struct sock *sk, struct sock *ssk)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
+	struct mptcp_sock *msk = mptcp_sk(sk);
 
 	/* The peer can send data while we are shutting down this
 	 * subflow at msk destruction time, but we must avoid enqueuing
@@ -833,13 +851,33 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
 		return;
 
 	mptcp_data_lock(sk);
-	if (!sock_owned_by_user(sk))
-		__mptcp_data_ready(sk, ssk);
-	else
-		__set_bit(MPTCP_DEQUEUE, &mptcp_sk(sk)->cb_flags);
+	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))
+			sk->sk_data_ready(sk);
+	} else {
+		__mptcp_move_skbs_from_subflow(msk, ssk, false);
+		if (unlikely(ssk->sk_err))
+			__set_bit(MPTCP_ERROR_REPORT,  &msk->cb_flags);
+	}
 	mptcp_data_unlock(sk);
 }
 
+static int mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
+{
+	struct mptcp_sock *msk = mptcp_sk(sk);
+
+	if (__mptcp_move_skb(sk, skb)) {
+		msk->last_data_recv = tcp_jiffies32;
+		__mptcp_ofo_queue(msk);
+		/* notify ack seq update */
+		mptcp_cleanup_rbuf(msk, 0);
+		mptcp_check_data_fin(sk);
+		sk->sk_data_ready(sk);
+	}
+	return 0;
+}
+
 static void mptcp_subflow_joined(struct mptcp_sock *msk, struct sock *ssk)
 {
 	mptcp_subflow_ctx(ssk)->map_seq = READ_ONCE(msk->ack_seq);
@@ -2085,7 +2123,7 @@ static bool __mptcp_move_skbs(struct sock *sk)
 
 		ssk = mptcp_subflow_tcp_sock(subflow);
 		slowpath = lock_sock_fast(ssk);
-		ret = __mptcp_move_skbs_from_subflow(msk, ssk) || ret;
+		ret = __mptcp_move_skbs_from_subflow(msk, ssk, true) || ret;
 		if (unlikely(ssk->sk_err))
 			__mptcp_error_report(sk);
 		unlock_sock_fast(ssk, slowpath);
@@ -2159,8 +2197,12 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 
 		copied += bytes_read;
 
-		if (skb_queue_empty(&sk->sk_receive_queue) && __mptcp_move_skbs(sk))
-			continue;
+		if (skb_queue_empty(&sk->sk_receive_queue)) {
+			__sk_flush_backlog(sk);
+			if (!skb_queue_empty(&sk->sk_receive_queue) ||
+			    __mptcp_move_skbs(sk))
+				continue;
+		}
 
 		/* only the MPTCP socket status is relevant here. The exit
 		 * conditions mirror closely tcp_recvmsg()
@@ -2508,7 +2550,6 @@ static void __mptcp_close_subflow(struct sock *sk)
 
 		mptcp_close_ssk(sk, ssk, subflow);
 	}
-
 }
 
 static bool mptcp_close_tout_expired(const struct sock *sk)
@@ -3092,6 +3133,13 @@ bool __mptcp_close(struct sock *sk, long timeout)
 	pr_debug("msk=%p state=%d\n", sk, sk->sk_state);
 	mptcp_pm_connection_closed(msk);
 
+	/* process the backlog; note that it never destroies the msk */
+	local_bh_disable();
+	bh_lock_sock(sk);
+	__release_sock(sk);
+	bh_unlock_sock(sk);
+	local_bh_enable();
+
 	if (sk->sk_state == TCP_CLOSE) {
 		__mptcp_destroy_sock(sk);
 		do_cancel_work = true;
@@ -3392,8 +3440,7 @@ void __mptcp_check_push(struct sock *sk, struct sock *ssk)
 
 #define MPTCP_FLAGS_PROCESS_CTX_NEED (BIT(MPTCP_PUSH_PENDING) | \
 				      BIT(MPTCP_RETRANSMIT) | \
-				      BIT(MPTCP_FLUSH_JOIN_LIST) | \
-				      BIT(MPTCP_DEQUEUE))
+				      BIT(MPTCP_FLUSH_JOIN_LIST))
 
 /* processes deferred events and flush wmem */
 static void mptcp_release_cb(struct sock *sk)
@@ -3427,11 +3474,6 @@ static void mptcp_release_cb(struct sock *sk)
 			__mptcp_push_pending(sk, 0);
 		if (flags & BIT(MPTCP_RETRANSMIT))
 			__mptcp_retrans(sk);
-		if ((flags & BIT(MPTCP_DEQUEUE)) && __mptcp_move_skbs(sk)) {
-			/* notify ack seq update */
-			mptcp_cleanup_rbuf(msk, 0);
-			sk->sk_data_ready(sk);
-		}
 
 		cond_resched();
 		spin_lock_bh(&sk->sk_lock.slock);
@@ -3668,8 +3710,6 @@ static int mptcp_ioctl(struct sock *sk, int cmd, int *karg)
 			return -EINVAL;
 
 		lock_sock(sk);
-		if (__mptcp_move_skbs(sk))
-			mptcp_cleanup_rbuf(msk, 0);
 		*karg = mptcp_inq_hint(sk);
 		release_sock(sk);
 		break;
@@ -3781,6 +3821,7 @@ static struct proto mptcp_prot = {
 	.sendmsg	= mptcp_sendmsg,
 	.ioctl		= mptcp_ioctl,
 	.recvmsg	= mptcp_recvmsg,
+	.backlog_rcv	= mptcp_move_skb,
 	.release_cb	= mptcp_release_cb,
 	.hash		= mptcp_hash,
 	.unhash		= mptcp_unhash,
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 128baea5b496e..a6e775d6412e5 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -124,7 +124,6 @@
 #define MPTCP_FLUSH_JOIN_LIST	5
 #define MPTCP_SYNC_STATE	6
 #define MPTCP_SYNC_SNDBUF	7
-#define MPTCP_DEQUEUE		8
 
 struct mptcp_skb_cb {
 	u64 map_seq;
@@ -407,6 +406,7 @@ static inline int mptcp_space_from_win(const struct sock *sk, int win)
 static inline int __mptcp_space(const struct sock *sk)
 {
 	return mptcp_win_from_space(sk, READ_ONCE(sk->sk_rcvbuf) -
+				    READ_ONCE(sk->sk_backlog.len) -
 				    sk_rmem_alloc_get(sk));
 }
 
-- 
2.51.0

[MPTCP next 10/12] mptcp: prevernt __mptcp_move_skbs() interferring with the fastpath

[Paolo Abeni]

skbs will be left waiting in the subflow only in exceptional cases,
we want to avoid messing with the fast path by unintentionally
processing in __mptcp_move_skbs() packets landed into the subflows
after the last check.

Use a separate flag to mark delayed skbs and only process subflow
with such flag set. Also add new mibs to track the exceptional events.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/mib.c      |  2 ++
 net/mptcp/mib.h      |  4 ++++
 net/mptcp/protocol.c | 40 ++++++++++++----------------------------
 net/mptcp/protocol.h |  1 +
 4 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index cf879c188ca26..7af9d35cde884 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -85,6 +85,8 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("DssFallback", MPTCP_MIB_DSSFALLBACK),
 	SNMP_MIB_ITEM("SimultConnectFallback", MPTCP_MIB_SIMULTCONNFALLBACK),
 	SNMP_MIB_ITEM("FallbackFailed", MPTCP_MIB_FALLBACKFAILED),
+	SNMP_MIB_ITEM("RcvDelayed", MPTCP_MIB_RCVDELAYED),
+	SNMP_MIB_ITEM("DelayedProcess", MPTCP_MIB_DELAYED_PROCESS),
 	SNMP_MIB_SENTINEL
 };
 
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 309bac6fea325..f6d0eaea463e5 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -88,6 +88,10 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_DSSFALLBACK,		/* Bad or missing DSS */
 	MPTCP_MIB_SIMULTCONNFALLBACK,	/* Simultaneous connect */
 	MPTCP_MIB_FALLBACKFAILED,	/* Can't fallback due to msk status */
+	MPTCP_MIB_RCVDELAYED,		/* Data move from subflow is delayed due to msk
+					 * receive buffer full
+					 */
+	MPTCP_MIB_DELAYED_PROCESS,	/* Delayed data moved in slowpath */
 	__MPTCP_MIB_MAX
 };
 
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 9c3baed948d1d..f211a939daf39 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -665,13 +665,17 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
 	tp = tcp_sk(ssk);
 	do {
 		int mem = own_msk ? sk_rmem_alloc_get(sk) : sk->sk_backlog.len;
+		bool over_limit = mem > READ_ONCE(sk->sk_rcvbuf);
 		u32 map_remaining, offset;
 		u32 seq = tp->copied_seq;
 		struct sk_buff *skb;
 		bool fin;
 
-		if (mem > READ_ONCE(sk->sk_rcvbuf))
+		WRITE_ONCE(subflow->data_delayed, over_limit);
+		if (subflow->data_delayed) {
+			MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVDELAYED);
 			break;
+		}
 
 		/* try to move as much data as available */
 		map_remaining = subflow->map_data_len -
@@ -2081,32 +2085,13 @@ static void mptcp_rcv_space_adjust(struct mptcp_sock *msk, int copied)
 	msk->rcvq_space.time = mstamp;
 }
 
-static struct mptcp_subflow_context *
-__mptcp_first_ready_from(struct mptcp_sock *msk,
-			 struct mptcp_subflow_context *subflow)
-{
-	struct mptcp_subflow_context *start_subflow = subflow;
-
-	while (!READ_ONCE(subflow->data_avail)) {
-		subflow = mptcp_next_subflow(msk, subflow);
-		if (subflow == start_subflow)
-			return NULL;
-	}
-	return subflow;
-}
-
 static bool __mptcp_move_skbs(struct sock *sk)
 {
 	struct mptcp_subflow_context *subflow;
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	bool ret = false;
 
-	if (list_empty(&msk->conn_list))
-		return false;
-
-	subflow = list_first_entry(&msk->conn_list,
-				   struct mptcp_subflow_context, node);
-	for (;;) {
+	mptcp_for_each_subflow(msk, subflow) {
 		struct sock *ssk;
 		bool slowpath;
 
@@ -2117,23 +2102,22 @@ static bool __mptcp_move_skbs(struct sock *sk)
 		if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
 			break;
 
-		subflow = __mptcp_first_ready_from(msk, subflow);
-		if (!subflow)
-			break;
+		if (!subflow->data_delayed)
+			continue;
 
 		ssk = mptcp_subflow_tcp_sock(subflow);
 		slowpath = lock_sock_fast(ssk);
-		ret = __mptcp_move_skbs_from_subflow(msk, ssk, true) || ret;
+		ret |= __mptcp_move_skbs_from_subflow(msk, ssk, true);
 		if (unlikely(ssk->sk_err))
 			__mptcp_error_report(sk);
 		unlock_sock_fast(ssk, slowpath);
-
-		subflow = mptcp_next_subflow(msk, subflow);
 	}
 
 	__mptcp_ofo_queue(msk);
-	if (ret)
+	if (ret) {
+		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DELAYED_PROCESS);
 		mptcp_check_data_fin((struct sock *)msk);
+	}
 	return ret;
 }
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index a6e775d6412e5..2905e4b250070 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -559,6 +559,7 @@ struct mptcp_subflow_context {
 	u8	reset_transient:1;
 	u8	reset_reason:4;
 	u8	stale_count;
+	bool	data_delayed;
 
 	u32	subflow_id;
 
-- 
2.51.0

[MPTCP next 11/12] mptcp: borrow forward memory from subflow

[Paolo Abeni]

In the MPTCP receive path, we release the subflow allocated
fwd memory just to allocate it again shortly after for the msk.

That could increases the change of failures, especially during
backlog processing, when other actions could consume the just
released memory before the msk socket has a chance to do the
rcv allocation.

Explicitly borrow, with a PAGE_SIZE granularity, the fwd memory
from the subflow socket instead of releasing it. During backlog
processing the borrowed memory is accounted at release_cb time.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 29 ++++++++++++++++++++++-------
 net/mptcp/protocol.h |  1 +
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f211a939daf39..b883e8548dcb8 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -335,11 +335,12 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
 		mptcp_rcvbuf_grow(sk);
 }
 
-static void mptcp_init_skb(struct sock *ssk,
-			   struct mptcp_subflow_context *subflow,
-			   struct sk_buff *skb, int offset, int copy_len)
+static int mptcp_init_skb(struct sock *ssk,
+			  struct mptcp_subflow_context *subflow,
+			  struct sk_buff *skb, int offset, int copy_len)
 {
 	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
+	int borrowed;
 
 	/* the skb map_seq accounts for the skb offset:
 	 * mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
@@ -355,6 +356,15 @@ static void mptcp_init_skb(struct sock *ssk,
 
 	skb_ext_reset(skb);
 	skb_dst_drop(skb);
+
+	/* "borrow" the fwd memory from the subflow, instead of reclaiming it */
+	skb->destructor = NULL;
+	skb->sk = NULL;
+	atomic_sub(skb->truesize, &ssk->sk_rmem_alloc);
+	borrowed = ssk->sk_forward_alloc - sk_unused_reserved_mem(ssk);
+	borrowed &= ~(PAGE_SIZE - 1);
+	sk_forward_alloc_add(ssk, skb->truesize - borrowed);
+	return borrowed;
 }
 
 static void __mptcp_add_backlog(struct sock *sk, struct sock *ssk,
@@ -701,14 +711,17 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
 
 		if (offset < skb->len) {
 			size_t len = skb->len - offset;
+			int bmem;
 
-			mptcp_init_skb(ssk, subflow, skb, offset, len);
-			skb_orphan(skb);
+			bmem = mptcp_init_skb(ssk, subflow, skb, offset, len);
 
-			if (own_msk)
+			if (own_msk) {
+				sk_forward_alloc_add(sk, bmem);
 				ret |= __mptcp_move_skb(sk, skb);
-			else
+			} else {
+				msk->borrowed_fwd_mem += bmem;
 				__mptcp_add_backlog(sk, ssk, skb);
+			}
 			seq += len;
 
 			if (unlikely(map_remaining < len)) {
@@ -3477,6 +3490,8 @@ static void mptcp_release_cb(struct sock *sk)
 		if (__test_and_clear_bit(MPTCP_SYNC_SNDBUF, &msk->cb_flags))
 			__mptcp_sync_sndbuf(sk);
 	}
+	sk_forward_alloc_add(sk, msk->borrowed_fwd_mem);
+	msk->borrowed_fwd_mem = 0;
 }
 
 /* MP_JOIN client subflow must wait for 4th ack before sending any data:
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 2905e4b250070..93e7b1b3fe359 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -297,6 +297,7 @@ struct mptcp_sock {
 	u32		last_data_sent;
 	u32		last_data_recv;
 	u32		last_ack_recv;
+	int		borrowed_fwd_mem;
 	unsigned long	timer_ival;
 	u32		token;
 	unsigned long	flags;
-- 
2.51.0

[MPTCP next 12/12] mptcp: make fallback backlog aware

[Paolo Abeni]

MPTCP can't relay on ack_seq outside the msk socket log scope. With
packets pending in the backlog, such value can be quite far from the
actual dummy correct sequence map value.

Fallback dummy mapping are in sequence by definition, generate the
data seq from the subflow sequence.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/subflow.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 5339a00528a7a..02bf89f6b5a1a 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -491,6 +491,9 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
 	mptcp_crypto_key_sha(subflow->remote_key, NULL, &subflow->iasn);
 	subflow->iasn++;
 
+	/* for fallback's sake */
+	subflow->map_seq = subflow->iasn;
+
 	WRITE_ONCE(msk->remote_key, subflow->remote_key);
 	WRITE_ONCE(msk->ack_seq, subflow->iasn);
 	WRITE_ONCE(msk->can_ack, true);
@@ -1421,9 +1424,12 @@ static bool subflow_check_data_avail(struct sock *ssk)
 
 	skb = skb_peek(&ssk->sk_receive_queue);
 	subflow->map_valid = 1;
-	subflow->map_seq = READ_ONCE(msk->ack_seq);
 	subflow->map_data_len = skb->len;
 	subflow->map_subflow_seq = tcp_sk(ssk)->copied_seq - subflow->ssn_offset;
+	subflow->map_seq = __mptcp_expand_seq(subflow->map_seq,
+					      subflow->iasn +
+					      TCP_SKB_CB(skb)->seq -
+					      subflow->ssn_offset - 1);
 	WRITE_ONCE(subflow->data_avail, true);
 
 	/* last skb in closed fallback subflow: we are at data fin */
-- 
2.51.0

Re: [MPTCP next 10/12] mptcp: prevernt __mptcp_move_skbs() interferring with the fastpath

[Geliang Tang]

Hi Paolo,

Thanks for this patch.

On Tue, 2025-09-16 at 18:27 +0200, Paolo Abeni wrote:
> skbs will be left waiting in the subflow only in exceptional cases,
> we want to avoid messing with the fast path by unintentionally
> processing in __mptcp_move_skbs() packets landed into the subflows
> after the last check.
> 
> Use a separate flag to mark delayed skbs and only process subflow
> with such flag set. Also add new mibs to track the exceptional
> events.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/mptcp/mib.c      |  2 ++
>  net/mptcp/mib.h      |  4 ++++
>  net/mptcp/protocol.c | 40 ++++++++++++----------------------------
>  net/mptcp/protocol.h |  1 +
>  4 files changed, 19 insertions(+), 28 deletions(-)
> 
> diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
> index cf879c188ca26..7af9d35cde884 100644
> --- a/net/mptcp/mib.c
> +++ b/net/mptcp/mib.c
> @@ -85,6 +85,8 @@ static const struct snmp_mib mptcp_snmp_list[] = {
>  	SNMP_MIB_ITEM("DssFallback", MPTCP_MIB_DSSFALLBACK),
>  	SNMP_MIB_ITEM("SimultConnectFallback",
> MPTCP_MIB_SIMULTCONNFALLBACK),
>  	SNMP_MIB_ITEM("FallbackFailed", MPTCP_MIB_FALLBACKFAILED),
> +	SNMP_MIB_ITEM("RcvDelayed", MPTCP_MIB_RCVDELAYED),
> +	SNMP_MIB_ITEM("DelayedProcess", MPTCP_MIB_DELAYED_PROCESS),
>  	SNMP_MIB_SENTINEL

I guess this line was accidentally pasted, which prevents this patch
from being applied.

Thanks,
-Geliang

>  };
>  
> diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
> index 309bac6fea325..f6d0eaea463e5 100644
> --- a/net/mptcp/mib.h
> +++ b/net/mptcp/mib.h
> @@ -88,6 +88,10 @@ enum linux_mptcp_mib_field {
>  	MPTCP_MIB_DSSFALLBACK,		/* Bad or missing DSS */
>  	MPTCP_MIB_SIMULTCONNFALLBACK,	/* Simultaneous connect */
>  	MPTCP_MIB_FALLBACKFAILED,	/* Can't fallback due to msk
> status */
> +	MPTCP_MIB_RCVDELAYED,		/* Data move from subflow is
> delayed due to msk
> +					 * receive buffer full
> +					 */
> +	MPTCP_MIB_DELAYED_PROCESS,	/* Delayed data moved in
> slowpath */
>  	__MPTCP_MIB_MAX
>  };
>  
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 9c3baed948d1d..f211a939daf39 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -665,13 +665,17 @@ static bool
> __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
>  	tp = tcp_sk(ssk);
>  	do {
>  		int mem = own_msk ? sk_rmem_alloc_get(sk) : sk-
> >sk_backlog.len;
> +		bool over_limit = mem > READ_ONCE(sk->sk_rcvbuf);
>  		u32 map_remaining, offset;
>  		u32 seq = tp->copied_seq;
>  		struct sk_buff *skb;
>  		bool fin;
>  
> -		if (mem > READ_ONCE(sk->sk_rcvbuf))
> +		WRITE_ONCE(subflow->data_delayed, over_limit);
> +		if (subflow->data_delayed) {
> +			MPTCP_INC_STATS(sock_net(sk),
> MPTCP_MIB_RCVDELAYED);
>  			break;
> +		}
>  
>  		/* try to move as much data as available */
>  		map_remaining = subflow->map_data_len -
> @@ -2081,32 +2085,13 @@ static void mptcp_rcv_space_adjust(struct
> mptcp_sock *msk, int copied)
>  	msk->rcvq_space.time = mstamp;
>  }
>  
> -static struct mptcp_subflow_context *
> -__mptcp_first_ready_from(struct mptcp_sock *msk,
> -			 struct mptcp_subflow_context *subflow)
> -{
> -	struct mptcp_subflow_context *start_subflow = subflow;
> -
> -	while (!READ_ONCE(subflow->data_avail)) {
> -		subflow = mptcp_next_subflow(msk, subflow);
> -		if (subflow == start_subflow)
> -			return NULL;
> -	}
> -	return subflow;
> -}
> -
>  static bool __mptcp_move_skbs(struct sock *sk)
>  {
>  	struct mptcp_subflow_context *subflow;
>  	struct mptcp_sock *msk = mptcp_sk(sk);
>  	bool ret = false;
>  
> -	if (list_empty(&msk->conn_list))
> -		return false;
> -
> -	subflow = list_first_entry(&msk->conn_list,
> -				   struct mptcp_subflow_context,
> node);
> -	for (;;) {
> +	mptcp_for_each_subflow(msk, subflow) {
>  		struct sock *ssk;
>  		bool slowpath;
>  
> @@ -2117,23 +2102,22 @@ static bool __mptcp_move_skbs(struct sock
> *sk)
>  		if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
>  			break;
>  
> -		subflow = __mptcp_first_ready_from(msk, subflow);
> -		if (!subflow)
> -			break;
> +		if (!subflow->data_delayed)
> +			continue;
>  
>  		ssk = mptcp_subflow_tcp_sock(subflow);
>  		slowpath = lock_sock_fast(ssk);
> -		ret = __mptcp_move_skbs_from_subflow(msk, ssk, true)
> || ret;
> +		ret |= __mptcp_move_skbs_from_subflow(msk, ssk,
> true);
>  		if (unlikely(ssk->sk_err))
>  			__mptcp_error_report(sk);
>  		unlock_sock_fast(ssk, slowpath);
> -
> -		subflow = mptcp_next_subflow(msk, subflow);
>  	}
>  
>  	__mptcp_ofo_queue(msk);
> -	if (ret)
> +	if (ret) {
> +		MPTCP_INC_STATS(sock_net(sk),
> MPTCP_MIB_DELAYED_PROCESS);
>  		mptcp_check_data_fin((struct sock *)msk);
> +	}
>  	return ret;
>  }
>  
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index a6e775d6412e5..2905e4b250070 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -559,6 +559,7 @@ struct mptcp_subflow_context {
>  	u8	reset_transient:1;
>  	u8	reset_reason:4;
>  	u8	stale_count;
> +	bool	data_delayed;
>  
>  	u32	subflow_id;
>  

Re: [MPTCP next 00/12] mptcp: receive path improvement

[Geliang Tang]

On Tue, 2025-09-16 at 18:27 +0200, Paolo Abeni wrote:
> This series includes several changes to the MPTCP RX path.
> 
> The main goals are improving the RX performances _and_ increase the
> long term maintainability.
> 
> Some changes reflects recent (or not so) improvements introduced in
> the
> TCP stack: patch 1, 2 and 3 are the MPTCP counter part of skb
> deferral
> free and auto-tuning improvements.
> 
> Note that patch 3 could possibly fix issues/574, and overall such
> patch
> should protect from similar issues to arise in the future.
> 
> All the others patches are aimed at introducing the socket backlog
> usage
> to process the packets received by the subflows while the msk socket
> is
> owned. That (almost completely) replace the processing currently
> happening in the mptcp_release_cb().
> 
> The actual job is done in patch 9, while the others are cleanups
> needed
> to make the change tidy and more follow-up cleanups.
> 
> Sharing earlier with known issues (at least on fallback socket) to
> raise
> awareness about this upcoming work.

Great! Thanks Paolo. Patch 3 "mptcp: rcvbuf auto-tuning improvement"
also fixes issue #487 "send() fails with EAGAIN in blocking IO mode"
that's been bothering me for quite a while. Therefore, please also add:

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/487

Meanwhile, I think this series closes issue #559 as well:

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/559

-Geliang

> 
> Paolo Abeni (12):
>   mptcp: leverage skb deferral free
>   tcp: make tcp_rcvbuf_grow() accessible to mptcp code
>   mptcp: rcvbuf auto-tuning improvement
>   mptcp: introduce the mptcp_init_skb helper.
>   mptcp: remove unneeded mptcp_move_skb()
>   mptcp: factor out a basic skb coalesce helper
>   mptcp: minor move_skbs_to_msk() cleanup
>   mptcp: cleanup fallback data fin reception
>   mptcp: leverage the sk backlog for RX packet processing.
>   mptcp: prevernt __mptcp_move_skbs() interferring with the fastpath
>   mptcp: borrow forward memory from subflow
>   mptcp: make fallback backlog aware
> 
>  include/net/tcp.h    |   1 +
>  net/ipv4/tcp_input.c |   2 +-
>  net/mptcp/ctrl.c     |   2 +
>  net/mptcp/mib.c      |   2 +
>  net/mptcp/mib.h      |   4 +
>  net/mptcp/protocol.c | 338 ++++++++++++++++++++++++-----------------
> --
>  net/mptcp/protocol.h |   8 +-
>  net/mptcp/subflow.c  |  24 +--
>  8 files changed, 220 insertions(+), 161 deletions(-)

Re: [MPTCP next 04/12] mptcp: introduce the mptcp_init_skb helper.

[Geliang Tang]

Hi Paolo,

Thanks for this patch. I've left some minor cleanup comments below.

On Tue, 2025-09-16 at 18:27 +0200, Paolo Abeni wrote:
> Factor out all the skb initialization step in a new helper and
> use it. Note that this change moves the MPTCP CB initialization
> earlier: we can do such step as soon as the skb leaves the
> subflow socket receive queues.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/mptcp/protocol.c | 46 ++++++++++++++++++++++++------------------
> --
>  1 file changed, 25 insertions(+), 21 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 671c51cb9539c..879157a1f4fb1 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -323,27 +323,11 @@ static void mptcp_data_queue_ofo(struct
> mptcp_sock *msk, struct sk_buff *skb)
>  		mptcp_rcvbuf_grow(sk);
>  }
>  
> -static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock
> *ssk,
> -			     struct sk_buff *skb, unsigned int
> offset,
> -			     size_t copy_len)
> +static void mptcp_init_skb(struct sock *ssk,
> +			   struct mptcp_subflow_context *subflow,

subflow and ssk parameters are redundant, we can just use ssk.

> +			   struct sk_buff *skb, int offset, int
> copy_len)
>  {
> -	struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
> -	struct sock *sk = (struct sock *)msk;
> -	struct sk_buff *tail;
> -	bool has_rxtstamp;
> -
> -	__skb_unlink(skb, &ssk->sk_receive_queue);
> -
> -	skb_ext_reset(skb);
> -	skb_orphan(skb);
> -
> -	/* try to fetch required memory from subflow */
> -	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> -		goto drop;
> -	}
> -
> -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> +	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
>  
>  	/* the skb map_seq accounts for the skb offset:
>  	 * mptcp_subflow_get_mapped_dsn() is based on the current
> tp->copied_seq
> @@ -355,6 +339,24 @@ static bool __mptcp_move_skb(struct mptcp_sock
> *msk, struct sock *ssk,
>  	MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
>  	MPTCP_SKB_CB(skb)->cant_coalesce = 0;
>  
> +	__skb_unlink(skb, &ssk->sk_receive_queue);
> +
> +	skb_ext_reset(skb);

skb_orphan(skb) call was moved from here to after mptcp_init_skb(), but
was then completely removed in patch 11. I'm a bit confused by this. I
think it might be better to retain skb_orphan(skb) call here. Or
perhaps the addition of skb_dst_drop(skb) has made skb_orphan(skb)
unnecessary. I'd like to consult on this point.

> +	skb_dst_drop(skb);
> +}
> +
> +static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sk_buff
> *skb)

The parameter 'msk' was changed to 'struct sock *sk' in patch 9. We can
directly adopt 'struct sock *sk' here as well.

Thanks,
-Geliang

> +{
> +	u64 copy_len = MPTCP_SKB_CB(skb)->end_seq -
> MPTCP_SKB_CB(skb)->map_seq;
> +	struct sock *sk = (struct sock *)msk;
> +	struct sk_buff *tail;
> +
> +	/* try to fetch required memory from subflow */
> +	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> +		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> +		goto drop;
> +	}
> +
>  	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
>  		/* in sequence */
>  		msk->bytes_received += copy_len;
> @@ -662,7 +664,9 @@ static bool __mptcp_move_skbs_from_subflow(struct
> mptcp_sock *msk,
>  		if (offset < skb->len) {
>  			size_t len = skb->len - offset;
>  
> -			ret = __mptcp_move_skb(msk, ssk, skb,
> offset, len) || ret;
> +			mptcp_init_skb(ssk, subflow, skb, offset,
> len);
> +			skb_orphan(skb);
> +			ret = __mptcp_move_skb(msk, skb) || ret;
>  			seq += len;
>  
>  			if (unlikely(map_remaining < len)) {

Re: [MPTCP next 09/12] mptcp: leverage the sk backlog for RX packet processing.

[Geliang Tang]

Hi Paolo,

On Tue, 2025-09-16 at 18:27 +0200, Paolo Abeni wrote:
> This streamline the RX path implementation and improves the RX
> performances by reducing the subflow-level locking and the amount of
> work done under the msk socket lock; the implementation mirror
> closely
> the TCP backlog processing.
> 
> Note that MPTCP needs now to traverse the existing subflow looking
> for
> data that was left there due to the msk receive buffer full, only
> after
> that recvmsg completely empties the receive queue.

This patch breaks my "implement mptcp read_sock v11" test case in
patchwork, and I haven't figured out the reason yet.

Thanks,
-Geliang

> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/mptcp/protocol.c | 107 ++++++++++++++++++++++++++++++-----------
> --
>  net/mptcp/protocol.h |   2 +-
>  2 files changed, 75 insertions(+), 34 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 251760183118a..9c3baed948d1d 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -357,10 +357,31 @@ static void mptcp_init_skb(struct sock *ssk,
>  	skb_dst_drop(skb);
>  }
>  
> -static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sk_buff
> *skb)
> +static void __mptcp_add_backlog(struct sock *sk, struct sock *ssk,
> +				struct sk_buff *skb)
> +{
> +	struct sk_buff *tail = sk->sk_backlog.tail;
> +	bool fragstolen;
> +	int delta;
> +
> +	if (tail && MPTCP_SKB_CB(skb)->map_seq ==
> MPTCP_SKB_CB(tail)->end_seq) {
> +		delta = __mptcp_try_coalesce(sk, tail, skb,
> &fragstolen);
> +		if (delta) {
> +			sk->sk_backlog.len += delta;
> +			kfree_skb_partial(skb, fragstolen);
> +			return;
> +		}
> +	}
> +
> +	/* mptcp checks the limit before adding the skb to the
> backlog */
> +	__sk_add_backlog(sk, skb);
> +	sk->sk_backlog.len += skb->truesize;
> +}
> +
> +static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
>  {
>  	u64 copy_len = MPTCP_SKB_CB(skb)->end_seq -
> MPTCP_SKB_CB(skb)->map_seq;
> -	struct sock *sk = (struct sock *)msk;
> +	struct mptcp_sock *msk = mptcp_sk(sk);
>  	struct sk_buff *tail;
>  
>  	/* try to fetch required memory from subflow */
> @@ -632,7 +653,7 @@ static void mptcp_dss_corruption(struct
> mptcp_sock *msk, struct sock *ssk)
>  }
>  
>  static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
> -					   struct sock *ssk)
> +					   struct sock *ssk, bool
> own_msk)
>  {
>  	struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
>  	struct sock *sk = (struct sock *)msk;
> @@ -643,12 +664,13 @@ static bool
> __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
>  	pr_debug("msk=%p ssk=%p\n", msk, ssk);
>  	tp = tcp_sk(ssk);
>  	do {
> +		int mem = own_msk ? sk_rmem_alloc_get(sk) : sk-
> >sk_backlog.len;
>  		u32 map_remaining, offset;
>  		u32 seq = tp->copied_seq;
>  		struct sk_buff *skb;
>  		bool fin;
>  
> -		if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
> +		if (mem > READ_ONCE(sk->sk_rcvbuf))
>  			break;
>  
>  		/* try to move as much data as available */
> @@ -678,7 +700,11 @@ static bool
> __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
>  
>  			mptcp_init_skb(ssk, subflow, skb, offset,
> len);
>  			skb_orphan(skb);
> -			ret = __mptcp_move_skb(msk, skb) || ret;
> +
> +			if (own_msk)
> +				ret |= __mptcp_move_skb(sk, skb);
> +			else
> +				__mptcp_add_backlog(sk, ssk, skb);
>  			seq += len;
>  
>  			if (unlikely(map_remaining < len)) {
> @@ -699,7 +725,7 @@ static bool __mptcp_move_skbs_from_subflow(struct
> mptcp_sock *msk,
>  
>  	} while (more_data_avail);
>  
> -	if (ret)
> +	if (ret && own_msk)
>  		msk->last_data_recv = tcp_jiffies32;
>  	return ret;
>  }
> @@ -797,7 +823,7 @@ static bool move_skbs_to_msk(struct mptcp_sock
> *msk, struct sock *ssk)
>  	struct sock *sk = (struct sock *)msk;
>  	bool moved;
>  
> -	moved = __mptcp_move_skbs_from_subflow(msk, ssk);
> +	moved = __mptcp_move_skbs_from_subflow(msk, ssk, true);
>  	__mptcp_ofo_queue(msk);
>  	if (unlikely(ssk->sk_err))
>  		__mptcp_subflow_error_report(sk, ssk);
> @@ -812,18 +838,10 @@ static bool move_skbs_to_msk(struct mptcp_sock
> *msk, struct sock *ssk)
>  	return moved;
>  }
>  
> -static void __mptcp_data_ready(struct sock *sk, struct sock *ssk)
> -{
> -	struct mptcp_sock *msk = mptcp_sk(sk);
> -
> -	/* Wake-up the reader only for in-sequence data */
> -	if (move_skbs_to_msk(msk, ssk) && mptcp_epollin_ready(sk))
> -		sk->sk_data_ready(sk);
> -}
> -
>  void mptcp_data_ready(struct sock *sk, struct sock *ssk)
>  {
>  	struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
> +	struct mptcp_sock *msk = mptcp_sk(sk);
>  
>  	/* The peer can send data while we are shutting down this
>  	 * subflow at msk destruction time, but we must avoid
> enqueuing
> @@ -833,13 +851,33 @@ void mptcp_data_ready(struct sock *sk, struct
> sock *ssk)
>  		return;
>  
>  	mptcp_data_lock(sk);
> -	if (!sock_owned_by_user(sk))
> -		__mptcp_data_ready(sk, ssk);
> -	else
> -		__set_bit(MPTCP_DEQUEUE, &mptcp_sk(sk)->cb_flags);
> +	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))
> +			sk->sk_data_ready(sk);
> +	} else {
> +		__mptcp_move_skbs_from_subflow(msk, ssk, false);
> +		if (unlikely(ssk->sk_err))
> +			__set_bit(MPTCP_ERROR_REPORT,  &msk-
> >cb_flags);
> +	}
>  	mptcp_data_unlock(sk);
>  }
>  
> +static int mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
> +{
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +
> +	if (__mptcp_move_skb(sk, skb)) {
> +		msk->last_data_recv = tcp_jiffies32;
> +		__mptcp_ofo_queue(msk);
> +		/* notify ack seq update */
> +		mptcp_cleanup_rbuf(msk, 0);
> +		mptcp_check_data_fin(sk);
> +		sk->sk_data_ready(sk);
> +	}
> +	return 0;
> +}
> +
>  static void mptcp_subflow_joined(struct mptcp_sock *msk, struct sock
> *ssk)
>  {
>  	mptcp_subflow_ctx(ssk)->map_seq = READ_ONCE(msk->ack_seq);
> @@ -2085,7 +2123,7 @@ static bool __mptcp_move_skbs(struct sock *sk)
>  
>  		ssk = mptcp_subflow_tcp_sock(subflow);
>  		slowpath = lock_sock_fast(ssk);
> -		ret = __mptcp_move_skbs_from_subflow(msk, ssk) ||
> ret;
> +		ret = __mptcp_move_skbs_from_subflow(msk, ssk, true)
> || ret;
>  		if (unlikely(ssk->sk_err))
>  			__mptcp_error_report(sk);
>  		unlock_sock_fast(ssk, slowpath);
> @@ -2159,8 +2197,12 @@ static int mptcp_recvmsg(struct sock *sk,
> struct msghdr *msg, size_t len,
>  
>  		copied += bytes_read;
>  
> -		if (skb_queue_empty(&sk->sk_receive_queue) &&
> __mptcp_move_skbs(sk))
> -			continue;
> +		if (skb_queue_empty(&sk->sk_receive_queue)) {
> +			__sk_flush_backlog(sk);
> +			if (!skb_queue_empty(&sk->sk_receive_queue)
> ||
> +			    __mptcp_move_skbs(sk))
> +				continue;
> +		}
>  
>  		/* only the MPTCP socket status is relevant here.
> The exit
>  		 * conditions mirror closely tcp_recvmsg()
> @@ -2508,7 +2550,6 @@ static void __mptcp_close_subflow(struct sock
> *sk)
>  
>  		mptcp_close_ssk(sk, ssk, subflow);
>  	}
> -
>  }
>  
>  static bool mptcp_close_tout_expired(const struct sock *sk)
> @@ -3092,6 +3133,13 @@ bool __mptcp_close(struct sock *sk, long
> timeout)
>  	pr_debug("msk=%p state=%d\n", sk, sk->sk_state);
>  	mptcp_pm_connection_closed(msk);
>  
> +	/* process the backlog; note that it never destroies the msk
> */
> +	local_bh_disable();
> +	bh_lock_sock(sk);
> +	__release_sock(sk);
> +	bh_unlock_sock(sk);
> +	local_bh_enable();
> +
>  	if (sk->sk_state == TCP_CLOSE) {
>  		__mptcp_destroy_sock(sk);
>  		do_cancel_work = true;
> @@ -3392,8 +3440,7 @@ void __mptcp_check_push(struct sock *sk, struct
> sock *ssk)
>  
>  #define MPTCP_FLAGS_PROCESS_CTX_NEED (BIT(MPTCP_PUSH_PENDING) | \
>  				      BIT(MPTCP_RETRANSMIT) | \
> -				      BIT(MPTCP_FLUSH_JOIN_LIST) | \
> -				      BIT(MPTCP_DEQUEUE))
> +				      BIT(MPTCP_FLUSH_JOIN_LIST))
>  
>  /* processes deferred events and flush wmem */
>  static void mptcp_release_cb(struct sock *sk)
> @@ -3427,11 +3474,6 @@ static void mptcp_release_cb(struct sock *sk)
>  			__mptcp_push_pending(sk, 0);
>  		if (flags & BIT(MPTCP_RETRANSMIT))
>  			__mptcp_retrans(sk);
> -		if ((flags & BIT(MPTCP_DEQUEUE)) &&
> __mptcp_move_skbs(sk)) {
> -			/* notify ack seq update */
> -			mptcp_cleanup_rbuf(msk, 0);
> -			sk->sk_data_ready(sk);
> -		}
>  
>  		cond_resched();
>  		spin_lock_bh(&sk->sk_lock.slock);
> @@ -3668,8 +3710,6 @@ static int mptcp_ioctl(struct sock *sk, int
> cmd, int *karg)
>  			return -EINVAL;
>  
>  		lock_sock(sk);
> -		if (__mptcp_move_skbs(sk))
> -			mptcp_cleanup_rbuf(msk, 0);
>  		*karg = mptcp_inq_hint(sk);
>  		release_sock(sk);
>  		break;
> @@ -3781,6 +3821,7 @@ static struct proto mptcp_prot = {
>  	.sendmsg	= mptcp_sendmsg,
>  	.ioctl		= mptcp_ioctl,
>  	.recvmsg	= mptcp_recvmsg,
> +	.backlog_rcv	= mptcp_move_skb,
>  	.release_cb	= mptcp_release_cb,
>  	.hash		= mptcp_hash,
>  	.unhash		= mptcp_unhash,
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 128baea5b496e..a6e775d6412e5 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -124,7 +124,6 @@
>  #define MPTCP_FLUSH_JOIN_LIST	5
>  #define MPTCP_SYNC_STATE	6
>  #define MPTCP_SYNC_SNDBUF	7
> -#define MPTCP_DEQUEUE		8
>  
>  struct mptcp_skb_cb {
>  	u64 map_seq;
> @@ -407,6 +406,7 @@ static inline int mptcp_space_from_win(const
> struct sock *sk, int win)
>  static inline int __mptcp_space(const struct sock *sk)
>  {
>  	return mptcp_win_from_space(sk, READ_ONCE(sk->sk_rcvbuf) -
> +				    READ_ONCE(sk->sk_backlog.len) -
>  				    sk_rmem_alloc_get(sk));
>  }
>  

Re: [MPTCP next 03/12] mptcp: rcvbuf auto-tuning improvement

[kernel test robot]

Hi Paolo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mptcp/export-net]
[also build test WARNING on linus/master v6.17-rc6 next-20250916]
[cannot apply to next-20250916 mptcp/export]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Paolo-Abeni/mptcp-leverage-skb-deferral-free/20250917-003012
base:   https://github.com/multipath-tcp/mptcp_net-next.git export-net
patch link:    https://lore.kernel.org/r/d3f96328a76e6fffcb871e8542c526ea99135ea5.1758039775.git.pabeni%40redhat.com
patch subject: [MPTCP next 03/12] mptcp: rcvbuf auto-tuning improvement
config: arc-nsimosci_hs_smp_defconfig (https://download.01.org/0day-ci/archive/20250917/202509171856.1KhVdgmP-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250917/202509171856.1KhVdgmP-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509171856.1KhVdgmP-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/mptcp/protocol.c: In function 'mptcp_rcvbuf_grow':
>> net/mptcp/protocol.c:199:21: warning: variable 'window_clamp' set but not used [-Wunused-but-set-variable]
     199 |                 u32 window_clamp;
         |                     ^~~~~~~~~~~~


vim +/window_clamp +199 net/mptcp/protocol.c

   180	
   181	static bool mptcp_rcvbuf_grow(struct sock *sk)
   182	{
   183		struct mptcp_sock *msk = mptcp_sk(sk);
   184		int rcvwin, rcvbuf;
   185	
   186		if (!READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_moderate_rcvbuf) ||
   187		    (sk->sk_userlocks & SOCK_RCVBUF_LOCK))
   188			return false;
   189	
   190		rcvwin = ((u64)msk->rcvq_space.space << 1);
   191	
   192		if (!RB_EMPTY_ROOT(&msk->out_of_order_queue))
   193			rcvwin += MPTCP_SKB_CB(msk->ooo_last_skb)->end_seq - msk->ack_seq;
   194	
   195		rcvbuf = min_t(u64, mptcp_space_from_win(sk, rcvwin),
   196			       READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_rmem[2]));
   197	
   198		if (rcvbuf > sk->sk_rcvbuf) {
 > 199			u32 window_clamp;
   200	
   201			window_clamp = mptcp_win_from_space(sk, rcvbuf);
   202			WRITE_ONCE(sk->sk_rcvbuf, rcvbuf);
   203			return true;
   204		}
   205		return false;
   206	}
   207	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [MPTCP next 00/12] mptcp: receive path improvement

[MPTCP CI]

Hi Paolo,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/17795536140

Status: failure
Initiator: Matthieu Baerts (NGI0)
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/f83d2e5ccf99
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1002996

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [MPTCP next 10/12] mptcp: prevernt __mptcp_move_skbs() interferring with the fastpath

[Matthieu Baerts]

Hi Geliang, Paolo,

On 17/09/2025 05:39, Geliang Tang wrote:
> Hi Paolo,
> 
> Thanks for this patch.
> 
> On Tue, 2025-09-16 at 18:27 +0200, Paolo Abeni wrote:
>> skbs will be left waiting in the subflow only in exceptional cases,
>> we want to avoid messing with the fast path by unintentionally
>> processing in __mptcp_move_skbs() packets landed into the subflows
>> after the last check.
>>
>> Use a separate flag to mark delayed skbs and only process subflow
>> with such flag set. Also add new mibs to track the exceptional
>> events.
>>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> ---
>>  net/mptcp/mib.c      |  2 ++
>>  net/mptcp/mib.h      |  4 ++++
>>  net/mptcp/protocol.c | 40 ++++++++++++----------------------------
>>  net/mptcp/protocol.h |  1 +
>>  4 files changed, 19 insertions(+), 28 deletions(-)
>>
>> diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
>> index cf879c188ca26..7af9d35cde884 100644
>> --- a/net/mptcp/mib.c
>> +++ b/net/mptcp/mib.c
>> @@ -85,6 +85,8 @@ static const struct snmp_mib mptcp_snmp_list[] = {
>>  	SNMP_MIB_ITEM("DssFallback", MPTCP_MIB_DSSFALLBACK),
>>  	SNMP_MIB_ITEM("SimultConnectFallback",
>> MPTCP_MIB_SIMULTCONNFALLBACK),
>>  	SNMP_MIB_ITEM("FallbackFailed", MPTCP_MIB_FALLBACKFAILED),
>> +	SNMP_MIB_ITEM("RcvDelayed", MPTCP_MIB_RCVDELAYED),
>> +	SNMP_MIB_ITEM("DelayedProcess", MPTCP_MIB_DELAYED_PROCESS),
>>  	SNMP_MIB_SENTINEL
> 
> I guess this line was accidentally pasted, which prevents this patch
> from being applied.

FYI, this line has been removed recently by commit 35cb2da0abaf ("mptcp:
snmp: do not use SNMP_MIB_SENTINEL anymore").

I manually fixed the conflict, and sent the series to the CI. I also
restarted my syzkaller instances to validate your series.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [MPTCP next 00/12] mptcp: receive path improvement

[MPTCP CI]

Hi Paolo,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Unstable: 5 failed test(s): packetdrill_mp_capable selftest_mptcp_connect selftest_mptcp_connect_checksum selftest_mptcp_connect_mmap selftest_mptcp_connect_sendfile 🔴
- KVM Validation: debug: Unstable: 5 failed test(s): packetdrill_mp_capable selftest_mptcp_connect selftest_mptcp_connect_checksum selftest_mptcp_connect_mmap selftest_mptcp_connect_sendfile 🔴
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/17795536153

Initiator: Matthieu Baerts (NGI0)
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/f83d2e5ccf99
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1002996


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [MPTCP next 04/12] mptcp: introduce the mptcp_init_skb helper.

[Paolo Abeni]

Hi,

On 9/17/25 9:48 AM, Geliang Tang wrote:
> On Tue, 2025-09-16 at 18:27 +0200, Paolo Abeni wrote:
>> Factor out all the skb initialization step in a new helper and
>> use it. Note that this change moves the MPTCP CB initialization
>> earlier: we can do such step as soon as the skb leaves the
>> subflow socket receive queues.
>>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> ---
>>  net/mptcp/protocol.c | 46 ++++++++++++++++++++++++------------------
>> --
>>  1 file changed, 25 insertions(+), 21 deletions(-)
>>
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index 671c51cb9539c..879157a1f4fb1 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -323,27 +323,11 @@ static void mptcp_data_queue_ofo(struct
>> mptcp_sock *msk, struct sk_buff *skb)
>>  		mptcp_rcvbuf_grow(sk);
>>  }
>>  
>> -static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock
>> *ssk,
>> -			     struct sk_buff *skb, unsigned int
>> offset,
>> -			     size_t copy_len)
>> +static void mptcp_init_skb(struct sock *ssk,
>> +			   struct mptcp_subflow_context *subflow,
> 
> subflow and ssk parameters are redundant, we can just use ssk.

I added the subflow argument explicitly since the caller already had it
handy to avoid an additional mptcp_subflow_ctx(). Perhaps the compiler
is smart enough to generate the same code, let me try...

> 
>> +			   struct sk_buff *skb, int offset, int
>> copy_len)
>>  {
>> -	struct mptcp_subflow_context *subflow =
>> mptcp_subflow_ctx(ssk);
>> -	struct sock *sk = (struct sock *)msk;
>> -	struct sk_buff *tail;
>> -	bool has_rxtstamp;
>> -
>> -	__skb_unlink(skb, &ssk->sk_receive_queue);
>> -
>> -	skb_ext_reset(skb);
>> -	skb_orphan(skb);
>> -
>> -	/* try to fetch required memory from subflow */
>> -	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
>> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
>> -		goto drop;
>> -	}
>> -
>> -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
>> +	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
>>  
>>  	/* the skb map_seq accounts for the skb offset:
>>  	 * mptcp_subflow_get_mapped_dsn() is based on the current
>> tp->copied_seq
>> @@ -355,6 +339,24 @@ static bool __mptcp_move_skb(struct mptcp_sock
>> *msk, struct sock *ssk,
>>  	MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
>>  	MPTCP_SKB_CB(skb)->cant_coalesce = 0;
>>  
>> +	__skb_unlink(skb, &ssk->sk_receive_queue);
>> +
>> +	skb_ext_reset(skb);
> 
> skb_orphan(skb) call was moved from here to after mptcp_init_skb(), but
> was then completely removed in patch 11. I'm a bit confused by this. I
> think it might be better to retain skb_orphan(skb) call here. Or
> perhaps the addition of skb_dst_drop(skb) has made skb_orphan(skb)
> unnecessary. I'd like to consult on this point.

Patch 11 replaces the skb_orphan() call with an open-code optimized
version of it, as skb_orphan() in this case is really just a skb_rfree()
I'll expand the commit message in patch 11 to make it clearer.

> 
>> +	skb_dst_drop(skb);
>> +}
>> +
>> +static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sk_buff
>> *skb)
> 
> The parameter 'msk' was changed to 'struct sock *sk' in patch 9. We can
> directly adopt 'struct sock *sk' here as well.

Will do.

Thanks,

Paolo

Re: [MPTCP next 10/12] mptcp: prevernt __mptcp_move_skbs() interferring with the fastpath

[Paolo Abeni]

On 9/17/25 2:53 PM, Matthieu Baerts wrote:
> On 17/09/2025 05:39, Geliang Tang wrote:
>> On Tue, 2025-09-16 at 18:27 +0200, Paolo Abeni wrote:
>>> skbs will be left waiting in the subflow only in exceptional cases,
>>> we want to avoid messing with the fast path by unintentionally
>>> processing in __mptcp_move_skbs() packets landed into the subflows
>>> after the last check.
>>>
>>> Use a separate flag to mark delayed skbs and only process subflow
>>> with such flag set. Also add new mibs to track the exceptional
>>> events.
>>>
>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>> ---
>>>  net/mptcp/mib.c      |  2 ++
>>>  net/mptcp/mib.h      |  4 ++++
>>>  net/mptcp/protocol.c | 40 ++++++++++++----------------------------
>>>  net/mptcp/protocol.h |  1 +
>>>  4 files changed, 19 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
>>> index cf879c188ca26..7af9d35cde884 100644
>>> --- a/net/mptcp/mib.c
>>> +++ b/net/mptcp/mib.c
>>> @@ -85,6 +85,8 @@ static const struct snmp_mib mptcp_snmp_list[] = {
>>>  	SNMP_MIB_ITEM("DssFallback", MPTCP_MIB_DSSFALLBACK),
>>>  	SNMP_MIB_ITEM("SimultConnectFallback",
>>> MPTCP_MIB_SIMULTCONNFALLBACK),
>>>  	SNMP_MIB_ITEM("FallbackFailed", MPTCP_MIB_FALLBACKFAILED),
>>> +	SNMP_MIB_ITEM("RcvDelayed", MPTCP_MIB_RCVDELAYED),
>>> +	SNMP_MIB_ITEM("DelayedProcess", MPTCP_MIB_DELAYED_PROCESS),
>>>  	SNMP_MIB_SENTINEL
>>
>> I guess this line was accidentally pasted, which prevents this patch
>> from being applied.
> 
> FYI, this line has been removed recently by commit 35cb2da0abaf ("mptcp:
> snmp: do not use SNMP_MIB_SENTINEL anymore").
> 
> I manually fixed the conflict, and sent the series to the CI. I also
> restarted my syzkaller instances to validate your series.

Oh wow! many thanks! I though I pulled and rebased the series just
before submitting it, but perhaps PEBKAC hit again.

In any case I'll send a v2 to address the comments on other patches.

/P

Re: [MPTCP next 09/12] mptcp: leverage the sk backlog for RX packet processing.

[Paolo Abeni]

On 9/17/25 12:07 PM, Geliang Tang wrote:
> On Tue, 2025-09-16 at 18:27 +0200, Paolo Abeni wrote:
>> This streamline the RX path implementation and improves the RX
>> performances by reducing the subflow-level locking and the amount of
>> work done under the msk socket lock; the implementation mirror
>> closely
>> the TCP backlog processing.
>>
>> Note that MPTCP needs now to traverse the existing subflow looking
>> for
>> data that was left there due to the msk receive buffer full, only
>> after
>> that recvmsg completely empties the receive queue.
> 
> This patch breaks my "implement mptcp read_sock v11" test case in
> patchwork, and I haven't figured out the reason yet.

Thanks for the head-up. On top of this patch, statement alike:

	if (skb_queue_empty(&sk->sk_receive_queue))
		__mptcp_move_skbs(sk);

should be changed into:

	if (skb_queue_empty(&sk->sk_receive_queue)) {
		__sk_flush_backlog(sk);
		if (skb_queue_empty(&sk->sk_receive_queue))
			__mptcp_move_skbs(sk))
	}

Possibly an helper for the above could help

/P