net: sockets: Remove async service support

As found in PR #75525, we should not modify the polled fd array in multiple places. Because of this fix, the async version of the socket service could start to trigger while it is being handled by the async handler. This basically means that the async version cannot work as intended so remove its support. Signed-off-by: Jukka Rissanen <[email protected]>
zephyrproject-rtos · Jul 7, 2024 · 4805b7a · 4805b7a
1 parent a912bb6
commit 4805b7a
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 77 deletions.
diff --git a/include/zephyr/net/socket_service.h b/include/zephyr/net/socket_service.h
@@ -89,17 +89,12 @@ extern void net_socket_service_callback(struct k_work *work);
 #define NET_SOCKET_SERVICE_OWNER
 #endif
 
-#define NET_SOCKET_SERVICE_CALLBACK_MODE(_flag)				\
-	IF_ENABLED(_flag,						\
-		   (.work = Z_WORK_INITIALIZER(net_socket_service_callback),))
-
-#define __z_net_socket_service_define(_name, _work_q, _cb, _count, _async, ...) \
+#define __z_net_socket_service_define(_name, _work_q, _cb, _count, ...) \
 	static int __z_net_socket_svc_get_idx(_name);			\
 	static struct net_socket_service_event				\
 			__z_net_socket_svc_get_name(_name)[_count] = {	\
 		[0 ... ((_count) - 1)] = {				\
 			.event.fd = -1, /* Invalid socket */		\
-			NET_SOCKET_SERVICE_CALLBACK_MODE(_async)	\
 			.callback = _cb,				\
 		}							\
 	};								\
@@ -114,44 +109,6 @@ extern void net_socket_service_callback(struct k_work *work);
 
 /** @endcond */
 
-/**
- * @brief Statically define a network socket service.
- *        The user callback is called asynchronously for this service meaning that
- *        the service API will not wait until the user callback returns before continuing
- *        with next socket service.
- *
- * The socket service can be accessed outside the module where it is defined using:
- *
- * @code extern struct net_socket_service_desc <name>; @endcode
- *
- * @note This macro cannot be used together with a static keyword.
- *       If such a use-case is desired, use NET_SOCKET_SERVICE_ASYNC_DEFINE_STATIC
- *       instead.
- *
- * @param name Name of the service.
- * @param work_q Pointer to workqueue where the work is done. Can be null in which case
- *        system workqueue is used.
- * @param cb Callback function that is called for socket activity.
- * @param count How many pollable sockets is needed for this service.
- */
-#define NET_SOCKET_SERVICE_ASYNC_DEFINE(name, work_q, cb, count)	\
-	__z_net_socket_service_define(name, work_q, cb, count, 1)
-
-/**
- * @brief Statically define a network socket service in a private (static) scope.
- *        The user callback is called asynchronously for this service meaning that
- *        the service API will not wait until the user callback returns before continuing
- *        with next socket service.
- *
- * @param name Name of the service.
- * @param work_q Pointer to workqueue where the work is done. Can be null in which case
- *        system workqueue is used.
- * @param cb Callback function that is called for socket activity.
- * @param count How many pollable sockets is needed for this service.
- */
-#define NET_SOCKET_SERVICE_ASYNC_DEFINE_STATIC(name, work_q, cb, count) \
-	__z_net_socket_service_define(name, work_q, cb, count, 1, static)
-
 /**
  * @brief Statically define a network socket service.
  *        The user callback is called synchronously for this service meaning that
@@ -173,7 +130,7 @@ extern void net_socket_service_callback(struct k_work *work);
  * @param count How many pollable sockets is needed for this service.
  */
 #define NET_SOCKET_SERVICE_SYNC_DEFINE(name, work_q, cb, count)	\
-	__z_net_socket_service_define(name, work_q, cb, count, 0)
+	__z_net_socket_service_define(name, work_q, cb, count)
 
 /**
  * @brief Statically define a network socket service in a private (static) scope.
@@ -188,7 +145,7 @@ extern void net_socket_service_callback(struct k_work *work);
  * @param count How many pollable sockets is needed for this service.
  */
 #define NET_SOCKET_SERVICE_SYNC_DEFINE_STATIC(name, work_q, cb, count)	\
