2:25 p.m.
Paolo says he prefers direct hooking of SOL_SOCKET instead of copying
values in bind/listen/connect, so do that here.
Similar to the v1 patchset, except that first patch is new (adds
needed 'do not use generic handler' for mptcp sake),
second patch adds minimal SOL_SOCKET support (rest yields -EOPNOTSUPP).
Last patch is almost the same as v1 (only fixes an error check and
extends scope of sk lock).
2:25 p.m.
New subject: [PATCH mptcp-next v2 1/3] net: use mptcp setsockopt function for SOL_SOCKET on mptcp sockets
setsockopt(mptcp_fd, SOL_SOCKET, ...)... appears to work (returns 0),
but it has no effect -- this is because the MPTCP layer never has a
chance to copy the settings to the subflow socket.
Skip the generic handling for the mptcp case and return -EOPNOTSUPP.
Next patch adds handling for SOL_SOCKET to mptcp.
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/protocol.c | 3 +++
net/socket.c | 12 +++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index faa804f63c81..f9429c6f5b96 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1636,6 +1636,9 @@ static int mptcp_setsockopt(struct sock *sk, int level, int
optname,
pr_debug("msk=%p", msk);
+ if (level == SOL_SOCKET)
+ return -EOPNOTSUPP;
+
/* @@ the meaning of setsockopt() when the socket is connected and
* there are multiple subflows is not yet defined. It is up to the
* MPTCP-level socket to configure the subflows until the subflow
diff --git a/net/socket.c b/net/socket.c
index d5b555845b37..98b70fcaff19 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2091,6 +2091,16 @@ SYSCALL_DEFINE4(recv, int, fd, void __user *, ubuf, size_t, size,
return __sys_recvfrom(fd, ubuf, size, flags, NULL, NULL);
}
+static bool sock_use_custom_sol_socket(const struct socket *sock)
+{
+ const struct sock *sk = sock->sk;
+
+ /* Use sock->ops->setsockopt() for MPTCP */
+ return sk->sk_protocol == IPPROTO_MPTCP &&
+ sk->sk_type == SOCK_STREAM &&
+ (sk->sk_family == AF_INET || sk->sk_family == AF_INET6);
+}
+
/*
* Set a socket option. Because we don't know the option lengths we have
* to pass the user mode parameter for the protocols to sort out.
@@ -2129,7 +2139,7 @@ static int __sys_setsockopt(int fd, int level, int optname,
optval = (char __user __force *)kernel_optval;
}
- if (level == SOL_SOCKET)
+ if (level == SOL_SOCKET && !sock_use_custom_sol_socket(sock))
err =
sock_setsockopt(sock, level, optname, optval,
optlen);
--
2.26.2
10:50 p.m.
New subject: [PATCH mptcp-next v2 1/3] net: use mptcp setsockopt function for SOL_SOCKET on mptcp sockets
On Tue, 16 Jun 2020, Florian Westphal wrote:
setsockopt(mptcp_fd, SOL_SOCKET, ...)... appears to work (returns
0),
but it has no effect -- this is because the MPTCP layer never has a
chance to copy the settings to the subflow socket.
Skip the generic handling for the mptcp case and return -EOPNOTSUPP.
Next patch adds handling for SOL_SOCKET to mptcp.
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/protocol.c | 3 +++
net/socket.c | 12 +++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index faa804f63c81..f9429c6f5b96 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1636,6 +1636,9 @@ static int mptcp_setsockopt(struct sock *sk, int level, int
optname,
pr_debug("msk=%p", msk);
+ if (level == SOL_SOCKET)
+ return -EOPNOTSUPP;
+
/* @@ the meaning of setsockopt() when the socket is connected and
* there are multiple subflows is not yet defined. It is up to the
* MPTCP-level socket to configure the subflows until the subflow
diff --git a/net/socket.c b/net/socket.c
index d5b555845b37..98b70fcaff19 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2091,6 +2091,16 @@ SYSCALL_DEFINE4(recv, int, fd, void __user *, ubuf, size_t, size,
return __sys_recvfrom(fd, ubuf, size, flags, NULL, NULL);
}
+static bool sock_use_custom_sol_socket(const struct socket *sock)
+{
+ const struct sock *sk = sock->sk;
+
+ /* Use sock->ops->setsockopt() for MPTCP */
+ return sk->sk_protocol == IPPROTO_MPTCP &&
Is it worth using IS_ENABLED(CONFIG_MPTCP) first in this boolean
expression so this all compiles out when MPTCP is not enabled? On one hand
it's a microoptimization on a path where performance doesn't matter much,
but on the other hand we have tried to make things compile out where we
can.
Mat
+ sk->sk_type == SOCK_STREAM &&
+ (sk->sk_family == AF_INET || sk->sk_family == AF_INET6);
+}
+
/*
* Set a socket option. Because we don't know the option lengths we have
* to pass the user mode parameter for the protocols to sort out.
@@ -2129,7 +2139,7 @@ static int __sys_setsockopt(int fd, int level, int optname,
optval = (char __user __force *)kernel_optval;
}
- if (level == SOL_SOCKET)
+ if (level == SOL_SOCKET && !sock_use_custom_sol_socket(sock))
err =
sock_setsockopt(sock, level, optname, optval,
optlen);
--
2.26.2
--
Mat Martineau
Intel
11:01 p.m.
New subject: [PATCH mptcp-next v2 1/3] net: use mptcp setsockopt function for SOL_SOCKET on mptcp sockets
On Tue, 16 Jun 2020, Mat Martineau wrote:
On Tue, 16 Jun 2020, Florian Westphal wrote:
> setsockopt(mptcp_fd, SOL_SOCKET, ...)... appears to work (returns 0),
> but it has no effect -- this is because the MPTCP layer never has a
> chance to copy the settings to the subflow socket.
>
> Skip the generic handling for the mptcp case and return -EOPNOTSUPP.
> Next patch adds handling for SOL_SOCKET to mptcp.
>
> Signed-off-by: Florian Westphal <fw(a)strlen.de>
> ---
> net/mptcp/protocol.c | 3 +++
> net/socket.c | 12 +++++++++++-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index faa804f63c81..f9429c6f5b96 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1636,6 +1636,9 @@ static int mptcp_setsockopt(struct sock *sk, int
> level, int optname,
>
> pr_debug("msk=%p", msk);
>
> + if (level == SOL_SOCKET)
> + return -EOPNOTSUPP;
One more thing: calling sock_setsockopt() here would be a better
placeholder, depending on what you think about my comments on patch 2/3.
Mat
> +
> /* @@ the meaning of setsockopt() when the socket is connected and
> * there are multiple subflows is not yet defined. It is up to the
> * MPTCP-level socket to configure the subflows until the subflow
> diff --git a/net/socket.c b/net/socket.c
> index d5b555845b37..98b70fcaff19 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -2091,6 +2091,16 @@ SYSCALL_DEFINE4(recv, int, fd, void __user *, ubuf,
> size_t, size,
> return __sys_recvfrom(fd, ubuf, size, flags, NULL, NULL);
> }
>
> +static bool sock_use_custom_sol_socket(const struct socket *sock)
> +{
> + const struct sock *sk = sock->sk;
> +
> + /* Use sock->ops->setsockopt() for MPTCP */
> + return sk->sk_protocol == IPPROTO_MPTCP &&
Is it worth using IS_ENABLED(CONFIG_MPTCP) first in this boolean expression
so this all compiles out when MPTCP is not enabled? On one hand it's a
microoptimization on a path where performance doesn't matter much, but on the
other hand we have tried to make things compile out where we can.
Mat
> + sk->sk_type == SOCK_STREAM &&
> + (sk->sk_family == AF_INET || sk->sk_family == AF_INET6);
> +}
> +
> /*
> * Set a socket option. Because we don't know the option lengths we have
> * to pass the user mode parameter for the protocols to sort out.
> @@ -2129,7 +2139,7 @@ static int __sys_setsockopt(int fd, int level, int
> optname,
> optval = (char __user __force *)kernel_optval;
> }
>
> - if (level == SOL_SOCKET)
> + if (level == SOL_SOCKET && !sock_use_custom_sol_socket(sock))
> err =
> sock_setsockopt(sock, level, optname, optval,
> optlen);
> --
> 2.26.2
--
Mat Martineau
Intel
2:25 p.m.
New subject: [PATCH mptcp-next v2 2/3] mptcp: add REUSEADDR/REUSEADDR support
This will e.g. make 'sshd restart' work when MPTCP is used, as we will
now set this option on the listener socket instead of only the mptcp
socket (where it has no effect).
We still need to copy the setting to the master socket so that a
subsequent getsockopt() returns the expected value.
Reported-by: Christoph Paasch <cpaasch(a)apple.com>
Suggested-by: Paolo Abeni <pabeni(a)redhat.com>
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/protocol.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f9429c6f5b96..b2338680497a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1628,6 +1628,37 @@ static void mptcp_destroy(struct sock *sk)
sk_sockets_allocated_dec(sk);
}
+static int mptcp_setsockopt_sol_socket(struct mptcp_sock *msk, int optname,
+ char __user *optval, unsigned int optlen)
+{
+ struct sock *sk = (struct sock *)msk;
+ int ret = -EOPNOTSUPP;
+ struct socket *ssock;
+
+ switch (optname) {
+ case SO_REUSEPORT:
+ case SO_REUSEADDR:
+ lock_sock(sk);
+ ssock = __mptcp_socket_create(msk, MPTCP_SAME_STATE);
+ if (IS_ERR(ssock)) {
+ release_sock(sk);
+ return PTR_ERR(ssock);
+ }
+
+ ret = sock_setsockopt(ssock, SOL_SOCKET, optname, optval, optlen);
+ if (ret == 0) {
+ if (optname == SO_REUSEPORT)
+ sk->sk_reuseport = ssock->sk->sk_reuseport;
+ else if (optname == SO_REUSEADDR)
+ sk->sk_reuse = ssock->sk->sk_reuse;
+ }
+ release_sock(sk);
+ break;
+ }
+
+ return ret;
+}
+
static int mptcp_setsockopt(struct sock *sk, int level, int optname,
char __user *optval, unsigned int optlen)
{
@@ -1637,7 +1668,8 @@ static int mptcp_setsockopt(struct sock *sk, int level, int
optname,
pr_debug("msk=%p", msk);
if (level == SOL_SOCKET)
- return -EOPNOTSUPP;
+ return mptcp_setsockopt_sol_socket(msk, optname,
+ optval, optlen);
/* @@ the meaning of setsockopt() when the socket is connected and
* there are multiple subflows is not yet defined. It is up to the
--
2.26.2
10:59 p.m.
New subject: [PATCH mptcp-next v2 2/3] mptcp: add REUSEADDR/REUSEADDR support
On Tue, 16 Jun 2020, Florian Westphal wrote:
This will e.g. make 'sshd restart' work when MPTCP is used,
as we will
now set this option on the listener socket instead of only the mptcp
socket (where it has no effect).
We still need to copy the setting to the master socket so that a
subsequent getsockopt() returns the expected value.
Reported-by: Christoph Paasch <cpaasch(a)apple.com>
Suggested-by: Paolo Abeni <pabeni(a)redhat.com>
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/protocol.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f9429c6f5b96..b2338680497a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1628,6 +1628,37 @@ static void mptcp_destroy(struct sock *sk)
sk_sockets_allocated_dec(sk);
}
+static int mptcp_setsockopt_sol_socket(struct mptcp_sock *msk, int optname,
+ char __user *optval, unsigned int optlen)
+{
+ struct sock *sk = (struct sock *)msk;
+ int ret = -EOPNOTSUPP;
+ struct socket *ssock;
+
+ switch (optname) {
+ case SO_REUSEPORT:
+ case SO_REUSEADDR:
+ lock_sock(sk);
+ ssock = __mptcp_socket_create(msk, MPTCP_SAME_STATE);
+ if (IS_ERR(ssock)) {
+ release_sock(sk);
+ return PTR_ERR(ssock);
+ }
+
+ ret = sock_setsockopt(ssock, SOL_SOCKET, optname, optval, optlen);
+ if (ret == 0) {
+ if (optname == SO_REUSEPORT)
+ sk->sk_reuseport = ssock->sk->sk_reuseport;
+ else if (optname == SO_REUSEADDR)
+ sk->sk_reuse = ssock->sk->sk_reuse;
+ }
+ release_sock(sk);
+ break;
Returning EOPNOTSUPP for all the other SOL_SOCKET options would be a
significant change in behavior from MPTCP in 5.7 (or 5.8, depending on
whether this goes to net or net-next), and also different from every other
kind of socket. Even of not all the SOL_SOCKET options end up doing much
yet, it seems more consistent for userspace if this switch statement has a
default case that calls sock_setsockopt() on the msk. What do you think?
Mat
+ }
+
+ return ret;
+}
+
static int mptcp_setsockopt(struct sock *sk, int level, int optname,
char __user *optval, unsigned int optlen)
{
@@ -1637,7 +1668,8 @@ static int mptcp_setsockopt(struct sock *sk, int level, int
optname,
pr_debug("msk=%p", msk);
if (level == SOL_SOCKET)
- return -EOPNOTSUPP;
+ return mptcp_setsockopt_sol_socket(msk, optname,
+ optval, optlen);
/* @@ the meaning of setsockopt() when the socket is connected and
* there are multiple subflows is not yet defined. It is up to the
--
2.26.2
--
Mat Martineau
Intel
2:25 p.m.
New subject: [PATCH mptcp-next v2 3/3] mptcp: support IPV6_V6ONLY setsockopt
Without this, Opensshd fails to open an ipv6 socket listening
socket:
error: setsockopt IPV6_V6ONLY: Operation not supported
error: Bind to port 22 on :: failed: Address already in use.
Opensshd opens an ipv4 and and ipv6 listening socket, but because
IPV6_V6ONLY setsockopt fails, the port number is already in use.
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/protocol.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index b2338680497a..9e7fc44f405d 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1659,6 +1659,33 @@ static int mptcp_setsockopt_sol_socket(struct mptcp_sock *msk, int
optname,
return ret;
}
+static int mptcp_setsockopt_v6(struct mptcp_sock *msk, int optname,
+ char __user *optval, unsigned int optlen)
+{
+ struct sock *sk = (struct sock *)msk;
+ int ret = -EOPNOTSUPP;
+ struct socket *ssock;
+
+ switch (optname) {
+ case IPV6_V6ONLY:
+ lock_sock(sk);
+ ssock = __mptcp_socket_create(msk, MPTCP_SAME_STATE);
+ if (IS_ERR(ssock)) {
+ release_sock(sk);
+ return PTR_ERR(ssock);
+ }
+
+ ret = tcp_setsockopt(ssock->sk, SOL_IPV6, optname, optval, optlen);
+ if (ret == 0)
+ sk->sk_ipv6only = ssock->sk->sk_ipv6only;
+
+ release_sock(sk);
+ break;
+ }
+
+ return ret;
+}
+
static int mptcp_setsockopt(struct sock *sk, int level, int optname,
char __user *optval, unsigned int optlen)
{
@@ -1684,6 +1711,9 @@ static int mptcp_setsockopt(struct sock *sk, int level, int
optname,
return tcp_setsockopt(ssock->sk, level, optname, optval,
optlen);
+ if (level == SOL_IPV6)
+ return mptcp_setsockopt_v6(msk, optname, optval, optlen);
+
return -EOPNOTSUPP;
}
--
2.26.2
5:46 p.m.
On 06/16/20 - 14:25, Florian Westphal wrote:
Paolo says he prefers direct hooking of SOL_SOCKET instead of
copying
values in bind/listen/connect, so do that here.
Similar to the v1 patchset, except that first patch is new (adds
needed 'do not use generic handler' for mptcp sake),
second patch adds minimal SOL_SOCKET support (rest yields -EOPNOTSUPP).
Last patch is almost the same as v1 (only fixes an error check and
extends scope of sk lock).
Nice! I gave it a shot and restarting sshd now works fine on my test-machines!
I also am running syzkaller on these to see if anything breaks.
Christoph
705
days inactive
705
days old
7 comments
3 participants
participants (3)
-
Christoph Paasch -
Florian Westphal -
Mat Martineau