[MPTCP] Re: [PATCH v2 2/4] subflow: wake parent mptcp socket on subflow state change

Mat Martineau <mathew.j.martineau(a)linux.intel.com&gt; wrote: > On Wed, 23 Oct 2019, Florian Westphal wrote: > >> We need to wakeup readers blocking for POLLIN, as no >> more data will arrive they will never be awoken otherwise. >> >> This is needed at least until proper MPTCP-Level fin/reset >> signalling gets added. >> >> Signed-off-by: Florian Westphal <fw(a)strlen.de&gt; >> --- >> net/mptcp/subflow.c | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c >> index 1f27a96d0439..7bae7c35ea6b 100644 >> --- a/net/mptcp/subflow.c >> +++ b/net/mptcp/subflow.c >> @@ -722,6 +722,28 @@ static struct mptcp_subflow_context *subflow_create_ctx(struct sock *sk, >> return ctx; >> } >> >> +static void __subflow_state_change(struct sock *sk) >> +{ >> + struct socket_wq *wq; >> + >> + rcu_read_lock(); >> + wq = rcu_dereference(sk->sk_wq); >> + if (skwq_has_sleeper(wq)) >> + wake_up_interruptible_all(&wq->wait); >> + rcu_read_unlock(); >> +} >> + >> +static void subflow_state_change(struct sock *sk) >> +{ >> + struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk); >> + struct sock *parent = subflow->conn; >> + >> + __subflow_state_change(sk); >> + >> + if (parent) >> + __subflow_state_change(parent); > > In the v1 patch I asked about the missing sk_wake_async() for the parent > socket - you proposed calling parent->sk_data_ready(parent) here. Can you > confirm which is intended? > > https://lists.01.org/hyperkitty/list/mptcp@lists.01.org/message/EMGGJL4WI... That comment refers to subflow_data_ready(). The patch here was not part of v1. An alternative would be to make sock_def_wakeup() non-static and use it but since the function is very small I don't think thats needed. We will also likely want to do something different once MPTCP is able to process mptcp-level reset/fin. The proposed change of if (mptcp_should_wake_parent(subflow)) parent->sk_data_ready(parent); inside subflow_data_ready isn't needed anymore because after Paolos refactoring the existing parent->sk_data_ready(parent); is already conditional on 'we have in-sequence data', so I dropped that part from the last patch.