Commit ee36df5b authored by Jacek Kalwas's avatar Jacek Kalwas Committed by Tomasz Zawadzki
Browse files

sock: simplify spdk_sock_[listen|connect] impl_name handling 



Previously, if NULL was provided and the connection or listen operation
failed the code would continue checking other sock implementations.

This behavior is no longer necessary, as VPP sock module was removed.
The remaining uring and posix implementations both rely on network
kernel stack and upcoming XLIO support shares the same addressing.

This legacy behavior complicates the implementation of asynchronous
connect, particularly when the socket object is returned immediately.

Now, when NULL is provided, the default implementation is selected and
terminates on failure without iterating over other sock implementations.

Change-Id: I57c68d131fac8951107d82d6b6e34db7ec2bafcc
Signed-off-by: default avatarJacek Kalwas <jacek.kalwas@nutanix.com>
Reviewed-on: https://review.spdk.io/c/spdk/spdk/+/26077


Community-CI: Mellanox Build Bot
Reviewed-by: default avatarTomasz Zawadzki <tomasz@tzawadzki.com>
Tested-by: default avatarSPDK Automated Test System <spdkbot@gmail.com>
Reviewed-by: default avatarJim Harris <jim.harris@nvidia.com>
parent 27c505fb

CHANGELOG.md

+9 −0
Original line number Diff line number Diff line
@@ -4,6 +4,15 @@ 


### sock



Simplified spdk_sock_[listen|connect] impl_name handling. Previously, if NULL was provided and the

connection or listen operation failed the code would continue checking other sock implementations.

This behavior is no longer necessary, as VPP sock module was removed. The remaining uring and posix

implementations both rely on network kernel stack and upcoming XLIO support shares the same

addressing. This legacy behavior complicates the implementation of asynchronous connect,

particularly when the socket object is returned immediately. Now, when NULL is provided, the default

implementation is selected and terminates on failure without iterating over other sock

implementations.



Changed the return behavior of `spdk_sock_flush`. The function now returns 0 on success, as relying

on the number of bytes returned was not recommended.

include/spdk/sock.h

+4 −16
Original line number Diff line number Diff line
@@ -307,10 +307,7 @@ const char *spdk_sock_get_impl_name(struct spdk_sock *sock); 
 *

 * \param ip IP address of the server.

 * \param port Port number of the server.

 * \param impl_name The sock_implementation to use, such as "posix". If impl_name is

 * specified, it will *only* try to connect on that impl. If it is NULL, it will try

 * all the sock implementations in order and uses the first sock implementation which

 * can connect.

 * \param impl_name The sock implementation to use, such as "posix", or NULL for default.

 *

 * \return a pointer to the connected socket on success, or NULL on failure.

 */
