From cf95d4a24fa2dbbd74a57828557bb3ca91a255b3 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 31 Jul 2019 11:08:30 +0900 Subject: [PATCH] sock: Fix return value of spdk_sock_group_poll to return number of events spdk_sock_group_poll() and spdk_sock_group_poll_count() had returned 0 on success. The implementation didn't match the specification described in the header file, and couldn't be used to collect stats correctly because 0 means idle. This patch fixes the return value of spdk_sock_group_poll() and spdk_sock_group_poll_count() to return number of events and the callers not to overwrite the return value by 0. Signed-off-by: Shuhei Matsumoto Change-Id: I7e2a17187fc74ea44d3acf2f35d63f5e5a254eda Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/463710 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu Reviewed-by: Ben Walker --- include/spdk/sock.h | 2 +- lib/iscsi/iscsi_subsystem.c | 2 +- lib/nvmf/tcp.c | 3 +-- lib/sock/sock.c | 12 +++++++----- test/unit/lib/sock/sock.c/sock_ut.c | 10 +++++----- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/include/spdk/sock.h b/include/spdk/sock.h index 501500bcb..282d224cb 100644 --- a/include/spdk/sock.h +++ b/include/spdk/sock.h @@ -249,7 +249,7 @@ int spdk_sock_group_remove_sock(struct spdk_sock_group *group, struct spdk_sock * * \param group Group to poll. * - * \return 0 on success, -1 on failure. + * \return the number of events on success, -1 on failure. */ int spdk_sock_group_poll(struct spdk_sock_group *group); diff --git a/lib/iscsi/iscsi_subsystem.c b/lib/iscsi/iscsi_subsystem.c index 39a6065f3..a4bdd6268 100644 --- a/lib/iscsi/iscsi_subsystem.c +++ b/lib/iscsi/iscsi_subsystem.c @@ -1177,7 +1177,7 @@ iscsi_poll_group_poll(void *ctx) } } - return -1; + return rc; } static int diff --git a/lib/nvmf/tcp.c b/lib/nvmf/tcp.c index a92212e86..a84621b2e 100644 --- a/lib/nvmf/tcp.c +++ b/lib/nvmf/tcp.c @@ -2776,10 +2776,9 @@ spdk_nvmf_tcp_poll_group_poll(struct spdk_nvmf_transport_poll_group *group) rc = spdk_sock_group_poll(tgroup->sock_group); if (rc < 0) { SPDK_ERRLOG("Failed to poll sock_group=%p\n", tgroup->sock_group); - return rc; } - return 0; + return rc; } static int diff --git a/lib/sock/sock.c b/lib/sock/sock.c index 29426b1b9..5339272a5 100644 --- a/lib/sock/sock.c +++ b/lib/sock/sock.c @@ -456,14 +456,14 @@ spdk_sock_group_impl_poll_count(struct spdk_sock_group_impl *group_impl, assert(sock->cb_fn != NULL); sock->cb_fn(sock->cb_arg, group, sock); } - return 0; + return num_events; } int spdk_sock_group_poll_count(struct spdk_sock_group *group, int max_events) { struct spdk_sock_group_impl *group_impl = NULL; - int rc, final_rc = 0; + int rc, num_events = 0; if (max_events < 1) { errno = -EINVAL; @@ -480,14 +480,16 @@ spdk_sock_group_poll_count(struct spdk_sock_group *group, int max_events) STAILQ_FOREACH_FROM(group_impl, &group->group_impls, link) { rc = spdk_sock_group_impl_poll_count(group_impl, group, max_events); - if (rc != 0) { - final_rc = rc; + if (rc < 0) { + num_events = -1; SPDK_ERRLOG("group_impl_poll_count for net(%s) failed\n", group_impl->net_impl->name); + } else if (num_events >= 0) { + num_events += rc; } } - return final_rc; + return num_events; } int diff --git a/test/unit/lib/sock/sock.c/sock_ut.c b/test/unit/lib/sock/sock.c/sock_ut.c index ca5c674aa..f7d8adfc1 100644 --- a/test/unit/lib/sock/sock.c/sock_ut.c +++ b/test/unit/lib/sock/sock.c/sock_ut.c @@ -510,7 +510,7 @@ _sock_group(const char *ip, int port) g_bytes_read = 0; rc = spdk_sock_group_poll(group); - CU_ASSERT(rc == 0); + CU_ASSERT(rc == 1); CU_ASSERT(g_read_data_called == true); CU_ASSERT(g_bytes_read == 7); @@ -621,7 +621,7 @@ posix_sock_group_fairness(void) */ g_server_sock_read = NULL; rc = spdk_sock_group_poll_count(group, 1); - CU_ASSERT(rc == 0); + CU_ASSERT(rc == 1); CU_ASSERT(g_server_sock_read == server_sock[0]); /* @@ -636,17 +636,17 @@ posix_sock_group_fairness(void) g_server_sock_read = NULL; rc = spdk_sock_group_poll_count(group, 1); - CU_ASSERT(rc == 0); + CU_ASSERT(rc == 1); CU_ASSERT(g_server_sock_read == server_sock[1]); g_server_sock_read = NULL; rc = spdk_sock_group_poll_count(group, 1); - CU_ASSERT(rc == 0); + CU_ASSERT(rc == 1); CU_ASSERT(g_server_sock_read == server_sock[2]); g_server_sock_read = NULL; rc = spdk_sock_group_poll_count(group, 1); - CU_ASSERT(rc == 0); + CU_ASSERT(rc == 1); CU_ASSERT(g_server_sock_read == server_sock[0]); for (i = 0; i < 3; i++) {