In the long run this will simplify the mptcp code and will
allow for more consistent behavior. Move the first subflow
allocation out of the sock->init ops into the __mptcp_nmpc_socket()
helper.
Since the first subflow creation can now happen after the first
setsockopt() we additionally need to invoke mptcp_sockopt_sync()
on it.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/pm_netlink.c | 4 +--
net/mptcp/protocol.c | 57 +++++++++++++++++++++++++++---------------
net/mptcp/protocol.h | 2 +-
net/mptcp/sockopt.c | 18 ++++++-------
4 files changed, 49 insertions(+), 32 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index e82a112b8779..290f88fffe5f 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1014,8 +1014,8 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
return -EINVAL;
ssock = __mptcp_nmpc_socket(msk);
- if (!ssock)
- return -EINVAL;
+ if (IS_ERR(ssock))
+ return PTR_ERR(ssock);
mptcp_info2sockaddr(&entry->addr, &addr, entry->addr.family);
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 9c4c729bf271..1b93624db62d 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -49,18 +49,6 @@ static void __mptcp_check_send_data_fin(struct sock *sk);
DEFINE_PER_CPU(struct mptcp_delegated_action, mptcp_delegated_actions);
static struct net_device mptcp_napi_dev;
-/* If msk has an initial subflow socket, and the MP_CAPABLE handshake has not
- * completed yet or has failed, return the subflow socket.
- * Otherwise return NULL.
- */
-struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk)
-{
- if (!msk->subflow || READ_ONCE(msk->can_ack))
- return NULL;
-
- return msk->subflow;
-}
-
/* Returns end sequence number of the receiver's advertised window */
static u64 mptcp_wnd_end(const struct mptcp_sock *msk)
{
@@ -116,6 +104,31 @@ static int __mptcp_socket_create(struct mptcp_sock *msk)
return 0;
}
+/* Returns the first subflow socket if available and the MPC
+ * handshake is not started yet.
+ */
+struct socket *__mptcp_nmpc_socket(struct mptcp_sock *msk)
+{
+ struct sock *sk = (struct sock *)msk;
+ int ret;
+
+ if (!((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)))
+ return ERR_PTR(-EINVAL);
+
+ if (!msk->subflow) {
+ if (msk->first)
+ return ERR_PTR(-EINVAL);
+
+ ret = __mptcp_socket_create(msk);
+ if (ret)
+ return ERR_PTR(ret);
+
+ mptcp_sockopt_sync(msk, msk->first);
+ }
+
+ return msk->subflow;
+}
+
static void mptcp_drop(struct sock *sk, struct sk_buff *skb)
{
sk_drops_add(sk, skb);
@@ -1699,6 +1712,7 @@ static int mptcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
{
unsigned int saved_flags = msg->msg_flags;
struct mptcp_sock *msk = mptcp_sk(sk);
+ struct socket *ssock;
struct sock *ssk;
int ret;
@@ -1708,8 +1722,11 @@ static int mptcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
* Since the defer_connect flag is cleared after the first succsful
* fastopen attempt, no need to check for additional subflow status.
*/
- if (msg->msg_flags & MSG_FASTOPEN && !__mptcp_nmpc_socket(msk))
- return -EINVAL;
+ if (msg->msg_flags & MSG_FASTOPEN) {
+ ssock = __mptcp_nmpc_socket(msk);
+ if (IS_ERR(ssock))
+ return PTR_ERR(ssock);
+ }
if (!msk->first)
return -EINVAL;
@@ -3592,8 +3609,8 @@ static int mptcp_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
int err = -EINVAL;
ssock = __mptcp_nmpc_socket(msk);
- if (!ssock)
- return -EINVAL;
+ if (IS_ERR(ssock))
+ return PTR_ERR(ssock);
mptcp_token_destroy(msk);
inet_sk_state_store(sk, TCP_SYN_SENT);
@@ -3681,8 +3698,8 @@ static int mptcp_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
lock_sock(sock->sk);
ssock = __mptcp_nmpc_socket(msk);
- if (!ssock) {
- err = -EINVAL;
+ if (IS_ERR(ssock)) {
+ err = PTR_ERR(ssock);
goto unlock;
}
@@ -3718,8 +3735,8 @@ static int mptcp_listen(struct socket *sock, int backlog)
lock_sock(sk);
ssock = __mptcp_nmpc_socket(msk);
- if (!ssock) {
- err = -EINVAL;
+ if (IS_ERR(ssock)) {
+ err = PTR_ERR(ssock);
goto unlock;
}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 5f1a30959b5c..3a055438c65e 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -632,7 +632,7 @@ void __mptcp_subflow_send_ack(struct sock *ssk);
void mptcp_subflow_reset(struct sock *ssk);
void mptcp_subflow_queue_clean(struct sock *sk, struct sock *ssk);
void mptcp_sock_graft(struct sock *sk, struct socket *parent);
-struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk);
+struct socket *__mptcp_nmpc_socket(struct mptcp_sock *msk);
bool __mptcp_close(struct sock *sk, long timeout);
void mptcp_cancel_work(struct sock *sk);
void mptcp_set_owner_r(struct sk_buff *skb, struct sock *sk);
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 9986681aaf40..022a6cad00c1 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -301,9 +301,9 @@ static int mptcp_setsockopt_sol_socket(struct mptcp_sock *msk, int optname,
case SO_BINDTOIFINDEX:
lock_sock(sk);
ssock = __mptcp_nmpc_socket(msk);
- if (!ssock) {
+ if (IS_ERR(ssock)) {
release_sock(sk);
- return -EINVAL;
+ return PTR_ERR(ssock);
}
ret = sock_setsockopt(ssock, SOL_SOCKET, optname, optval, optlen);
@@ -396,9 +396,9 @@ static int mptcp_setsockopt_v6(struct mptcp_sock *msk, int optname,
case IPV6_FREEBIND:
lock_sock(sk);
ssock = __mptcp_nmpc_socket(msk);
- if (!ssock) {
+ if (IS_ERR(ssock)) {
release_sock(sk);
- return -EINVAL;
+ return PTR_ERR(ssock);
}
ret = tcp_setsockopt(ssock->sk, SOL_IPV6, optname, optval, optlen);
@@ -693,9 +693,9 @@ static int mptcp_setsockopt_sol_ip_set_transparent(struct mptcp_sock *msk, int o
lock_sock(sk);
ssock = __mptcp_nmpc_socket(msk);
- if (!ssock) {
+ if (IS_ERR(ssock)) {
release_sock(sk);
- return -EINVAL;
+ return PTR_ERR(ssock);
}
issk = inet_sk(ssock->sk);
@@ -764,8 +764,8 @@ static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
/* Limit to first subflow, before the connection establishment */
sock = __mptcp_nmpc_socket(msk);
- if (!sock)
- return -EINVAL;
+ if (IS_ERR(sock))
+ return PTR_ERR(sock);
return tcp_setsockopt(sock->sk, level, optname, optval, optlen);
}
@@ -865,7 +865,7 @@ static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
}
ssock = __mptcp_nmpc_socket(msk);
- if (!ssock)
+ if (IS_ERR(ssock))
goto out;
ret = tcp_getsockopt(ssock->sk, level, optname, optval, optlen);
--
2.38.1
On Fri, 2023-01-13 at 19:20 +0100, Paolo Abeni wrote: > In the long run this will simplify the mptcp code and will > allow for more consistent behavior. Move the first subflow > allocation out of the sock->init ops into the __mptcp_nmpc_socket() > helper. > > Since the first subflow creation can now happen after the first > setsockopt() we additionally need to invoke mptcp_sockopt_sync() > on it. > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > net/mptcp/pm_netlink.c | 4 +-- > net/mptcp/protocol.c | 57 +++++++++++++++++++++++++++--------------- > net/mptcp/protocol.h | 2 +- > net/mptcp/sockopt.c | 18 ++++++------- > 4 files changed, 49 insertions(+), 32 deletions(-) > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index e82a112b8779..290f88fffe5f 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -1014,8 +1014,8 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk, > return -EINVAL; > > ssock = __mptcp_nmpc_socket(msk); > - if (!ssock) > - return -EINVAL; > + if (IS_ERR(ssock)) > + return PTR_ERR(ssock); > > mptcp_info2sockaddr(&entry->addr, &addr, entry->addr.family); > #if IS_ENABLED(CONFIG_MPTCP_IPV6) > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c > index 9c4c729bf271..1b93624db62d 100644 > --- a/net/mptcp/protocol.c > +++ b/net/mptcp/protocol.c > @@ -49,18 +49,6 @@ static void __mptcp_check_send_data_fin(struct sock *sk); > DEFINE_PER_CPU(struct mptcp_delegated_action, mptcp_delegated_actions); > static struct net_device mptcp_napi_dev; > > -/* If msk has an initial subflow socket, and the MP_CAPABLE handshake has not > - * completed yet or has failed, return the subflow socket. > - * Otherwise return NULL. > - */ > -struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk) > -{ > - if (!msk->subflow || READ_ONCE(msk->can_ack)) > - return NULL; > - > - return msk->subflow; > -} > - > /* Returns end sequence number of the receiver's advertised window */ > static u64 mptcp_wnd_end(const struct mptcp_sock *msk) > { > @@ -116,6 +104,31 @@ static int __mptcp_socket_create(struct mptcp_sock *msk) > return 0; > } > > +/* Returns the first subflow socket if available and the MPC > + * handshake is not started yet. > + */ > +struct socket *__mptcp_nmpc_socket(struct mptcp_sock *msk) > +{ > + struct sock *sk = (struct sock *)msk; > + int ret; > + > + if (!((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) > + return ERR_PTR(-EINVAL); > + > + if (!msk->subflow) { > + if (msk->first) > + return ERR_PTR(-EINVAL); > + > + ret = __mptcp_socket_create(msk); > + if (ret) > + return ERR_PTR(ret); > + > + mptcp_sockopt_sync(msk, msk->first); > + } > + > + return msk->subflow; > +} > + > static void mptcp_drop(struct sock *sk, struct sk_buff *skb) > { > sk_drops_add(sk, skb); > @@ -1699,6 +1712,7 @@ static int mptcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, > { > unsigned int saved_flags = msg->msg_flags; > struct mptcp_sock *msk = mptcp_sk(sk); > + struct socket *ssock; > struct sock *ssk; > int ret; > > @@ -1708,8 +1722,11 @@ static int mptcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, > * Since the defer_connect flag is cleared after the first succsful > * fastopen attempt, no need to check for additional subflow status. > */ > - if (msg->msg_flags & MSG_FASTOPEN && !__mptcp_nmpc_socket(msk)) > - return -EINVAL; > + if (msg->msg_flags & MSG_FASTOPEN) { > + ssock = __mptcp_nmpc_socket(msk); > + if (IS_ERR(ssock)) > + return PTR_ERR(ssock); > + } > if (!msk->first) > return -EINVAL; > > @@ -3592,8 +3609,8 @@ static int mptcp_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len) > int err = -EINVAL; > > ssock = __mptcp_nmpc_socket(msk); > - if (!ssock) > - return -EINVAL; > + if (IS_ERR(ssock)) > + return PTR_ERR(ssock); > > mptcp_token_destroy(msk); > inet_sk_state_store(sk, TCP_SYN_SENT); > @@ -3681,8 +3698,8 @@ static int mptcp_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) > > lock_sock(sock->sk); > ssock = __mptcp_nmpc_socket(msk); > - if (!ssock) { > - err = -EINVAL; > + if (IS_ERR(ssock)) { > + err = PTR_ERR(ssock); > goto unlock; > } > > @@ -3718,8 +3735,8 @@ static int mptcp_listen(struct socket *sock, int backlog) > > lock_sock(sk); > ssock = __mptcp_nmpc_socket(msk); > - if (!ssock) { > - err = -EINVAL; > + if (IS_ERR(ssock)) { > + err = PTR_ERR(ssock); > goto unlock; > } > > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h > index 5f1a30959b5c..3a055438c65e 100644 > --- a/net/mptcp/protocol.h > +++ b/net/mptcp/protocol.h > @@ -632,7 +632,7 @@ void __mptcp_subflow_send_ack(struct sock *ssk); > void mptcp_subflow_reset(struct sock *ssk); > void mptcp_subflow_queue_clean(struct sock *sk, struct sock *ssk); > void mptcp_sock_graft(struct sock *sk, struct socket *parent); > -struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk); > +struct socket *__mptcp_nmpc_socket(struct mptcp_sock *msk); > bool __mptcp_close(struct sock *sk, long timeout); > void mptcp_cancel_work(struct sock *sk); > void mptcp_set_owner_r(struct sk_buff *skb, struct sock *sk); > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c > index 9986681aaf40..022a6cad00c1 100644 > --- a/net/mptcp/sockopt.c > +++ b/net/mptcp/sockopt.c > @@ -301,9 +301,9 @@ static int mptcp_setsockopt_sol_socket(struct mptcp_sock *msk, int optname, > case SO_BINDTOIFINDEX: > lock_sock(sk); > ssock = __mptcp_nmpc_socket(msk); > - if (!ssock) { > + if (IS_ERR(ssock)) { > release_sock(sk); > - return -EINVAL; > + return PTR_ERR(ssock); > } > > ret = sock_setsockopt(ssock, SOL_SOCKET, optname, optval, optlen); > @@ -396,9 +396,9 @@ static int mptcp_setsockopt_v6(struct mptcp_sock *msk, int optname, > case IPV6_FREEBIND: > lock_sock(sk); > ssock = __mptcp_nmpc_socket(msk); > - if (!ssock) { > + if (IS_ERR(ssock)) { > release_sock(sk); > - return -EINVAL; > + return PTR_ERR(ssock); > } > > ret = tcp_setsockopt(ssock->sk, SOL_IPV6, optname, optval, optlen); > @@ -693,9 +693,9 @@ static int mptcp_setsockopt_sol_ip_set_transparent(struct mptcp_sock *msk, int o > lock_sock(sk); > > ssock = __mptcp_nmpc_socket(msk); > - if (!ssock) { > + if (IS_ERR(ssock)) { > release_sock(sk); > - return -EINVAL; > + return PTR_ERR(ssock); > } > > issk = inet_sk(ssock->sk); > @@ -764,8 +764,8 @@ static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int > > /* Limit to first subflow, before the connection establishment */ > sock = __mptcp_nmpc_socket(msk); > - if (!sock) > - return -EINVAL; > + if (IS_ERR(sock)) > + return PTR_ERR(sock); > > return tcp_setsockopt(sock->sk, level, optname, optval, optlen); > } > @@ -865,7 +865,7 @@ static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int > } > > ssock = __mptcp_nmpc_socket(msk); > - if (!ssock) > + if (IS_ERR(ssock)) > goto out; > > ret = tcp_getsockopt(ssock->sk, level, optname, optval, optlen); Self-nack. While rebasing the LSM/selinux patches on top of this series, I noticed I actually forgot (ahem ...) to remove the first subflow initialization from mptcp_init_sock(). The relative low number of issues found while testing should have warned me... I'm updating the series and I'll post after some more testings. Cheers, Paolo
© 2016 - 2024 Red Hat, Inc.