Skip to contentSkip to conversation

mptcp: just another receive path refactor

PATCH Superseded Paolo Abeni 3 participants 12 messages Jul 29, 2022

Subsystems (3)
Networking TCP Networking MPTCP

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Paolo Abeni

author maintainer
#0 lore raw

cover mptcp: just another receive path refactor

This is a refresh and rebase of an already shared work:

https://lore.kernel.org/mptcp/[email protected]/
[1]

The motiviation for refreshing this is:

https://lore.kernel.org/mptcp/YtVhyGSsv1CWvPz4@xsang-OptiPlex-9020/

specifically it looks like that properly attaching mem_cg to the msk socket would be (much?) easier if the RX path and the fwd memory allocation would be under msk socket lock protection.

The first 2 patches are proably worthy even without the refactor, but specifically the 2nd one is required to get a good mptcp-level acking behavior when we move the rx under the socket lock.

The 3rd patch does the real work, and the 4th is a follow-up cleanup.

Back at [1], I measured relevant performance regressions in some specific cases. I've done the same test here and I now see little to noise changes. I guess that is mostly due to the better acking strategy already introduce with commit 949dfdcf343c ("Merge branch 'mptcp-improve-mptcp-level-window-tracking'") and refined here.

Paolo Abeni (4):
mptcp: move RCVPRUNE event later
mptcp: more accurate receive buffer updates
mptcp: move msk input path under full msk socket lock
mptcp: use common helper for rmem memory accounting

 include/net/mptcp.h  |   2 +
 net/ipv4/tcp.c       |   3 +
 net/mptcp/options.c  |   1 -
 net/mptcp/protocol.c | 220 +++++++++++--------------------------------
 net/mptcp/protocol.h |  12 ++-
 5 files changed, 69 insertions(+), 169 deletions(-)
Signature · 1 line
2.35.3
author maintainer
#1 lore raw

PATCH 1/4 mptcp: move RCVPRUNE event later

This clean the code a bit, and avoid skipping msk receive buffer update on some weird corner case.

Signed-off-by Paolo Abeni <[email protected]>
---
 net/mptcp/protocol.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)
net/mptcp/protocol.c +5 -14
index 970da88cd04f..5af3d591a20b 100644
@@ -642,6 +642,10 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
 		}
 	}
 
+	/* over limit? can't append more skbs to msk */
+	if (__mptcp_rmem(sk) > sk_rbuf)
+		return true;
+
 	pr_debug("msk=%p ssk=%p", msk, ssk);
 	tp = tcp_sk(ssk);
 	do {

@@ -786,8 +790,6 @@ static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
 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);
-	int sk_rbuf, ssk_rbuf;
 
 	/* The peer can send data while we are shutting down this
 	 * subflow at msk destruction time, but we must avoid enqueuing

@@ -796,20 +798,9 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
 	if (unlikely(subflow->disposable))
 		return;
 
-	ssk_rbuf = READ_ONCE(ssk->sk_rcvbuf);
-	sk_rbuf = READ_ONCE(sk->sk_rcvbuf);
-	if (unlikely(ssk_rbuf > sk_rbuf))
-		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);
-		return;
-	}
-
 	/* Wake-up the reader only for in-sequence data */
 	mptcp_data_lock(sk);
-	if (move_skbs_to_msk(msk, ssk))
+	if (move_skbs_to_msk(mptcp_sk(sk), ssk))
 		sk->sk_data_ready(sk);
 
 	mptcp_data_unlock(sk);
Signature · 1 line
2.35.3
author maintainer
#2 lore raw

PATCH 2/4 mptcp: more accurate receive buffer updates

In reply to Paolo Abeni

Currently mptcp_cleanup_rbuf() makes a significant effort to avoid acquiring the subflow socket lock, estimating if the tcp level cleanup could actually send an ack.

Such estimate is a bit rough when accounting for receive window change, as it consider the msk available buffer space instead of the announced mptcp-level window.

Let's consider the announced window instead, mirroring closely the plain TCP implementation. We need to lockless access a bunch of additional, tcp fields.

Signed-off-by Paolo Abeni <[email protected]>
---
 include/net/mptcp.h  |  2 ++
 net/ipv4/tcp.c       |  3 +++
 net/mptcp/options.c  |  1 -
 net/mptcp/protocol.c | 14 +++++++++++---
 net/mptcp/protocol.h |  6 +++++-
 5 files changed, 21 insertions(+), 5 deletions(-)
include/net/mptcp.h +2 -0
index 7af7fd48acc7..4b6c66b73bf4 100644
@@ -136,6 +136,7 @@ static inline bool rsk_drop_req(const struct request_sock *req)
 	return tcp_rsk(req)->is_mptcp && tcp_rsk(req)->drop_req;
 }
 
+void mptcp_receive_window(const struct sock *ssk, int *rwnd);
 void mptcp_space(const struct sock *ssk, int *space, int *full_space);
 bool mptcp_syn_options(struct sock *sk, const struct sk_buff *skb,
 		       unsigned int *size, struct mptcp_out_options *opts);

@@ -284,6 +285,7 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
 	return true;
 }
 
+static inline void mptcp_receive_window(const struct sock *ssk, int *rwnd) { }
 static inline void mptcp_space(const struct sock *ssk, int *s, int *fs) { }
 static inline void mptcp_seq_show(struct seq_file *seq) { }