@@ -323,10 +320,7 @@ struct spdk_sock *spdk_sock_connect(const char *ip, int port, const char *impl_n 
 *

 * \param ip IP address of the server.

 * \param port Port number of the server.

 * \param impl_name The sock_implementation to use, such as "posix". If impl_name is

 * specified, it will *only* try to connect on that impl. If it is NULL, it will try

 * all the sock implementations in order and uses the first sock implementation which

 * can connect.

 * \param impl_name The sock implementation to use, such as "posix", or NULL for default.

 * \param opts The sock option pointer provided by the user which should not be NULL pointer.

 *

 * \return a pointer to the connected socket on success, or NULL on failure.
@@ -341,10 +335,7 @@ struct spdk_sock *spdk_sock_connect_ext(const char *ip, int port, const char *im 
 *

 * \param ip IP address to listen on.

 * \param port Port number.

 * \param impl_name The sock_implementation to use, such as "posix". If impl_name is

 * specified, it will *only* try to listen on that impl. If it is NULL, it will try

 * all the sock implementations in order and uses the first sock implementation which

 * can listen.

 * \param impl_name The sock implementation to use, such as "posix", or NULL for default.

 *

 * \return a pointer to the listened socket on success, or NULL on failure.

 */
@@ -357,10 +348,7 @@ struct spdk_sock *spdk_sock_listen(const char *ip, int port, const char *impl_na 
 *

 * \param ip IP address to listen on.

 * \param port Port number.

 * \param impl_name The sock_implementation to use, such as "posix". If impl_name is

 * specified, it will *only* try to listen on that impl. If it is NULL, it will try

 * all the sock implementations in order and uses the first sock implementation which

 * can listen.

 * \param impl_name The sock implementation to use, such as "posix", or NULL for default.

 * \param opts The sock option pointer provided by the user, which should not be NULL pointer.

 *

 * \return a pointer to the listened socket on success, or NULL on failure.

lib/sock/sock.c

+48 −39
Original line number Diff line number Diff line
@@ -624,8 +624,14 @@ spdk_sock_connect_ext(const char *ip, int port, const char *_impl_name, struct s 
	}



	STAILQ_FOREACH_FROM(impl, &g_net_impls, link) {

		if (impl_name && strncmp(impl_name, impl->name, strlen(impl->name) + 1)) {

			continue;

		if (impl_name && strncmp(impl_name, impl->name, strlen(impl->name) + 1) == 0) {

			break;

		}

	}



	if (!impl) {

		SPDK_ERRLOG("Cannot find %s sock implementation\n", impl_name ? impl_name : "any");

		return NULL;

	}



	SPDK_DEBUGLOG(sock, "Creating a client socket using impl %s\n", impl->name);
@@ -636,7 +642,10 @@ spdk_sock_connect_ext(const char *ip, int port, const char *_impl_name, struct s 
	}



	sock = impl->connect(ip, port, &opts_local);

		if (sock != NULL) {

	if (!sock) {

		return NULL;

	}



	/* Copy the contents, both the two structures are the same ABI version */

	memcpy(&sock->opts, &opts_local, sizeof(sock->opts));

	/* Clear out impl_opts to make sure we don't keep reference to a dangling
@@ -645,13 +654,8 @@ spdk_sock_connect_ext(const char *ip, int port, const char *_impl_name, struct s 
	sock->net_impl = impl;

	TAILQ_INIT(&sock->queued_reqs);

	TAILQ_INIT(&sock->pending_reqs);



	return sock;

}

	}



	return NULL;

}



struct spdk_sock *

spdk_sock_listen(const char *ip, int port, const char *impl_name)
@@ -683,14 +687,23 @@ spdk_sock_listen_ext(const char *ip, int port, const char *_impl_name, struct sp 
	}



	STAILQ_FOREACH_FROM(impl, &g_net_impls, link) {

		if (impl_name && strncmp(impl_name, impl->name, strlen(impl->name) + 1)) {

			continue;

		if (impl_name && strncmp(impl_name, impl->name, strlen(impl->name) + 1) == 0) {

			break;

		}

	}



	if (!impl) {

		SPDK_ERRLOG("Cannot find %s sock implementation\n", impl_name ? impl_name : "any");

		return NULL;

	}



	SPDK_DEBUGLOG(sock, "Creating a listening socket using impl %s\n", impl->name);

	sock_init_opts(&opts_local, opts);

	sock = impl->listen(ip, port, &opts_local);

		if (sock != NULL) {

	if (!sock) {

		return NULL;

	}



	/* Copy the contents, both the two structures are the same ABI version */

	memcpy(&sock->opts, &opts_local, sizeof(sock->opts));

	/* Clear out impl_opts to make sure we don't keep reference to a dangling
@@ -701,10 +714,6 @@ spdk_sock_listen_ext(const char *ip, int port, const char *_impl_name, struct sp 
	 * sockets. */

	return sock;

}

	}



	return NULL;

}



struct spdk_sock *

spdk_sock_accept(struct spdk_sock *sock)