[PATCH net-next 0/3] mptcp: improve MSG_* flags handling

[PATCH net-next 0/3] mptcp: improve MSG_* flags handling

[Paolo Abeni]

The current handling is a bit fuzzy. Sometimes we silently ignore,
sometimes we bail on unsupported even on no-op ones.

Let's silently provide a provide a reasonable implementation or 
bail (currently only for MSG_FASTOPEN) for the flags with user-visible
side effects. Finally silently ignore the unsupported flags with no
user visible side effects.

Paolo Abeni (3):
  mptcp: implement dummy MSG_ERRQUEUE support
  mptcp: implement MSG_TRUNC support
  mptcp: ignore unsupported msg flags

 net/mptcp/protocol.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

-- 
2.26.2

[PATCH net-next 1/3] mptcp: implement dummy MSG_ERRQUEUE support

[Paolo Abeni]

mptcp_recvmsg() currently silently ignores MSG_ERRQUEUE, returning
input data instead of error cmsg.

This change provides a dummy implementation for MSG_ERRQUEUE - always
returns no data. That is consistent with the current lack of a suitable
IP_RECVERR setsockopt() support.

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

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 0437fe4f0552..ed9ada31f2da 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1945,6 +1945,10 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 	int target;
 	long timeo;
 
+	/* MSG_ERRQUEUE is really a no-op till we support IP_RECVERR */
+	if (unlikely(flags & MSG_ERRQUEUE))
+		return inet_recv_error(sk, msg, len, addr_len);
+
 	if (msg->msg_flags & ~(MSG_WAITALL | MSG_DONTWAIT))
 		return -EOPNOTSUPP;
 
-- 
2.26.2

[PATCH net-next 2/3] mptcp: implement MSG_TRUNC support

[Paolo Abeni]

The mentioned flag is currently silenlty ignored. This
change implementes the TCP-like behaviour, dropping the
pending data up to the specified length.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Note: this will conflict with the pending MSG_PEEK,
with hopefully simple conflict resolutions (same change in both patches)
---
 net/mptcp/protocol.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index ed9ada31f2da..e6d1c293a615 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1739,7 +1739,7 @@ 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;
 	int copied = 0;
@@ -1750,11 +1750,13 @@ static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
 		u32 count = min_t(size_t, len - copied, data_len);
 		int err;
 
-		err = skb_copy_datagram_msg(skb, offset, msg, count);
-		if (unlikely(err < 0)) {
-			if (!copied)
-				return err;
-			break;
+		if (!(flags & MSG_TRUNC)) {
+			err = skb_copy_datagram_msg(skb, offset, msg, count);
+			if (unlikely(err < 0)) {
+				if (!copied)
+					return err;
+				break;
+			}
 		}
 
 		copied += count;
@@ -1966,7 +1968,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;
-- 
2.26.2

[PATCH net-next 3/3] mptcp: ignore unsupported msg flags

[Paolo Abeni]

Currently mptcp_sendmsg() fails with EOPNOTSUPP if the
user-space provides some unsupported flag. That is unexpected
and may foul existing applications migrated to MPTCP, which
expect a different behavior.

Change the mentioned function to silently ignore the unsupported
flags except MSG_FASTOPEN. This is the only flags currently not
supported by MPTCP with user-space visible side-effects.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/162
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
 - bail on MSG_FASTOPEN only

Note: this will conflict with the pending MSG_PEEK changes, with
hopefully simple resolution
---
 net/mptcp/protocol.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index e6d1c293a615..04cb937630f6 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1614,8 +1614,12 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	int ret = 0;
 	long timeo;
 
-	if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL))
-		return -EOPNOTSUPP;
+	/* we don't support FASTOPEN yet */
+	if (msg->msg_flags & MSG_FASTOPEN)
+		return -ENOTSUPP;
+
+	/* silently ignore everything else */
+	msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL;
 
 	mptcp_lock_sock(sk, __mptcp_wmem_reserve(sk, min_t(size_t, 1 << 20, len)));
 
@@ -1951,9 +1955,6 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 	if (unlikely(flags & MSG_ERRQUEUE))
 		return inet_recv_error(sk, msg, len, addr_len);
 
-	if (msg->msg_flags & ~(MSG_WAITALL | MSG_DONTWAIT))
-		return -EOPNOTSUPP;
-
 	mptcp_lock_sock(sk, __mptcp_splice_receive_queue(sk));
 	if (unlikely(sk->sk_state == TCP_LISTEN)) {
 		copied = -ENOTCONN;
-- 
2.26.2

Re: [PATCH net-next 0/3] mptcp: improve MSG_* flags handling

[Mat Martineau]

On Thu, 15 Apr 2021, Paolo Abeni wrote:

> The current handling is a bit fuzzy. Sometimes we silently ignore,
> sometimes we bail on unsupported even on no-op ones.
>
> Let's silently provide a provide a reasonable implementation or
> bail (currently only for MSG_FASTOPEN) for the flags with user-visible
> side effects. Finally silently ignore the unsupported flags with no
> user visible side effects.
>
> Paolo Abeni (3):
>  mptcp: implement dummy MSG_ERRQUEUE support
>  mptcp: implement MSG_TRUNC support
>  mptcp: ignore unsupported msg flags
>
> net/mptcp/protocol.c | 29 ++++++++++++++++++-----------
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> -- 
> 2.26.2

Hi Paolo -

Thanks, series looks good to me.

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

--
Mat Martineau
Intel

Re: [PATCH net-next 0/3] mptcp: improve MSG_* flags handling

[Matthieu Baerts]

Hi Paolo, Mat,

On 15/04/2021 18:02, Paolo Abeni wrote:
> The current handling is a bit fuzzy. Sometimes we silently ignore,
> sometimes we bail on unsupported even on no-op ones.
> 
> Let's silently provide a provide a reasonable implementation or 
> bail (currently only for MSG_FASTOPEN) for the flags with user-visible
> side effects. Finally silently ignore the unsupported flags with no
> user visible side effects.

Thank you for the patches and the review!

Applied in our tree with Mat's RvB tags, without a spelling mistake and
a switch from ENOTSUPP to EOPNOTSUPP:

- 79f8f01a2fbe: mptcp: implement dummy MSG_ERRQUEUE support



- 5ea72f542f69: mptcp: implement MSG_TRUNC support



- faf109acd959: mptcp: ignore unsupported msg flags



- Results: 8da9d3d6f081..0bc569d1592d

Tests + export are in progress!

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