net/can: replace net_lock with conn_lock and conn_lock_dev

Protect can resources through netdev_lock, conn_lock, and can_list_lock

Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
This commit is contained in:
zhanghongyu
2025-06-19 14:03:24 +08:00
committed by Xiang Xiao
parent 4fa74e4d18
commit 02e7ed7cc4
10 changed files with 90 additions and 29 deletions
+16
View File
@@ -188,6 +188,22 @@ void can_free(FAR struct can_conn_s *conn);
FAR struct can_conn_s *can_nextconn(FAR struct can_conn_s *conn); FAR struct can_conn_s *can_nextconn(FAR struct can_conn_s *conn);
/****************************************************************************
* Name: can_conn_list_lock
* can_conn_list_unlock
*
* Description:
* Lock and unlock the CAN connection list. This is used to protect
* the list of active connections.
*
* Assumptions:
* This function is called from driver.
*
****************************************************************************/
void can_conn_list_lock(void);
void can_conn_list_unlock(void);
/**************************************************************************** /****************************************************************************
* Name: can_active() * Name: can_active()
* *
+7 -7
View File
@@ -37,6 +37,7 @@
#include <nuttx/net/can.h> #include <nuttx/net/can.h>
#include "devif/devif.h" #include "devif/devif.h"
#include "utils/utils.h"
#include "can/can.h" #include "can/can.h"
#ifdef CONFIG_NET_TIMESTAMP #ifdef CONFIG_NET_TIMESTAMP
@@ -149,13 +150,9 @@ uint16_t can_callback(FAR struct net_driver_s *dev,
} }
#endif #endif
/* Try to lock the network when successful send data to the listener */ conn_lock(&conn->sconn);
flags = devif_conn_event(dev, flags, conn->sconn.list);
if (net_trylock() == OK) conn_unlock(&conn->sconn);
{
flags = devif_conn_event(dev, flags, conn->sconn.list);
net_unlock();
}
/* Either we did not get the lock or there is no application listening /* Either we did not get the lock or there is no application listening
* If we did not get a lock we store the frame in the read-ahead buffer * If we did not get a lock we store the frame in the read-ahead buffer
@@ -202,6 +199,7 @@ uint16_t can_datahandler(FAR struct net_driver_s *dev,
FAR struct iob_s *iob = dev->d_iob; FAR struct iob_s *iob = dev->d_iob;
int ret = 0; int ret = 0;
conn_lock(&conn->sconn);
#if CONFIG_NET_RECV_BUFSIZE > 0 #if CONFIG_NET_RECV_BUFSIZE > 0
# if CONFIG_NET_CAN_NBUFFERS > 0 # if CONFIG_NET_CAN_NBUFFERS > 0
int bufnum = div_const_roundup(conn->rcvbufs, NET_CAN_PKTSIZE); int bufnum = div_const_roundup(conn->rcvbufs, NET_CAN_PKTSIZE);
@@ -244,10 +242,12 @@ uint16_t can_datahandler(FAR struct net_driver_s *dev,
goto errout; goto errout;
} }
conn_unlock(&conn->sconn);
return ret; return ret;
errout: errout:
netdev_iob_release(dev); netdev_iob_release(dev);
conn_unlock(&conn->sconn);
return ret; return ret;
} }
+26
View File
@@ -138,6 +138,7 @@ void can_free(FAR struct can_conn_s *conn)
/* Remove the connection from the active list */ /* Remove the connection from the active list */
dq_rem(&conn->sconn.node, &g_active_can_connections); dq_rem(&conn->sconn.node, &g_active_can_connections);
nxmutex_destroy(&conn->sconn.s_lock);
#ifdef CONFIG_NET_CAN_WRITE_BUFFERS #ifdef CONFIG_NET_CAN_WRITE_BUFFERS
/* Free the write queue */ /* Free the write queue */
@@ -243,6 +244,29 @@ FAR struct can_conn_s *can_nextconn(FAR struct can_conn_s *conn)
} }
} }
/****************************************************************************
* Name: can_conn_list_lock
* can_conn_list_unlock
*
* Description:
* Lock and unlock the CAN connection list. This is used to protect
* the list of active connections.
*
* Assumptions:
* This function is called from driver.
*
****************************************************************************/
void can_conn_list_lock(void)
{
NET_BUFPOOL_LOCK(g_can_connections);
}
void can_conn_list_unlock(void)
{
NET_BUFPOOL_UNLOCK(g_can_connections);
}
/**************************************************************************** /****************************************************************************
* Name: can_active() * Name: can_active()
* *
@@ -267,6 +291,7 @@ FAR struct can_conn_s *can_active(FAR struct net_driver_s *dev,
memcpy(&can_id, NETLLBUF, sizeof(canid_t)); memcpy(&can_id, NETLLBUF, sizeof(canid_t));
#endif #endif
can_conn_list_lock();
while ((conn = can_nextconn(conn)) != NULL) while ((conn = can_nextconn(conn)) != NULL)
{ {
if ((conn->dev == NULL && _SS_ISBOUND(conn->sconn.s_flags)) || if ((conn->dev == NULL && _SS_ISBOUND(conn->sconn.s_flags)) ||
@@ -282,6 +307,7 @@ FAR struct can_conn_s *can_active(FAR struct net_driver_s *dev,
} }
} }
can_conn_list_unlock();
return conn; return conn;
} }
+9 -1
View File
@@ -35,6 +35,7 @@
#include <nuttx/net/netstats.h> #include <nuttx/net/netstats.h>
#include "devif/devif.h" #include "devif/devif.h"
#include "utils/utils.h"
#include "can/can.h" #include "can/can.h"
/**************************************************************************** /****************************************************************************
@@ -218,6 +219,7 @@ static int can_in(FAR struct net_driver_s *dev)
{ {
FAR struct can_conn_s *conn = can_active(dev, NULL); FAR struct can_conn_s *conn = can_active(dev, NULL);
FAR struct can_conn_s *nextconn; FAR struct can_conn_s *nextconn;
int ret;
if (conn == NULL) if (conn == NULL)
{ {
@@ -226,6 +228,8 @@ static int can_in(FAR struct net_driver_s *dev)
return OK; return OK;
} }
can_conn_list_lock();
/* Do we have second connection that can hold this packet? */ /* Do we have second connection that can hold this packet? */
while ((nextconn = can_active(dev, conn)) != NULL) while ((nextconn = can_active(dev, conn)) != NULL)
@@ -250,7 +254,11 @@ static int can_in(FAR struct net_driver_s *dev)
/* We can deliver the packet directly to the last listener. */ /* We can deliver the packet directly to the last listener. */
return can_input_conn(dev, conn); ret = can_input_conn(dev, conn);
can_conn_list_unlock();
return ret;
} }
/**************************************************************************** /****************************************************************************
+9 -3
View File
@@ -47,6 +47,7 @@
#include "devif/devif.h" #include "devif/devif.h"
#include "can/can.h" #include "can/can.h"
#include "socket/socket.h" #include "socket/socket.h"
#include "utils/utils.h"
#include <netpacket/packet.h> #include <netpacket/packet.h>
#ifdef CONFIG_NET_TIMESTAMP #ifdef CONFIG_NET_TIMESTAMP
@@ -419,7 +420,7 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
int flags) int flags)
{ {
FAR struct can_conn_s *conn; FAR struct can_conn_s *conn;
FAR struct net_driver_s *dev; FAR struct net_driver_s *dev = NULL;
struct can_recvfrom_s state; struct can_recvfrom_s state;
int ret; int ret;
@@ -436,7 +437,7 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
return -ENOTSUP; return -ENOTSUP;
} }
net_lock(); conn_lock(&conn->sconn);
/* Initialize the state structure. */ /* Initialize the state structure. */
@@ -504,6 +505,9 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
goto errout_with_state; goto errout_with_state;
} }
conn_unlock(&conn->sconn);
conn_dev_lock(&conn->sconn, dev);
/* Set up the callback in the connection */ /* Set up the callback in the connection */
state.pr_cb = can_callback_alloc(dev, conn); state.pr_cb = can_callback_alloc(dev, conn);
@@ -519,7 +523,9 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
* the task sleeps and automatically re-locked when the task restarts. * the task sleeps and automatically re-locked when 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 */
@@ -532,7 +538,7 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
} }
errout_with_state: errout_with_state:
net_unlock(); conn_dev_unlock(&conn->sconn, dev);
nxsem_destroy(&state.pr_sem); nxsem_destroy(&state.pr_sem);
return ret; return ret;
} }
+7 -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 "can/can.h" #include "can/can.h"
#include <sys/time.h> #include <sys/time.h>
@@ -225,7 +226,7 @@ ssize_t can_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 */
@@ -258,6 +259,8 @@ ssize_t can_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);
@@ -275,13 +278,15 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
ret = net_sem_timedwait(&state.snd_sem, UINT_MAX); ret = net_sem_timedwait(&state.snd_sem, UINT_MAX);
} }
conn_dev_lock(&conn->sconn, dev);
/* Make sure that no further events are processed */ /* Make sure that no further events are processed */
can_callback_free(dev, conn, state.snd_cb); can_callback_free(dev, conn, state.snd_cb);
} }
nxsem_destroy(&state.snd_sem); nxsem_destroy(&state.snd_sem);
net_unlock(); conn_dev_unlock(&conn->sconn, dev);
/* Check for a errors, Errors are signalled by negative errno values /* Check for a errors, Errors are signalled by negative errno values
* for the send length * for the send length
+6 -12
View File
@@ -248,7 +248,7 @@ ssize_t can_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);
#if CONFIG_NET_SEND_BUFSIZE > 0 #if CONFIG_NET_SEND_BUFSIZE > 0
if ((iob_get_queue_size(&conn->write_q) + msg->msg_iov->iov_len) > if ((iob_get_queue_size(&conn->write_q) + msg->msg_iov->iov_len) >
@@ -313,21 +313,15 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR 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(wb_iob, (FAR uint8_t *)msg->msg_iov->iov_base, ret = iob_copyin(wb_iob, (FAR uint8_t *)msg->msg_iov->iov_base,
msg->msg_iov->iov_len, 0, false); msg->msg_iov->iov_len, 0, false);
if (blresult >= 0) conn_dev_lock(&conn->sconn, dev);
{
net_restorelock(count);
}
} }
if (ret < 0) if (ret < 0)
@@ -389,7 +383,7 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
/* unlock */ /* unlock */
net_unlock(); 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. */
@@ -399,7 +393,7 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
{ {
/* unlock */ /* unlock */
net_unlock(); conn_dev_unlock(&conn->sconn, dev);
} }
return msg->msg_iov->iov_len; return msg->msg_iov->iov_len;
@@ -411,7 +405,7 @@ errout_with_wq:
iob_free_queue(&conn->write_q); iob_free_queue(&conn->write_q);
errout_with_lock: errout_with_lock:
net_unlock(); conn_dev_unlock(&conn->sconn, dev);
return ret; return ret;
} }
+2 -2
View File
@@ -158,7 +158,7 @@ int can_setsockopt(FAR struct socket *psock, int level, int option,
* options. * options.
*/ */
net_lock(); conn_lock(&conn->sconn);
/* Set or clear the option bit */ /* Set or clear the option bit */
@@ -171,7 +171,7 @@ int can_setsockopt(FAR struct socket *psock, int level, int option,
_SO_CLROPT(conn->sconn.s_options, option); _SO_CLROPT(conn->sconn.s_options, option);
} }
net_unlock(); conn_unlock(&conn->sconn);
break; break;
#if CONFIG_NET_RECV_BUFSIZE > 0 #if CONFIG_NET_RECV_BUFSIZE > 0
+6 -2
View File
@@ -43,6 +43,7 @@
#include "can/can.h" #include "can/can.h"
#include "netdev/netdev.h" #include "netdev/netdev.h"
#include "utils/utils.h"
#ifdef CONFIG_NET_CAN #ifdef CONFIG_NET_CAN
@@ -235,6 +236,7 @@ static int can_setup(FAR struct socket *psock)
conn->sndbufs = CONFIG_NET_SEND_BUFSIZE; conn->sndbufs = CONFIG_NET_SEND_BUFSIZE;
nxsem_init(&conn->sndsem, 0, 0); nxsem_init(&conn->sndsem, 0, 0);
#endif #endif
nxmutex_init(&conn->sconn.s_lock);
/* Attach the connection instance to the socket */ /* Attach the connection instance to the socket */
@@ -388,7 +390,7 @@ static int can_poll_local(FAR struct socket *psock, FAR struct pollfd *fds,
if (setup) if (setup)
{ {
net_lock(); conn_dev_lock(&conn->sconn, conn->dev);
info->dev = conn->dev; info->dev = conn->dev;
@@ -451,7 +453,7 @@ static int can_poll_local(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, conn->dev);
} }
else else
{ {
@@ -461,7 +463,9 @@ errout_with_lock:
{ {
/* Cancel any response notifications */ /* Cancel any response notifications */
conn_dev_lock(&conn->sconn, info->dev);
can_callback_free(info->dev, conn, info->cb); can_callback_free(info->dev, conn, info->cb);
conn_dev_unlock(&conn->sconn, info->dev);
/* Release the poll/select data slot */ /* Release the poll/select data slot */
+2
View File
@@ -288,6 +288,7 @@ static int devif_poll_can_connections(FAR struct net_driver_s *dev,
* perform the poll action * perform the poll action
*/ */
can_conn_list_lock();
while (!bstop && (can_conn = can_nextconn(can_conn))) while (!bstop && (can_conn = can_nextconn(can_conn)))
{ {
/* Skip connections that are bound to other polling devices */ /* Skip connections that are bound to other polling devices */
@@ -307,6 +308,7 @@ static int devif_poll_can_connections(FAR struct net_driver_s *dev,
} }
} }
can_conn_list_unlock();
return bstop; return bstop;
} }
#endif /* CONFIG_NET_PKT */ #endif /* CONFIG_NET_PKT */