[PATCH v4 1/2] mptcp: add MSG_PEEK support

[PATCH v4 1/2] mptcp: add MSG_PEEK support

[Yonglong Li]

This patch adds support for MSG_PEEK flag. Packets are not removed
from the receive_queue if MSG_PEEK set in recv() system call.

Signed-off-by: Yonglong Li <liyonglong@chinatelecom.cn>
---
 net/mptcp/protocol.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 16d73ec..75f4420 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1739,12 +1739,12 @@ static void mptcp_wait_data(struct sock *sk, long *timeo)
 
 static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
 				struct msghdr *msg,
-				size_t len)
+				size_t len, int flags)
 {
-	struct sk_buff *skb;
+	struct sk_buff *skb, *tmp;
 	int copied = 0;
 
-	while ((skb = skb_peek(&msk->receive_queue)) != NULL) {
+	skb_queue_walk_safe(&msk->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);
@@ -1760,15 +1760,18 @@ static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
 		copied += count;
 
 		if (count < data_len) {
-			MPTCP_SKB_CB(skb)->offset += count;
+			if (!(flags & MSG_PEEK))
+				MPTCP_SKB_CB(skb)->offset += count;
 			break;
 		}
 
-		/* we will bulk release the skb memory later */
-		skb->destructor = NULL;
-		msk->rmem_released += skb->truesize;
-		__skb_unlink(skb, &msk->receive_queue);
-		__kfree_skb(skb);
+		if (!(flags & MSG_PEEK)) {
+			/* we will bulk release the skb memory later */
+			skb->destructor = NULL;
+			msk->rmem_released += skb->truesize;
+			__skb_unlink(skb, &msk->receive_queue);
+			__kfree_skb(skb);
+		}
 
 		if (copied >= len)
 			break;
@@ -1945,7 +1948,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 	int target;
 	long timeo;
 
-	if (msg->msg_flags & ~(MSG_WAITALL | MSG_DONTWAIT))
+	if (flags & ~(MSG_PEEK | MSG_WAITALL | MSG_DONTWAIT))
 		return -EOPNOTSUPP;
 
 	mptcp_lock_sock(sk, __mptcp_splice_receive_queue(sk));
@@ -1962,7 +1965,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);
+		bytes_read = __mptcp_recvmsg_mskq(msk, msg, len - copied, flags);
 		if (unlikely(bytes_read < 0)) {
 			if (!copied)
 				copied = bytes_read;
@@ -2046,7 +2049,8 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 	pr_debug("msk=%p data_ready=%d rx queue empty=%d copied=%d",
 		 msk, test_bit(MPTCP_DATA_READY, &msk->flags),
 		 skb_queue_empty_lockless(&sk->sk_receive_queue), copied);
-	mptcp_rcv_space_adjust(msk, copied);
+	if (!(flags & MSG_PEEK))
+		mptcp_rcv_space_adjust(msk, copied);
 
 	release_sock(sk);
 	return copied;
-- 
1.8.3.1

[PATCH v4 2/2] selftests: mptcp: add a test case for MSG_PEEK

[Yonglong Li]

Extend mptcp_connect tool with MSG_PEEK support and add a test case in
mptcp_connect.sh that checks the data recvived from/after recv() with
MSG_PEEK.

Signed-off-by: Yonglong Li <liyonglong@chinatelecom.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.c  | 48 +++++++++++++++++++++-
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 17 ++++++++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 2f207cf..d88e1fd 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -45,7 +45,14 @@ enum cfg_mode {
 	CFG_MODE_SENDFILE,
 };
 
+enum cfg_peek {
+	CFG_NONE_PEEK,
+	CFG_WITH_PEEK,
+	CFG_AFTER_PEEK,
+};
+
 static enum cfg_mode cfg_mode = CFG_MODE_POLL;
+static enum cfg_peek cfg_peek = CFG_NONE_PEEK;
 static const char *cfg_host;
 static const char *cfg_port	= "12000";
 static int cfg_sock_proto	= IPPROTO_MPTCP;
@@ -73,6 +80,8 @@ static void die_usage(void)
 	fprintf(stderr, "\t-M mark -- set socket packet mark\n");
 	fprintf(stderr, "\t-u -- check mptcp ulp\n");
 	fprintf(stderr, "\t-w num -- wait num sec before closing the socket\n");
+	fprintf(stderr,
+		"\t-P [saveWithPeek|saveAfterPeek] -- save data with/after MSG_PEEK form tcp socket\n");
 	exit(1);
 }
 
@@ -331,6 +340,8 @@ static size_t do_write(const int fd, char *buf, const size_t len)
 
 static ssize_t do_rnd_read(const int fd, char *buf, const size_t len)
 {
+	int ret = 0;
+	char tmp[16384];
 	size_t cap = rand();
 
 	cap &= 0xffff;
@@ -340,7 +351,17 @@ static ssize_t do_rnd_read(const int fd, char *buf, const size_t len)
 	else if (cap > len)
 		cap = len;
 
-	return read(fd, buf, cap);
+	if (cfg_peek == CFG_WITH_PEEK) {
+		ret = recv(fd, buf, cap, MSG_PEEK);
+		ret = (ret < 0) ? ret : read(fd, tmp, ret);
+	} else if (cfg_peek == CFG_AFTER_PEEK) {
+		ret = recv(fd, buf, cap, MSG_PEEK);
+		ret = (ret < 0) ? ret : read(fd, buf, cap);
+	} else {
+		ret = read(fd, buf, cap);
+	}
+
+	return ret;
 }
 
 static void set_nonblock(int fd)
@@ -819,6 +840,26 @@ int parse_mode(const char *mode)
 	return 0;
 }
 