net/ipv4/tcp.c +3 -0
index 970e9a2cca4a..9e2df51c0558 100644
@@ -1606,6 +1606,9 @@ void tcp_cleanup_rbuf(struct sock *sk, int copied)
 	if (copied > 0 && !time_to_ack && !(sk->sk_shutdown & RCV_SHUTDOWN)) {
 		__u32 rcv_window_now = tcp_receive_window(tp);
 
+		if (is_tcpsk(sk))
+			mptcp_receive_window(sk, &rcv_window_now);
+
 		/* Optimize, __tcp_select_window() is not cheap. */
 		if (2*rcv_window_now <= tp->window_clamp) {
 			__u32 new_window = __tcp_select_window(sk);
net/mptcp/options.c +0 -1
index 30d289044e71..563ef8fe5a85 100644
@@ -604,7 +604,6 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
 	}
 	opts->ext_copy.use_ack = 1;
 	opts->suboptions = OPTION_MPTCP_DSS;
-	WRITE_ONCE(msk->old_wspace, __mptcp_space((struct sock *)msk));
 
 	/* Add kind/length/subtype/flag overhead if mapping is not populated */
 	if (dss_size == 0)
net/mptcp/protocol.c +11 -3
index 5af3d591a20b..17e2dbe43639 100644
@@ -535,6 +535,14 @@ static void mptcp_subflow_cleanup_rbuf(struct sock *ssk)
 	unlock_sock_fast(ssk, slow);
 }
 
