sock: Don't cache placement_id in generic sock struct

Instead, move it down to the modules. This allows modules
to potentially change the value, if they are able.

Change-Id: I08f5fbadf5d1e96b489ddaaca72aa051ce2cb85c
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7212
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

include/spdk_internal/sock.h

@@ -66,7 +66,6 @@ struct spdk_sock {
 	int				cb_cnt;
 	spdk_sock_cb			cb_fn;
 	void				*cb_arg;
-	int				placement_id;
 	struct {
 		uint8_t		closed		: 1;
 		uint8_t		reserved	: 7;
@@ -271,6 +270,35 @@ end:
 	return iovcnt;
 }
 
+static inline void
+spdk_sock_get_placement_id(int fd, enum spdk_placement_mode mode, int *placement_id)
+{
+	*placement_id = -1;
+
+	switch (mode) {
+	case PLACEMENT_NONE:
+		break;
+	case PLACEMENT_NAPI: {
+#if defined(SO_INCOMING_NAPI_ID)
+		socklen_t len = sizeof(int);
+
+		getsockopt(fd, SOL_SOCKET, SO_INCOMING_NAPI_ID, placement_id, &len);
+#endif
+		break;
+	}
+	case PLACEMENT_CPU: {
+#if defined(SO_INCOMING_CPU)
+		socklen_t len = sizeof(int);
+
+		getsockopt(fd, SOL_SOCKET, SO_INCOMING_CPU, placement_id, &len);
+#endif
+		break;
+	}
+	default:
+		break;
+	}
+}
+
 #ifdef __cplusplus
 }
 #endif

lib/sock/sock.c

@@ -166,16 +166,14 @@ static int
 sock_get_placement_id(struct spdk_sock *sock)
 {
 	int rc;
-	int placement_id = -1;
+	int placement_id;
 
-	if (sock->placement_id == -1) {
 	rc = sock->net_impl->get_placement_id(sock, &placement_id);
-		if (!rc && (placement_id != -1)) {
+	if (rc) {
-			sock->placement_id = placement_id;
+		placement_id = -1;
-		}
 	}
 
-	return sock->placement_id;
+	return placement_id;
 }
 
 int
@@ -278,8 +276,6 @@ spdk_sock_connect_ext(const char *ip, int port, char *_impl_name, struct spdk_so
 			/* Copy the contents, both the two structures are the same ABI version */
 			memcpy(&sock->opts, &opts_local, sizeof(sock->opts));
 			sock->net_impl = impl;
-			/* Set the placement_id to -1 explicitly */
-			sock->placement_id = -1;
 			TAILQ_INIT(&sock->queued_reqs);
 			TAILQ_INIT(&sock->pending_reqs);
 			return sock;
@@ -350,8 +346,6 @@ spdk_sock_accept(struct spdk_sock *sock)
 		new_sock->opts = sock->opts;
 		memcpy(&new_sock->opts, &sock->opts, sizeof(new_sock->opts));
 		new_sock->net_impl = sock->net_impl;
-		/* Set the placement_id to -1 explicitly */
-		new_sock->placement_id = -1;
 		TAILQ_INIT(&new_sock->queued_reqs);
 		TAILQ_INIT(&new_sock->pending_reqs);
 	}

module/sock/posix/posix.c

@@ -70,6 +70,8 @@ struct spdk_posix_sock {
 	bool			pending_recv;
 	bool			zcopy;
 
+	int			placement_id;
+
 	TAILQ_ENTRY(spdk_posix_sock)	link;
 };
 
@@ -349,6 +351,9 @@ posix_sock_alloc(int fd, bool enable_zero_copy)
 			SPDK_ERRLOG("quickack was failed to set\n");
 		}
 	}
+
+	spdk_sock_get_placement_id(sock->fd, g_spdk_posix_sock_impl_opts.enable_placement_id,
+				   &sock->placement_id);
 #endif
 
 	return sock;
