Messages in this thread | | | Subject | Re: [PATCH net] net: mptcp: fix unreleased socket in accept queue | From | Paolo Abeni <> | Date | Mon, 05 Sep 2022 10:26:30 +0200 |
| |
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?!?
> 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?
> 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?
Perhpas even providing a __mptcp_close() variant not acquiring the socket lock and move such close call inside the existing sk socket lock above?
Thanks,
Paolo
| |