Re: [PATCH] handshake: set hr_sk field earlier in handshake_req_submit

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2023-03-28 at 18:12 +0000, Chuck Lever III wrote:
> 
> > On Mar 28, 2023, at 2:10 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > Otherwise, the request is always hashed with the socket set to a NULL
> > pointer.
> 
> Sounds plausible. I will need to check the clean-up paths too.
> 

You're right about the error paths. The sock refcounting will need to be
fixed up for sure.

With this but fixed on both ends though, it works for me pretty
consistently now. I guess I was just unlucky with the hashing!


> 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > net/handshake/request.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > I think this is the problem I was hitting earlier. Chuck, feel free to
> > fold this into the original patch that adds this code.
> > 
> > diff --git a/net/handshake/request.c b/net/handshake/request.c
> > index 54ba304b5bef..44a097210e59 100644
> > --- a/net/handshake/request.c
> > +++ b/net/handshake/request.c
> > @@ -207,6 +207,7 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req,
> > 	if (READ_ONCE(hn->hn_pending) >= hn->hn_pending_max)
> > 		goto out_err;
> > 
> > +	req->hr_sk = sk;
> > 	req->hr_odestruct = sk->sk_destruct;
> > 	sk->sk_destruct = handshake_sk_destruct;
> > 	spin_lock(&hn->hn_lock);
> > @@ -230,7 +231,6 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req,
> > 
> > 	/* Prevent socket release while a handshake request is pending */
> > 	sock_hold(sk);
> > -	req->hr_sk = sk;
> > 
> > 	trace_handshake_submit(net, req, sk);
> > 	return 0;
> > -- 
> > 2.39.2
> > 
> 
> --
> Chuck Lever
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>