* [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV
@ 2016-07-26 13:51 Trond Myklebust
2016-07-26 13:51 ` [PATCH 2/2] SUNRPC: Detect immediate closure of accepted sockets Trond Myklebust
2016-07-26 15:43 ` [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV J. Bruce Fields
0 siblings, 2 replies; 8+ messages in thread
From: Trond Myklebust @ 2016-07-26 13:51 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
We're seeing traces of the following form:
[10952.396347] svc: transport ffff88042ba4a 000 dequeued, inuse=2
[10952.396351] svc: tcp_accept ffff88042ba4 a000 sock ffff88042a6e4c80
[10952.396362] nfsd: connect from 10.2.6.1, port=187
[10952.396364] svc: svc_setup_socket ffff8800b99bcf00
[10952.396368] setting up TCP socket for reading
[10952.396370] svc: svc_setup_socket created ffff8803eb10a000 (inet ffff88042b75b800)
[10952.396373] svc: transport ffff8803eb10a000 put into queue
[10952.396375] svc: transport ffff88042ba4a000 put into queue
[10952.396377] svc: server ffff8800bb0ec000 waiting for data (to = 3600000)
[10952.396380] svc: transport ffff8803eb10a000 dequeued, inuse=2
[10952.396381] svc_recv: found XPT_CLOSE
[10952.396397] svc: svc_delete_xprt(ffff8803eb10a000)
[10952.396398] svc: svc_tcp_sock_detach(ffff8803eb10a000)
[10952.396399] svc: svc_sock_detach(ffff8803eb10a000)
[10952.396412] svc: svc_sock_free(ffff8803eb10a000)
i.e. an immediate close of the socket after initialisation.
The culprit appears to be the test at the end of svc_tcp_init, which
checks if the newly created socket is in the TCP_ESTABLISHED state,
and immediately closes it if not. The evidence appears to suggest that
the socket might still be in the SYN_RECV state at this time.
The fix is to check for both states, and then to add a check in
svc_tcp_state_change() to ensure we don't close the socket when
it transitions into TCP_ESTABLISHED.
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
net/sunrpc/svcsock.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index df61a4ec21a4..38b132ff6dce 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -767,8 +767,10 @@ static void svc_tcp_state_change(struct sock *sk)
printk("svc: socket %p: no user data\n", sk);
else {
svsk->sk_ostate(sk);
- set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
- svc_xprt_enqueue(&svsk->sk_xprt);
+ if (sk->sk_state != TCP_ESTABLISHED) {
+ set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
+ svc_xprt_enqueue(&svsk->sk_xprt);
+ }
}
}
@@ -1294,8 +1296,13 @@ static void svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)
tcp_sk(sk)->nonagle |= TCP_NAGLE_OFF;
set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags);
- if (sk->sk_state != TCP_ESTABLISHED)
+ switch (sk->sk_state) {
+ case TCP_SYN_RECV:
+ case TCP_ESTABLISHED:
+ break;
+ default:
set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
+ }
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] SUNRPC: Detect immediate closure of accepted sockets
2016-07-26 13:51 [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV Trond Myklebust
@ 2016-07-26 13:51 ` Trond Myklebust
2016-07-26 15:43 ` [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV J. Bruce Fields
1 sibling, 0 replies; 8+ messages in thread
From: Trond Myklebust @ 2016-07-26 13:51 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
This modification is useful for debugging issues that happen while
the socket is being initialised.
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
net/sunrpc/svcsock.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 38b132ff6dce..e84321a09880 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1365,8 +1365,11 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
else
svc_tcp_init(svsk, serv);
- dprintk("svc: svc_setup_socket created %p (inet %p)\n",
- svsk, svsk->sk_sk);
+ dprintk("svc: svc_setup_socket created %p (inet %p), "
+ "listen %d close %d\n",
+ svsk, svsk->sk_sk,
+ test_bit(XPT_LISTENER, &svsk->sk_xprt.xpt_flags),
+ test_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags));
return svsk;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV
2016-07-26 13:51 [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV Trond Myklebust
2016-07-26 13:51 ` [PATCH 2/2] SUNRPC: Detect immediate closure of accepted sockets Trond Myklebust
@ 2016-07-26 15:43 ` J. Bruce Fields
2016-07-26 16:08 ` Trond Myklebust
1 sibling, 1 reply; 8+ messages in thread
From: J. Bruce Fields @ 2016-07-26 15:43 UTC (permalink / raw)
To: Trond Myklebust; +Cc: linux-nfs
On Tue, Jul 26, 2016 at 09:51:19AM -0400, Trond Myklebust wrote:
> We're seeing traces of the following form:
>
> [10952.396347] svc: transport ffff88042ba4a 000 dequeued, inuse=2
> [10952.396351] svc: tcp_accept ffff88042ba4 a000 sock ffff88042a6e4c80
> [10952.396362] nfsd: connect from 10.2.6.1, port=187
> [10952.396364] svc: svc_setup_socket ffff8800b99bcf00
> [10952.396368] setting up TCP socket for reading
> [10952.396370] svc: svc_setup_socket created ffff8803eb10a000 (inet ffff88042b75b800)
> [10952.396373] svc: transport ffff8803eb10a000 put into queue
> [10952.396375] svc: transport ffff88042ba4a000 put into queue
> [10952.396377] svc: server ffff8800bb0ec000 waiting for data (to = 3600000)
> [10952.396380] svc: transport ffff8803eb10a000 dequeued, inuse=2
> [10952.396381] svc_recv: found XPT_CLOSE
> [10952.396397] svc: svc_delete_xprt(ffff8803eb10a000)
> [10952.396398] svc: svc_tcp_sock_detach(ffff8803eb10a000)
> [10952.396399] svc: svc_sock_detach(ffff8803eb10a000)
> [10952.396412] svc: svc_sock_free(ffff8803eb10a000)
>
> i.e. an immediate close of the socket after initialisation.
Interesting, thanks!
So the one thing I don't understand is why this is correct behavior for
accept--I thought it wasn't supposed to return a socket until it was
fully established.
--b.
>
> The culprit appears to be the test at the end of svc_tcp_init, which
> checks if the newly created socket is in the TCP_ESTABLISHED state,
> and immediately closes it if not. The evidence appears to suggest that
> the socket might still be in the SYN_RECV state at this time.
>
> The fix is to check for both states, and then to add a check in
> svc_tcp_state_change() to ensure we don't close the socket when
> it transitions into TCP_ESTABLISHED.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
> net/sunrpc/svcsock.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index df61a4ec21a4..38b132ff6dce 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -767,8 +767,10 @@ static void svc_tcp_state_change(struct sock *sk)
> printk("svc: socket %p: no user data\n", sk);
> else {
> svsk->sk_ostate(sk);
> - set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
> - svc_xprt_enqueue(&svsk->sk_xprt);
> + if (sk->sk_state != TCP_ESTABLISHED) {
> + set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
> + svc_xprt_enqueue(&svsk->sk_xprt);
> + }
> }
> }
>
> @@ -1294,8 +1296,13 @@ static void svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)
> tcp_sk(sk)->nonagle |= TCP_NAGLE_OFF;
>
> set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags);
> - if (sk->sk_state != TCP_ESTABLISHED)
> + switch (sk->sk_state) {
> + case TCP_SYN_RECV:
> + case TCP_ESTABLISHED:
> + break;
> + default:
> set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
> + }
> }
> }
>
> --
> 2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV
2016-07-26 15:43 ` [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV J. Bruce Fields
@ 2016-07-26 16:08 ` Trond Myklebust
2016-07-27 18:48 ` Fields Bruce James
0 siblings, 1 reply; 8+ messages in thread
From: Trond Myklebust @ 2016-07-26 16:08 UTC (permalink / raw)
To: Fields Bruce James; +Cc: List Linux NFS Mailing
> On Jul 26, 2016, at 11:43, J. Bruce Fields <bfields@fieldses.org> wrote:
>
> On Tue, Jul 26, 2016 at 09:51:19AM -0400, Trond Myklebust wrote:
>> We're seeing traces of the following form:
>>
>> [10952.396347] svc: transport ffff88042ba4a 000 dequeued, inuse=2
>> [10952.396351] svc: tcp_accept ffff88042ba4 a000 sock ffff88042a6e4c80
>> [10952.396362] nfsd: connect from 10.2.6.1, port=187
>> [10952.396364] svc: svc_setup_socket ffff8800b99bcf00
>> [10952.396368] setting up TCP socket for reading
>> [10952.396370] svc: svc_setup_socket created ffff8803eb10a000 (inet ffff88042b75b800)
>> [10952.396373] svc: transport ffff8803eb10a000 put into queue
>> [10952.396375] svc: transport ffff88042ba4a000 put into queue
>> [10952.396377] svc: server ffff8800bb0ec000 waiting for data (to = 3600000)
>> [10952.396380] svc: transport ffff8803eb10a000 dequeued, inuse=2
>> [10952.396381] svc_recv: found XPT_CLOSE
>> [10952.396397] svc: svc_delete_xprt(ffff8803eb10a000)
>> [10952.396398] svc: svc_tcp_sock_detach(ffff8803eb10a000)
>> [10952.396399] svc: svc_sock_detach(ffff8803eb10a000)
>> [10952.396412] svc: svc_sock_free(ffff8803eb10a000)
>>
>> i.e. an immediate close of the socket after initialisation.
>
> Interesting, thanks!
>
> So the one thing I don't understand is why this is correct behavior for
> accept--I thought it wasn't supposed to return a socket until it was
> fully established.
inet_accept() appears to allow SYN_RECV:
int inet_accept(struct socket *sock, struct socket *newsock, int flags)
{
struct sock *sk1 = sock->sk;
int err = -EINVAL;
struct sock *sk2 = sk1->sk_prot->accept(sk1, flags, &err);
if (!sk2)
goto do_err;
lock_sock(sk2);
sock_rps_record_flow(sk2);
WARN_ON(!((1 << sk2->sk_state) &
(TCPF_ESTABLISHED | TCPF_SYN_RECV |
TCPF_CLOSE_WAIT | TCPF_CLOSE)));
sock_graft(sk2, newsock);
newsock->state = SS_CONNECTED;
err = 0;
release_sock(sk2);
do_err:
return err;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV
2016-07-26 16:08 ` Trond Myklebust
@ 2016-07-27 18:48 ` Fields Bruce James
2016-07-27 18:59 ` Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: Fields Bruce James @ 2016-07-27 18:48 UTC (permalink / raw)
To: Trond Myklebust; +Cc: List Linux NFS Mailing, netdev
On Tue, Jul 26, 2016 at 04:08:29PM +0000, Trond Myklebust wrote:
>
> > On Jul 26, 2016, at 11:43, J. Bruce Fields <bfields@fieldses.org> wrote:
> >
> > On Tue, Jul 26, 2016 at 09:51:19AM -0400, Trond Myklebust wrote:
> >> We're seeing traces of the following form:
> >>
> >> [10952.396347] svc: transport ffff88042ba4a 000 dequeued, inuse=2
> >> [10952.396351] svc: tcp_accept ffff88042ba4 a000 sock ffff88042a6e4c80
> >> [10952.396362] nfsd: connect from 10.2.6.1, port=187
> >> [10952.396364] svc: svc_setup_socket ffff8800b99bcf00
> >> [10952.396368] setting up TCP socket for reading
> >> [10952.396370] svc: svc_setup_socket created ffff8803eb10a000 (inet ffff88042b75b800)
> >> [10952.396373] svc: transport ffff8803eb10a000 put into queue
> >> [10952.396375] svc: transport ffff88042ba4a000 put into queue
> >> [10952.396377] svc: server ffff8800bb0ec000 waiting for data (to = 3600000)
> >> [10952.396380] svc: transport ffff8803eb10a000 dequeued, inuse=2
> >> [10952.396381] svc_recv: found XPT_CLOSE
> >> [10952.396397] svc: svc_delete_xprt(ffff8803eb10a000)
> >> [10952.396398] svc: svc_tcp_sock_detach(ffff8803eb10a000)
> >> [10952.396399] svc: svc_sock_detach(ffff8803eb10a000)
> >> [10952.396412] svc: svc_sock_free(ffff8803eb10a000)
> >>
> >> i.e. an immediate close of the socket after initialisation.
> >
> > Interesting, thanks!
> >
> > So the one thing I don't understand is why this is correct behavior for
> > accept--I thought it wasn't supposed to return a socket until it was
> > fully established.
>
> inet_accept() appears to allow SYN_RECV:
OK. Cc'ing netdev just to make sure we didn't overlook anything.
(Also: what were user-visible symptoms? Mounts failing, or unexpected
delays?)
--b.
>
> int inet_accept(struct socket *sock, struct socket *newsock, int flags)
> {
> struct sock *sk1 = sock->sk;
> int err = -EINVAL;
> struct sock *sk2 = sk1->sk_prot->accept(sk1, flags, &err);
>
> if (!sk2)
> goto do_err;
>
> lock_sock(sk2);
>
> sock_rps_record_flow(sk2);
> WARN_ON(!((1 << sk2->sk_state) &
> (TCPF_ESTABLISHED | TCPF_SYN_RECV |
> TCPF_CLOSE_WAIT | TCPF_CLOSE)));
>
> sock_graft(sk2, newsock);
>
> newsock->state = SS_CONNECTED;
> err = 0;
> release_sock(sk2);
> do_err:
> return err;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV
2016-07-27 18:48 ` Fields Bruce James
@ 2016-07-27 18:59 ` Eric Dumazet
2016-07-27 19:11 ` Trond Myklebust
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2016-07-27 18:59 UTC (permalink / raw)
To: Fields Bruce James
Cc: Trond Myklebust, List Linux NFS Mailing, netdev, Yuchung Cheng
On Wed, 2016-07-27 at 14:48 -0400, Fields Bruce James wrote:
> On Tue, Jul 26, 2016 at 04:08:29PM +0000, Trond Myklebust wrote:
> >
> > > On Jul 26, 2016, at 11:43, J. Bruce Fields <bfields@fieldses.org> wrote:
> > >
> > > On Tue, Jul 26, 2016 at 09:51:19AM -0400, Trond Myklebust wrote:
> > >> We're seeing traces of the following form:
> > >>
> > >> [10952.396347] svc: transport ffff88042ba4a 000 dequeued, inuse=2
> > >> [10952.396351] svc: tcp_accept ffff88042ba4 a000 sock ffff88042a6e4c80
> > >> [10952.396362] nfsd: connect from 10.2.6.1, port=187
> > >> [10952.396364] svc: svc_setup_socket ffff8800b99bcf00
> > >> [10952.396368] setting up TCP socket for reading
> > >> [10952.396370] svc: svc_setup_socket created ffff8803eb10a000 (inet ffff88042b75b800)
> > >> [10952.396373] svc: transport ffff8803eb10a000 put into queue
> > >> [10952.396375] svc: transport ffff88042ba4a000 put into queue
> > >> [10952.396377] svc: server ffff8800bb0ec000 waiting for data (to = 3600000)
> > >> [10952.396380] svc: transport ffff8803eb10a000 dequeued, inuse=2
> > >> [10952.396381] svc_recv: found XPT_CLOSE
> > >> [10952.396397] svc: svc_delete_xprt(ffff8803eb10a000)
> > >> [10952.396398] svc: svc_tcp_sock_detach(ffff8803eb10a000)
> > >> [10952.396399] svc: svc_sock_detach(ffff8803eb10a000)
> > >> [10952.396412] svc: svc_sock_free(ffff8803eb10a000)
> > >>
> > >> i.e. an immediate close of the socket after initialisation.
> > >
> > > Interesting, thanks!
> > >
> > > So the one thing I don't understand is why this is correct behavior for
> > > accept--I thought it wasn't supposed to return a socket until it was
> > > fully established.
> >
> > inet_accept() appears to allow SYN_RECV:
>
> OK. Cc'ing netdev just to make sure we didn't overlook anything.
>
SYN_RECV after accept() is a TCP Fast Open property I think.
Maybe you are playing with some global TCP Fast Open settings ?
> (Also: what were user-visible symptoms? Mounts failing, or unexpected
> delays?)
>
> --b.
>
> >
> > int inet_accept(struct socket *sock, struct socket *newsock, int flags)
> > {
> > struct sock *sk1 = sock->sk;
> > int err = -EINVAL;
> > struct sock *sk2 = sk1->sk_prot->accept(sk1, flags, &err);
> >
> > if (!sk2)
> > goto do_err;
> >
> > lock_sock(sk2);
> >
> > sock_rps_record_flow(sk2);
> > WARN_ON(!((1 << sk2->sk_state) &
> > (TCPF_ESTABLISHED | TCPF_SYN_RECV |
> > TCPF_CLOSE_WAIT | TCPF_CLOSE)));
> >
> > sock_graft(sk2, newsock);
> >
> > newsock->state = SS_CONNECTED;
> > err = 0;
> > release_sock(sk2);
> > do_err:
> > return err;
> > }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV
2016-07-27 18:59 ` Eric Dumazet
@ 2016-07-27 19:11 ` Trond Myklebust
2016-07-28 14:21 ` Fields Bruce James
0 siblings, 1 reply; 8+ messages in thread
From: Trond Myklebust @ 2016-07-27 19:11 UTC (permalink / raw)
To: Eric Dumazet
Cc: Fields Bruce James, List Linux NFS Mailing,
List Linux Network Devel Mailing, Yuchung Cheng
SGkgRXJpYywNCg0KPiBPbiBKdWwgMjcsIDIwMTYsIGF0IDE0OjU5LCBFcmljIER1bWF6ZXQgPGVy
aWMuZHVtYXpldEBnbWFpbC5jb20+IHdyb3RlOg0KPiANCj4gT24gV2VkLCAyMDE2LTA3LTI3IGF0
IDE0OjQ4IC0wNDAwLCBGaWVsZHMgQnJ1Y2UgSmFtZXMgd3JvdGU6DQo+PiBPbiBUdWUsIEp1bCAy
NiwgMjAxNiBhdCAwNDowODoyOVBNICswMDAwLCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6DQo+Pj4g
DQo+Pj4+IE9uIEp1bCAyNiwgMjAxNiwgYXQgMTE6NDMsIEouIEJydWNlIEZpZWxkcyA8YmZpZWxk
c0BmaWVsZHNlcy5vcmc+IHdyb3RlOg0KPj4+PiANCj4+Pj4gT24gVHVlLCBKdWwgMjYsIDIwMTYg
YXQgMDk6NTE6MTlBTSAtMDQwMCwgVHJvbmQgTXlrbGVidXN0IHdyb3RlOg0KPj4+Pj4gV2UncmUg
c2VlaW5nIHRyYWNlcyBvZiB0aGUgZm9sbG93aW5nIGZvcm06DQo+Pj4+PiANCj4+Pj4+IFsxMDk1
Mi4zOTYzNDddIHN2YzogdHJhbnNwb3J0IGZmZmY4ODA0MmJhNGEgMDAwIGRlcXVldWVkLCBpbnVz
ZT0yDQo+Pj4+PiBbMTA5NTIuMzk2MzUxXSBzdmM6IHRjcF9hY2NlcHQgZmZmZjg4MDQyYmE0IGEw
MDAgc29jayBmZmZmODgwNDJhNmU0YzgwDQo+Pj4+PiBbMTA5NTIuMzk2MzYyXSBuZnNkOiBjb25u
ZWN0IGZyb20gMTAuMi42LjEsIHBvcnQ9MTg3DQo+Pj4+PiBbMTA5NTIuMzk2MzY0XSBzdmM6IHN2
Y19zZXR1cF9zb2NrZXQgZmZmZjg4MDBiOTliY2YwMA0KPj4+Pj4gWzEwOTUyLjM5NjM2OF0gc2V0
dGluZyB1cCBUQ1Agc29ja2V0IGZvciByZWFkaW5nDQo+Pj4+PiBbMTA5NTIuMzk2MzcwXSBzdmM6
IHN2Y19zZXR1cF9zb2NrZXQgY3JlYXRlZCBmZmZmODgwM2ViMTBhMDAwIChpbmV0IGZmZmY4ODA0
MmI3NWI4MDApDQo+Pj4+PiBbMTA5NTIuMzk2MzczXSBzdmM6IHRyYW5zcG9ydCBmZmZmODgwM2Vi
MTBhMDAwIHB1dCBpbnRvIHF1ZXVlDQo+Pj4+PiBbMTA5NTIuMzk2Mzc1XSBzdmM6IHRyYW5zcG9y
dCBmZmZmODgwNDJiYTRhMDAwIHB1dCBpbnRvIHF1ZXVlDQo+Pj4+PiBbMTA5NTIuMzk2Mzc3XSBz
dmM6IHNlcnZlciBmZmZmODgwMGJiMGVjMDAwIHdhaXRpbmcgZm9yIGRhdGEgKHRvID0gMzYwMDAw
MCkNCj4+Pj4+IFsxMDk1Mi4zOTYzODBdIHN2YzogdHJhbnNwb3J0IGZmZmY4ODAzZWIxMGEwMDAg
ZGVxdWV1ZWQsIGludXNlPTINCj4+Pj4+IFsxMDk1Mi4zOTYzODFdIHN2Y19yZWN2OiBmb3VuZCBY
UFRfQ0xPU0UNCj4+Pj4+IFsxMDk1Mi4zOTYzOTddIHN2Yzogc3ZjX2RlbGV0ZV94cHJ0KGZmZmY4
ODAzZWIxMGEwMDApDQo+Pj4+PiBbMTA5NTIuMzk2Mzk4XSBzdmM6IHN2Y190Y3Bfc29ja19kZXRh
Y2goZmZmZjg4MDNlYjEwYTAwMCkNCj4+Pj4+IFsxMDk1Mi4zOTYzOTldIHN2Yzogc3ZjX3NvY2tf
ZGV0YWNoKGZmZmY4ODAzZWIxMGEwMDApDQo+Pj4+PiBbMTA5NTIuMzk2NDEyXSBzdmM6IHN2Y19z
b2NrX2ZyZWUoZmZmZjg4MDNlYjEwYTAwMCkNCj4+Pj4+IA0KPj4+Pj4gaS5lLiBhbiBpbW1lZGlh
dGUgY2xvc2Ugb2YgdGhlIHNvY2tldCBhZnRlciBpbml0aWFsaXNhdGlvbi4NCj4+Pj4gDQo+Pj4+
IEludGVyZXN0aW5nLCB0aGFua3MhDQo+Pj4+IA0KPj4+PiBTbyB0aGUgb25lIHRoaW5nIEkgZG9u
J3QgdW5kZXJzdGFuZCBpcyB3aHkgdGhpcyBpcyBjb3JyZWN0IGJlaGF2aW9yIGZvcg0KPj4+PiBh
Y2NlcHQtLUkgdGhvdWdodCBpdCB3YXNuJ3Qgc3VwcG9zZWQgdG8gcmV0dXJuIGEgc29ja2V0IHVu
dGlsIGl0IHdhcw0KPj4+PiBmdWxseSBlc3RhYmxpc2hlZC4NCj4+PiANCj4+PiBpbmV0X2FjY2Vw
dCgpIGFwcGVhcnMgdG8gYWxsb3cgU1lOX1JFQ1Y6DQo+PiANCj4+IE9LLiAgQ2MnaW5nIG5ldGRl
diBqdXN0IHRvIG1ha2Ugc3VyZSB3ZSBkaWRuJ3Qgb3Zlcmxvb2sgYW55dGhpbmcuDQo+PiANCj4g
DQo+IFNZTl9SRUNWIGFmdGVyIGFjY2VwdCgpIGlzIGEgVENQIEZhc3QgT3BlbiBwcm9wZXJ0eSBJ
IHRoaW5rLg0KPiANCj4gTWF5YmUgeW91IGFyZSBwbGF5aW5nIHdpdGggc29tZSBnbG9iYWwgVENQ
IEZhc3QgT3BlbiBzZXR0aW5ncyA/DQo+IA0KDQpUaGUgTGludXgga2VybmVsIGNsaWVudCBzaG91
bGQgbm90IGJlIHVzaW5nIFRDUCBmYXN0IG9wZW4sIGJ1dCBpdCBpcyBwb3NzaWJsZSB0aGF0IHNv
bWUgb2YgdGhlIG90aGVyIE5GU3YzIGNsaWVudHMgd2XigJlyZSB1c2luZyBhcmUuDQpXb3VsZCBh
IHN0YW5kYXJkIGtuZnNkIGxpc3RlbmVyIHJlc3BvbmQgdG8gYSBUQ1AgZmFzdCBvcGVuIHJlcXVl
c3QsIG9yIHdvdWxkIHRoZSBkZWZhdWx0IGJlaGF2aW91ciBiZSB0byBpZ25vcmUgaXQ/DQoNCklm
IHRoZSBkZWZhdWx0IGJlaGF2aW91ciBmb3IgdGhlIHNlcnZlciBpcyB0byBhbGxvdyBmYXN0IG9w
ZW4sIHRoZW4gd2UgZG8gbmVlZCB0aGVzZSBwYXRjaGVzLCBJTU8uDQoNCj4gDQo+PiAoQWxzbzog
d2hhdCB3ZXJlIHVzZXItdmlzaWJsZSBzeW1wdG9tcz8gIE1vdW50cyBmYWlsaW5nLCBvciB1bmV4
cGVjdGVkDQo+PiBkZWxheXM/KQ0KPj4gDQoNCkNvbm5lY3Rpb24gcmV0cnkgc3Rvcm1zIG9uIHRo
ZSBzZXJ2ZXIuDQoNCj4+IC0tYi4NCj4+IA0KPj4+IA0KPj4+IGludCBpbmV0X2FjY2VwdChzdHJ1
Y3Qgc29ja2V0ICpzb2NrLCBzdHJ1Y3Qgc29ja2V0ICpuZXdzb2NrLCBpbnQgZmxhZ3MpDQo+Pj4g
ew0KPj4+ICAgICAgICBzdHJ1Y3Qgc29jayAqc2sxID0gc29jay0+c2s7DQo+Pj4gICAgICAgIGlu
dCBlcnIgPSAtRUlOVkFMOw0KPj4+ICAgICAgICBzdHJ1Y3Qgc29jayAqc2syID0gc2sxLT5za19w
cm90LT5hY2NlcHQoc2sxLCBmbGFncywgJmVycik7DQo+Pj4gDQo+Pj4gICAgICAgIGlmICghc2sy
KQ0KPj4+ICAgICAgICAgICAgICAgIGdvdG8gZG9fZXJyOw0KPj4+IA0KPj4+ICAgICAgICBsb2Nr
X3NvY2soc2syKTsNCj4+PiANCj4+PiAgICAgICAgc29ja19ycHNfcmVjb3JkX2Zsb3coc2syKTsN
Cj4+PiAgICAgICAgV0FSTl9PTighKCgxIDw8IHNrMi0+c2tfc3RhdGUpICYNCj4+PiAgICAgICAg
ICAgICAgICAgIChUQ1BGX0VTVEFCTElTSEVEIHwgVENQRl9TWU5fUkVDViB8DQo+Pj4gICAgICAg
ICAgICAgICAgICBUQ1BGX0NMT1NFX1dBSVQgfCBUQ1BGX0NMT1NFKSkpOw0KPj4+IA0KPj4+ICAg
ICAgICBzb2NrX2dyYWZ0KHNrMiwgbmV3c29jayk7DQo+Pj4gDQo+Pj4gICAgICAgIG5ld3NvY2st
PnN0YXRlID0gU1NfQ09OTkVDVEVEOw0KPj4+ICAgICAgICBlcnIgPSAwOw0KPj4+ICAgICAgICBy
ZWxlYXNlX3NvY2soc2syKTsNCj4+PiBkb19lcnI6DQo+Pj4gICAgICAgIHJldHVybiBlcnI7DQo+
Pj4gfQ0KPiANCj4gDQoNCg==
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV
2016-07-27 19:11 ` Trond Myklebust
@ 2016-07-28 14:21 ` Fields Bruce James
0 siblings, 0 replies; 8+ messages in thread
From: Fields Bruce James @ 2016-07-28 14:21 UTC (permalink / raw)
To: Trond Myklebust
Cc: Eric Dumazet, List Linux NFS Mailing,
List Linux Network Devel Mailing, Yuchung Cheng
On Wed, Jul 27, 2016 at 07:11:23PM +0000, Trond Myklebust wrote:
> Hi Eric,
>
> > On Jul 27, 2016, at 14:59, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > On Wed, 2016-07-27 at 14:48 -0400, Fields Bruce James wrote:
> >> On Tue, Jul 26, 2016 at 04:08:29PM +0000, Trond Myklebust wrote:
> >>>
> >>>> On Jul 26, 2016, at 11:43, J. Bruce Fields <bfields@fieldses.org> wrote:
> >>>>
> >>>> On Tue, Jul 26, 2016 at 09:51:19AM -0400, Trond Myklebust wrote:
> >>>>> We're seeing traces of the following form:
> >>>>>
> >>>>> [10952.396347] svc: transport ffff88042ba4a 000 dequeued, inuse=2
> >>>>> [10952.396351] svc: tcp_accept ffff88042ba4 a000 sock ffff88042a6e4c80
> >>>>> [10952.396362] nfsd: connect from 10.2.6.1, port=187
> >>>>> [10952.396364] svc: svc_setup_socket ffff8800b99bcf00
> >>>>> [10952.396368] setting up TCP socket for reading
> >>>>> [10952.396370] svc: svc_setup_socket created ffff8803eb10a000 (inet ffff88042b75b800)
> >>>>> [10952.396373] svc: transport ffff8803eb10a000 put into queue
> >>>>> [10952.396375] svc: transport ffff88042ba4a000 put into queue
> >>>>> [10952.396377] svc: server ffff8800bb0ec000 waiting for data (to = 3600000)
> >>>>> [10952.396380] svc: transport ffff8803eb10a000 dequeued, inuse=2
> >>>>> [10952.396381] svc_recv: found XPT_CLOSE
> >>>>> [10952.396397] svc: svc_delete_xprt(ffff8803eb10a000)
> >>>>> [10952.396398] svc: svc_tcp_sock_detach(ffff8803eb10a000)
> >>>>> [10952.396399] svc: svc_sock_detach(ffff8803eb10a000)
> >>>>> [10952.396412] svc: svc_sock_free(ffff8803eb10a000)
> >>>>>
> >>>>> i.e. an immediate close of the socket after initialisation.
> >>>>
> >>>> Interesting, thanks!
> >>>>
> >>>> So the one thing I don't understand is why this is correct behavior for
> >>>> accept--I thought it wasn't supposed to return a socket until it was
> >>>> fully established.
> >>>
> >>> inet_accept() appears to allow SYN_RECV:
> >>
> >> OK. Cc'ing netdev just to make sure we didn't overlook anything.
> >>
> >
> > SYN_RECV after accept() is a TCP Fast Open property I think.
> >
> > Maybe you are playing with some global TCP Fast Open settings ?
> >
>
> The Linux kernel client should not be using TCP fast open, but it is possible that some of the other NFSv3 clients we’re using are.
> Would a standard knfsd listener respond to a TCP fast open request, or would the default behaviour be to ignore it?
>
> If the default behaviour for the server is to allow fast open, then we do need these patches, IMO.
Even if it's not a default, if there's a configuration that allows
accept to return a socket in SYN_RECV state, then knfsd should handle it
gracefully, especially as long as it's this easy.
It'd still be useful to understand why this is happening, though....
--b.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-07-28 14:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-26 13:51 [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV Trond Myklebust
2016-07-26 13:51 ` [PATCH 2/2] SUNRPC: Detect immediate closure of accepted sockets Trond Myklebust
2016-07-26 15:43 ` [PATCH 1/2] SUNRPC: accept() may return sockets that are still in SYN_RECV J. Bruce Fields
2016-07-26 16:08 ` Trond Myklebust
2016-07-27 18:48 ` Fields Bruce James
2016-07-27 18:59 ` Eric Dumazet
2016-07-27 19:11 ` Trond Myklebust
2016-07-28 14:21 ` Fields Bruce James
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).