This is an automated email from the ASF dual-hosted git repository. xiaoxiang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 9befb8ae4afd2017018105e1f15c1a94a109f5b7 Author: zhanghongyu <[email protected]> AuthorDate: Wed May 14 17:40:43 2025 +0800 net/udp: remove net_lock protect UDP resources through netdev_lock, conn_lock, and udp_list_lock. Signed-off-by: zhanghongyu <[email protected]> --- net/devif/devif_callback.c | 22 ++++--------- net/devif/devif_poll.c | 2 ++ net/procfs/net_udp.c | 10 ++---- net/udp/udp.h | 26 +++++++++++++++ net/udp/udp_callback.c | 10 +++++- net/udp/udp_close.c | 9 +++--- net/udp/udp_conn.c | 64 +++++++++++++++++++++++++++---------- net/udp/udp_input.c | 3 ++ net/udp/udp_ioctl.c | 5 +-- net/udp/udp_netpoll.c | 26 ++++++++------- net/udp/udp_recvfrom.c | 21 ++++++------ net/udp/udp_sendto_buffered.c | 71 ++++++++++++++++++++++------------------- net/udp/udp_sendto_unbuffered.c | 20 ++++++------ net/udp/udp_txdrain.c | 7 ++-- 14 files changed, 183 insertions(+), 113 deletions(-) diff --git a/net/devif/devif_callback.c b/net/devif/devif_callback.c index 0f3a1142c32..37782cfdc8b 100644 --- a/net/devif/devif_callback.c +++ b/net/devif/devif_callback.c @@ -93,11 +93,10 @@ static void devif_callback_free(FAR struct net_driver_s *dev, if (cb) { - net_lock(); - #ifdef CONFIG_DEBUG_FEATURES /* Check for double freed callbacks */ + NET_BUFPOOL_LOCK(g_cbprealloc); curr = (FAR struct devif_callback_s *)g_cbprealloc.freebuffers.head; while (curr != NULL) @@ -105,6 +104,8 @@ static void devif_callback_free(FAR struct net_driver_s *dev, DEBUGASSERT(cb != curr); curr = curr->nxtconn; } + + NET_BUFPOOL_UNLOCK(g_cbprealloc); #endif /* Remove the callback structure from the data notification list if @@ -164,7 +165,6 @@ static void devif_callback_free(FAR struct net_driver_s *dev, if (cb->free_flags & DEVIF_CB_DONT_FREE) { cb->free_flags |= DEVIF_CB_PEND_FREE; - net_unlock(); return; } @@ -200,8 +200,6 @@ static void devif_callback_free(FAR struct net_driver_s *dev, /* Free the callback structure */ NET_BUFPOOL_FREE(g_cbprealloc, cb); - - net_unlock(); } } @@ -265,14 +263,12 @@ static bool devif_event_trigger(uint16_t events, uint16_t triggers) ****************************************************************************/ FAR struct devif_callback_s * - devif_callback_alloc(FAR struct net_driver_s *dev, - FAR struct devif_callback_s **list_head, - FAR struct devif_callback_s **list_tail) +devif_callback_alloc(FAR struct net_driver_s *dev, + FAR struct devif_callback_s **list_head, + FAR struct devif_callback_s **list_tail) { FAR struct devif_callback_s *ret; - net_lock(); - /* Verify that the device pointer is valid, i.e., that it still * points to a registered network device and also that the network * device in the UP state. @@ -288,7 +284,6 @@ FAR struct devif_callback_s * if (dev && !(netdev_verify(dev) && (dev->d_flags & IFF_UP) != 0)) { - net_unlock(); return NULL; } @@ -339,7 +334,6 @@ FAR struct devif_callback_s * } #endif - net_unlock(); return ret; } @@ -466,7 +460,6 @@ uint16_t devif_conn_event(FAR struct net_driver_s *dev, uint16_t flags, * set in the flags set. */ - net_lock(); while (list && flags) { /* Save the pointer to the next callback in the lists. This is done @@ -493,7 +486,6 @@ uint16_t devif_conn_event(FAR struct net_driver_s *dev, uint16_t flags, list = next; } - net_unlock(); return flags; } @@ -525,7 +517,6 @@ uint16_t devif_dev_event(FAR struct net_driver_s *dev, uint16_t flags) * set in the flags set. */ - net_lock(); for (cb = dev->d_devcb; cb != NULL && flags != 0; cb = next) { /* Save the pointer to the next callback in the lists. This is done @@ -562,7 +553,6 @@ uint16_t devif_dev_event(FAR struct net_driver_s *dev, uint16_t flags) } } - net_unlock(); return flags; } diff --git a/net/devif/devif_poll.c b/net/devif/devif_poll.c index 8b762bfacce..9f7d6ca9d1e 100644 --- a/net/devif/devif_poll.c +++ b/net/devif/devif_poll.c @@ -592,6 +592,7 @@ static int devif_poll_udp_connections(FAR struct net_driver_s *dev, * action. */ + udp_conn_list_lock(); while (!bstop && (conn = udp_nextconn(conn))) { #ifdef CONFIG_NET_UDP_WRITE_BUFFERS @@ -614,6 +615,7 @@ static int devif_poll_udp_connections(FAR struct net_driver_s *dev, } } + udp_conn_list_unlock(); return bstop; } #endif /* NET_UDP_HAVE_STACK */ diff --git a/net/procfs/net_udp.c b/net/procfs/net_udp.c index 6bae9e75831..73c9044f86e 100644 --- a/net/procfs/net_udp.c +++ b/net/procfs/net_udp.c @@ -68,8 +68,6 @@ static ssize_t netprocfs_udpstats(FAR struct netprocfs_file_s *priv, FAR void *laddr; FAR void *raddr; - net_lock(); - while ((conn = udp_nextconn(conn)) != NULL) { #if defined(CONFIG_NET_IPv4) && defined(CONFIG_NET_IPv6) @@ -116,8 +114,6 @@ static ssize_t netprocfs_udpstats(FAR struct netprocfs_file_s *priv, ntohs(conn->rport)); } - net_unlock(); - return len; } @@ -149,8 +145,7 @@ ssize_t netprocfs_read_udpstats(FAR struct netprocfs_file_s *priv, int skip = 1; int len = 0; - net_lock(); - + udp_conn_list_lock(); if (udp_nextconn(NULL) != NULL) { if (priv->offset == 0) @@ -183,8 +178,7 @@ ssize_t netprocfs_read_udpstats(FAR struct netprocfs_file_s *priv, #endif /* CONFIG_NET_IPv6 */ } - net_unlock(); - + udp_conn_list_unlock(); return len; } diff --git a/net/udp/udp.h b/net/udp/udp.h index 78314bbbe07..ebfd5500468 100644 --- a/net/udp/udp.h +++ b/net/udp/udp.h @@ -253,6 +253,32 @@ FAR struct udp_conn_s *udp_active(FAR struct net_driver_s *dev, FAR struct udp_conn_s *udp_nextconn(FAR struct udp_conn_s *conn); +/**************************************************************************** + * Name: udp_conn_list_lock + * + * Description: + * Lock the UDP connection list + * + * Assumptions: + * This function must be called by driver thread. + * + ****************************************************************************/ + +void udp_conn_list_lock(void); + +/**************************************************************************** + * Name: udp_conn_list_unlock + * + * Description: + * Unlock the UDP connection list + * + * Assumptions: + * This function must be called by driver thread. + * + ****************************************************************************/ + +void udp_conn_list_unlock(void); + /**************************************************************************** * Name: udp_select_port * diff --git a/net/udp/udp_callback.c b/net/udp/udp_callback.c index 970439c0f90..710bd95657e 100644 --- a/net/udp/udp_callback.c +++ b/net/udp/udp_callback.c @@ -78,9 +78,11 @@ static uint16_t udp_datahandler(FAR struct net_driver_s *dev, FAR void *src_addr; int offset; + conn_lock(&conn->sconn); #if CONFIG_NET_RECV_BUFSIZE > 0 if (conn->readahead && conn->readahead->io_pktlen > conn->rcvbufs) { + conn_unlock(&conn->sconn); netdev_iob_release(dev); return 0; } @@ -214,6 +216,7 @@ static uint16_t udp_datahandler(FAR struct net_driver_s *dev, /* Concat the iob to readahead */ net_iob_concat(&conn->readahead, &iob); + conn_unlock(&conn->sconn); #ifdef CONFIG_NET_UDP_NOTIFIER ninfo("Buffered %d bytes\n", buflen); @@ -230,6 +233,7 @@ static uint16_t udp_datahandler(FAR struct net_driver_s *dev, errout: nerr("ERROR: Failed to queue the I/O buffer chain: %d\n", ret); + conn_unlock(&conn->sconn); netdev_iob_release(dev); return 0; @@ -313,7 +317,9 @@ uint16_t udp_callback(FAR struct net_driver_s *dev, { /* Perform the callback */ + conn_lock(&conn->sconn); flags = devif_conn_event(dev, flags, conn->sconn.list); + conn_unlock(&conn->sconn); if ((flags & UDP_NEWDATA) != 0) { @@ -343,11 +349,13 @@ void udp_callback_cleanup(FAR void *arg) nerr("ERROR: pthread is being canceled, need to cleanup cb\n"); - udp_callback_free(cb->dev, cb->conn, cb->udp_cb); + conn_dev_lock(&cb->conn->sconn, cb->dev); if (cb->sem) { nxsem_destroy(cb->sem); } + + conn_dev_unlock(&cb->conn->sconn, cb->dev); } #endif /* CONFIG_NET && CONFIG_NET_UDP */ diff --git a/net/udp/udp_close.c b/net/udp/udp_close.c index ea0ee25abc3..130761df2f4 100644 --- a/net/udp/udp_close.c +++ b/net/udp/udp_close.c @@ -35,8 +35,9 @@ #include <nuttx/net/udp.h> #include "devif/devif.h" -#include "udp/udp.h" #include "socket/socket.h" +#include "utils/utils.h" +#include "udp/udp.h" /**************************************************************************** * Public Functions @@ -67,8 +68,6 @@ int udp_close(FAR struct socket *psock) /* Lock the network to avoid race conditions */ - net_lock(); - conn = psock->s_conn; DEBUGASSERT(conn != NULL); @@ -112,8 +111,10 @@ int udp_close(FAR struct socket *psock) if (conn->sndcb != NULL) { + conn_dev_lock(&conn->sconn, conn->dev); udp_callback_free(conn->dev, conn, conn->sndcb); conn->sndcb = NULL; + conn_dev_unlock(&conn->sconn, conn->dev); } #endif @@ -121,7 +122,7 @@ int udp_close(FAR struct socket *psock) conn->crefs = 0; udp_free(psock->s_conn); - net_unlock(); + return OK; } diff --git a/net/udp/udp_conn.c b/net/udp/udp_conn.c index ee3f64762d4..8cdebd41743 100644 --- a/net/udp/udp_conn.c +++ b/net/udp/udp_conn.c @@ -129,6 +129,7 @@ static FAR struct udp_conn_s *udp_find_conn(uint8_t domain, /* Now search each connection structure. */ + udp_conn_list_lock(); while ((conn = udp_nextconn(conn)) != NULL) { /* With SO_REUSEADDR set for both sockets, we do not need to check its @@ -157,7 +158,7 @@ static FAR struct udp_conn_s *udp_find_conn(uint8_t domain, (net_ipv4addr_cmp(conn->u.ipv4.laddr, ipaddr->ipv4.laddr) || net_ipv4addr_cmp(conn->u.ipv4.laddr, INADDR_ANY))) { - return conn; + break; } } #endif /* CONFIG_NET_IPv4 */ @@ -171,13 +172,14 @@ static FAR struct udp_conn_s *udp_find_conn(uint8_t domain, (net_ipv6addr_cmp(conn->u.ipv6.laddr, ipaddr->ipv6.laddr) || net_ipv6addr_cmp(conn->u.ipv6.laddr, g_ipv6_unspecaddr))) { - return conn; + break; } } #endif /* CONFIG_NET_IPv6 */ } - return NULL; + udp_conn_list_unlock(); + return conn; } /**************************************************************************** @@ -316,7 +318,7 @@ udp_ipv4_active(FAR struct net_driver_s *dev, FAR struct udp_conn_s *conn, /* Look at the next active connection */ - conn = (FAR struct udp_conn_s *)conn->sconn.node.flink; + conn = udp_nextconn(conn); } return conn; @@ -456,7 +458,7 @@ udp_ipv6_active(FAR struct net_driver_s *dev, FAR struct udp_conn_s *conn, /* Look at the next active connection */ - conn = (FAR struct udp_conn_s *)conn->sconn.node.flink; + conn = udp_nextconn(conn); } return conn; @@ -492,8 +494,6 @@ uint16_t udp_select_port(uint8_t domain, FAR union ip_binding_u *u) static uint16_t g_last_udp_port; uint16_t portno; - net_lock(); - /* Generate port base dynamically */ if (g_last_udp_port == 0) @@ -514,8 +514,7 @@ uint16_t udp_select_port(uint8_t domain, FAR union ip_binding_u *u) { /* We have looped back, failed. */ - portno = 0; - goto errout; + return 0; } } while (udp_find_conn(domain, u, HTONS(g_last_udp_port), 0) != NULL @@ -531,9 +530,6 @@ uint16_t udp_select_port(uint8_t domain, FAR union ip_binding_u *u) portno = g_last_udp_port; -errout: - net_unlock(); - return portno; } @@ -575,6 +571,7 @@ FAR struct udp_conn_s *udp_alloc(uint8_t domain) nxsem_init(&conn->sndsem, 0, 0); #endif + nxmutex_init(&conn->sconn.s_lock); #ifdef CONFIG_NET_UDP_WRITE_BUFFERS /* Initialize the write buffer lists */ @@ -614,6 +611,7 @@ void udp_free(FAR struct udp_conn_s *conn) /* Remove the connection from the active list */ dq_rem(&conn->sconn.node, &g_active_udp_connections); + nxmutex_destroy(&conn->sconn.s_lock); /* Release any read-ahead buffers attached to the connection, NULL is ok */ @@ -682,6 +680,38 @@ FAR struct udp_conn_s *udp_active(FAR struct net_driver_s *dev, #endif /* CONFIG_NET_IPv4 */ } +/**************************************************************************** + * Name: udp_conn_list_lock + * + * Description: + * Lock the UDP connection list + * + * Assumptions: + * This function must be called by driver thread. + * + ****************************************************************************/ + +void udp_conn_list_lock(void) +{ + NET_BUFPOOL_LOCK(g_udp_connections); +} + +/**************************************************************************** + * Name: udp_conn_list_unlock + * + * Description: + * Unlock the UDP connection list + * + * Assumptions: + * This function must be called by driver thread. + * + ****************************************************************************/ + +void udp_conn_list_unlock(void) +{ + NET_BUFPOOL_UNLOCK(g_udp_connections); +} + /**************************************************************************** * Name: udp_nextconn * @@ -689,7 +719,7 @@ FAR struct udp_conn_s *udp_active(FAR struct net_driver_s *dev, * Traverse the list of allocated UDP connections * * Assumptions: - * This function must be called with the network locked. + * This function must be called with the udp_conn_list_lock. * ****************************************************************************/ @@ -734,7 +764,7 @@ int udp_bind(FAR struct udp_conn_s *conn, FAR const struct sockaddr *addr) /* Interrupts must be disabled while access the UDP connection list */ - net_lock(); + conn_lock(&conn->sconn); #ifdef CONFIG_NET_IPv4 #ifdef CONFIG_NET_IPv6 @@ -764,7 +794,7 @@ int udp_bind(FAR struct udp_conn_s *conn, FAR const struct sockaddr *addr) netdev_list_unlock(); if (ret == -EADDRNOTAVAIL) { - net_unlock(); + conn_unlock(&conn->sconn); return ret; } } @@ -814,7 +844,7 @@ int udp_bind(FAR struct udp_conn_s *conn, FAR const struct sockaddr *addr) netdev_list_unlock(); if (ret == -EADDRNOTAVAIL) { - net_unlock(); + conn_unlock(&conn->sconn); return ret; } } @@ -880,7 +910,7 @@ int udp_bind(FAR struct udp_conn_s *conn, FAR const struct sockaddr *addr) } } - net_unlock(); + conn_unlock(&conn->sconn); return ret; } diff --git a/net/udp/udp_input.c b/net/udp/udp_input.c index f6fd0b1fc7c..22ec8a065e3 100644 --- a/net/udp/udp_input.c +++ b/net/udp/udp_input.c @@ -297,6 +297,7 @@ static int udp_input(FAR struct net_driver_s *dev, unsigned int iplen) * that, however. */ + udp_conn_list_lock(); conn = udp_active(dev, NULL, udp); if (conn) { @@ -389,6 +390,8 @@ static int udp_input(FAR struct net_driver_s *dev, unsigned int iplen) # endif /* CONFIG_NET_IPv6*/ #endif } + + udp_conn_list_unlock(); } return ret; diff --git a/net/udp/udp_ioctl.c b/net/udp/udp_ioctl.c index df26d5a45d4..adb8473df56 100644 --- a/net/udp/udp_ioctl.c +++ b/net/udp/udp_ioctl.c @@ -38,6 +38,7 @@ #include <nuttx/mm/iob.h> #include <nuttx/net/net.h> +#include "utils/utils.h" #include "udp/udp.h" /**************************************************************************** @@ -117,7 +118,7 @@ int udp_ioctl(FAR struct udp_conn_s *conn, int cmd, unsigned long arg) FAR struct iob_s *iob; int ret = OK; - net_lock(); + conn_lock(&conn->sconn); switch (cmd) { @@ -155,7 +156,7 @@ int udp_ioctl(FAR struct udp_conn_s *conn, int cmd, unsigned long arg) break; } - net_unlock(); + conn_unlock(&conn->sconn); return ret; } diff --git a/net/udp/udp_netpoll.c b/net/udp/udp_netpoll.c index 0a64bb820c8..213832b80c6 100644 --- a/net/udp/udp_netpoll.c +++ b/net/udp/udp_netpoll.c @@ -37,6 +37,7 @@ #include "devif/devif.h" #include "netdev/netdev.h" #include "socket/socket.h" +#include "utils/utils.h" #include "udp/udp.h" /**************************************************************************** @@ -132,23 +133,25 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) FAR struct udp_conn_s *conn; FAR struct udp_poll_s *info; FAR struct devif_callback_s *cb; + FAR struct net_driver_s *dev; pollevent_t eventset = 0; int ret = OK; /* Some of the following must be atomic */ - net_lock(); - conn = psock->s_conn; /* Sanity check */ if (conn == NULL || fds == NULL) { - ret = -EINVAL; - goto errout_with_lock; + return -EINVAL; } + dev = udp_find_laddr_device(conn); + + conn_dev_lock(&conn->sconn, dev); + /* Find a container to hold the poll information */ info = conn->pollinfo; @@ -166,7 +169,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) * dev value will be zero and there will be no NETDEV_DOWN notifications. */ - info->dev = udp_find_laddr_device(conn); + info->dev = dev; /* Allocate a UDP callback structure */ @@ -229,7 +232,8 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) poll_notify(&fds, 1, eventset); errout_with_lock: - net_unlock(); + conn_dev_unlock(&conn->sconn, dev); + return ret; } @@ -254,9 +258,7 @@ int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) FAR struct udp_conn_s *conn; FAR struct udp_poll_s *info; - /* Some of the following must be atomic */ - - net_lock(); + /* Some of the following must be atomic? */ conn = psock->s_conn; @@ -264,7 +266,6 @@ int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) if (!conn || !fds->priv) { - net_unlock(); return -EINVAL; } @@ -274,6 +275,8 @@ int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) DEBUGASSERT(info->fds != NULL && info->cb != NULL); if (info != NULL) { + conn_dev_lock(&conn->sconn, info->dev); + /* Release the callback */ udp_callback_free(info->dev, conn, info->cb); @@ -285,9 +288,8 @@ int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) /* Then free the poll info container */ info->conn = NULL; + conn_dev_unlock(&conn->sconn, info->dev); } - net_unlock(); - return OK; } diff --git a/net/udp/udp_recvfrom.c b/net/udp/udp_recvfrom.c index b7fcca74ec5..a5d18d68e9f 100644 --- a/net/udp/udp_recvfrom.c +++ b/net/udp/udp_recvfrom.c @@ -694,9 +694,17 @@ ssize_t psock_udp_recvfrom(FAR struct socket *psock, FAR struct msghdr *msg, * because we don't want anything to happen until we are ready. */ - net_lock(); udp_recvfrom_initialize(conn, msg, &state, flags); + /* Get the device that will handle the packet transfers. This may be + * NULL if the UDP socket is bound to INADDR_ANY. In that case, no + * NETDEV_DOWN notifications will be received. + */ + + dev = udp_find_laddr_device(conn); + + conn_dev_lock(&conn->sconn, dev); + /* Copy the read-ahead data from the packet */ udp_readahead(&state); @@ -735,13 +743,6 @@ ssize_t psock_udp_recvfrom(FAR struct socket *psock, FAR struct msghdr *msg, else if (state.ir_recvlen <= 0) { - /* Get the device that will handle the packet transfers. This may be - * NULL if the UDP socket is bound to INADDR_ANY. In that case, no - * NETDEV_DOWN notifications will be received. - */ - - dev = udp_find_laddr_device(conn); - /* Set up the callback in the connection */ state.ir_cb = udp_callback_alloc(dev, conn); @@ -768,8 +769,10 @@ ssize_t psock_udp_recvfrom(FAR struct socket *psock, FAR struct msghdr *msg, * received. */ + conn_dev_unlock(&conn->sconn, dev); ret = net_sem_timedwait(&state.ir_sem, _SO_TIMEOUT(conn->sconn.s_rcvtimeo)); + conn_dev_lock(&conn->sconn, dev); tls_cleanup_pop(tls_get_info(), 0); if (ret == -ETIMEDOUT) { @@ -787,9 +790,9 @@ ssize_t psock_udp_recvfrom(FAR struct socket *psock, FAR struct msghdr *msg, } } + conn_dev_unlock(&conn->sconn, dev); udp_notify_recvcpu(conn); - net_unlock(); udp_recvfrom_uninitialize(&state); return ret; } diff --git a/net/udp/udp_sendto_buffered.c b/net/udp/udp_sendto_buffered.c index 5dba74a504b..e4cbef8cfaf 100644 --- a/net/udp/udp_sendto_buffered.c +++ b/net/udp/udp_sendto_buffered.c @@ -118,16 +118,16 @@ static uint16_t sendto_eventhandler(FAR struct net_driver_s *dev, * ****************************************************************************/ -static void sendto_writebuffer_release(FAR struct udp_conn_s *conn) +static void sendto_writebuffer_release(FAR struct udp_conn_s *conn, + FAR struct udp_wrbuffer_s *wrb) { - FAR struct udp_wrbuffer_s *wrb; int ret = OK; do { /* Check if the write queue became empty */ - if (sq_empty(&conn->write_q)) + if (wrb == NULL && sq_empty(&conn->write_q)) { /* Yes.. stifle any further callbacks until more write data is * enqueued. @@ -136,7 +136,7 @@ static void sendto_writebuffer_release(FAR struct udp_conn_s *conn) conn->sndcb->flags = 0; conn->sndcb->priv = NULL; conn->sndcb->event = NULL; - wrb = NULL; + ret = OK; if (conn->txdrain_sem != NULL) { @@ -153,10 +153,17 @@ static void sendto_writebuffer_release(FAR struct udp_conn_s *conn) * and release it. */ - wrb = (FAR struct udp_wrbuffer_s *)sq_remfirst(&conn->write_q); + if (wrb == NULL) + { + /* Get the write buffer at the head of the queue */ + + wrb = (FAR struct udp_wrbuffer_s *)sq_remfirst(&conn->write_q); + } + DEBUGASSERT(wrb != NULL); udp_wrbuffer_release(wrb); + wrb = NULL; /* Set up for the next packet transfer by setting the connection * address to the address of the next packet now at the header of @@ -166,7 +173,7 @@ static void sendto_writebuffer_release(FAR struct udp_conn_s *conn) ret = sendto_next_transfer(conn); } } - while (wrb != NULL && ret < 0); + while (ret < 0); #if CONFIG_NET_SEND_BUFSIZE > 0 /* Notify the send buffer available if wrbbuffer drained */ @@ -303,6 +310,8 @@ static int sendto_next_transfer(FAR struct udp_conn_s *conn) } #endif + conn_unlock(&conn->sconn); + /* If this is not the same device that we used in the last call to * udp_callback_alloc(), then we need to release and reallocate the old * callback instance. @@ -310,8 +319,10 @@ static int sendto_next_transfer(FAR struct udp_conn_s *conn) if (conn->sndcb != NULL && conn->dev != dev) { + conn_dev_lock(&conn->sconn, conn->dev); udp_callback_free(conn->dev, conn, conn->sndcb); conn->sndcb = NULL; + conn_dev_unlock(&conn->sconn, conn->dev); } /* Allocate resources to receive a callback from this device if the @@ -320,7 +331,9 @@ static int sendto_next_transfer(FAR struct udp_conn_s *conn) if (conn->sndcb == NULL) { + conn_dev_lock(&conn->sconn, dev); conn->sndcb = udp_callback_alloc(dev, conn); + conn_dev_unlock(&conn->sconn, dev); } /* Test if the callback has been allocated */ @@ -330,6 +343,7 @@ static int sendto_next_transfer(FAR struct udp_conn_s *conn) /* A buffer allocation error occurred */ nerr("ERROR: Failed to allocate callback\n"); + conn_lock(&conn->sconn); return -ENOMEM; } @@ -344,6 +358,9 @@ static int sendto_next_transfer(FAR struct udp_conn_s *conn) /* Notify the device driver of the availability of TX data */ netdev_txnotify_dev(dev); + + conn_lock(&conn->sconn); + return OK; } @@ -386,7 +403,7 @@ static uint16_t sendto_eventhandler(FAR struct net_driver_s *dev, * the next transfer. */ - sendto_writebuffer_release(conn); + sendto_writebuffer_release(conn, NULL); return flags; } @@ -428,7 +445,7 @@ static uint16_t sendto_eventhandler(FAR struct net_driver_s *dev, * the write_q is not empty. */ - wrb = (FAR struct udp_wrbuffer_s *)sq_peek(&conn->write_q); + wrb = (FAR struct udp_wrbuffer_s *)sq_remfirst(&conn->write_q); DEBUGASSERT(wrb != NULL); /* If the udp socket not connected, it is possible to have @@ -474,7 +491,7 @@ static uint16_t sendto_eventhandler(FAR struct net_driver_s *dev, * setup the next transfer. */ - sendto_writebuffer_release(conn); + sendto_writebuffer_release(conn, wrb); /* Only one data can be sent by low level driver at once, * tell the caller stop polling the other connections. @@ -690,19 +707,18 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, BUF_DUMP("psock_udp_sendto", buf, len); - net_lock(); - #if CONFIG_NET_SEND_BUFSIZE > 0 /* If the send buffer size exceeds the send limit, * wait for the write buffer to be released */ + conn_lock(&conn->sconn); while (udp_wrbuffer_inqueue_size(conn) + len > conn->sndbufs) { + conn_unlock(&conn->sconn); if (nonblock) { - ret = -EAGAIN; - goto errout_with_lock; + return -EAGAIN; } ret = net_sem_timedwait_uninterruptible(&conn->sndsem, @@ -714,9 +730,13 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, ret = -EAGAIN; } - goto errout_with_lock; + return ret; } + + conn_lock(&conn->sconn); } + + conn_unlock(&conn->sconn); #endif /* CONFIG_NET_SEND_BUFSIZE */ /* Allocate a write buffer. Careful, the network will be momentarily @@ -755,7 +775,7 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, ret = -ENOMEM; } - goto errout_with_lock; + return ret; } /* Initialize the write buffer @@ -825,21 +845,8 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, } else { - unsigned int count; - int blresult; - - /* iob_copyin might wait for buffers to be freed, but if - * network is locked this might never happen, since network - * driver is also locked, therefore we need to break the lock - */ - - blresult = net_breaklock(&count); ret = iob_copyin(wrb->wb_iob, (FAR uint8_t *)buf, len, udpiplen, false); - if (blresult >= 0) - { - net_restorelock(count); - } } if (ret < 0) @@ -863,6 +870,7 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, * not a very common use case, however. */ + conn_lock(&conn->sconn); empty = sq_empty(&conn->write_q); sq_addlast(&wrb->wb_node, &conn->write_q); @@ -882,11 +890,12 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, if (ret < 0) { sq_remlast(&conn->write_q); + conn_unlock(&conn->sconn); goto errout_with_wrb; } - } - net_unlock(); + conn_unlock(&conn->sconn); + } /* Return the number of bytes that will be sent */ @@ -895,8 +904,6 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, errout_with_wrb: udp_wrbuffer_release(wrb); -errout_with_lock: - net_unlock(); return ret; } diff --git a/net/udp/udp_sendto_unbuffered.c b/net/udp/udp_sendto_unbuffered.c index 5248b5dc153..f80aa1e9f48 100644 --- a/net/udp/udp_sendto_unbuffered.c +++ b/net/udp/udp_sendto_unbuffered.c @@ -44,6 +44,7 @@ #include "arp/arp.h" #include "icmpv6/icmpv6.h" #include "socket/socket.h" +#include "utils/utils.h" #include "udp/udp.h" /**************************************************************************** @@ -415,7 +416,6 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, * ready. */ - net_lock(); memset(&state, 0, sizeof(struct sendto_s)); nxsem_init(&state.st_sem, 0, 0); @@ -440,7 +440,7 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, if (ret < 0) { nerr("ERROR: udp_connect failed: %d\n", ret); - goto errout_with_lock; + return ret; } } @@ -452,8 +452,7 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, if (state.st_dev == NULL) { nerr("ERROR: udp_find_raddr_device failed\n"); - ret = -ENETUNREACH; - goto errout_with_lock; + return -ENETUNREACH; } /* Make sure that the device is in the UP state */ @@ -461,10 +460,11 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, if ((state.st_dev->d_flags & IFF_UP) == 0) { nwarn("WARNING: device is DOWN\n"); - ret = -EHOSTUNREACH; - goto errout_with_lock; + return -EHOSTUNREACH; } + conn_dev_lock(&conn->sconn, state.st_dev); + /* Set up the callback in the connection */ ret = -ENOMEM; /* Assume allocation failure */ @@ -475,6 +475,8 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, state.st_cb->priv = (FAR void *)&state; state.st_cb->event = sendto_eventhandler; + conn_dev_unlock(&conn->sconn, state.st_dev); + /* Notify the device driver of the availability of TX data */ netdev_txnotify_dev(state.st_dev); @@ -495,20 +497,20 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, ret = state.st_sndlen; } + conn_dev_lock(&conn->sconn, state.st_dev); + /* Make sure that no further events are processed */ udp_callback_free(state.st_dev, conn, state.st_cb); } -errout_with_lock: - /* Release the semaphore */ nxsem_destroy(&state.st_sem); /* Unlock the network and return the result of the sendto() operation */ - net_unlock(); + conn_dev_unlock(&conn->sconn, state.st_dev); return ret; } diff --git a/net/udp/udp_txdrain.c b/net/udp/udp_txdrain.c index b98be298dd9..f5013eaa64a 100644 --- a/net/udp/udp_txdrain.c +++ b/net/udp/udp_txdrain.c @@ -81,16 +81,17 @@ int udp_txdrain(FAR struct socket *psock, unsigned int timeout) /* The following needs to be done with the network stable */ - net_lock(); - + conn_lock(&conn->sconn); if (!sq_empty(&conn->write_q)) { conn->txdrain_sem = &waitsem; + conn_unlock(&conn->sconn); ret = net_sem_timedwait_uninterruptible(&waitsem, timeout); + conn_lock(&conn->sconn); conn->txdrain_sem = NULL; } - net_unlock(); + conn_unlock(&conn->sconn); nxsem_destroy(&waitsem); leave_cancellation_point(); return ret;
