Re: [MPTCP] [PATCH mptcp v3 0/3] mptcp: plug mptcp_accept memory leaks

Re: [MPTCP] [PATCH mptcp v3 0/3] mptcp: plug mptcp_accept memory leaks

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 2973 bytes --]

Hi Florian,

Thank you for the patches and the update.
And thanks Peter and Mat for the reviews!

On 16/05/2019 11:23, Florian Westphal wrote:
> v3: rebase & fix erroneous conditional in patch #3.
> 
> v2 cover letter:
> v2:
> - rename helper in patch 2
> - add back crucial mptcp_init_sock() after clone in patch 3.
>   This was lost during rebase and broke the tests but i forgot
>   to refresh this patch when i sent v1.                     
> 
> This series resolves the memory leaks that occur when running
> mptcp_connect.sh.
> 
> I don't mind if they get squashed.
> I've decided to split this into 3 distinct changes to make it
> a bit clearer what is going on.  

Thank you for this!

Because the 1st one was fixing a bug introduced in the patch series and
the 2nd one change the architecture, I have squashed them in different
commits. For the 3rd one, I guess it made more sense to create a
specific commit to explain the decision that you took.

Here are the details of the squashes if someone wants to see them:


Patch 1: mptcp: release mptcp socket in mptcp_close:
	2bb08839b911 → squash in "mptcp: Add MPTCP socket stubs"
	419b1ecc8760 → sign-off
	af1f253ddd49 → conflict 1
	5a85931b0875 → conflict 2
	14035afa54ce..e58d56c3bdb0 → result patch 1


Patch 2: mptcp: store tcp socket, not tcp sock in subflow ctx
	278ebcdf1fcd → squash in "mptcp: Associate MPTCP context with TCP socket"
	1cf900c3d339 → sign-off
	e5fabdff04da → conflict 1
	0867ab696817 → conflict 2
	3f9772cce9fb → squash in "mptcp: Create SUBFLOW socket for incoming
connections"
	fdf93ec159c6 → sign-off
	67fb09ba1e9f → conflict 3
	adb0b1e24c8e → conflict 4
	9127b8ba2843 + 5ca554150c60 → squash in "mptcp: Make connection_list a
real list of subflows" (use the new function instead of the old one +
remove unneeded semi-colon)
	b2f24974d1b2 → conflict 5
	5f525e95f57f + 907383369eab → fix-up 1
	e58d56c3bdb0..be0a3d56b3bb → result patch 2


Patch 3: mptcp: accept: don't leak mptcp socket structure
	a7bc3b13590e → added at the end (without the updates notes, I hope you
don't mind)
	be0a3d56b3bb..d90f77dd19c4 → result patch 3


Cheers,
Matt

> 
> Second patch is not a bug fix, i can resend and drop it if
> the consensus is to continue using the sk->sk_socket back reference.
> 
> Third patch is a bit large due to the change in how the new mptcp
> socket is created.
> 
> See the commit messages for more details.                      
> After these changes kmemleak doesn't report any leaks when the  
> test script has completed.
> 
> 
> _______________________________________________
> mptcp mailing list
> mptcp(a)lists.01.org
> https://lists.01.org/mailman/listinfo/mptcp
> 

-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

[MPTCP] [PATCH mptcp v3 0/3] mptcp: plug mptcp_accept memory leaks

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

v3: rebase & fix erroneous conditional in patch #3.

v2 cover letter:
v2:
- rename helper in patch 2
- add back crucial mptcp_init_sock() after clone in patch 3.
  This was lost during rebase and broke the tests but i forgot
  to refresh this patch when i sent v1.                     

This series resolves the memory leaks that occur when running
mptcp_connect.sh.

I don't mind if they get squashed.
I've decided to split this into 3 distinct changes to make it
a bit clearer what is going on.  

Second patch is not a bug fix, i can resend and drop it if
the consensus is to continue using the sk->sk_socket back reference.

Third patch is a bit large due to the change in how the new mptcp
socket is created.

See the commit messages for more details.                      
After these changes kmemleak doesn't report any leaks when the  
test script has completed.