[PATCH net-next] tcp: diag: add support for request sockets to tcp_abort()

[PATCH net-next] tcp: diag: add support for request sockets to tcp_abort()

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Adding support for SYN_RECV request sockets to tcp_abort()
is quite easy after our tcp listener rewrite.

Note that we also need to better handle listeners, or we might
leak not yet accepted children, because of a missing
inet_csk_listen_stop() call.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
---
 net/ipv4/tcp.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2c0e340518d2..cc7aaa507abf 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3083,6 +3083,15 @@ EXPORT_SYMBOL_GPL(tcp_done);
 int tcp_abort(struct sock *sk, int err)
 {
 	if (!sk_fullsock(sk)) {
+		if (sk->sk_state == TCP_NEW_SYN_RECV) {
+			struct request_sock *req = inet_reqsk(sk);
+
+			local_bh_disable();
+			inet_csk_reqsk_queue_drop_and_put(req->rsk_listener,
+							  req);
+			local_bh_enable();
+			return 0;
+		}
 		sock_gen_put(sk);
 		return -EOPNOTSUPP;
 	}

Re: [PATCH net-next] tcp: diag: add support for request sockets to tcp_abort()

[Lorenzo Colitti]

On Fri, Dec 18, 2015 at 9:14 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Adding support for SYN_RECV request sockets to tcp_abort()
> is quite easy after our tcp listener rewrite.

I added test coverage for this to our tests.

Without this patch, attempting to destroy an SYN_RECV socket using
SOCK_DESTROY results in EOPNOTSUPP. With this patch, SOCK_DESTROY
succeeds, and after it does, sock_diag reports no child sockets.

Tested-by: Lorenzo Colitti <lorenzo@google.com>

Re: [PATCH net-next] tcp: diag: add support for request sockets to tcp_abort()

[Eric Dumazet]

On Fri, 2015-12-18 at 17:38 +0900, Lorenzo Colitti wrote:
> On Fri, Dec 18, 2015 at 9:14 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Adding support for SYN_RECV request sockets to tcp_abort()
> > is quite easy after our tcp listener rewrite.
> 
> I added test coverage for this to our tests.
> 
> Without this patch, attempting to destroy an SYN_RECV socket using
> SOCK_DESTROY results in EOPNOTSUPP. With this patch, SOCK_DESTROY
> succeeds, and after it does, sock_diag reports no child sockets.
> 
> Tested-by: Lorenzo Colitti <lorenzo@google.com>

I am curious, did you use packetdrill for this ?

I was about to write a packetdrill test as well ;)

Thanks !

Re: [PATCH net-next] tcp: diag: add support for request sockets to tcp_abort()

[Lorenzo Colitti]

On Fri, Dec 18, 2015 at 9:46 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Tested-by: Lorenzo Colitti <lorenzo@google.com>
>
> I am curious, did you use packetdrill for this ?

No, I added this to our existing kernel networking tests:

https://android-review.googlesource.com/#/c/187491/

The tests are written in Python and run under ARCH=um. They use Python
code to exercise the kernel socket and netlink APIs on one side, and
use scapy on multiple tap interfaces to simulate the networks. They
were initially written to check that routing on a multinetwork device
would work properly.

https://android.googlesource.com/platform/system/extras/+/master/tests/net_test

Re: [PATCH net-next] tcp: diag: add support for request sockets to tcp_abort()

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 17 Dec 2015 16:14:11 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> Adding support for SYN_RECV request sockets to tcp_abort()
> is quite easy after our tcp listener rewrite.
> 
> Note that we also need to better handle listeners, or we might
> leak not yet accepted children, because of a missing
> inet_csk_listen_stop() call.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks Eric.