+int parse_peek(const char *mode)
+{
+	if (!strcasecmp(mode, "saveWithPeek"))
+		return CFG_WITH_PEEK;
+	if (!strcasecmp(mode, "saveAfterPeek"))
+		return CFG_AFTER_PEEK;
+
+	fprintf(stderr, "Unknown: %s\n", mode);
+	fprintf(stderr, "Supported MSG_PEEK mode are:\n");
+	fprintf(stderr,
+		"\t\t\"saveWithPeek\" - recv data with flags 'MSG_PEEK' and save the peek data into file\n");
+	fprintf(stderr,
+		"\t\t\"saveAfterPeek\" - read and save data into file after recv with flags 'MSG_PEEK'\n");
+
+	die_usage();
+
+	/* silence compiler warning */
+	return 0;
+}
+
 static int parse_int(const char *size)
 {
 	unsigned long s;
@@ -846,7 +887,7 @@ static void parse_opts(int argc, char **argv)
 {
 	int c;
 
-	while ((c = getopt(argc, argv, "6jr:lp:s:hut:m:S:R:w:M:")) != -1) {
+	while ((c = getopt(argc, argv, "6jr:lp:s:hut:m:S:R:w:M:P:")) != -1) {
 		switch (c) {
 		case 'j':
 			cfg_join = true;
@@ -899,6 +940,9 @@ static void parse_opts(int argc, char **argv)
 		case 'M':
 			cfg_mark = strtol(optarg, NULL, 0);
 			break;
+		case 'P':
+			cfg_peek = parse_peek(optarg);
+			break;
 		}
 	}
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 385cdc9..319d166 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -23,6 +23,8 @@ rcvbuf=0
 options_log=true
 do_tcp=0
 filesize=0
+peekmode=""
+testpeek=false
 
 if [ $tc_loss -eq 100 ];then
 	tc_loss=1%
@@ -393,6 +395,10 @@ do_transfer()
 		extra_args="$extra_args -m $testmode"
 	fi
 
+	if $testpeek; then
+		extra_args="$extra_args -P $peekmode"
+	fi
+
 	if [ -n "$extra_args" ] && $options_log; then
 		options_log=false
 		echo "INFO: extra options: $extra_args"
@@ -732,6 +738,17 @@ for sender in $ns1 $ns2 $ns3 $ns4;do
 	run_tests "$ns4" $sender dead:beef:3::1
 done
 
+testpeek=true
+options_log=true
+peekmode="saveAfterPeek"
+run_tests_lo "$ns1" "$ns1" 10.0.1.1 1
+run_tests_lo "$ns1" $sender dead:beef:1::1 1
+
+options_log=true
+peekmode="saveAfterPeek"
+run_tests_lo "$ns1" "$ns1" 10.0.1.1 1
+run_tests_lo "$ns1" $sender dead:beef:1::1 1
+
 time_end=$(date +%s)
 time_run=$((time_end-time_start))
 
-- 
1.8.3.1

Re: [PATCH v4 1/2] mptcp: add MSG_PEEK support

[Paolo Abeni]

On Mon, 2021-04-19 at 10:53 +0800, Yonglong Li wrote:
> This patch adds support for MSG_PEEK flag. Packets are not removed
> from the receive_queue if MSG_PEEK set in recv() system call.
> 
> Signed-off-by: Yonglong Li <liyonglong@chinatelecom.cn>
> ---
>  net/mptcp/protocol.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 16d73ec..75f4420 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1739,12 +1739,12 @@ static void mptcp_wait_data(struct sock *sk, long *timeo)
>  
>  static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
>  				struct msghdr *msg,
> -				size_t len)
> +				size_t len, int flags)
>  {
> -	struct sk_buff *skb;
> +	struct sk_buff *skb, *tmp;
>  	int copied = 0;
>  
> -	while ((skb = skb_peek(&msk->receive_queue)) != NULL) {
> +	skb_queue_walk_safe(&msk->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);
> @@ -1760,15 +1760,18 @@ static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
>  		copied += count;
>  
>  		if (count < data_len) {
> -			MPTCP_SKB_CB(skb)->offset += count;
> +			if (!(flags & MSG_PEEK))
> +				MPTCP_SKB_CB(skb)->offset += count;
>  			break;
>  		}
>  
> -		/* we will bulk release the skb memory later */
> -		skb->destructor = NULL;
> -		msk->rmem_released += skb->truesize;
> -		__skb_unlink(skb, &msk->receive_queue);
> -		__kfree_skb(skb);
> +		if (!(flags & MSG_PEEK)) {
> +			/* we will bulk release the skb memory later */
> +			skb->destructor = NULL;
> +			msk->rmem_released += skb->truesize;
> +			__skb_unlink(skb, &msk->receive_queue);
> +			__kfree_skb(skb);
> +		}
>  
>  		if (copied >= len)
>  			break;
> @@ -1945,7 +1948,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  	int target;
>  	long timeo;
>  
> -	if (msg->msg_flags & ~(MSG_WAITALL | MSG_DONTWAIT))
> +	if (flags & ~(MSG_PEEK | MSG_WAITALL | MSG_DONTWAIT))
>  		return -EOPNOTSUPP;
>  
>  	mptcp_lock_sock(sk, __mptcp_splice_receive_queue(sk));
> @@ -1962,7 +1965,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);
> +		bytes_read = __mptcp_recvmsg_mskq(msk, msg, len - copied, flags);
>  		if (unlikely(bytes_read < 0)) {
>  			if (!copied)
>  				copied = bytes_read;
> @@ -2046,7 +2049,8 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  	pr_debug("msk=%p data_ready=%d rx queue empty=%d copied=%d",
>  		 msk, test_bit(MPTCP_DATA_READY, &msk->flags),
>  		 skb_queue_empty_lockless(&sk->sk_receive_queue), copied);
> -	mptcp_rcv_space_adjust(msk, copied);
> +	if (!(flags & MSG_PEEK))
> +		mptcp_rcv_space_adjust(msk, copied);
>  
>  	release_sock(sk);
>  	return copied;

LGTM,

Acked-by: Paolo Abeni <pabeni@redhat.com>

Note that the first chunk mptcp_recvmsg() will conflict with other
pending changes ("mptcp: ignore unsupported msg flags"), but I guess
Mattbe can handle that ;) - likely no need to resubmit.

Thanks!

Paolo

Re: [PATCH v4 2/2] selftests: mptcp: add a test case for MSG_PEEK

[Paolo Abeni]

On Mon, 2021-04-19 at 10:53 +0800, Yonglong Li wrote:
> Extend mptcp_connect tool with MSG_PEEK support and add a test case in
> mptcp_connect.sh that checks the data recvived from/after recv() with
> MSG_PEEK.
> 
> Signed-off-by: Yonglong Li <liyonglong@chinatelecom.cn>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.c  | 48 +++++++++++++++++++++-
>  tools/testing/selftests/net/mptcp/mptcp_connect.sh | 17 ++++++++
>  2 files changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 2f207cf..d88e1fd 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -45,7 +45,14 @@ enum cfg_mode {
>  	CFG_MODE_SENDFILE,
>  };
>  
> +enum cfg_peek {
> +	CFG_NONE_PEEK,
> +	CFG_WITH_PEEK,
> +	CFG_AFTER_PEEK,
> +};
> +
>  static enum cfg_mode cfg_mode = CFG_MODE_POLL;
> +static enum cfg_peek cfg_peek = CFG_NONE_PEEK;
>  static const char *cfg_host;
>  static const char *cfg_port	= "12000";
>  static int cfg_sock_proto	= IPPROTO_MPTCP;
> @@ -73,6 +80,8 @@ static void die_usage(void)
>  	fprintf(stderr, "\t-M mark -- set socket packet mark\n");
>  	fprintf(stderr, "\t-u -- check mptcp ulp\n");
>  	fprintf(stderr, "\t-w num -- wait num sec before closing the socket\n");
> +	fprintf(stderr,
> +		"\t-P [saveWithPeek|saveAfterPeek] -- save data with/after MSG_PEEK form tcp socket\n");
>  	exit(1);
>  }
>  
> @@ -331,6 +340,8 @@ static size_t do_write(const int fd, char *buf, const size_t len)
>  
>  static ssize_t do_rnd_read(const int fd, char *buf, const size_t len)
>  {
> +	int ret = 0;
> +	char tmp[16384];
>  	size_t cap = rand();
>  
>  	cap &= 0xffff;
> @@ -340,7 +351,17 @@ static ssize_t do_rnd_read(const int fd, char *buf, const size_t len)
>  	else if (cap > len)
>  		cap = len;
>  
> -	return read(fd, buf, cap);
> +	if (cfg_peek == CFG_WITH_PEEK) {
> +		ret = recv(fd, buf, cap, MSG_PEEK);
> +		ret = (ret < 0) ? ret : read(fd, tmp, ret);
> +	} else if (cfg_peek == CFG_AFTER_PEEK) {
> +		ret = recv(fd, buf, cap, MSG_PEEK);
> +		ret = (ret < 0) ? ret : read(fd, buf, cap);
> +	} else {
> +		ret = read(fd, buf, cap);
> +	}
> +
> +	return ret;
>  }
>  
>  static void set_nonblock(int fd)
> @@ -819,6 +840,26 @@ int parse_mode(const char *mode)
>  	return 0;
>  }
>  
> +int parse_peek(const char *mode)
> +{
> +	if (!strcasecmp(mode, "saveWithPeek"))
> +		return CFG_WITH_PEEK;
> +	if (!strcasecmp(mode, "saveAfterPeek"))
> +		return CFG_AFTER_PEEK;
> +
> +	fprintf(stderr, "Unknown: %s\n", mode);
> +	fprintf(stderr, "Supported MSG_PEEK mode are:\n");
> +	fprintf(stderr,
> +		"\t\t\"saveWithPeek\" - recv data with flags 'MSG_PEEK' and save the peek data into file\n");
> +	fprintf(stderr,
> +		"\t\t\"saveAfterPeek\" - read and save data into file after recv with flags 'MSG_PEEK'\n");
> +
> +	die_usage();
> +
> +	/* silence compiler warning */
> +	return 0;
> +}
> +
>  static int parse_int(const char *size)
>  {
>  	unsigned long s;
> @@ -846,7 +887,7 @@ static void parse_opts(int argc, char **argv)
>  {
>  	int c;
>  
> -	while ((c = getopt(argc, argv, "6jr:lp:s:hut:m:S:R:w:M:")) != -1) {
> +	while ((c = getopt(argc, argv, "6jr:lp:s:hut:m:S:R:w:M:P:")) != -1) {
>  		switch (c) {
>  		case 'j':
>  			cfg_join = true;
> @@ -899,6 +940,9 @@ static void parse_opts(int argc, char **argv)
>  		case 'M':
>  			cfg_mark = strtol(optarg, NULL, 0);
>  			break;
> +		case 'P':
> +			cfg_peek = parse_peek(optarg);
> +			break;
>  		}
>  	}
>  
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index 385cdc9..319d166 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> @@ -23,6 +23,8 @@ rcvbuf=0
>  options_log=true
>  do_tcp=0
>  filesize=0
> +peekmode=""
> +testpeek=false
>  
>  if [ $tc_loss -eq 100 ];then
>  	tc_loss=1%
> @@ -393,6 +395,10 @@ do_transfer()
>  		extra_args="$extra_args -m $testmode"
>  	fi
>  
> +	if $testpeek; then
> +		extra_args="$extra_args -P $peekmode"
> +	fi
> +
>  	if [ -n "$extra_args" ] && $options_log; then
>  		options_log=false
>  		echo "INFO: extra options: $extra_args"
> @@ -732,6 +738,17 @@ for sender in $ns1 $ns2 $ns3 $ns4;do
>  	run_tests "$ns4" $sender dead:beef:3::1
>  done
>  
> +testpeek=true
> +options_log=true
> +peekmode="saveAfterPeek"
> +run_tests_lo "$ns1" "$ns1" 10.0.1.1 1
> +run_tests_lo "$ns1" $sender dead:beef:1::1 1
> +
> +options_log=true
> +peekmode="saveAfterPeek"
> +run_tests_lo "$ns1" "$ns1" 10.0.1.1 1
> +run_tests_lo "$ns1" $sender dead:beef:1::1 1
> +
>  time_end=$(date +%s)
>  time_run=$((time_end-time_start))

LGTM! and to checkpatch, also!

Acked-by: Paolo Abeni <pabeni@redhat.com>

Re: [PATCH v4 1/2] mptcp: add MSG_PEEK support

[Matthieu Baerts]

Hi Yonglong, Paolo,

On 19/04/2021 04:53, Yonglong Li wrote:
> This patch adds support for MSG_PEEK flag. Packets are not removed
> from the receive_queue if MSG_PEEK set in recv() system call.

Thank you for the patches and the reviews!

Now in our tree:

- e7fc5d2f8302: mptcp: add MSG_PEEK support
- 3a43a9f8d6df: selftests: mptcp: add a test case for MSG_PEEK
- Results: 0bc569d1592d..41136bc7e8d8

Tests + export are in progress!

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net