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 075eb6a6d254bda1d38d1d3459a7108e5c6c4eea Author: Zhe Weng <[email protected]> AuthorDate: Tue Jul 4 13:24:51 2023 +0800 net/udp: Change conn->readahead to I/O buffer chain When using IOB queue to store readahead data, we use one IOB for each UDP packet. Then if the packets are very small, like 10Bytes per packet, we'll use ~1600 IOBs just for 16KB recv buffer size, which is wasteful and dangerous. So change conn->readahead to a single IOB chain like TCP. Benefits: - Using memory and IOBs more efficiently (small packets are common in UDP) Side effects: - UDP recv buffer size may count the overhead - A little bit drop in performance (<1%, more seek & copy) Signed-off-by: Zhe Weng <[email protected]> --- net/procfs/net_udp.c | 2 +- net/tcp/tcp.h | 5 ++-- net/udp/udp.h | 5 ++-- net/udp/udp_callback.c | 81 ++++++++++++++++++++++++++++++++++---------------- net/udp/udp_conn.c | 4 +-- net/udp/udp_ioctl.c | 6 ++-- net/udp/udp_netpoll.c | 2 +- net/udp/udp_notifier.c | 2 +- net/udp/udp_recvfrom.c | 62 ++++++++++++++++++++++++++------------ 9 files changed, 111 insertions(+), 58 deletions(-) diff --git a/net/procfs/net_udp.c b/net/procfs/net_udp.c index deca5ffaeb..26ba034dbd 100644 --- a/net/procfs/net_udp.c +++ b/net/procfs/net_udp.c @@ -122,7 +122,7 @@ static ssize_t netprocfs_udpstats(FAR struct netprocfs_file_s *priv, #if CONFIG_NET_SEND_BUFSIZE > 0 udp_wrbuffer_inqueue_size(conn), #endif - iob_get_queue_size(&conn->readahead)); + (conn->readahead) ? conn->readahead->io_pktlen : 0); len += snprintf(buffer + len, buflen - len, " %*s:%-6" PRIu16 " %*s:%-6" PRIu16 "\n", diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h index 301d5ea722..2907f63bd3 100644 --- a/net/tcp/tcp.h +++ b/net/tcp/tcp.h @@ -289,11 +289,10 @@ struct tcp_conn_s /* Read-ahead buffering. * - * readahead - A singly linked list of type struct iob_s - * where the TCP/IP read-ahead data is retained. + * readahead - An IOB chain where the TCP/IP read-ahead data is retained. */ - struct iob_s *readahead; /* Read-ahead buffering */ + FAR struct iob_s *readahead; /* Read-ahead buffering */ #ifdef CONFIG_NET_TCP_OUT_OF_ORDER diff --git a/net/udp/udp.h b/net/udp/udp.h index 2e325ccbe6..622897cd0b 100644 --- a/net/udp/udp.h +++ b/net/udp/udp.h @@ -127,11 +127,10 @@ struct udp_conn_s /* Read-ahead buffering. * - * readahead - A singly linked list of type struct iob_qentry_s - * where the UDP/IP read-ahead data is retained. + * readahead - An IOB chain where the UDP/IP read-ahead data is retained. */ - struct iob_queue_s readahead; /* Read-ahead buffering */ + FAR struct iob_s *readahead; /* Read-ahead buffering */ #ifdef CONFIG_NET_UDP_WRITE_BUFFERS /* Write buffering diff --git a/net/udp/udp_callback.c b/net/udp/udp_callback.c index 6ccf330cd2..309f494775 100644 --- a/net/udp/udp_callback.c +++ b/net/udp/udp_callback.c @@ -36,6 +36,7 @@ #include "devif/devif.h" #include "udp/udp.h" +#include "utils/utils.h" /**************************************************************************** * Private Functions @@ -71,11 +72,11 @@ static uint16_t udp_datahandler(FAR struct net_driver_s *dev, #endif uint8_t src_addr_size; - FAR void *src_addr; - uint8_t offset = 0; + FAR void *src_addr; + int offset; #if CONFIG_NET_RECV_BUFSIZE > 0 - if (iob_get_queue_size(&conn->readahead) > conn->rcvbufs) + if (conn->readahead && conn->readahead->io_pktlen > conn->rcvbufs) { netdev_iob_release(dev); #ifdef CONFIG_NET_STATISTICS @@ -151,44 +152,72 @@ static uint16_t udp_datahandler(FAR struct net_driver_s *dev, } #endif /* CONFIG_NET_IPv4 */ - /* Override the address info begin of io_data */ - -#ifdef CONFIG_NETDEV_IFINDEX - iob->io_data[offset++] = dev->d_ifindex; -#endif - iob->io_data[offset++] = src_addr_size; - memcpy(&iob->io_data[offset], src_addr, src_addr_size); + /* Copy the meta info into the I/O buffer chain, just before data. + * Layout: |datalen|ifindex|src_addr_size|src_addr|data| + */ - /* Trim l3/l4 offset */ + offset = (dev->d_appdata - iob->io_data) - iob->io_offset; - iob = iob_trimhead(iob, (dev->d_appdata - iob->io_data) - - iob->io_offset); + offset -= src_addr_size; + ret = iob_trycopyin(iob, src_addr, src_addr_size, offset, true); + if (ret < 0) + { + goto errout; + } - /* Add the new I/O buffer chain to the tail of the read-ahead queue */ + offset -= sizeof(src_addr_size); + ret = iob_trycopyin(iob, &src_addr_size, sizeof(src_addr_size), + offset, true); + if (ret < 0) + { + goto errout; + } - ret = iob_tryadd_queue(iob, &conn->readahead); +#ifdef CONFIG_NETDEV_IFINDEX + offset -= sizeof(dev->d_ifindex); + ret = iob_trycopyin(iob, &dev->d_ifindex, sizeof(dev->d_ifindex), + offset, true); if (ret < 0) { - nerr("ERROR: Failed to queue the I/O buffer chain: %d\n", ret); + goto errout; + } +#endif - iob_free_chain(iob); - buflen = 0; + offset -= sizeof(buflen); + ret = iob_trycopyin(iob, (FAR const uint8_t *)&buflen, sizeof(buflen), + offset, true); + if (ret < 0) + { + goto errout; } + + /* Trim l3/l4 offset, src_addr + 4Bytes should be less than header size. */ + + DEBUGASSERT(offset >= 0); + iob = iob_trimhead(iob, offset); + + /* Concat the iob to readahead */ + + net_iob_concat(&conn->readahead, &iob); + #ifdef CONFIG_NET_UDP_NOTIFIER - else - { - ninfo("Buffered %d bytes\n", buflen); + ninfo("Buffered %d bytes\n", buflen); - /* Provided notification(s) that additional UDP read-ahead data is - * available. - */ + /* Provided notification(s) that additional UDP read-ahead data is + * available. + */ - udp_readahead_signal(conn); - } + udp_readahead_signal(conn); #endif netdev_iob_clear(dev); return buflen; + +errout: + nerr("ERROR: Failed to queue the I/O buffer chain: %d\n", ret); + + netdev_iob_release(dev); + return 0; } /**************************************************************************** diff --git a/net/udp/udp_conn.c b/net/udp/udp_conn.c index b567789452..cc6c166fbe 100644 --- a/net/udp/udp_conn.c +++ b/net/udp/udp_conn.c @@ -691,9 +691,9 @@ void udp_free(FAR struct udp_conn_s *conn) dq_rem(&conn->sconn.node, &g_active_udp_connections); - /* Release any read-ahead buffers attached to the connection */ + /* Release any read-ahead buffers attached to the connection, NULL is ok */ - iob_free_queue(&conn->readahead); + iob_free_chain(conn->readahead); #ifdef CONFIG_NET_UDP_WRITE_BUFFERS /* Release any write buffers attached to the connection */ diff --git a/net/udp/udp_ioctl.c b/net/udp/udp_ioctl.c index 2f29b9549c..5b5fdc17e2 100644 --- a/net/udp/udp_ioctl.c +++ b/net/udp/udp_ioctl.c @@ -64,10 +64,12 @@ int udp_ioctl(FAR struct udp_conn_s *conn, int cmd, unsigned long arg) switch (cmd) { case FIONREAD: - iob = iob_peek_queue(&conn->readahead); + iob = conn->readahead; if (iob) { - *(FAR int *)((uintptr_t)arg) = iob->io_pktlen; + uint16_t datalen; + iob_copyout((FAR uint8_t *)&datalen, iob, sizeof(datalen), 0); + *(FAR int *)((uintptr_t)arg) = datalen; } else { diff --git a/net/udp/udp_netpoll.c b/net/udp/udp_netpoll.c index e3e2c563c3..11f1b5cf6c 100644 --- a/net/udp/udp_netpoll.c +++ b/net/udp/udp_netpoll.c @@ -208,7 +208,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) /* Check for read data availability now */ - if (!IOB_QEMPTY(&conn->readahead)) + if (conn->readahead != NULL) { /* Normal data may be read without blocking. */ diff --git a/net/udp/udp_notifier.c b/net/udp/udp_notifier.c index 21f16b0548..b1726ab512 100644 --- a/net/udp/udp_notifier.c +++ b/net/udp/udp_notifier.c @@ -76,7 +76,7 @@ int udp_readahead_notifier_setup(worker_t worker, * setting up the notification. */ - if (conn->readahead.qh_head != NULL) + if (conn->readahead != NULL) { return 0; } diff --git a/net/udp/udp_recvfrom.c b/net/udp/udp_recvfrom.c index e96543ff89..82f08d2938 100644 --- a/net/udp/udp_recvfrom.c +++ b/net/udp/udp_recvfrom.c @@ -164,34 +164,56 @@ static inline void udp_readahead(struct udp_recvfrom_s *pstate) pstate->ir_recvlen = -1; - if ((iob = iob_peek_queue(&conn->readahead)) != NULL) + if ((iob = conn->readahead) != NULL) { - int recvlen = pstate->ir_msg->msg_iov->iov_len; + int recvlen; + int offset = 0; + uint16_t datalen; uint8_t src_addr_size; - uint8_t offset = 0; - FAR void *srcaddr; uint8_t ifindex; +#ifdef CONFIG_NET_IPv6 + uint8_t srcaddr[sizeof(struct sockaddr_in6)]; +#else + uint8_t srcaddr[sizeof(struct sockaddr_in)]; +#endif - /* Unflatten saved connection information */ + /* Unflatten saved connection information + * Layout: |datalen|ifindex|src_addr_size|src_addr|data| + */ + + recvlen = iob_copyout((FAR uint8_t *)&datalen, iob, + sizeof(datalen), offset); + offset += sizeof(datalen); + DEBUGASSERT(recvlen == sizeof(datalen)); #ifdef CONFIG_NETDEV_IFINDEX - ifindex = iob->io_data[offset++]; + recvlen = iob_copyout(&ifindex, iob, sizeof(ifindex), offset); + offset += sizeof(ifindex); + DEBUGASSERT(recvlen == sizeof(ifindex)); #else ifindex = 1; #endif - src_addr_size = iob->io_data[offset++]; - srcaddr = &iob->io_data[offset]; + recvlen = iob_copyout(&src_addr_size, iob, + sizeof(src_addr_size), offset); + offset += sizeof(src_addr_size); + DEBUGASSERT(recvlen == sizeof(src_addr_size)); + + recvlen = iob_copyout(srcaddr, iob, src_addr_size, offset); + offset += src_addr_size; + DEBUGASSERT(recvlen == src_addr_size); /* Copy to user */ - recvlen = iob_copyout(pstate->ir_msg->msg_iov->iov_base, - iob, recvlen, 0); + recvlen = iob_copyout(pstate->ir_msg->msg_iov->iov_base, iob, + MIN(pstate->ir_msg->msg_iov->iov_len, datalen), + offset); /* Update the accumulated size of the data read */ pstate->ir_recvlen = recvlen; - ninfo("Received %d bytes (of %d)\n", recvlen, iob->io_pktlen); + ninfo("Received %d bytes (of %d, total %d)\n", + recvlen, datalen, iob->io_pktlen); if (pstate->ir_msg->msg_name) { @@ -205,17 +227,19 @@ static inline void udp_readahead(struct udp_recvfrom_s *pstate) udp_recvpktinfo(pstate, srcaddr, ifindex); - /* Remove the I/O buffer chain from the head of the read-ahead - * buffer queue. - */ + /* Remove the packet from the head of the I/O buffer chain. */ if (!(pstate->ir_flags & MSG_PEEK)) { - iob_remove_queue(&conn->readahead); - - /* And free the I/O buffer chain */ - - iob_free_chain(iob); + if (offset + datalen >= iob->io_pktlen) + { + iob_free_chain(iob); + conn->readahead = NULL; + } + else + { + conn->readahead = iob_trimhead(iob, offset + datalen); + } } } }