+void mptcp_receive_window(const struct sock *ssk, int *rwnd)
+{
+	const struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
+	const struct sock *sk = subflow->conn;
+
+	*rwnd = __mptcp_receive_window(mptcp_sk(sk));
+}
+
 static bool mptcp_subflow_could_cleanup(const struct sock *ssk, bool rx_empty)
 {
 	const struct inet_connection_sock *icsk = inet_csk(ssk);

@@ -550,13 +558,13 @@ static bool mptcp_subflow_could_cleanup(const struct sock *ssk, bool rx_empty)
 
 static void mptcp_cleanup_rbuf(struct mptcp_sock *msk)
 {
-	int old_space = READ_ONCE(msk->old_wspace);
+	int cur_window = __mptcp_receive_window(msk);
 	struct mptcp_subflow_context *subflow;
 	struct sock *sk = (struct sock *)msk;
-	int space =  __mptcp_space(sk);
+	int new_window =  __mptcp_space(sk);
 	bool cleanup, rx_empty;
 
-	cleanup = (space > 0) && (space >= (old_space << 1));
+	cleanup = new_window > 0 && new_window >= (cur_window << 1);
 	rx_empty = !__mptcp_rmem(sk);
 
 	mptcp_for_each_subflow(msk, subflow) {
net/mptcp/protocol.h +5 -1
index 1bc9f6e77ddd..a54f42462a71 100644
@@ -260,7 +260,6 @@ struct mptcp_sock {
 	int		rmem_fwd_alloc;
 	struct sock	*last_snd;
 	int		snd_burst;
-	int		old_wspace;
 	u64		recovery_snd_nxt;	/* in recovery mode accept up to this seq;
 						 * recovery related fields are under data_lock
 						 * protection

@@ -336,6 +335,11 @@ static inline int __mptcp_rmem(const struct sock *sk)
 	return atomic_read(&sk->sk_rmem_alloc) - READ_ONCE(mptcp_sk(sk)->rmem_released);
 }
 
+static inline int __mptcp_receive_window(const struct mptcp_sock *msk)
+{
+	return atomic64_read(&msk->rcv_wnd_sent) - msk->ack_seq;
+}
+
 static inline int __mptcp_space(const struct sock *sk)
 {
 	return tcp_win_from_space(sk, READ_ONCE(sk->sk_rcvbuf) - __mptcp_rmem(sk));
Signature · 1 line
2.35.3
build bot
#3 lore raw

Re: [PATCH mptcp-next 2/4] mptcp: more accurate receive buffer updates

I love your patch! Yet something to improve:

Hi Paolo,

I love your patch! Yet something to improve:

[auto build test ERROR on mptcp/export]
[also build test ERROR on linus/master v5.19-rc8 next-20220728] [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-just-another-receive-path-refactor/20220729-233501
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20220730/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/c330a583c6d306ab637d187f0f981bfe4408caba
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Paolo-Abeni/mptcp-just-another-receive-path-refactor/20220729-233501
        git checkout c330a583c6d306ab637d187f0f981bfe4408caba
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable

Reported-by kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

net/ipv4/tcp.c: In function 'tcp_cleanup_rbuf':

>> net/ipv4/tcp.c:1609:21: error: implicit declaration of function 'is_tcpsk' [-Werror=implicit-function-declaration]

1609 | if (is_tcpsk(sk))
| ^~~~~~~~
cc1: some warnings being treated as errors

vim +/is_tcpsk +1609 net/ipv4/tcp.c

1563
1564 /* Clean up the receive buffer for full frames taken by the user,
1565 * then send an ACK if necessary. COPIED is the number of bytes
1566 * tcp_recvmsg has given to the user so far, it speeds up the
1567 * calculation of whether or not we must ACK for the sake of
1568 * a window update.
1569 */
1570 void tcp_cleanup_rbuf(struct sock *sk, int copied)
1571 {
1572 struct tcp_sock *tp = tcp_sk(sk);
1573 bool time_to_ack = false;
1574
1575 struct sk_buff *skb = skb_peek(&sk->sk_receive_queue);
1576
1577 WARN(skb && !before(tp->copied_seq, TCP_SKB_CB(skb)->end_seq),
1578 "cleanup rbuf bug: copied %X seq %X rcvnxt %X\n",
1579 tp->copied_seq, TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt);
1580
1581 if (inet_csk_ack_scheduled(sk)) {
1582 const struct inet_connection_sock *icsk = inet_csk(sk);
1583
1584 if (/* Once-per-two-segments ACK was not sent by tcp_input.c */
1585 tp->rcv_nxt - tp->rcv_wup > icsk->icsk_ack.rcv_mss ||
1586 /*

  1587			     * If this read emptied read buffer, we send ACK, if
  1588			     * connection is not bidirectional, user drained
  1589			     * receive buffer and there was a small segment
  1590			     * in queue.
  1591			     */
  1592			    (copied > 0 &&
  1593			     ((icsk->icsk_ack.pending & ICSK_ACK_PUSHED2) ||
  1594			      ((icsk->icsk_ack.pending & ICSK_ACK_PUSHED) &&
  1595			       !inet_csk_in_pingpong_mode(sk))) &&
  1596			      !atomic_read(&sk->sk_rmem_alloc)))
  1597				time_to_ack = true;
  1598		}
  1599	
  1600		/* We send an ACK if we can now advertise a non-zero window
  1601		 * which has been raised "significantly".
  1602		 *
  1603		 * Even if window raised up to infinity, do not send window open ACK
  1604		 * in states, where we will not receive more. It is useless.
  1605		 */
  1606		if (copied > 0 && !time_to_ack && !(sk->sk_shutdown & RCV_SHUTDOWN)) {
  1607			__u32 rcv_window_now = tcp_receive_window(tp);
  1608	
> 1609			if (is_tcpsk(sk))

1610 mptcp_receive_window(sk, &rcv_window_now);
1611
1612 /* Optimize, __tcp_select_window() is not cheap. */
1613 if (2*rcv_window_now <= tp->window_clamp) {
1614 __u32 new_window = __tcp_select_window(sk);
1615
1616 /* Send ACK now, if this read freed lots of space
1617 * in our buffer. Certainly, new_window is new window.
1618 * We can advertise it now, if it is not less than current one.
1619 * "Lots" means "at least twice" here.
1620 */
1621 if (new_window && new_window >= 2 * rcv_window_now)
1622 time_to_ack = true;
1623 }
1624 }
1625 if (time_to_ack)
1626 tcp_send_ack(sk);
1627 }
1628

Signature · 2 lines
0-DAY CI Kernel Test Service
https://01.org/lkp
build bot
#4 lore raw

Re: [PATCH mptcp-next 2/4] mptcp: more accurate receive buffer updates

In reply to Paolo Abeni
I love your patch! Yet something to improve:

Hi Paolo,

I love your patch! Yet something to improve:

[auto build test ERROR on mptcp/export]
[also build test ERROR on linus/master v5.19-rc8 next-20220728] [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-just-another-receive-path-refactor/20220729-233501
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
config: hexagon-randconfig-r045-20220729 (https://download.01.org/0day-ci/archive/20220730/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 8dfaecc4c24494337933aff9d9166486ca0949f1)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c330a583c6d306ab637d187f0f981bfe4408caba
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paolo-Abeni/mptcp-just-another-receive-path-refactor/20220729-233501 git checkout c330a583c6d306ab637d187f0f981bfe4408caba
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash net/ipv4/ net/mptcp/

If you fix the issue, kindly add following tag where applicable

Reported-by kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> net/ipv4/tcp.c:1609:7: error: call to undeclared function 'is_tcpsk'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                   if (is_tcpsk(sk))
                       ^

1 error generated.

vim +/is_tcpsk +1609 net/ipv4/tcp.c

1563
1564 /* Clean up the receive buffer for full frames taken by the user,
1565 * then send an ACK if necessary. COPIED is the number of bytes
1566 * tcp_recvmsg has given to the user so far, it speeds up the
1567 * calculation of whether or not we must ACK for the sake of
1568 * a window update.
1569 */
1570 void tcp_cleanup_rbuf(struct sock *sk, int copied)
1571 {
1572 struct tcp_sock *tp = tcp_sk(sk);
1573 bool time_to_ack = false;
1574
1575 struct sk_buff *skb = skb_peek(&sk->sk_receive_queue);
1576
1577 WARN(skb && !before(tp->copied_seq, TCP_SKB_CB(skb)->end_seq),
1578 "cleanup rbuf bug: copied %X seq %X rcvnxt %X\n",
1579 tp->copied_seq, TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt);
1580
1581 if (inet_csk_ack_scheduled(sk)) {
1582 const struct inet_connection_sock *icsk = inet_csk(sk);
1583
1584 if (/* Once-per-two-segments ACK was not sent by tcp_input.c */
1585 tp->rcv_nxt - tp->rcv_wup > icsk->icsk_ack.rcv_mss ||
1586 /*

  1587			     * If this read emptied read buffer, we send ACK, if
  1588			     * connection is not bidirectional, user drained
  1589			     * receive buffer and there was a small segment
  1590			     * in queue.
  1591			     */
  1592			    (copied > 0 &&
  1593			     ((icsk->icsk_ack.pending & ICSK_ACK_PUSHED2) ||
  1594			      ((icsk->icsk_ack.pending & ICSK_ACK_PUSHED) &&
  1595			       !inet_csk_in_pingpong_mode(sk))) &&
  1596			      !atomic_read(&sk->sk_rmem_alloc)))
  1597				time_to_ack = true;
  1598		}
  1599	
  1600		/* We send an ACK if we can now advertise a non-zero window
  1601		 * which has been raised "significantly".
  1602		 *
  1603		 * Even if window raised up to infinity, do not send window open ACK
  1604		 * in states, where we will not receive more. It is useless.
  1605		 */
  1606		if (copied > 0 && !time_to_ack && !(sk->sk_shutdown & RCV_SHUTDOWN)) {
  1607			__u32 rcv_window_now = tcp_receive_window(tp);
  1608	
> 1609			if (is_tcpsk(sk))

1610 mptcp_receive_window(sk, &rcv_window_now);
1611
1612 /* Optimize, __tcp_select_window() is not cheap. */
1613 if (2*rcv_window_now <= tp->window_clamp) {
1614 __u32 new_window = __tcp_select_window(sk);
1615
1616 /* Send ACK now, if this read freed lots of space
1617 * in our buffer. Certainly, new_window is new window.
1618 * We can advertise it now, if it is not less than current one.
1619 * "Lots" means "at least twice" here.
1620 */
1621 if (new_window && new_window >= 2 * rcv_window_now)
1622 time_to_ack = true;
1623 }
1624 }
1625 if (time_to_ack)
1626 tcp_send_ack(sk);
1627 }
1628

Signature · 2 lines
0-DAY CI Kernel Test Service
https://01.org/lkp
author maintainer
#5 lore raw

PATCH 3/4 mptcp: move msk input path under full msk socket lock

In reply to Paolo Abeni
Signed-off-by Paolo Abeni <[email protected]>
---
 net/mptcp/protocol.c | 79 ++++++++++++++++++++------------------------
 net/mptcp/protocol.h |  2 +-
 2 files changed, 36 insertions(+), 45 deletions(-)
net/mptcp/protocol.c +35 -44
index 17e2dbe43639..b9402a13a69d 100644
@@ -795,7 +795,7 @@ static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
 	return moved > 0;
 }
 
-void mptcp_data_ready(struct sock *sk, struct sock *ssk)
+void __mptcp_data_ready(struct sock *sk, struct sock *ssk)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
 

@@ -807,10 +807,17 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
 		return;
 
 	/* Wake-up the reader only for in-sequence data */
-	mptcp_data_lock(sk);
 	if (move_skbs_to_msk(mptcp_sk(sk), ssk))
 		sk->sk_data_ready(sk);
+}
 
+void mptcp_data_ready(struct sock *sk, struct sock *ssk)
+{
+	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);
 	mptcp_data_unlock(sk);
 }
 

@@ -1059,6 +1066,7 @@ static void mptcp_clean_una_wakeup(struct sock *sk)
 	mptcp_data_unlock(sk);
 }
 
+
 static void mptcp_enter_memory_pressure(struct sock *sk)
 {
 	struct mptcp_subflow_context *subflow;

@@ -1768,16 +1776,22 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	return copied ? : ret;
 }
 
-static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
+static bool __mptcp_move_skbs(struct sock *sk);
+
+static int __mptcp_recvmsg_mskq(struct sock *sk,
 				struct msghdr *msg,
 				size_t len, int flags,
 				struct scm_timestamping_internal *tss,
 				int *cmsg_flags)
 {
+	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct sk_buff *skb, *tmp;
 	int copied = 0;
 
-	skb_queue_walk_safe(&msk->receive_queue, skb, tmp) {
+	if (skb_queue_empty(&sk->sk_receive_queue) && !__mptcp_move_skbs(sk))
+		return 0;
+
+	skb_queue_walk_safe(&sk->sk_receive_queue, skb, tmp) {
 		u32 offset = MPTCP_SKB_CB(skb)->offset;
 		u32 data_len = skb->len - offset;
 		u32 count = min_t(size_t, len - copied, data_len);

@@ -1811,7 +1825,7 @@ static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
 			/* we will bulk release the skb memory later */
 			skb->destructor = NULL;
 			WRITE_ONCE(msk->rmem_released, msk->rmem_released + skb->truesize);
-			__skb_unlink(skb, &msk->receive_queue);
+			__skb_unlink(skb, &sk->sk_receive_queue);
 			__kfree_skb(skb);
 		}
 

@@ -1932,16 +1946,9 @@ static void __mptcp_update_rmem(struct sock *sk)
 	WRITE_ONCE(msk->rmem_released, 0);
 }
 
-static void __mptcp_splice_receive_queue(struct sock *sk)
+static bool __mptcp_move_skbs(struct sock *sk)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
-
-	skb_queue_splice_tail_init(&sk->sk_receive_queue, &msk->receive_queue);
-}
-
-static bool __mptcp_move_skbs(struct mptcp_sock *msk)
-{
-	struct sock *sk = (struct sock *)msk;
 	unsigned int moved = 0;
 	bool ret, done;
 

@@ -1949,37 +1956,29 @@ static bool __mptcp_move_skbs(struct mptcp_sock *msk)
 		struct sock *ssk = mptcp_subflow_recv_lookup(msk);
 		bool slowpath;
 
-		/* we can have data pending in the subflows only if the msk
-		 * receive buffer was full at subflow_data_ready() time,
-		 * that is an unlikely slow path.
-		 */
-		if (likely(!ssk))
+		if (unlikely(!ssk))
 			break;
 
 		slowpath = lock_sock_fast(ssk);
-		mptcp_data_lock(sk);
 		__mptcp_update_rmem(sk);
 		done = __mptcp_move_skbs_from_subflow(msk, ssk, &moved);
-		mptcp_data_unlock(sk);
 
 		if (unlikely(ssk->sk_err))
 			__mptcp_error_report(sk);
 		unlock_sock_fast(ssk, slowpath);
 	} while (!done);
 
-	/* acquire the data lock only if some input data is pending */
 	ret = moved > 0;
 	if (!RB_EMPTY_ROOT(&msk->out_of_order_queue) ||
-	    !skb_queue_empty_lockless(&sk->sk_receive_queue)) {
-		mptcp_data_lock(sk);
+	    !skb_queue_empty(&sk->sk_receive_queue)) {
 		__mptcp_update_rmem(sk);
 		ret |= __mptcp_ofo_queue(msk);
-		__mptcp_splice_receive_queue(sk);
-		mptcp_data_unlock(sk);
 	}
-	if (ret)
+	if (ret) {
+		mptcp_cleanup_rbuf(msk);
 		mptcp_check_data_fin((struct sock *)msk);
-	return !skb_queue_empty(&msk->receive_queue);
+	}
+	return ret;
 }
 
 static unsigned int mptcp_inq_hint(const struct sock *sk)

@@ -1987,7 +1986,7 @@ static unsigned int mptcp_inq_hint(const struct sock *sk)
 	const struct mptcp_sock *msk = mptcp_sk(sk);
 	const struct sk_buff *skb;
 
-	skb = skb_peek(&msk->receive_queue);
+	skb = skb_peek(&sk->sk_receive_queue);
 	if (skb) {
 		u64 hint_val = msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
 

@@ -2033,7 +2032,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 	while (copied < len) {
 		int bytes_read;
 
-		bytes_read = __mptcp_recvmsg_mskq(msk, msg, len - copied, flags, &tss, &cmsg_flags);
+		bytes_read = __mptcp_recvmsg_mskq(sk, msg, len - copied, flags, &tss, &cmsg_flags);
 		if (unlikely(bytes_read < 0)) {
 			if (!copied)
 				copied = bytes_read;

@@ -2045,9 +2044,6 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		/* be sure to advertise window change */
 		mptcp_cleanup_rbuf(msk);
 
-		if (skb_queue_empty(&msk->receive_queue) && __mptcp_move_skbs(msk))
-			continue;
-
 		/* only the master socket status is relevant here. The exit
 		 * conditions mirror closely tcp_recvmsg()
 		 */

@@ -2074,7 +2070,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 				/* race breaker: the shutdown could be after the
 				 * previous receive queue check
 				 */
-				if (__mptcp_move_skbs(msk))
+				if (__mptcp_move_skbs(sk))
 					continue;
 				break;
 			}

@@ -2111,9 +2107,8 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		}
 	}
 
-	pr_debug("msk=%p rx queue empty=%d:%d copied=%d",
-		 msk, skb_queue_empty_lockless(&sk->sk_receive_queue),
-		 skb_queue_empty(&msk->receive_queue), copied);
+	pr_debug("msk=%p rx queue empty=%d copied=%d",
+		 msk, skb_queue_empty(&sk->sk_receive_queue), copied);
 	if (!(flags & MSG_PEEK))
 		mptcp_rcv_space_adjust(msk, copied);
 

@@ -2566,7 +2561,6 @@ static int __mptcp_init_sock(struct sock *sk)
 	INIT_LIST_HEAD(&msk->join_list);
 	INIT_LIST_HEAD(&msk->rtx_queue);
 	INIT_WORK(&msk->work, mptcp_worker);
-	__skb_queue_head_init(&msk->receive_queue);
 	msk->out_of_order_queue = RB_ROOT;
 	msk->first_pending = NULL;
 	msk->rmem_fwd_alloc = 0;

@@ -3048,12 +3042,8 @@ void mptcp_destroy_common(struct mptcp_sock *msk, unsigned int flags)
 	mptcp_for_each_subflow_safe(msk, subflow, tmp)
 		__mptcp_close_ssk(sk, mptcp_subflow_tcp_sock(subflow), subflow, flags);
 
-	/* move to sk_receive_queue, sk_stream_kill_queues will purge it */
-	mptcp_data_lock(sk);
-	skb_queue_splice_tail_init(&msk->receive_queue, &sk->sk_receive_queue);
 	__skb_queue_purge(&sk->sk_receive_queue);
 	skb_rbtree_purge(&msk->out_of_order_queue);
-	mptcp_data_unlock(sk);
 
 	/* move all the rx fwd alloc into the sk_mem_reclaim_final in
 	 * inet_sock_destruct() will dispose it

@@ -3135,6 +3125,8 @@ static void mptcp_release_cb(struct sock *sk)
 			__mptcp_flush_join_list(sk);
 		if (flags & BIT(MPTCP_PUSH_PENDING))
 			__mptcp_push_pending(sk, 0);
+		if ((flags & BIT(MPTCP_DEQUEUE)) && __mptcp_move_skbs(sk))
+			sk->sk_data_ready(sk);
 		if (flags & BIT(MPTCP_RETRANSMIT))
 			__mptcp_retrans(sk);
 

@@ -3383,7 +3375,7 @@ static int mptcp_ioctl(struct sock *sk, int cmd, unsigned long arg)
 			return -EINVAL;
 
 		lock_sock(sk);
-		__mptcp_move_skbs(msk);
+		__mptcp_move_skbs(sk);
 		answ = mptcp_inq_hint(sk);
 		release_sock(sk);
 		break;

@@ -3619,8 +3611,7 @@ static __poll_t mptcp_check_readable(struct mptcp_sock *msk)
 	/* Concurrent splices from sk_receive_queue into receive_queue will
 	 * always show at least one non-empty queue when checked in this order.
 	 */
-	if (skb_queue_empty_lockless(&((struct sock *)msk)->sk_receive_queue) &&
-	    skb_queue_empty_lockless(&msk->receive_queue))
+	if (skb_queue_empty_lockless(&((struct sock *)msk)->sk_receive_queue))
 		return 0;
 
 	return EPOLLIN | EPOLLRDNORM;
net/mptcp/protocol.h +1 -1
index a54f42462a71..99c710e1ff5c 100644
@@ -125,6 +125,7 @@
 #define MPTCP_FLUSH_JOIN_LIST	5
 #define MPTCP_CONNECTED		6
 #define MPTCP_RESET_SCHEDULER	7
+#define MPTCP_DEQUEUE		8
 
 static inline bool before64(__u64 seq1, __u64 seq2)
 {

@@ -288,7 +289,6 @@ struct mptcp_sock {
 	struct work_struct work;
 	struct sk_buff  *ooo_last_skb;
 	struct rb_root  out_of_order_queue;
-	struct sk_buff_head receive_queue;
 	struct list_head conn_list;
 	struct list_head rtx_queue;
 	struct mptcp_data_frag *first_pending;
Signature · 1 line
2.35.3
build bot
#6 lore raw

Re: [PATCH mptcp-next 3/4] mptcp: move msk input path under full msk socket lock

I love your patch! Perhaps something to improve:

Hi Paolo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mptcp/export]
[cannot apply to linus/master v5.19-rc8 next-20220728]
[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-just-another-receive-path-refactor/20220729-233501
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20220730/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/120ee56b1ad70bbb3d1b35c107110b0f83b2e4e1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Paolo-Abeni/mptcp-just-another-receive-path-refactor/20220729-233501
        git checkout 120ee56b1ad70bbb3d1b35c107110b0f83b2e4e1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash net/ipv4/ net/mptcp/

If you fix the issue, kindly add following tag where applicable

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> net/mptcp/protocol.c:798:6: warning: no previous prototype for '__mptcp_data_ready' [-Wmissing-prototypes]

798 | void __mptcp_data_ready(struct sock *sk, struct sock *ssk)
| ^~~~~~~~~~~~~~~~~~

vim +/__mptcp_data_ready +798 net/mptcp/protocol.c

797

 > 798	void __mptcp_data_ready(struct sock *sk, struct sock *ssk)

799 {
800 struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
801
802 /* The peer can send data while we are shutting down this
803 * subflow at msk destruction time, but we must avoid enqueuing
804 * more data to the msk receive queue
805 */
806 if (unlikely(subflow->disposable))
807 return;
808
809 /* Wake-up the reader only for in-sequence data */
810 if (move_skbs_to_msk(mptcp_sk(sk), ssk))
811 sk->sk_data_ready(sk);
812 }
813

Signature · 2 lines
0-DAY CI Kernel Test Service
https://01.org/lkp
build bot
#7 lore raw

Re: [PATCH mptcp-next 3/4] mptcp: move msk input path under full msk socket lock

In reply to Paolo Abeni
I love your patch! Perhaps something to improve:

Hi Paolo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mptcp/export]
[cannot apply to linus/master v5.19-rc8 next-20220728]
[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-just-another-receive-path-refactor/20220729-233501
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
config: hexagon-randconfig-r045-20220729 (https://download.01.org/0day-ci/archive/20220730/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 8dfaecc4c24494337933aff9d9166486ca0949f1)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/120ee56b1ad70bbb3d1b35c107110b0f83b2e4e1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paolo-Abeni/mptcp-just-another-receive-path-refactor/20220729-233501 git checkout 120ee56b1ad70bbb3d1b35c107110b0f83b2e4e1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash net/mptcp/

If you fix the issue, kindly add following tag where applicable

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> net/mptcp/protocol.c:798:6: warning: no previous prototype for function '__mptcp_data_ready' [-Wmissing-prototypes]
   void __mptcp_data_ready(struct sock *sk, struct sock *ssk)
        ^

net/mptcp/protocol.c:798:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void __mptcp_data_ready(struct sock *sk, struct sock *ssk)
^
static
1 warning generated.

vim +/__mptcp_data_ready +798 net/mptcp/protocol.c

797

 > 798	void __mptcp_data_ready(struct sock *sk, struct sock *ssk)

799 {
800 struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
801
802 /* The peer can send data while we are shutting down this
803 * subflow at msk destruction time, but we must avoid enqueuing
804 * more data to the msk receive queue
805 */
806 if (unlikely(subflow->disposable))
807 return;
808
809 /* Wake-up the reader only for in-sequence data */
810 if (move_skbs_to_msk(mptcp_sk(sk), ssk))
811 sk->sk_data_ready(sk);
812 }
813

Signature · 2 lines
0-DAY CI Kernel Test Service
https://01.org/lkp
author maintainer
#8 lore raw

PATCH 4/4 mptcp: use common helper for rmem memory accounting

In reply to Paolo Abeni
Signed-off-by Paolo Abeni <[email protected]>
---
 net/mptcp/protocol.c | 112 ++++---------------------------------------
 net/mptcp/protocol.h |   4 +-
 2 files changed, 9 insertions(+), 107 deletions(-)
net/mptcp/protocol.c +8 -104
index b9402a13a69d..130afd239ecf 100644
@@ -131,11 +131,6 @@ static void mptcp_drop(struct sock *sk, struct sk_buff *skb)
 	__kfree_skb(skb);
 }
 
-static void mptcp_rmem_charge(struct sock *sk, int size)
-{
-	mptcp_sk(sk)->rmem_fwd_alloc -= size;
-}
-
 static bool mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
 			       struct sk_buff *from)
 {

@@ -152,7 +147,7 @@ static bool mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
 	MPTCP_SKB_CB(to)->end_seq = MPTCP_SKB_CB(from)->end_seq;
 	kfree_skb_partial(from, fragstolen);
 	atomic_add(delta, &sk->sk_rmem_alloc);
-	mptcp_rmem_charge(sk, delta);
+	sk->sk_forward_alloc -= delta;
 	return true;
 }
 

@@ -165,44 +160,6 @@ static bool mptcp_ooo_try_coalesce(struct mptcp_sock *msk, struct sk_buff *to,
 	return mptcp_try_coalesce((struct sock *)msk, to, from);
 }
 
-static void __mptcp_rmem_reclaim(struct sock *sk, int amount)
-{
-	amount >>= PAGE_SHIFT;
-	mptcp_sk(sk)->rmem_fwd_alloc -= amount << PAGE_SHIFT;
-	__sk_mem_reduce_allocated(sk, amount);
-}
-
-static void mptcp_rmem_uncharge(struct sock *sk, int size)
-{
-	struct mptcp_sock *msk = mptcp_sk(sk);
-	int reclaimable;
-
-	msk->rmem_fwd_alloc += size;
-	reclaimable = msk->rmem_fwd_alloc - sk_unused_reserved_mem(sk);
-
-	/* see sk_mem_uncharge() for the rationale behind the following schema */
-	if (unlikely(reclaimable >= PAGE_SIZE))
-		__mptcp_rmem_reclaim(sk, reclaimable);
-}
-
-static void mptcp_rfree(struct sk_buff *skb)
-{
-	unsigned int len = skb->truesize;
-	struct sock *sk = skb->sk;
-
-	atomic_sub(len, &sk->sk_rmem_alloc);
-	mptcp_rmem_uncharge(sk, len);
-}
-
-static void mptcp_set_owner_r(struct sk_buff *skb, struct sock *sk)
-{
-	skb_orphan(skb);
-	skb->sk = sk;
-	skb->destructor = mptcp_rfree;
-	atomic_add(skb->truesize, &sk->sk_rmem_alloc);
-	mptcp_rmem_charge(sk, skb->truesize);
-}
-
 /* "inspired" by tcp_data_queue_ofo(), main differences:
  * - use mptcp seqs
  * - don't cope with sacks

@@ -315,25 +272,7 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
 
 end:
 	skb_condense(skb);
-	mptcp_set_owner_r(skb, sk);
-}
-
-static bool mptcp_rmem_schedule(struct sock *sk, struct sock *ssk, int size)
-{
-	struct mptcp_sock *msk = mptcp_sk(sk);
-	int amt, amount;
-
-	if (size <= msk->rmem_fwd_alloc)
-		return true;
-
-	size -= msk->rmem_fwd_alloc;
-	amt = sk_mem_pages(size);
-	amount = amt << PAGE_SHIFT;
-	if (!__sk_mem_raise_allocated(sk, size, amt, SK_MEM_RECV))
-		return false;
-
-	msk->rmem_fwd_alloc += amount;
-	return true;
+	skb_set_owner_r(skb, sk);
 }
 
 static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,

@@ -350,8 +289,7 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
 	skb_ext_reset(skb);
 	skb_orphan(skb);
 
-	/* try to fetch required memory from subflow */
-	if (!mptcp_rmem_schedule(sk, ssk, skb->truesize))
+	if (!sk_rmem_schedule(sk, skb, skb->truesize))
 		goto drop;
 
 	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;

@@ -372,7 +310,7 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
 		if (tail && mptcp_try_coalesce(sk, tail, skb))
 			return true;
 
-		mptcp_set_owner_r(skb, sk);
+		skb_set_owner_r(skb, sk);
 		__skb_queue_tail(&sk->sk_receive_queue, skb);
 		return true;
 	} else if (after64(MPTCP_SKB_CB(skb)->map_seq, msk->ack_seq)) {

@@ -1784,7 +1722,6 @@ static int __mptcp_recvmsg_mskq(struct sock *sk,
 				struct scm_timestamping_internal *tss,
 				int *cmsg_flags)
 {
-	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct sk_buff *skb, *tmp;
 	int copied = 0;
 

@@ -1822,9 +1759,10 @@ static int __mptcp_recvmsg_mskq(struct sock *sk,
 		}
 
 		if (!(flags & MSG_PEEK)) {
-			/* we will bulk release the skb memory later */
+			/* avoid the indirect call, we know the destructor is sock_wfree */
 			skb->destructor = NULL;
-			WRITE_ONCE(msk->rmem_released, msk->rmem_released + skb->truesize);
+			atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
+			sk_mem_uncharge(sk, skb->truesize);
 			__skb_unlink(skb, &sk->sk_receive_queue);
 			__kfree_skb(skb);
 		}

@@ -1934,18 +1872,6 @@ static void mptcp_rcv_space_adjust(struct mptcp_sock *msk, int copied)
 	msk->rcvq_space.time = mstamp;
 }
 
-static void __mptcp_update_rmem(struct sock *sk)
-{
-	struct mptcp_sock *msk = mptcp_sk(sk);
-
-	if (!msk->rmem_released)
-		return;
-
-	atomic_sub(msk->rmem_released, &sk->sk_rmem_alloc);
-	mptcp_rmem_uncharge(sk, msk->rmem_released);
-	WRITE_ONCE(msk->rmem_released, 0);
-}
-
 static bool __mptcp_move_skbs(struct sock *sk)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);

@@ -1960,7 +1886,6 @@ static bool __mptcp_move_skbs(struct sock *sk)
 			break;
 
 		slowpath = lock_sock_fast(ssk);
-		__mptcp_update_rmem(sk);
 		done = __mptcp_move_skbs_from_subflow(msk, ssk, &moved);
 
 		if (unlikely(ssk->sk_err))

@@ -1969,11 +1894,7 @@ static bool __mptcp_move_skbs(struct sock *sk)
 	} while (!done);
 
 	ret = moved > 0;
