From 0954302091d19064fb179223bd029683b8962301 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 11 Oct 2022 17:50:00 +0900 Subject: [PATCH] sock: Ensure recv/send_buf_size to be larger than g_sock_impl_opts.recv/send_buf_size In a use case, a custom sock module supports zero copy for read. The custom sock module wants to keep the recv_buf_size to be sufficiently large, for example 16MB. However, most upper layers overwrite the recv_buf_size by a smaller value via spdk_sock_set_recvbuf() later. This is not desirable. To fix the described issue, change the meaning of impl_opts->recv_buf_size to be the minimum size, and spdk_sock_set_recvbuf() uses the maximum value among the requested size, g_spdk_sock_impl_opts->recv_buf_size, and MIN_SO_RCVBUF_SIZE. We may have to change the code to initially create a socket. However, for most cases, the upper layer calls spdk_sock_set_recvbuf() anyway. Hence this fix will be minimal and enough. For the use case, it is enough to change recv_buf_size of the posix sock module. However, the custom sock module may support I/O uring in future. Hence, change I/O uring sock module together. Additionally, for consistency, change the meaning of impl_opts->send_buf_size to be the minimum size, and spdk_sock_set_sendbuf() uses the maximum value among the requested size, g_spdk_sock_impl_opts->send_buf_size, and MIN_SO_SNDBUF_SIZE. Signed-off-by: Shuhei Matsumoto Change-Id: I051ba7cb50bc9dcad229e922198b04fe45335219 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/14915 Tested-by: SPDK CI Jenkins Reviewed-by: Konrad Sztyber Reviewed-by: Aleksey Marchuk Community-CI: Mellanox Build Bot --- include/spdk/sock.h | 4 ++-- module/sock/posix/posix.c | 19 ++++++++++++++----- module/sock/uring/uring.c | 18 ++++++++++++++---- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/include/spdk/sock.h b/include/spdk/sock.h index 6677cf8ed..81bd823d4 100644 --- a/include/spdk/sock.h +++ b/include/spdk/sock.h @@ -83,12 +83,12 @@ enum spdk_placement_mode { */ struct spdk_sock_impl_opts { /** - * Size of sock receive buffer. Used by posix and uring socket modules. + * Minimum size of sock receive buffer. Used by posix and uring socket modules. */ uint32_t recv_buf_size; /** - * Size of sock send buffer. Used by posix and uring socket modules. + * Minimum size of sock send buffer. Used by posix and uring socket modules. */ uint32_t send_buf_size; diff --git a/module/sock/posix/posix.c b/module/sock/posix/posix.c index 934a33b27..435ebf699 100644 --- a/module/sock/posix/posix.c +++ b/module/sock/posix/posix.c @@ -317,6 +317,7 @@ static int posix_sock_set_recvbuf(struct spdk_sock *_sock, int sz) { struct spdk_posix_sock *sock = __posix_sock(_sock); + int min_size; int rc; assert(sock != NULL); @@ -328,9 +329,12 @@ posix_sock_set_recvbuf(struct spdk_sock *_sock, int sz) } } - /* Set kernel buffer size to be at least MIN_SO_RCVBUF_SIZE */ - if (sz < MIN_SO_RCVBUF_SIZE) { - sz = MIN_SO_RCVBUF_SIZE; + /* Set kernel buffer size to be at least MIN_SO_RCVBUF_SIZE and + * g_spdk_posix_sock_impl_opts.recv_buf_size. */ + min_size = spdk_max(MIN_SO_RCVBUF_SIZE, g_spdk_posix_sock_impl_opts.recv_buf_size); + + if (sz < min_size) { + sz = min_size; } rc = setsockopt(sock->fd, SOL_SOCKET, SO_RCVBUF, &sz, sizeof(sz)); @@ -347,12 +351,17 @@ static int posix_sock_set_sendbuf(struct spdk_sock *_sock, int sz) { struct spdk_posix_sock *sock = __posix_sock(_sock); + int min_size; int rc; assert(sock != NULL); - if (sz < MIN_SO_SNDBUF_SIZE) { - sz = MIN_SO_SNDBUF_SIZE; + /* Set kernel buffer size to be at least MIN_SO_SNDBUF_SIZE and + * g_spdk_posix_sock_impl_opts.send_buf_size. */ + min_size = spdk_max(MIN_SO_SNDBUF_SIZE, g_spdk_posix_sock_impl_opts.send_buf_size); + + if (sz < min_size) { + sz = min_size; } rc = setsockopt(sock->fd, SOL_SOCKET, SO_SNDBUF, &sz, sizeof(sz)); diff --git a/module/sock/uring/uring.c b/module/sock/uring/uring.c index 5a43873d0..a82dd1a05 100644 --- a/module/sock/uring/uring.c +++ b/module/sock/uring/uring.c @@ -337,6 +337,7 @@ static int uring_sock_set_recvbuf(struct spdk_sock *_sock, int sz) { struct spdk_uring_sock *sock = __uring_sock(_sock); + int min_size; int rc; assert(sock != NULL); @@ -349,8 +350,12 @@ uring_sock_set_recvbuf(struct spdk_sock *_sock, int sz) } } - if (sz < MIN_SO_RCVBUF_SIZE) { - sz = MIN_SO_RCVBUF_SIZE; + /* Set kernel buffer size to be at least MIN_SO_RCVBUF_SIZE and + * g_spdk_uring_sock_impl_opts.recv_buf_size. */ + min_size = spdk_max(MIN_SO_RCVBUF_SIZE, g_spdk_uring_sock_impl_opts.recv_buf_size); + + if (sz < min_size) { + sz = min_size; } rc = setsockopt(sock->fd, SOL_SOCKET, SO_RCVBUF, &sz, sizeof(sz)); @@ -367,12 +372,17 @@ static int uring_sock_set_sendbuf(struct spdk_sock *_sock, int sz) { struct spdk_uring_sock *sock = __uring_sock(_sock); + int min_size; int rc; assert(sock != NULL); - if (sz < MIN_SO_SNDBUF_SIZE) { - sz = MIN_SO_SNDBUF_SIZE; + /* Set kernel buffer size to be at least MIN_SO_SNDBUF_SIZE and + * g_spdk_uring_sock_impl_opts.seend_buf_size. */ + min_size = spdk_max(MIN_SO_SNDBUF_SIZE, g_spdk_uring_sock_impl_opts.send_buf_size); + + if (sz < min_size) { + sz = min_size; } rc = setsockopt(sock->fd, SOL_SOCKET, SO_SNDBUF, &sz, sizeof(sz));