-	__z_net_socket_service_define(name, work_q, cb, count, 0, static)
+	__z_net_socket_service_define(name, work_q, cb, count, static)
 
 /**
  * @brief Register pollable sockets.

diff --git a/samples/net/sockets/echo_service/src/main.c b/samples/net/sockets/echo_service/src/main.c
@@ -57,14 +57,8 @@ static void udp_service_handler(struct k_work *work)
 	receive_data(true, pev, buf, sizeof(buf));
 }
 
-/* In this example we create two services, one with async behavior and one with
- * sync one. The async is for TCP and sync is for UDP (this is just an arbitrary
- * choice).
- * This is an artificial example, both UDP and TCP sockets could be served by the
- * same service.
- */
 NET_SOCKET_SERVICE_SYNC_DEFINE_STATIC(service_udp, NULL, udp_service_handler, MAX_SERVICES);
-NET_SOCKET_SERVICE_ASYNC_DEFINE_STATIC(service_tcp, NULL, tcp_service_handler, MAX_SERVICES);
+NET_SOCKET_SERVICE_SYNC_DEFINE_STATIC(service_tcp, NULL, tcp_service_handler, MAX_SERVICES);
 
 static void receive_data(bool is_udp, struct net_socket_service_event *pev,
 			 char *buf, size_t buflen)
@@ -102,8 +96,13 @@ static void receive_data(bool is_udp, struct net_socket_service_event *pev,
 
 	p = buf;
 	do {
-		out_len = sendto(client, p, len, 0,
-				 (struct sockaddr *)&addr, addrlen);
+		if (is_udp) {
+			out_len = sendto(client, p, len, 0,
+					 (struct sockaddr *)&addr, addrlen);
+		} else {
+			out_len = send(client, p, len, 0);
+		}
+
 		if (out_len < 0) {
 			LOG_ERR("sendto: %d", -errno);
 			break;

diff --git a/subsys/net/lib/sockets/sockets_service.c b/subsys/net/lib/sockets/sockets_service.c
@@ -151,18 +151,8 @@ static int call_work(struct zsock_pollfd *pev, struct k_work_q *work_q,
 	 */
 	pev->fd = -1;
 
-	if (work->handler == NULL) {
-		/* Synchronous call */
-		net_socket_service_callback(work);
-	} else {
-		if (work_q != NULL) {
-			ret = k_work_submit_to_queue(work_q, work);
-		} else {
-			ret = k_work_submit(work);
-		}
-
-		k_yield();
-	}
+	/* Synchronous call */
+	net_socket_service_callback(work);
 
 	return ret;
 

diff --git a/tests/net/socket/service/src/main.c b/tests/net/socket/service/src/main.c
@@ -55,10 +55,6 @@ static void tcp_server_handler(struct k_work *work)
 	Z_SPIN_DELAY(100);
 }
 
-NET_SOCKET_SERVICE_ASYNC_DEFINE(udp_service_async, NULL, server_handler, 2);
-NET_SOCKET_SERVICE_ASYNC_DEFINE(tcp_service_small_async, NULL, tcp_server_handler, 1);
-NET_SOCKET_SERVICE_ASYNC_DEFINE_STATIC(tcp_service_async, NULL, tcp_server_handler, 2);
-
 NET_SOCKET_SERVICE_SYNC_DEFINE(udp_service_sync, NULL, server_handler, 2);
 NET_SOCKET_SERVICE_SYNC_DEFINE(tcp_service_small_sync, NULL, tcp_server_handler, 1);
 NET_SOCKET_SERVICE_SYNC_DEFINE_STATIC(tcp_service_sync, NULL, tcp_server_handler, 2);
@@ -190,10 +186,4 @@ ZTEST(net_socket_service, test_service_sync)
 			 &tcp_service_sync);
 }
 
-ZTEST(net_socket_service, test_service_async)
-{
-	run_test_service(&udp_service_async, &tcp_service_small_async,
-			 &tcp_service_async);
-}
-
 ZTEST_SUITE(net_socket_service, NULL, NULL, NULL, NULL, NULL);