-	if (!RB_EMPTY_ROOT(&msk->out_of_order_queue) ||
-	    !skb_queue_empty(&sk->sk_receive_queue)) {
-		__mptcp_update_rmem(sk);
-		ret |= __mptcp_ofo_queue(msk);
-	}
+	ret |= __mptcp_ofo_queue(msk);
 	if (ret) {
 		mptcp_cleanup_rbuf(msk);
 		mptcp_check_data_fin((struct sock *)msk);

@@ -2563,8 +2484,6 @@ static int __mptcp_init_sock(struct sock *sk)
 	INIT_WORK(&msk->work, mptcp_worker);
 	msk->out_of_order_queue = RB_ROOT;
 	msk->first_pending = NULL;
-	msk->rmem_fwd_alloc = 0;
-	WRITE_ONCE(msk->rmem_released, 0);
 	msk->timer_ival = TCP_RTO_MIN;
 
 	msk->first = NULL;

@@ -2776,8 +2695,6 @@ static void __mptcp_destroy_sock(struct sock *sk)
 
 	sk->sk_prot->destroy(sk);
 
-	WARN_ON_ONCE(msk->rmem_fwd_alloc);
-	WARN_ON_ONCE(msk->rmem_released);
 	sk_stream_kill_queues(sk);
 	xfrm_sk_free_policy(sk);
 

@@ -3045,11 +2962,6 @@ void mptcp_destroy_common(struct mptcp_sock *msk, unsigned int flags)
 	__skb_queue_purge(&sk->sk_receive_queue);
 	skb_rbtree_purge(&msk->out_of_order_queue);
 
-	/* move all the rx fwd alloc into the sk_mem_reclaim_final in
-	 * inet_sock_destruct() will dispose it
-	 */
-	sk->sk_forward_alloc += msk->rmem_fwd_alloc;
-	msk->rmem_fwd_alloc = 0;
 	mptcp_token_destroy(msk);
 	mptcp_pm_free_anno_list(msk);
 	mptcp_free_local_addr_list(msk);

@@ -3147,8 +3059,6 @@ static void mptcp_release_cb(struct sock *sk)
 		if (__test_and_clear_bit(MPTCP_RESET_SCHEDULER, &msk->cb_flags))
 			msk->last_snd = NULL;
 	}
