Commit 1621809e authored by Ben Walker's avatar Ben Walker Committed by Tomasz Zawadzki
Browse files

nvmf/tcp: Correctly size the socket receive buffer 



The code used to do this but it was removed when the buffering was
shifted down to the posix layer. Add a way for users of sockets
to still properly size the buffers.

This also means that by default, the receive buffering is not enabled
on sockets. That matches the behavior of the previous release.

Signed-off-by: default avatarBen Walker <benjamin.walker@intel.com>
Change-Id: I20ce875be2efd841fe3a900047b4655a317d7799
Signed-off-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1560


Community-CI: Broadcom CI
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent ae6519e4

lib/nvmf/tcp.c

+19 −0
Original line number Diff line number Diff line
@@ -209,6 +209,7 @@ struct spdk_nvmf_tcp_qpair { 


	/* PDU being actively received */

	struct nvme_tcp_pdu			pdu_in_progress;

	uint32_t				recv_buf_size;



	/* This is a spare PDU used for sending special management

	 * operations. Primarily, this is used for the initial
@@ -818,6 +819,9 @@ spdk_nvmf_tcp_qpair_init_mem_resource(struct spdk_nvmf_tcp_qpair *tqpair) 
		tqpair->state_cntr[TCP_REQUEST_STATE_FREE]++;

	}



	tqpair->recv_buf_size = (in_capsule_data_size + sizeof(struct spdk_nvme_tcp_cmd) + 2 *

				 SPDK_NVME_TCP_DIGEST_LEN) * SPDK_NVMF_TCP_RECV_BUF_SIZE_FACTOR;



	return 0;

}
@@ -1493,7 +1497,22 @@ spdk_nvmf_tcp_icreq_handle(struct spdk_nvmf_tcp_transport *ttransport, 
	SPDK_DEBUGLOG(SPDK_LOG_NVMF_TCP, "maxr2t =%u\n", (ic_req->maxr2t + 1u));



	tqpair->host_hdgst_enable = ic_req->dgst.bits.hdgst_enable ? true : false;

	if (!tqpair->host_hdgst_enable) {

		tqpair->recv_buf_size -= SPDK_NVME_TCP_DIGEST_LEN * SPDK_NVMF_TCP_RECV_BUF_SIZE_FACTOR;

	}



	tqpair->host_ddgst_enable = ic_req->dgst.bits.ddgst_enable ? true : false;

	if (!tqpair->host_ddgst_enable) {

		tqpair->recv_buf_size -= SPDK_NVME_TCP_DIGEST_LEN * SPDK_NVMF_TCP_RECV_BUF_SIZE_FACTOR;

	}



	/* Now that we know whether digests are enabled, properly size the receive buffer */

	if (spdk_sock_set_recvbuf(tqpair->sock, tqpair->recv_buf_size) < 0) {

		SPDK_WARNLOG("Unable to allocate enough memory for receive buffer on tqpair=%p with size=%d\n",

			     tqpair,

			     tqpair->recv_buf_size);

		/* Not fatal. */

	}



	tqpair->cpda = spdk_min(ic_req->hpda, SPDK_NVME_TCP_CPDA_MAX);

	SPDK_DEBUGLOG(SPDK_LOG_NVMF_TCP, "cpda of tqpair=(%p) is : %u\n", tqpair, tqpair->cpda);

module/sock/posix/posix.c

+10 −12
Original line number Diff line number Diff line
@@ -257,6 +257,15 @@ spdk_posix_sock_set_recvbuf(struct spdk_sock *_sock, int sz) 


	assert(sock != NULL);



#ifndef __aarch64__

	/* On ARM systems, this buffering does not help. Skip it. */

	rc = spdk_posix_sock_alloc_pipe(sock, sz);

	if (rc) {

		return rc;

	}

#endif



	/* Set kernel buffer size to be at least SO_RCVBUF_SIZE */

	if (sz < SO_RCVBUF_SIZE) {

		sz = SO_RCVBUF_SIZE;

	}
@@ -293,8 +302,8 @@ static struct spdk_posix_sock * 
_spdk_posix_sock_alloc(int fd)

{

	struct spdk_posix_sock *sock;

	int rc;

#ifdef SPDK_ZEROCOPY

	int rc;

	int flag;

#endif
@@ -306,17 +315,6 @@ _spdk_posix_sock_alloc(int fd) 


	sock->fd = fd;



#ifndef __aarch64__

	/* On ARM systems, this buffering does not help. Skip it. */

	/* The size of the pipe is purely derived from benchmarks. It seems to work well. */

	rc = spdk_posix_sock_alloc_pipe(sock, 8192);

	if (rc) {

		SPDK_ERRLOG("unable to allocate sufficient recvbuf\n");

		free(sock);

		return NULL;

	}

#endif



#ifdef SPDK_ZEROCOPY

	/* Try to turn on zero copy sends */

	flag = 1;