@@ -1104,41 +1109,13 @@ posix_sock_is_connected(struct spdk_sock *_sock)
 static int
 posix_sock_get_placement_id(struct spdk_sock *_sock, int *placement_id)
 {
-	int rc = -1;
-
-	if (!g_spdk_posix_sock_impl_opts.enable_placement_id) {
-		return rc;
-	}
-
-	if (g_spdk_posix_sock_impl_opts.enable_placement_id != PLACEMENT_NONE) {
-		switch (g_spdk_posix_sock_impl_opts.enable_placement_id) {
-		case PLACEMENT_NAPI: {
-#if defined(SO_INCOMING_NAPI_ID)
 	struct spdk_posix_sock *sock = __posix_sock(_sock);
-			socklen_t len = sizeof(int);
 
-			rc = getsockopt(sock->fd, SOL_SOCKET, SO_INCOMING_NAPI_ID, placement_id, &len);
+	assert(placement_id);
-#endif
-			break;
-		}
-		case PLACEMENT_CPU: {
-#if defined(SO_INCOMING_CPU)
-			struct spdk_posix_sock *sock = __posix_sock(_sock);
-			socklen_t len = sizeof(int);
 
-			rc = getsockopt(sock->fd, SOL_SOCKET, SO_INCOMING_CPU, placement_id, &len);
+	*placement_id = sock->placement_id;
-#endif
-			break;
-		}
-		default:
-			break;
-		}
-	}
 
-	if (rc != 0) {
+	return 0;
-		SPDK_ERRLOG("getsockopt() failed (errno=%d)\n", errno);
-	}
-	return rc;
 }
 
 static struct spdk_sock_group_impl *

module/sock/uring/uring.c

@@ -85,6 +85,7 @@ struct spdk_uring_sock {
 	int					recv_buf_sz;
 	bool					pending_recv;
 	int					connection_status;
+	int					placement_id;
 	TAILQ_ENTRY(spdk_uring_sock)		link;
 };
 
@@ -356,7 +357,11 @@ uring_sock_alloc(int fd)
 			SPDK_ERRLOG("quickack was failed to set\n");
 		}
 	}
+
+	spdk_sock_get_placement_id(sock->fd, g_spdk_uring_sock_impl_opts.enable_placement_id,
+				   &sock->placement_id);
 #endif
+
 	return sock;
 }
 
@@ -1092,41 +1097,13 @@ uring_sock_is_connected(struct spdk_sock *_sock)
 static int
 uring_sock_get_placement_id(struct spdk_sock *_sock, int *placement_id)
 {
-	int rc = -1;
-
-	if (!g_spdk_uring_sock_impl_opts.enable_placement_id) {
-		return rc;
-	}
-
-	if (g_spdk_uring_sock_impl_opts.enable_placement_id != PLACEMENT_NONE) {
-		switch (g_spdk_uring_sock_impl_opts.enable_placement_id) {
-		case PLACEMENT_NAPI: {
-#if defined(SO_INCOMING_NAPI_ID)
 	struct spdk_uring_sock *sock = __uring_sock(_sock);
-			socklen_t len = sizeof(int);
 
-			rc = getsockopt(sock->fd, SOL_SOCKET, SO_INCOMING_NAPI_ID, placement_id, &len);
+	assert(placement_id);
-#endif
-			break;
-		}
-		case PLACEMENT_CPU: {
-#if defined(SO_INCOMING_CPU)
-			struct spdk_uring_sock *sock = __uring_sock(_sock);
-			socklen_t len = sizeof(int);
 
-			rc = getsockopt(sock->fd, SOL_SOCKET, SO_INCOMING_CPU, placement_id, &len);
+	*placement_id = sock->placement_id;
-#endif
-			break;
-		}
-		default:
-			break;
-		}
-	}
 
-	if (rc != 0) {
+	return 0;
-		SPDK_ERRLOG("getsockopt() failed (errno=%d)\n", errno);
-	}
-	return rc;
 }
 
 static struct spdk_sock_group_impl *