From 2cbc9d4dff224dc1f5b4a3b01b0314d461e6f1a0 Mon Sep 17 00:00:00 2001 From: Sudheer Mogilappagari Date: Tue, 6 Apr 2021 14:48:51 -0700 Subject: [PATCH] posix: Group connections of sock group on host side based on placement_id On host side the connections are created and then added to thread's poll group. Those connections could use different NIC queues underneath. To route all connections of poll group through single queue a unique placement id is chosen as group_placement_id and each socket of poll group is marked with group_placment_id using getsockopt(SO_MARK) option. The driver could use so_mark value of skb to determine the queue to use. Change-Id: I06bda777fe07a62133b80b2491fa7772150b3b5d Signed-off-by: Sudheer Mogilappagari Signed-off-by: Ben Walker Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6160 Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris Reviewed-by: Tomasz Zawadzki Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins --- CHANGELOG.md | 5 +++ include/spdk/sock.h | 1 + include/spdk_internal/sock.h | 1 + module/sock/posix/posix.c | 67 ++++++++++++++++++++++++++++++++++-- 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4443da775..241417bc72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -174,6 +174,11 @@ bool to int. We can use RPC to configure different value of `enable_placement_id Then we can leverage SO_INCOMING_CPU to get placement_id, which aims to utilize CPU cache locality, enabled by setting enable_placement_id=2. +A new socket placement mode called PLACEMENT_MARK has been added. Some NICs allow +sockets to be marked using the SO_MARK socket option as a hint for which hardware +queue they should be associated with. This mode leverages that by setting the same +value for all sockets within a poll group. + ### thread A new API `spdk_io_channel_get_io_device` was added to get the io_device for the specified diff --git a/include/spdk/sock.h b/include/spdk/sock.h index 249020d55b..73b6a2046c 100644 --- a/include/spdk/sock.h +++ b/include/spdk/sock.h @@ -84,6 +84,7 @@ enum spdk_placement_mode { PLACEMENT_NONE, PLACEMENT_NAPI, PLACEMENT_CPU, + PLACEMENT_MARK, }; /** diff --git a/include/spdk_internal/sock.h b/include/spdk_internal/sock.h index 138805ac4c..841b1cf5c4 100644 --- a/include/spdk_internal/sock.h +++ b/include/spdk_internal/sock.h @@ -284,6 +284,7 @@ spdk_sock_get_placement_id(int fd, enum spdk_placement_mode mode, int *placement switch (mode) { case PLACEMENT_NONE: break; + case PLACEMENT_MARK: case PLACEMENT_NAPI: { #if defined(SO_INCOMING_NAPI_ID) socklen_t len = sizeof(int); diff --git a/module/sock/posix/posix.c b/module/sock/posix/posix.c index 9b763952ab..53dc1d4983 100644 --- a/module/sock/posix/posix.c +++ b/module/sock/posix/posix.c @@ -82,6 +82,7 @@ struct spdk_posix_sock_group_impl { struct spdk_sock_group_impl base; int fd; struct spdk_pending_events_list pending_events; + int placement_id; }; static struct spdk_sock_impl_opts g_spdk_posix_sock_impl_opts = { @@ -368,6 +369,11 @@ posix_sock_alloc(int fd, bool enable_zero_copy) spdk_sock_get_placement_id(sock->fd, g_spdk_posix_sock_impl_opts.enable_placement_id, &sock->placement_id); + + if (g_spdk_posix_sock_impl_opts.enable_placement_id == PLACEMENT_MARK) { + /* Save placement_id */ + spdk_sock_map_insert(&g_map, sock->placement_id, NULL); + } #endif return sock; @@ -1148,14 +1154,69 @@ posix_sock_group_impl_create(void) group_impl->fd = fd; TAILQ_INIT(&group_impl->pending_events); + group_impl->placement_id = -1; if (g_spdk_posix_sock_impl_opts.enable_placement_id == PLACEMENT_CPU) { spdk_sock_map_insert(&g_map, spdk_env_get_current_core(), &group_impl->base); + group_impl->placement_id = spdk_env_get_current_core(); } return &group_impl->base; } +static void +posix_sock_mark(struct spdk_posix_sock_group_impl *group, struct spdk_posix_sock *sock, + int placement_id) +{ +#if defined(SO_MARK) + int rc; + + rc = setsockopt(sock->fd, SOL_SOCKET, SO_MARK, + &placement_id, sizeof(placement_id)); + if (rc != 0) { + /* Not fatal */ + SPDK_ERRLOG("Error setting SO_MARK\n"); + return; + } + + rc = spdk_sock_map_insert(&g_map, placement_id, &group->base); + if (rc != 0) { + /* Not fatal */ + SPDK_ERRLOG("Failed to insert sock group into map: %d\n", rc); + return; + } + + sock->placement_id = placement_id; +#endif +} + +static void +posix_sock_update_mark(struct spdk_sock_group_impl *_group, struct spdk_sock *_sock) +{ + struct spdk_posix_sock_group_impl *group = __posix_group_impl(_group); + + if (group->placement_id == -1) { + group->placement_id = spdk_sock_map_find_free(&g_map); + + /* If a free placement id is found, update existing sockets in this group */ + if (group->placement_id != -1) { + struct spdk_sock *sock, *tmp; + + TAILQ_FOREACH_SAFE(sock, &_group->socks, link, tmp) { + posix_sock_mark(group, __posix_sock(sock), group->placement_id); + } + } + } + + if (group->placement_id != -1) { + /* + * group placement id is already determined for this poll group. + * Mark socket with group's placement id. + */ + posix_sock_mark(group, __posix_sock(_sock), group->placement_id); + } +} + static int posix_sock_group_impl_add_sock(struct spdk_sock_group_impl *_group, struct spdk_sock *_sock) { @@ -1193,10 +1254,12 @@ posix_sock_group_impl_add_sock(struct spdk_sock_group_impl *_group, struct spdk_ TAILQ_INSERT_TAIL(&group->pending_events, sock, link); } - if (sock->placement_id != -1) { + if (g_spdk_posix_sock_impl_opts.enable_placement_id == PLACEMENT_MARK) { + posix_sock_update_mark(_group, _sock); + } else if (sock->placement_id != -1) { rc = spdk_sock_map_insert(&g_map, sock->placement_id, &group->base); if (rc != 0) { - SPDK_ERRLOG("Failed to insert sock group into map: %d", rc); + SPDK_ERRLOG("Failed to insert sock group into map: %d\n", rc); /* Do not treat this as an error. The system will continue running. */ } }