net/pkt: replace net_lock with netdev_lock

protect PKT resources through netdev_lock, conn_lock, and pkt_list_lock

Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
This commit is contained in:
zhanghongyu
2025-06-18 11:15:51 +08:00
committed by Xiang Xiao
parent 9befb8ae4a
commit e431cb00ca
10 changed files with 128 additions and 50 deletions
+2
View File
@@ -236,6 +236,7 @@ static int devif_poll_pkt_connections(FAR struct net_driver_s *dev,
* action. * action.
*/ */
pkt_conn_list_lock();
while (!bstop && (pkt_conn = pkt_nextconn(pkt_conn))) while (!bstop && (pkt_conn = pkt_nextconn(pkt_conn)))
{ {
/* Skip packet connections that are bound to other polling devices */ /* Skip packet connections that are bound to other polling devices */
@@ -259,6 +260,7 @@ static int devif_poll_pkt_connections(FAR struct net_driver_s *dev,
} }
} }
pkt_conn_list_unlock();
return bstop; return bstop;
} }
#endif /* CONFIG_NET_PKT */ #endif /* CONFIG_NET_PKT */
+26
View File
@@ -205,6 +205,32 @@ int pkt_sendmsg_is_valid(FAR struct socket *psock,
FAR const struct msghdr *msg, FAR const struct msghdr *msg,
FAR struct net_driver_s **dev); FAR struct net_driver_s **dev);
/****************************************************************************
* Name: pkt_conn_list_lock()
*
* Description:
* Lock the packet connection list
*
* Assumptions:
* This function must be called by driver thread.
*
****************************************************************************/
void pkt_conn_list_lock(void);
/****************************************************************************
* Name: pkt_conn_list_unlock()
*
* Description:
* Unlock the packet connection list
*
* Assumptions:
* This function must be called by driver thread.
*
****************************************************************************/
void pkt_conn_list_unlock(void);
/**************************************************************************** /****************************************************************************
* Name: pkt_callback * Name: pkt_callback
* *
+3
View File
@@ -35,6 +35,7 @@
#include "devif/devif.h" #include "devif/devif.h"
#include "pkt/pkt.h" #include "pkt/pkt.h"
#include "utils/utils.h"
/**************************************************************************** /****************************************************************************
* Public Functions * Public Functions
@@ -65,7 +66,9 @@ uint16_t pkt_callback(FAR struct net_driver_s *dev,
{ {
/* Perform the callback */ /* Perform the callback */
conn_lock(&conn->sconn);
flags = devif_conn_event(dev, flags, conn->sconn.list); flags = devif_conn_event(dev, flags, conn->sconn.list);
conn_unlock(&conn->sconn);
} }
return flags; return flags;
+33
View File
@@ -127,6 +127,7 @@ void pkt_free(FAR struct pkt_conn_s *conn)
/* Remove the connection from the active list */ /* Remove the connection from the active list */
dq_rem(&conn->sconn.node, &g_active_pkt_connections); dq_rem(&conn->sconn.node, &g_active_pkt_connections);
nxmutex_destroy(&conn->sconn.s_lock);
#ifdef CONFIG_NET_PKT_WRITE_BUFFERS #ifdef CONFIG_NET_PKT_WRITE_BUFFERS
/* Free the write queue */ /* Free the write queue */
@@ -279,4 +280,36 @@ int pkt_sendmsg_is_valid(FAR struct socket *psock,
return OK; return OK;
} }
/****************************************************************************
* Name: pkt_conn_list_lock()
*
* Description:
* Lock the packet connection list
*
* Assumptions:
* This function must be called by driver thread.
*
****************************************************************************/
void pkt_conn_list_lock(void)
{
NET_BUFPOOL_LOCK(g_pkt_connections);
}
/****************************************************************************
* Name: pkt_conn_list_unlock()
*
* Description:
* Unlock the packet connection list
*
* Assumptions:
* This function must be called by driver thread.
*
****************************************************************************/
void pkt_conn_list_unlock(void)
{
NET_BUFPOOL_UNLOCK(g_pkt_connections);
}
#endif /* CONFIG_NET && CONFIG_NET_PKT */ #endif /* CONFIG_NET && CONFIG_NET_PKT */
+9 -1
View File
@@ -36,6 +36,7 @@
#include "devif/devif.h" #include "devif/devif.h"
#include "pkt/pkt.h" #include "pkt/pkt.h"
#include "utils/utils.h"
#include "socket/socket.h" #include "socket/socket.h"
/**************************************************************************** /****************************************************************************
@@ -102,7 +103,11 @@ static uint16_t pkt_datahandler(FAR struct net_driver_s *dev,
* without waiting). * without waiting).
*/ */
if ((ret = iob_tryadd_queue(iob, &conn->readahead)) < 0) conn_lock(&conn->sconn);
ret = iob_tryadd_queue(iob, &conn->readahead);
conn_unlock(&conn->sconn);
if (ret < 0)
{ {
nerr("ERROR: Failed to queue the I/O buffer chain: %d\n", ret); nerr("ERROR: Failed to queue the I/O buffer chain: %d\n", ret);
goto errout; goto errout;
@@ -151,6 +156,7 @@ static int pkt_in(FAR struct net_driver_s *dev)
FAR struct pkt_conn_s *conn; FAR struct pkt_conn_s *conn;
int ret = OK; int ret = OK;
pkt_conn_list_lock();
conn = pkt_active(dev); conn = pkt_active(dev);
if (conn) if (conn)
{ {
@@ -161,6 +167,7 @@ static int pkt_in(FAR struct net_driver_s *dev)
/* Do not read back the packet sent by oneself */ /* Do not read back the packet sent by oneself */
conn->pendiob = NULL; conn->pendiob = NULL;
pkt_conn_list_unlock();
return OK; return OK;
} }
@@ -207,6 +214,7 @@ static int pkt_in(FAR struct net_driver_s *dev)
ninfo("No PKT listener\n"); ninfo("No PKT listener\n");
} }
pkt_conn_list_unlock();
return ret; return ret;
} }
+22 -19
View File
@@ -38,6 +38,7 @@
#include "devif/devif.h" #include "devif/devif.h"
#include "netdev/netdev.h" #include "netdev/netdev.h"
#include "socket/socket.h" #include "socket/socket.h"
#include "utils/utils.h"
#include "pkt/pkt.h" #include "pkt/pkt.h"
/**************************************************************************** /****************************************************************************
@@ -172,18 +173,24 @@ int pkt_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
/* Some of the following must be atomic */ /* Some of the following must be atomic */
net_lock();
conn = psock->s_conn; conn = psock->s_conn;
/* Sanity check */ /* Sanity check */
if (conn == NULL || fds == NULL) if (conn == NULL || fds == NULL)
{ {
ret = -EINVAL; return -EINVAL;
goto errout_with_lock;
} }
dev = pkt_find_device(conn);
if (dev == NULL)
{
nerr("ERROR: No device found for PKT connection\n");
return -ENODEV;
}
conn_dev_lock(&conn->sconn, dev);
/* Find a container to hold the poll information */ /* Find a container to hold the poll information */
info = conn->pollinfo; info = conn->pollinfo;
@@ -196,10 +203,6 @@ int pkt_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
} }
} }
/* Get the device that will provide the provide the NETDEV_DOWN event. */
dev = pkt_find_device(conn);
/* Allocate a PKT callback structure */ /* Allocate a PKT callback structure */
cb = pkt_callback_alloc(dev, conn); cb = pkt_callback_alloc(dev, conn);
@@ -263,7 +266,7 @@ int pkt_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
poll_notify(&fds, 1, eventset); poll_notify(&fds, 1, eventset);
errout_with_lock: errout_with_lock:
net_unlock(); conn_dev_unlock(&conn->sconn, dev);
return ret; return ret;
} }
@@ -289,30 +292,30 @@ int pkt_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds)
FAR struct pkt_poll_s *info; FAR struct pkt_poll_s *info;
FAR struct net_driver_s *dev; FAR struct net_driver_s *dev;
/* Some of the following must be atomic */
net_lock();
conn = psock->s_conn; conn = psock->s_conn;
/* Sanity check */ /* Sanity check */
if (!conn || !fds->priv) if (!conn || !fds->priv)
{ {
net_unlock();
return -EINVAL; return -EINVAL;
} }
dev = pkt_find_device(conn);
if (dev == NULL)
{
nerr("ERROR: No device found for PKT connection\n");
return -ENODEV;
}
conn_dev_lock(&conn->sconn, dev);
/* Recover the socket descriptor poll state info from the poll structure */ /* Recover the socket descriptor poll state info from the poll structure */
info = (FAR struct pkt_poll_s *)fds->priv; info = (FAR struct pkt_poll_s *)fds->priv;
DEBUGASSERT(info->fds != NULL && info->cb != NULL); DEBUGASSERT(info->fds != NULL && info->cb != NULL);
if (info != NULL) if (info != NULL)
{ {
/* Get the device that will provide the NETDEV_DOWN event. */
dev = pkt_find_device(conn);
/* Release the callback */ /* Release the callback */
pkt_callback_free(dev, conn, info->cb); pkt_callback_free(dev, conn, info->cb);
@@ -326,7 +329,7 @@ int pkt_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds)
info->conn = NULL; info->conn = NULL;
} }
net_unlock(); conn_dev_unlock(&conn->sconn, dev);
return OK; return OK;
} }
+13 -12
View File
@@ -47,6 +47,7 @@
#include "devif/devif.h" #include "devif/devif.h"
#include "pkt/pkt.h" #include "pkt/pkt.h"
#include "socket/socket.h" #include "socket/socket.h"
#include "utils/utils.h"
#include <netpacket/packet.h> #include <netpacket/packet.h>
/**************************************************************************** /****************************************************************************
@@ -500,6 +501,14 @@ ssize_t pkt_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
ret = -ENOSYS; ret = -ENOSYS;
} }
/* Get the device driver that will service this transfer */
dev = pkt_find_device(conn);
if (dev == NULL)
{
return -ENODEV;
}
/* Perform the packet recvfrom() operation */ /* Perform the packet recvfrom() operation */
/* Initialize the state structure. This is done with the network /* Initialize the state structure. This is done with the network
@@ -508,7 +517,7 @@ ssize_t pkt_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
pkt_recvfrom_initialize(conn, msg, &state, psock->s_type); pkt_recvfrom_initialize(conn, msg, &state, psock->s_type);
net_lock(); conn_dev_lock(&conn->sconn, dev);
/* Check if there is buffered read-ahead data for this socket. We may have /* Check if there is buffered read-ahead data for this socket. We may have
* already received the response to previous command. * already received the response to previous command.
@@ -528,15 +537,6 @@ ssize_t pkt_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
} }
else else
{ {
/* Get the device driver that will service this transfer */
dev = pkt_find_device(conn);
if (dev == NULL)
{
ret = -ENODEV;
goto errout_with_state;
}
/* TODO pkt_recvfrom_initialize() expects from to be of type /* TODO pkt_recvfrom_initialize() expects from to be of type
* sockaddr_in, but in our case is sockaddr_ll * sockaddr_in, but in our case is sockaddr_ll
*/ */
@@ -565,7 +565,9 @@ ssize_t pkt_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
* the task restarts. * the task restarts.
*/ */
conn_dev_unlock(&conn->sconn, dev);
ret = net_sem_wait(&state.pr_sem); ret = net_sem_wait(&state.pr_sem);
conn_dev_lock(&conn->sconn, dev);
/* Make sure that no further events are processed */ /* Make sure that no further events are processed */
@@ -578,9 +580,8 @@ ssize_t pkt_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
} }
} }
net_unlock(); conn_dev_unlock(&conn->sconn, dev);
errout_with_state:
pkt_recvfrom_uninitialize(&state); pkt_recvfrom_uninitialize(&state);
return ret; return ret;
+5 -13
View File
@@ -230,8 +230,6 @@ ssize_t pkt_sendmsg(FAR struct socket *psock, FAR const struct msghdr *msg,
return 0; return 0;
} }
net_lock();
conn = psock->s_conn; conn = psock->s_conn;
if (psock->s_type == SOCK_DGRAM) if (psock->s_type == SOCK_DGRAM)
{ {
@@ -240,6 +238,7 @@ ssize_t pkt_sendmsg(FAR struct socket *psock, FAR const struct msghdr *msg,
conn->ifindex = addr->sll_ifindex; conn->ifindex = addr->sll_ifindex;
} }
conn_dev_lock(&conn->sconn, dev);
nonblock = _SS_ISNONBLOCK(conn->sconn.s_flags) || nonblock = _SS_ISNONBLOCK(conn->sconn.s_flags) ||
(flags & MSG_DONTWAIT) != 0; (flags & MSG_DONTWAIT) != 0;
@@ -309,20 +308,14 @@ ssize_t pkt_sendmsg(FAR struct socket *psock, FAR const struct msghdr *msg,
} }
else else
{ {
unsigned int count;
int blresult;
/* iob_copyin might wait for buffers to be freed, but if /* iob_copyin might wait for buffers to be freed, but if
* network is locked this might never happen, since network * network is locked this might never happen, since network
* driver is also locked, therefore we need to break the lock * driver is also locked, therefore we need to break the lock
*/ */
blresult = net_breaklock(&count); conn_dev_unlock(&conn->sconn, dev);
ret = iob_copyin(iob, buf, len, offset, false); ret = iob_copyin(iob, buf, len, offset, false);
if (blresult >= 0) conn_dev_lock(&conn->sconn, dev);
{
net_restorelock(count);
}
} }
if (ret < 0) if (ret < 0)
@@ -379,21 +372,20 @@ ssize_t pkt_sendmsg(FAR struct socket *psock, FAR const struct msghdr *msg,
conn->sndcb->flags = PKT_POLL; conn->sndcb->flags = PKT_POLL;
conn->sndcb->priv = conn; conn->sndcb->priv = conn;
conn->sndcb->event = psock_send_eventhandler; conn->sndcb->event = psock_send_eventhandler;
conn_dev_unlock(&conn->sconn, dev);
/* Notify the device driver that new TX data is available. */ /* Notify the device driver that new TX data is available. */
netdev_txnotify_dev(dev); netdev_txnotify_dev(dev);
} }
net_unlock();
return len; return len;
errout_with_iob: errout_with_iob:
iob_free_chain(iob); iob_free_chain(iob);
errout_with_lock: errout_with_lock:
net_unlock(); conn_dev_unlock(&conn->sconn, dev);
return ret; return ret;
} }
+6 -2
View File
@@ -47,6 +47,7 @@
#include "netdev/netdev.h" #include "netdev/netdev.h"
#include "devif/devif.h" #include "devif/devif.h"
#include "socket/socket.h" #include "socket/socket.h"
#include "utils/utils.h"
#include "pkt/pkt.h" #include "pkt/pkt.h"
/**************************************************************************** /****************************************************************************
@@ -213,7 +214,7 @@ ssize_t pkt_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
* because we don't want anything to happen until we are ready. * because we don't want anything to happen until we are ready.
*/ */
net_lock(); conn_dev_lock(&conn->sconn, dev);
memset(&state, 0, sizeof(struct send_s)); memset(&state, 0, sizeof(struct send_s));
nxsem_init(&state.snd_sem, 0, 0); /* Doesn't really fail */ nxsem_init(&state.snd_sem, 0, 0); /* Doesn't really fail */
@@ -236,6 +237,8 @@ ssize_t pkt_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
state.snd_cb->priv = (FAR void *)&state; state.snd_cb->priv = (FAR void *)&state;
state.snd_cb->event = psock_send_eventhandler; state.snd_cb->event = psock_send_eventhandler;
conn_dev_unlock(&conn->sconn, dev);
/* Notify the device driver that new TX data is available. */ /* Notify the device driver that new TX data is available. */
netdev_txnotify_dev(dev); netdev_txnotify_dev(dev);
@@ -245,6 +248,7 @@ ssize_t pkt_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
*/ */
ret = net_sem_wait(&state.snd_sem); ret = net_sem_wait(&state.snd_sem);
conn_dev_lock(&conn->sconn, dev);
/* Make sure that no further events are processed */ /* Make sure that no further events are processed */
@@ -253,7 +257,7 @@ ssize_t pkt_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
} }
nxsem_destroy(&state.snd_sem); nxsem_destroy(&state.snd_sem);
net_unlock(); conn_dev_unlock(&conn->sconn, dev);
/* Check for errors. Errors are signalled by negative errno values /* Check for errors. Errors are signalled by negative errno values
* for the send length * for the send length
+9 -3
View File
@@ -41,6 +41,7 @@
#include "devif/devif.h" #include "devif/devif.h"
#include "netdev/netdev.h" #include "netdev/netdev.h"
#include "utils/utils.h"
#include <socket/socket.h> #include <socket/socket.h>
#include "pkt/pkt.h" #include "pkt/pkt.h"
@@ -132,6 +133,8 @@ static int pkt_sockif_alloc(FAR struct socket *psock)
nxsem_init(&conn->sndsem, 0, 0); nxsem_init(&conn->sndsem, 0, 0);
#endif #endif
nxmutex_init(&conn->sconn.s_lock);
/* Save the pre-allocated connection in the socket structure */ /* Save the pre-allocated connection in the socket structure */
psock->s_conn = conn; psock->s_conn = conn;
@@ -354,6 +357,7 @@ static int pkt_close(FAR struct socket *psock)
case SOCK_CTRL: case SOCK_CTRL:
{ {
FAR struct pkt_conn_s *conn = psock->s_conn; FAR struct pkt_conn_s *conn = psock->s_conn;
FAR struct net_driver_s *dev = pkt_find_device(conn);
/* Is this the last reference to the connection structure (there /* Is this the last reference to the connection structure (there
* could be more if the socket was dup'ed). * could be more if the socket was dup'ed).
@@ -361,6 +365,8 @@ static int pkt_close(FAR struct socket *psock)
if (conn->crefs <= 1) if (conn->crefs <= 1)
{ {
conn_dev_lock(&conn->sconn, dev);
/* Yes... free any read-ahead data */ /* Yes... free any read-ahead data */
iob_free_queue(&conn->readahead); iob_free_queue(&conn->readahead);
@@ -370,21 +376,20 @@ static int pkt_close(FAR struct socket *psock)
if (conn->sndcb != NULL) if (conn->sndcb != NULL)
{ {
FAR struct net_driver_s *dev;
int ret; int ret;
while (iob_get_queue_entry_count(&conn->write_q) != 0) while (iob_get_queue_entry_count(&conn->write_q) != 0)
{ {
conn_dev_unlock(&conn->sconn, dev);
ret = net_sem_timedwait_uninterruptible(&conn->sndsem, ret = net_sem_timedwait_uninterruptible(&conn->sndsem,
_SO_TIMEOUT(conn->sconn.s_sndtimeo)); _SO_TIMEOUT(conn->sconn.s_sndtimeo));
conn_dev_lock(&conn->sconn, dev);
if (ret < 0) if (ret < 0)
{ {
break; break;
} }
} }
dev = pkt_find_device(conn);
pkt_callback_free(dev, conn, conn->sndcb); pkt_callback_free(dev, conn, conn->sndcb);
conn->sndcb = NULL; conn->sndcb = NULL;
} }
@@ -393,6 +398,7 @@ static int pkt_close(FAR struct socket *psock)
/* Then free the connection structure */ /* Then free the connection structure */
conn->crefs = 0; /* No more references on the connection */ conn->crefs = 0; /* No more references on the connection */
conn_dev_unlock(&conn->sconn, dev);
pkt_free(psock->s_conn); /* Free network resources */ pkt_free(psock->s_conn); /* Free network resources */
} }
else else