From 1621809e7e07dc648db431496e5785776f820e44 Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Fri, 27 Mar 2020 14:56:41 -0700 Subject: [PATCH] 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: Ben Walker Change-Id: I20ce875be2efd841fe3a900047b4655a317d7799 Signed-off-by: Ben Walker Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1560 Community-CI: Broadcom CI Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Jim Harris --- lib/nvmf/tcp.c | 19 +++++++++++++++++++ module/sock/posix/posix.c | 22 ++++++++++------------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/lib/nvmf/tcp.c b/lib/nvmf/tcp.c index f06cc9732..7e565c733 100644 --- a/lib/nvmf/tcp.c +++ b/lib/nvmf/tcp.c @@ -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); diff --git a/module/sock/posix/posix.c b/module/sock/posix/posix.c index 01b73c1c0..f3c4774ea 100644 --- a/module/sock/posix/posix.c +++ b/module/sock/posix/posix.c @@ -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;