-
-	__mptcp_update_rmem(sk);
 }
 
 /* MP_JOIN client subflow must wait for 4th ack before sending any data:

@@ -3329,11 +3239,6 @@ static void mptcp_shutdown(struct sock *sk, int how)
 		__mptcp_wr_shutdown(sk);
 }
 
-static int mptcp_forward_alloc_get(const struct sock *sk)
-{
-	return sk->sk_forward_alloc + mptcp_sk(sk)->rmem_fwd_alloc;
-}
-
 static int mptcp_ioctl_outq(const struct mptcp_sock *msk, u64 v)
 {
 	const struct sock *sk = (void *)msk;

@@ -3414,7 +3319,6 @@ static struct proto mptcp_prot = {
 	.hash		= mptcp_hash,
 	.unhash		= mptcp_unhash,
 	.get_port	= mptcp_get_port,
-	.forward_alloc_get	= mptcp_forward_alloc_get,
 	.sockets_allocated	= &mptcp_sockets_allocated,
 
 	.memory_allocated	= &tcp_memory_allocated,
net/mptcp/protocol.h +1 -3
index 99c710e1ff5c..9ddee7eac00d 100644
@@ -258,7 +258,6 @@ struct mptcp_sock {
 	u64		ack_seq;
 	atomic64_t	rcv_wnd_sent;
 	u64		rcv_data_fin_seq;
-	int		rmem_fwd_alloc;
 	struct sock	*last_snd;
 	int		snd_burst;
 	u64		recovery_snd_nxt;	/* in recovery mode accept up to this seq;

@@ -269,7 +268,6 @@ struct mptcp_sock {
 	u64		wnd_end;
 	unsigned long	timer_ival;
 	u32		token;
-	int		rmem_released;
 	unsigned long	flags;
 	unsigned long	cb_flags;
 	unsigned long	push_pending;

@@ -332,7 +330,7 @@ static inline struct mptcp_sock *mptcp_sk(const struct sock *sk)
  */
 static inline int __mptcp_rmem(const struct sock *sk)
 {
-	return atomic_read(&sk->sk_rmem_alloc) - READ_ONCE(mptcp_sk(sk)->rmem_released);
+	return atomic_read(&sk->sk_rmem_alloc);
 }
 
 static inline int __mptcp_receive_window(const struct mptcp_sock *msk)
