From dc651e090eb366f9abb18c48be1dbb92b7084bdc Mon Sep 17 00:00:00 2001 From: gaohedong Date: Thu, 6 Jun 2024 19:54:54 +0800 Subject: [PATCH] net/can: Add SO_RCVBUF option for can socket If the CAN stack receiving packets fast, but the application layer reading packets slow. Then `conn->readahead` will continue to grow, leading to memory leaks. Finally CAN stack potentially starve out all IOB buffers. To prevent memory leaks, users can restrict can socket buffer length. Signed-off-by: gaohedong --- include/nuttx/mm/iob.h | 1 + mm/iob/CMakeLists.txt | 2 +- mm/iob/Make.defs | 2 +- ..._get_queue_size.c => iob_get_queue_info.c} | 21 +++++++++++- net/can/can.h | 4 +++ net/can/can_callback.c | 20 +++++++++-- net/can/can_getsockopt.c | 16 +++++++++ net/can/can_setsockopt.c | 33 +++++++++++++++++++ net/can/can_sockif.c | 10 ++++++ 9 files changed, 104 insertions(+), 5 deletions(-) rename mm/iob/{iob_get_queue_size.c => iob_get_queue_info.c} (78%) diff --git a/include/nuttx/mm/iob.h b/include/nuttx/mm/iob.h index a69be9803e..0db63d0801 100644 --- a/include/nuttx/mm/iob.h +++ b/include/nuttx/mm/iob.h @@ -456,6 +456,7 @@ void iob_free_queue_qentry(FAR struct iob_s *iob, #if CONFIG_IOB_NCHAINS > 0 unsigned int iob_get_queue_size(FAR struct iob_queue_s *queue); +unsigned int iob_get_queue_entry_count(FAR struct iob_queue_s *queue); #endif /* CONFIG_IOB_NCHAINS > 0 */ /**************************************************************************** diff --git a/mm/iob/CMakeLists.txt b/mm/iob/CMakeLists.txt index dbe9234dbd..6f2159845a 100644 --- a/mm/iob/CMakeLists.txt +++ b/mm/iob/CMakeLists.txt @@ -44,7 +44,7 @@ if(CONFIG_MM_IOB) iob_navail.c iob_free_queue_qentry.c iob_tailroom.c - iob_get_queue_size.c + iob_get_queue_info.c iob_reserve.c iob_update_pktlen.c iob_count.c) diff --git a/mm/iob/Make.defs b/mm/iob/Make.defs index 6359014e36..e2c61f37bf 100644 --- a/mm/iob/Make.defs +++ b/mm/iob/Make.defs @@ -28,7 +28,7 @@ CSRCS += iob_free_chain.c iob_free_qentry.c iob_free_queue.c CSRCS += iob_initialize.c iob_pack.c iob_peek_queue.c iob_remove_queue.c CSRCS += iob_statistics.c iob_trimhead.c iob_trimhead_queue.c iob_trimtail.c CSRCS += iob_navail.c iob_free_queue_qentry.c iob_tailroom.c -CSRCS += iob_get_queue_size.c iob_reserve.c iob_update_pktlen.c +CSRCS += iob_get_queue_info.c iob_reserve.c iob_update_pktlen.c CSRCS += iob_count.c ifeq ($(CONFIG_IOB_NOTIFIER),y) diff --git a/mm/iob/iob_get_queue_size.c b/mm/iob/iob_get_queue_info.c similarity index 78% rename from mm/iob/iob_get_queue_size.c rename to mm/iob/iob_get_queue_info.c index 26146c2fa1..73b12208af 100644 --- a/mm/iob/iob_get_queue_size.c +++ b/mm/iob/iob_get_queue_info.c @@ -1,5 +1,5 @@ /**************************************************************************** - * mm/iob/iob_get_queue_size.c + * mm/iob/iob_get_queue_info.c * * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with @@ -55,4 +55,23 @@ unsigned int iob_get_queue_size(FAR struct iob_queue_s *queue) return total; } +/**************************************************************************** + * Name: iob_get_queue_entry_count + * + * Description: + * Queue helper for get the iob queue entry count. + * + ****************************************************************************/ + +unsigned int iob_get_queue_entry_count(FAR struct iob_queue_s *queue) +{ + FAR struct iob_qentry_s *iobq; + unsigned int count; + + for (iobq = queue->qh_head, count = 0; iobq != NULL; + iobq = iobq->qe_flink, count++); + + return count; +} + #endif /* CONFIG_IOB_NCHAINS > 0 */ diff --git a/net/can/can.h b/net/can/can.h index 1c0a78c006..979fdb1aa2 100644 --- a/net/can/can.h +++ b/net/can/can.h @@ -88,6 +88,10 @@ struct can_conn_s struct iob_queue_s readahead; /* remove Read-ahead buffering */ +#if CONFIG_NET_RECV_BUFSIZE > 0 + int32_t recv_buffnum; /* Recv buffer number */ +#endif + /* CAN-specific content follows */ int16_t crefs; /* Reference count */ diff --git a/net/can/can_callback.c b/net/can/can_callback.c index 0ca5363406..7fff08e8bc 100644 --- a/net/can/can_callback.c +++ b/net/can/can_callback.c @@ -199,7 +199,19 @@ uint16_t can_datahandler(FAR struct net_driver_s *dev, FAR struct can_conn_s *conn) { FAR struct iob_s *iob = dev->d_iob; - int ret; + int ret = 0; + +#if CONFIG_NET_RECV_BUFSIZE > 0 + /* Check the frame count pending on conn->readahead */ + + if (iob_get_queue_entry_count(&conn->readahead) >= conn->recv_buffnum) + { + nwarn("WARNNING: There are no free recive buffer to retain the data. " + "Recive buffer number:%"PRId32", recived frames:%"PRIuPTR" \n", + conn->recv_buffnum, iob_get_queue_entry_count(&conn->readahead)); + goto errout; + } +#endif /* Concat the iob to readahead */ @@ -222,10 +234,14 @@ uint16_t can_datahandler(FAR struct net_driver_s *dev, else { nerr("ERROR: Failed to queue the I/O buffer chain: %d\n", ret); - netdev_iob_release(dev); + goto errout; } return ret; + +errout: + netdev_iob_release(dev); + return ret; } #endif /* CONFIG_NET && CONFIG_NET_CAN */ diff --git a/net/can/can_getsockopt.c b/net/can/can_getsockopt.c index a979c5098d..f7049a5381 100644 --- a/net/can/can_getsockopt.c +++ b/net/can/can_getsockopt.c @@ -234,6 +234,22 @@ int can_getsockopt(FAR struct socket *psock, int level, int option, break; #endif +#if CONFIG_NET_RECV_BUFSIZE > 0 + case SO_RCVBUF: + /* Verify that option is the size of an 'int'. Should also check + * that 'value' is properly aligned for an 'int' + */ + + if (*value_len != sizeof(int)) + { + return -EINVAL; + } + + *(FAR int *)value = conn->recv_buffnum * CONFIG_IOB_BUFSIZE; + + break; +#endif + default: nerr("ERROR: Unrecognized RAW CAN socket option: %d\n", option); ret = -ENOPROTOOPT; diff --git a/net/can/can_setsockopt.c b/net/can/can_setsockopt.c index 2b1ce233ac..a256ba657e 100644 --- a/net/can/can_setsockopt.c +++ b/net/can/can_setsockopt.c @@ -210,6 +210,39 @@ int can_setsockopt(FAR struct socket *psock, int level, int option, break; #endif +#if CONFIG_NET_RECV_BUFSIZE > 0 + case SO_RCVBUF: + { + int buffersize; + + /* Verify that option is the size of an 'int'. Should also check + * that 'value' is properly aligned for an 'int' + */ + + if (value_len != sizeof(int)) + { + return -EINVAL; + } + + /* Get the value. Is the option being set or cleared? */ + + buffersize = *(FAR int *)value; + if (buffersize < 0) + { + return -EINVAL; + } + +#if CONFIG_NET_MAX_RECV_BUFSIZE > 0 + buffersize = MIN(buffersize, CONFIG_NET_MAX_RECV_BUFSIZE); +#endif + + conn->recv_buffnum = (buffersize + CONFIG_IOB_BUFSIZE - 1) + / CONFIG_IOB_BUFSIZE; + + break; + } +#endif + default: nerr("ERROR: Unrecognized CAN option: %d\n", option); ret = -ENOPROTOOPT; diff --git a/net/can/can_sockif.c b/net/can/can_sockif.c index 3a9013bf24..1ab0f8f5f0 100644 --- a/net/can/can_sockif.c +++ b/net/can/can_sockif.c @@ -220,6 +220,16 @@ static int can_setup(FAR struct socket *psock) conn->crefs = 1; + /* If Can Socket Stack recive can frame and pending on the readahead, + * but the application layer did not read the frame. This will cause + * a memory leak, and it is necessary to limit the readahead. + */ + +#if CONFIG_NET_RECV_BUFSIZE > 0 + conn->recv_buffnum = (CONFIG_NET_RECV_BUFSIZE + CONFIG_IOB_BUFSIZE - 1) + / CONFIG_IOB_BUFSIZE; +#endif + /* Attach the connection instance to the socket */ psock->s_conn = conn;