11:57 a.m.
This is a (very simplified) version of the mptcp poll refactoring.
I've removed all of the code from mptcp_should_wake_parent(), the reason
is that Paolo is working on moving more code from mptcp_recvmsg to the
subflow data_ready callback so we can weed out duplicate data before
waking up userspace.
So this only contains the new 'subflow_write_space' callback to notify
mptcp layer when a subflow isn't blocked anymore on write-side, plus the
__tcp_poll split so we can call the tcp layer from mptcp without placing
the subflow socket on a wait list.
Comments welcome.
11:57 a.m.
New subject: [PATCH mptcp-next 1/3] mptcp: add and use subflow_write_space
This will be used to unblock tasks waiting for POLLOUT event
once mptcp_poll is altered to not wait for each subflow anymore.
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/subflow.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 7a94049587cc..fdec4f6208b7 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -320,6 +320,16 @@ static void subflow_data_ready(struct sock *sk)
}
}
+static void subflow_write_space(struct sock *sk)
+{
+ struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
+ struct sock *parent = subflow->conn;
+
+ sk_stream_write_space(sk);
+ if (parent)
+ parent->sk_write_space(parent);
+}
+
int mptcp_subflow_connect(struct sock *sk, struct sockaddr_in *local,
struct sockaddr_in *remote, u8 remote_id)
{
@@ -438,6 +448,7 @@ static int subflow_ulp_init(struct sock *sk)
ctx->tcp_sk_data_ready = sk->sk_data_ready;
ctx->use_checksum = 0;
sk->sk_data_ready = subflow_data_ready;
+ sk->sk_write_space = subflow_write_space;
out:
return err;
}
--
2.21.0
11:57 a.m.
New subject: [PATCH mptcp-next 2/3] net: tcp: add __tcp_poll helper
MPTCP will need to be able to call tcp_poll to check if the tcp socket is
available for reading/writing, but without adding the task to the waitlist.
This splits tcp_poll into two functions so __tcp_poll can be called by
mptcp in followup patch.
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
include/net/tcp.h | 1 +
net/ipv4/tcp.c | 12 ++++++++----
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 3994e6b0db27..53720bb66a93 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -396,6 +396,7 @@ void tcp_init_sock(struct sock *sk);
void tcp_init_transfer(struct sock *sk, int bpf_op);
__poll_t tcp_poll(struct file *file, struct socket *sock,
struct poll_table_struct *wait);
+__poll_t __tcp_poll(struct sock *sk);
int tcp_getsockopt(struct sock *sk, int level, int optname,
char __user *optval, int __user *optlen);
int tcp_setsockopt(struct sock *sk, int level, int optname,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5450e91dd82c..6f20a9d9181e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -490,15 +490,12 @@ static inline bool tcp_stream_is_readable(const struct tcp_sock
*tp,
* take care of normal races (between the test and the event) and we don't
* go look at any of the socket buffers directly.
*/
-__poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
+__poll_t __tcp_poll(struct sock *sk)
{
__poll_t mask;
- struct sock *sk = sock->sk;
const struct tcp_sock *tp = tcp_sk(sk);
int state;
- sock_poll_wait(file, sock, wait);
-
state = inet_sk_state_load(sk);
if (state == TCP_LISTEN)
return inet_csk_listen_poll(sk);
@@ -590,6 +587,13 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table
*wait)
return mask;
}
+
+__poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
+{
+ sock_poll_wait(file, sock, wait);
+
+ return __tcp_poll(sock->sk);
+}
EXPORT_SYMBOL(tcp_poll);
int tcp_ioctl(struct sock *sk, int cmd, unsigned long arg)
--
2.21.0
11:57 a.m.
New subject: [PATCH mptcp-next 3/3] mptcp: switch mptcp_poll to mptcp socket wait queue
tcp_poll places current task on a wait queue. This can cause us to block
forever -- some subflows might never receive any data because they're
used as a backup/fallback path.
This makes mptcp_poll wait on the mptcp socket wait queue instead.
__tcp_poll is used instead to avoid blocking current task again after
parent mptcp socket has been woken from the subflow layer.
The series is incomplete, mptcp_should_wake_parent() should be extended
to take packets off the subflow queue and add them to the mptcp socket
queue, i.e. move most of the work currently done at recvmsg time into
the wakeup function.
This is also needed to not wake userspace in case the subflow tcp data
was stale (e.g. because of too early mptcp-level retransmit).
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/protocol.c | 9 ++++++++-
net/mptcp/protocol.h | 1 +
net/mptcp/subflow.c | 12 +++++++++---
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index b54fecf528b9..908146b7f300 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -531,6 +531,11 @@ static enum mapping_status mptcp_get_mapping(struct sock *ssk)
return ret;
}
+bool mptcp_should_wake_parent(struct mptcp_subflow_context *subflow)
+{
+ return true;
+}
+
static void mptcp_wait_data(struct sock *sk, long *timeo)
{
DEFINE_WAIT_FUNC(wait, woken_wake_function);
@@ -1378,11 +1383,13 @@ static __poll_t mptcp_poll(struct file *file, struct socket
*sock,
return ret;
}
+ sock_poll_wait(file, sock, wait);
+
mptcp_for_each_subflow(msk, subflow) {
struct socket *tcp_sock;
tcp_sock = mptcp_subflow_tcp_socket(subflow);
- ret |= tcp_poll(file, tcp_sock, wait);
+ ret |= __tcp_poll(tcp_sock->sk);
}
release_sock(sk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 0c6bc8617cf4..eaa48fe90629 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -265,6 +265,7 @@ void mptcp_get_options(const struct sk_buff *skb,
void mptcp_finish_connect(struct sock *sk, int mp_capable);
void mptcp_finish_join(struct sock *sk);
void mptcp_reset_timer(struct sock *sk);
+bool mptcp_should_wake_parent(struct mptcp_subflow_context *ctx);
int mptcp_token_new_request(struct request_sock *req);
void mptcp_token_destroy_request(u32 token);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index fdec4f6208b7..2db2529ad323 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -310,14 +310,20 @@ static void subflow_data_ready(struct sock *sk)
subflow->tcp_sk_data_ready(sk);
if (parent) {
- pr_debug("parent=%p", parent);
+ struct socket_wq *wq;
smp_mb__before_atomic();
set_bit(MPTCP_DATA_READY, &mptcp_sk(parent)->flags);
smp_mb__after_atomic();
- parent->sk_data_ready(parent);
- }
+ rcu_read_lock();
+ wq = rcu_dereference(parent->sk_wq);
+ if (skwq_has_sleeper(wq) &&
+ mptcp_should_wake_parent(subflow)) {
+ wake_up_interruptible_all(&wq->wait);
+ }
+ rcu_read_unlock();
+ }
}
static void subflow_write_space(struct sock *sk)
--
2.21.0
12:13 a.m.
New subject: [PATCH mptcp-next 3/3] mptcp: switch mptcp_poll to mptcp socket wait queue
On Tue, 1 Oct 2019, Florian Westphal wrote:
tcp_poll places current task on a wait queue. This can cause us to
block
forever -- some subflows might never receive any data because they're
used as a backup/fallback path.
This makes mptcp_poll wait on the mptcp socket wait queue instead.
__tcp_poll is used instead to avoid blocking current task again after
parent mptcp socket has been woken from the subflow layer.
The series is incomplete, mptcp_should_wake_parent() should be extended
to take packets off the subflow queue and add them to the mptcp socket
queue, i.e. move most of the work currently done at recvmsg time into
the wakeup function.
This is also needed to not wake userspace in case the subflow tcp data
was stale (e.g. because of too early mptcp-level retransmit).
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/protocol.c | 9 ++++++++-
net/mptcp/protocol.h | 1 +
net/mptcp/subflow.c | 12 +++++++++---
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index b54fecf528b9..908146b7f300 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -531,6 +531,11 @@ static enum mapping_status mptcp_get_mapping(struct sock *ssk)
return ret;
}
+bool mptcp_should_wake_parent(struct mptcp_subflow_context *subflow)
+{
+ return true;
+}
+
static void mptcp_wait_data(struct sock *sk, long *timeo)
{
DEFINE_WAIT_FUNC(wait, woken_wake_function);
@@ -1378,11 +1383,13 @@ static __poll_t mptcp_poll(struct file *file, struct socket
*sock,
return ret;
}
+ sock_poll_wait(file, sock, wait);
+
mptcp_for_each_subflow(msk, subflow) {
struct socket *tcp_sock;
tcp_sock = mptcp_subflow_tcp_socket(subflow);
- ret |= tcp_poll(file, tcp_sock, wait);
+ ret |= __tcp_poll(tcp_sock->sk);
}
release_sock(sk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 0c6bc8617cf4..eaa48fe90629 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -265,6 +265,7 @@ void mptcp_get_options(const struct sk_buff *skb,
void mptcp_finish_connect(struct sock *sk, int mp_capable);
void mptcp_finish_join(struct sock *sk);
void mptcp_reset_timer(struct sock *sk);
+bool mptcp_should_wake_parent(struct mptcp_subflow_context *ctx);
int mptcp_token_new_request(struct request_sock *req);
void mptcp_token_destroy_request(u32 token);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index fdec4f6208b7..2db2529ad323 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -310,14 +310,20 @@ static void subflow_data_ready(struct sock *sk)
subflow->tcp_sk_data_ready(sk);
if (parent) {
- pr_debug("parent=%p", parent);
+ struct socket_wq *wq;
smp_mb__before_atomic();
set_bit(MPTCP_DATA_READY, &mptcp_sk(parent)->flags);
smp_mb__after_atomic();
- parent->sk_data_ready(parent);
- }
+ rcu_read_lock();
+ wq = rcu_dereference(parent->sk_wq);
+ if (skwq_has_sleeper(wq) &&
+ mptcp_should_wake_parent(subflow)) {
+ wake_up_interruptible_all(&wq->wait);
+ }
+ rcu_read_unlock();
Compared to sock_def_readable, this wakeup code doesn't call
sk_wake_async. Is SOCK_FASYNC irrelevant here (sk_wake_async doesn't do
anything without that sock flag)?
Mat
+ }
}
static void subflow_write_space(struct sock *sk)
--
2.21.0
_______________________________________________
mptcp mailing list -- mptcp(a)lists.01.org
To unsubscribe send an email to mptcp-leave(a)lists.01.org
--
Mat Martineau
Intel
10:06 a.m.
New subject: [PATCH mptcp-next 3/3] mptcp: switch mptcp_poll to mptcp socket wait queue
Mat Martineau <mathew.j.martineau(a)linux.intel.com> wrote:
> - pr_debug("parent=%p", parent);
> + struct socket_wq *wq;
>
> smp_mb__before_atomic();
> set_bit(MPTCP_DATA_READY, &mptcp_sk(parent)->flags);
> smp_mb__after_atomic();
>
> - parent->sk_data_ready(parent);
> - }
> + rcu_read_lock();
> + wq = rcu_dereference(parent->sk_wq);
> + if (skwq_has_sleeper(wq) &&
> + mptcp_should_wake_parent(subflow)) {
> + wake_up_interruptible_all(&wq->wait);
> + }
> + rcu_read_unlock();
Compared to sock_def_readable, this wakeup code doesn't call sk_wake_async.
Is SOCK_FASYNC irrelevant here (sk_wake_async doesn't do anything without
that sock flag)?
Its not. Perhaps best to make this a simpler
if (mptcp_should_wake_parent(subflow))
parent->sk_data_ready(parent);
I will make this change in the next iteration.
1040
days inactive
1050
days old
5 comments
2 participants
participants (2)
-
Florian Westphal -
Mat Martineau