Commit 975f05a7 authored by Stefan Metzmacher's avatar Stefan Metzmacher Committed by Steve French
Browse files

smb: server: call smb_direct_post_recv_credits() when the negotiation is done 



We now activate sc->recv_io.posted.refill_work and sc->idle.immediate_work
only after a successful negotiation, before sending the negotiation
response.

It means the queue_work(sc->workqueue, &sc->recv_io.posted.refill_work)
in put_recvmsg() of the negotiate request, is a no-op now.

It also means our explicit smb_direct_post_recv_credits() will
have queue_work(sc->workqueue, &sc->idle.immediate_work) as no-op.

This should make sure we don't have races and post any immediate
data_transfer message that tries to grant credits to the peer,
before we send the negotiation response, as that will grant
the initial credits to the peer.

Fixes: 0626e664 ("cifsd: add server handler for central processing and tranport layers")
Fixes: 1cde0a74 ("smb: server: don't use delayed_work for post_recv_credits_work")
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Steve French <smfrench@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Signed-off-by: default avatarStefan Metzmacher <metze@samba.org>
Acked-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 6f40e50c

fs/smb/server/transport_rdma.c

+28 −8
Original line number Diff line number Diff line
@@ -418,9 +418,6 @@ static struct smb_direct_transport *alloc_transport(struct rdma_cm_id *cm_id) 


	sc->ib.dev = sc->rdma.cm_id->device;



	INIT_WORK(&sc->recv_io.posted.refill_work,

		  smb_direct_post_recv_credits);

	INIT_WORK(&sc->idle.immediate_work, smb_direct_send_immediate_work);

	INIT_DELAYED_WORK(&sc->idle.timer_work, smb_direct_idle_connection_timer);



	conn = ksmbd_conn_alloc();
@@ -1904,7 +1901,6 @@ static int smb_direct_prepare_negotiation(struct smbdirect_socket *sc) 
		goto out_err;

	}



	smb_direct_post_recv_credits(&sc->recv_io.posted.refill_work);

	return 0;

out_err:

	put_recvmsg(sc, recvmsg);
@@ -2249,8 +2245,8 @@ static int smb_direct_prepare(struct ksmbd_transport *t) 
		return -ECONNABORTED;



	ret = smb_direct_check_recvmsg(recvmsg);

	if (ret == -ECONNABORTED)

		goto out;

	if (ret)

		goto put;



	req = (struct smbdirect_negotiate_req *)recvmsg->packet;

	sp->max_recv_size = min_t(int, sp->max_recv_size,
@@ -2265,14 +2261,38 @@ static int smb_direct_prepare(struct ksmbd_transport *t) 
	sc->recv_io.credits.target = min_t(u16, sc->recv_io.credits.target, sp->recv_credit_max);

	sc->recv_io.credits.target = max_t(u16, sc->recv_io.credits.target, 1);



	ret = smb_direct_send_negotiate_response(sc, ret);

out:

put:

	spin_lock_irqsave(&sc->recv_io.reassembly.lock, flags);

	sc->recv_io.reassembly.queue_length--;

	list_del(&recvmsg->list);

	spin_unlock_irqrestore(&sc->recv_io.reassembly.lock, flags);

	put_recvmsg(sc, recvmsg);



	if (ret == -ECONNABORTED)

		return ret;



	if (ret)

		goto respond;



	/*

	 * We negotiated with success, so we need to refill the recv queue.

	 * We do that with sc->idle.immediate_work still being disabled

	 * via smbdirect_socket_init(), so that queue_work(sc->workqueue,

	 * &sc->idle.immediate_work) in smb_direct_post_recv_credits()

	 * is a no-op.

	 *

	 * The message that grants the credits to the client is

	 * the negotiate response.

	 */

	INIT_WORK(&sc->recv_io.posted.refill_work, smb_direct_post_recv_credits);

	smb_direct_post_recv_credits(&sc->recv_io.posted.refill_work);

	if (unlikely(sc->first_error))

		return sc->first_error;

	INIT_WORK(&sc->idle.immediate_work, smb_direct_send_immediate_work);



respond:

	ret = smb_direct_send_negotiate_response(sc, ret);



	return ret;

}