From 91f2725291dabf66c09a9220a5824c790dbb3cfc Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Thu, 16 Dec 2021 09:15:31 +0100 Subject: [PATCH] lib/sock: fix lookup on placement_id with NULL sock_group spdk_sock_map_insert() allows for allocating a sock_map entry, without assigning any sock_group. This is useful for cases where placement_id determined by the component using spdk_sock_map_*. See PLACEMENT_MARK mode. Placement_id's are allocated first, then an empty one is found using spdk_sock_map_find_free(). Since the above is a valid use case, then entry in sock_map can exist without a group assigned. spdk_sock_map_lookup() has to handle such cases, rather than trigger an assert. Signed-off-by: Tomasz Zawadzki Change-Id: Ia717c38fef5e71fe44471ea12f61a5548463f0cf Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10725 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: wanghailiang Reviewed-by: Jim Harris Reviewed-by: Dong Yi Reviewed-by: Aleksey Marchuk --- lib/sock/sock.c | 5 +++-- test/unit/lib/sock/sock.c/sock_ut.c | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/sock/sock.c b/lib/sock/sock.c index 9f96be111..0e22a8695 100644 --- a/lib/sock/sock.c +++ b/lib/sock/sock.c @@ -145,9 +145,10 @@ spdk_sock_map_lookup(struct spdk_sock_map *map, int placement_id, pthread_mutex_lock(&map->mtx); STAILQ_FOREACH(entry, &map->entries, link) { if (placement_id == entry->placement_id) { - assert(entry->group != NULL); *group = entry->group; - rc = 0; + if (*group != NULL) { + rc = 0; + } break; } } diff --git a/test/unit/lib/sock/sock.c/sock_ut.c b/test/unit/lib/sock/sock.c/sock_ut.c index 40bcaf117..0fa7c91ab 100644 --- a/test/unit/lib/sock/sock.c/sock_ut.c +++ b/test/unit/lib/sock/sock.c/sock_ut.c @@ -1019,6 +1019,10 @@ ut_sock_map(void) /* Release entry second and final time. */ spdk_sock_map_release(&map, 1); + test_group = NULL; + rc = spdk_sock_map_lookup(&map, 1, &test_group); + CU_ASSERT(rc == -EINVAL); + CU_ASSERT(test_group == NULL); spdk_sock_map_cleanup(&map); @@ -1062,6 +1066,29 @@ ut_sock_map(void) spdk_sock_map_cleanup(&map); + /* Test 6 + * Insert single entry without a sock_group */ + rc = spdk_sock_map_insert(&map, 1, NULL); + CU_ASSERT(rc == 0); + + test_group = NULL; + rc = spdk_sock_map_lookup(&map, 1, &test_group); + CU_ASSERT(rc == -EINVAL); + CU_ASSERT(test_group == NULL); + + test_id = spdk_sock_map_find_free(&map); + CU_ASSERT(test_id == 1); + + rc = spdk_sock_map_insert(&map, test_id, group_1); + CU_ASSERT(rc == 0); + + test_group = NULL; + rc = spdk_sock_map_lookup(&map, test_id, &test_group); + CU_ASSERT(rc == 0); + CU_ASSERT(test_group == group_1); + + spdk_sock_map_cleanup(&map); + spdk_ut_sock_group_impl_close(group_2); spdk_ut_sock_group_impl_close(group_1); }