Signature · 1 line
2.35.3
#9 lore raw

Re: mptcp: use common helper for rmem memory accounting: Build Failure

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://patchwork.kernel.org/project/mptcp/patch/f253e24227ce660aae19714c2cca1726ee087184.1659107989.git.pabeni@redhat.com/
https://github.com/multipath-tcp/mptcp_net-next/actions/runs/2761676692

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/1f45050af065

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 (Tessares)

author maintainer
#10 lore raw

Re: mptcp: use common helper for rmem memory accounting: Build Failure

Quoted 15 lines · 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://patchwork.kernel.org/project/mptcp/patch/f253e24227ce660aae19714c2cca1726ee087184.1659107989.git.pabeni@redhat.com/
https://github.com/multipath-tcp/mptcp_net-next/actions/runs/2761676692

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/1f45050af065

Quoted 7 lines

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 (Tessares)

Darn me! some latest local change did not land into this series, I'll submit a v2 soon. Sorry for the noise...

/P

#11 lore raw

Re: mptcp: use common helper for rmem memory accounting: Tests Results

In reply to Paolo Abeni

Hi Paolo,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: Script error! ❓:
- :
- Task: https://cirrus-ci.com/task/5166180951392256
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5166180951392256/summary/summary.txt

- KVM Validation: Script error! ❓:
- :
- Task: https://cirrus-ci.com/task/6292080858234880
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6292080858234880/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/1f45050af065

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-debug

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 (Tessares)