Messages in this thread Patch in this message | | | Subject | Re: [PATCH net] net: mptcp: fix unreleased socket in accept queue | From | Paolo Abeni <> | Date | Tue, 06 Sep 2022 09:02:50 +0200 |
| |
On Mon, 2022-09-05 at 17:03 +0800, Menglong Dong wrote: > On Mon, Sep 5, 2022 at 4:26 PM Paolo Abeni <pabeni@redhat.com> wrote: > > > > Hello, > > > > On Mon, 2022-09-05 at 13:04 +0800, menglong8.dong@gmail.com wrote: > > > From: Menglong Dong <imagedong@tencent.com> > > > > > > The mptcp socket and its subflow sockets in accept queue can't be > > > released after the process exit. > > > > > > While the release of a mptcp socket in listening state, the > > > corresponding tcp socket will be released too. Meanwhile, the tcp > > > socket in the unaccept queue will be released too. However, only init > > > subflow is in the unaccept queue, and the joined subflow is not in the > > > unaccept queue, which makes the joined subflow won't be released, and > > > therefore the corresponding unaccepted mptcp socket will not be released > > > to. > > > > > > This can be reproduced easily with following steps: > > > > > > 1. create 2 namespace and veth: > > > $ ip netns add mptcp-client > > > $ ip netns add mptcp-server > > > $ sysctl -w net.ipv4.conf.all.rp_filter=0 > > > $ ip netns exec mptcp-client sysctl -w net.mptcp.enabled=1 > > > $ ip netns exec mptcp-server sysctl -w net.mptcp.enabled=1 > > > $ ip link add red-client netns mptcp-client type veth peer red-server \ > > > netns mptcp-server > > > $ ip -n mptcp-server address add 10.0.0.1/24 dev red-server > > > $ ip -n mptcp-server address add 192.168.0.1/24 dev red-server > > > $ ip -n mptcp-client address add 10.0.0.2/24 dev red-client > > > $ ip -n mptcp-client address add 192.168.0.2/24 dev red-client > > > $ ip -n mptcp-server link set red-server up > > > $ ip -n mptcp-client link set red-client up > > > > > > 2. configure the endpoint and limit for client and server: > > > $ ip -n mptcp-server mptcp endpoint flush > > > $ ip -n mptcp-server mptcp limits set subflow 2 add_addr_accepted 2 > > > $ ip -n mptcp-client mptcp endpoint flush > > > $ ip -n mptcp-client mptcp limits set subflow 2 add_addr_accepted 2 > > > $ ip -n mptcp-client mptcp endpoint add 192.168.0.2 dev red-client id \ > > > 1 subflow > > > > > > 3. listen and accept on a port, such as 9999. The nc command we used > > > here is modified, which makes it uses mptcp protocol by default. > > > And the default backlog is 1: > > > ip netns exec mptcp-server nc -l -k -p 9999 > > > > > > 4. open another *two* terminal and connect to the server with the > > > following command: > > > $ ip netns exec mptcp-client nc 10.0.0.1 9999 > > > input something after connect, to triger the connection of the second > > > subflow > > > > > > 5. exit all the nc command, and check the tcp socket in server namespace. > > > And you will find that there is one tcp socket in CLOSE_WAIT state > > > and can't release forever. > > > > Thank you for the report! > > > > I have a doubt WRT the above scenario: AFAICS 'nc' will accept the > > incoming sockets ASAP, so the unaccepted queue should be empty at > > shutdown, but that does not fit with your description?!? > > > > By default, as far as in my case, nc won't accept the new connection > until the first connection closes with the '-k' set. Therefor, the second > connection will stay in the unaccepted queue.
I missed the fact you opened 2 connections. I guess that is point 4 above. Please rephrase that sentence with something alike:
--- 4. open another *two* terminal and use each of them to connect to the server with the following command: ... So that there are two established mptcp connections, with the second one still unaccepted. --- > > > > There are some solutions that I thought: > > > > > > 1. release all unaccepted mptcp socket with mptcp_close() while the > > > listening tcp socket release in mptcp_subflow_queue_clean(). This is > > > what we do in this commit. > > > 2. release the mptcp socket with mptcp_close() in subflow_ulp_release(). > > > 3. etc > > > > > > > Can you please point to a commit introducing the issue? > > > > In fact, I'm not sure. In my case, I found this issue in kernel 5.10. > And I wanted to find the solution in the upstream, but find that > upstream has this issue too. > > Hmm...I am curious if this issue exists in the beginning? I > can't find the opportunity that the joined subflow which are > unaccepted can be released.
It looks like the problem is there since MPJ support, commit f296234c98a8fcec94eec80304a873f635d350ea
> > > > Signed-off-by: Menglong Dong <imagedong@tencent.com> > > > --- > > > net/mptcp/subflow.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c > > > index c7d49fb6e7bd..e39dff5d5d84 100644 > > > --- a/net/mptcp/subflow.c > > > +++ b/net/mptcp/subflow.c > > > @@ -1770,6 +1770,10 @@ void mptcp_subflow_queue_clean(struct sock *listener_ssk) > > > msk->first = NULL; > > > msk->dl_next = NULL; > > > unlock_sock_fast(sk, slow); > > > + > > > + /* */ > > > + sock_hold(sk); > > > + sk->sk_prot->close(sk); > > > > You can call mptcp_close() directly here. > > > > Perhaps we could as well drop the mptcp_sock_destruct() hack? > > Do you mean to call mptcp_sock_destruct() directly here?
I suspect that with this change setting msk->sk_destruct to mptcp_sock_destruct in subflow_syn_recv_sock() is not needed anymore, and the relevant intialization (and callback definition) could be removed.
> Cheers,
Paolo
| |