From 3c54d82d811f5f056fa793b50bb29ad00326d052 Mon Sep 17 00:00:00 2001 From: Fotis Panagiotopoulos Date: Tue, 30 May 2023 00:26:19 +0300 Subject: [PATCH] net: Fix task block when devif_send fails. When a task needs to send data, a callback is allocated and the transmission is happening in a worker task through devif_send. Synchronization between the two tasks (sender & worker) is achieved by a semaphore. If devif_send fails, this semaphore was never posted, leaving the sending task blocked indefinitely. This commit fixes this by checking the return code of netif_send, and posting this semaphore in case of failure. Polling then stops, and execution is resumed on the sending task. --- net/can/can_sendmsg.c | 10 +++++--- net/devif/devif.h | 41 ++++++++++++++++++++------------- net/devif/devif_iobsend.c | 9 ++++---- net/devif/devif_send.c | 7 +++--- net/pkt/pkt_sendmsg.c | 10 +++++--- net/tcp/tcp_send_buffered.c | 14 ++++++----- net/tcp/tcp_send_unbuffered.c | 17 +++++++++----- net/udp/udp_sendto_unbuffered.c | 11 +++++---- 8 files changed, 74 insertions(+), 45 deletions(-) diff --git a/net/can/can_sendmsg.c b/net/can/can_sendmsg.c index ce1c2bc033..bddfa734b2 100644 --- a/net/can/can_sendmsg.c +++ b/net/can/can_sendmsg.c @@ -106,11 +106,13 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev, { /* Copy the packet data into the device packet buffer and send it */ - devif_send(dev, pstate->snd_buffer, pstate->snd_buflen, 0); + int ret = devif_send(dev, pstate->snd_buffer, + pstate->snd_buflen, 0); dev->d_len = dev->d_sndlen; - if (dev->d_sndlen == 0) + if (ret <= 0) { - return flags; + pstate->snd_sent = ret; + goto end_wait; } pstate->snd_sent = pstate->snd_buflen; @@ -122,6 +124,8 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev, } } +end_wait: + /* Don't allow any further call backs. */ pstate->snd_cb->flags = 0; diff --git a/net/devif/devif.h b/net/devif/devif.h index f6972cba2e..611f20f088 100644 --- a/net/devif/devif.h +++ b/net/devif/devif.h @@ -432,31 +432,40 @@ uint16_t devif_conn_event(FAR struct net_driver_s *dev, uint16_t flags, uint16_t devif_dev_event(FAR struct net_driver_s *dev, uint16_t flags); /**************************************************************************** - * Send data on the current connection. + * Name: devif_send * - * This function is used to send out a single segment of TCP data. Only - * socket logic that have been invoked by the lower level for event - * processing can send data. + * Description: + * Send data on the current connection. * - * The amount of data that actually is sent out after a call to this - * function is determined by the maximum amount of data TCP allows. The - * network will automatically crop the data so that only the appropriate - * amount of data is sent. The mss field of the TCP connection structure - * can be used to determine the amount of data that actually will be sent. + * This function is used to send out a single segment of TCP data. Only + * socket logic that have been invoked by the lower level for event + * processing can send data. + * + * The amount of data that actually is sent out after a call to this + * function is determined by the maximum amount of data TCP allows. The + * network will automatically crop the data so that only the appropriate + * amount of data is sent. The mss field of the TCP connection structure + * can be used to determine the amount of data that actually will be sent. * * Note: This function does not guarantee that the sent data will * arrive at the destination. If the data is lost in the network, the * TCP socket layer will be invoked with the TCP_REXMIT flag set. The * socket layer will then have to resend the data using this function. * - * data A pointer to the data which is to be sent. + * Input Parameters: + * dev - The network device state structure associated with the network + * device that initiated the callback event. + * buf - A pointer to the data which is to be sent. + * len - The maximum amount of data bytes to be sent. + * offset - Offset of data in buffer. * - * len The maximum amount of data bytes to be sent. + * Returned Value: + * The amount of data sent, or negated ERRNO in case of failure. * ****************************************************************************/ -void devif_send(FAR struct net_driver_s *dev, FAR const void *buf, - int len, unsigned int offset); +int devif_send(FAR struct net_driver_s *dev, FAR const void *buf, + int len, unsigned int offset); /**************************************************************************** * Name: devif_iob_send @@ -475,9 +484,9 @@ void devif_send(FAR struct net_driver_s *dev, FAR const void *buf, #ifdef CONFIG_MM_IOB struct iob_s; -void devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *buf, - unsigned int len, unsigned int offset, - unsigned int target_offset); +int devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *buf, + unsigned int len, unsigned int offset, + unsigned int target_offset); #endif /**************************************************************************** diff --git a/net/devif/devif_iobsend.c b/net/devif/devif_iobsend.c index 1dd068841a..87068f8bfe 100644 --- a/net/devif/devif_iobsend.c +++ b/net/devif/devif_iobsend.c @@ -53,9 +53,9 @@ * ****************************************************************************/ -void devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *iob, - unsigned int len, unsigned int offset, - unsigned int target_offset) +int devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *iob, + unsigned int len, unsigned int offset, + unsigned int target_offset) { int ret; @@ -105,10 +105,11 @@ void devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *iob, lib_dumpbuffer("devif_iob_send", dev->d_appdata, len); #endif - return; + return dev->d_sndlen; errout: nerr("ERROR: devif_iob_send error: %d\n", ret); + return ret; } #endif /* CONFIG_MM_IOB */ diff --git a/net/devif/devif_send.c b/net/devif/devif_send.c index 54e0980365..7ba5fd114b 100644 --- a/net/devif/devif_send.c +++ b/net/devif/devif_send.c @@ -66,8 +66,8 @@ * ****************************************************************************/ -void devif_send(FAR struct net_driver_s *dev, FAR const void *buf, - int len, unsigned int offset) +int devif_send(FAR struct net_driver_s *dev, FAR const void *buf, + int len, unsigned int offset) { int ret; @@ -113,8 +113,9 @@ void devif_send(FAR struct net_driver_s *dev, FAR const void *buf, dev->d_sndlen = len; - return; + return dev->d_sndlen; errout: nerr("ERROR: devif_send error: %d\n", ret); + return ret; } diff --git a/net/pkt/pkt_sendmsg.c b/net/pkt/pkt_sendmsg.c index 6f4539d707..08ed662c52 100644 --- a/net/pkt/pkt_sendmsg.c +++ b/net/pkt/pkt_sendmsg.c @@ -105,10 +105,12 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev, { /* Copy the packet data into the device packet buffer and send it */ - devif_send(dev, pstate->snd_buffer, pstate->snd_buflen, 0); - if (dev->d_sndlen == 0) + int ret = devif_send(dev, pstate->snd_buffer, + pstate->snd_buflen, 0); + if (ret <= 0) { - return flags; + pstate->snd_sent = ret; + goto end_wait; } pstate->snd_sent = pstate->snd_buflen; @@ -120,6 +122,8 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev, IFF_SET_NOARP(dev->d_flags); } +end_wait: + /* Don't allow any further call backs. */ pstate->snd_cb->flags = 0; diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c index f81b0895dc..614c675ea5 100644 --- a/net/tcp/tcp_send_buffered.c +++ b/net/tcp/tcp_send_buffered.c @@ -695,6 +695,7 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev, FAR sq_entry_t *entry; FAR sq_entry_t *next; size_t sndlen; + int ret; /* According to RFC 6298 (5.4), retransmit the earliest segment * that has not been acknowledged by the TCP receiver. @@ -742,9 +743,9 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev, tcp_setsequence(conn->sndseq, TCP_WBSEQNO(wrb)); - devif_iob_send(dev, TCP_WBIOB(wrb), sndlen, - 0, tcpip_hdrsize(conn)); - if (dev->d_sndlen == 0) + ret = devif_iob_send(dev, TCP_WBIOB(wrb), sndlen, + 0, tcpip_hdrsize(conn)); + if (ret <= 0) { return flags; } @@ -1005,6 +1006,7 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev, if (TCP_SEQ_LT(seq, snd_wnd_edge)) { uint32_t remaining_snd_wnd; + int ret; sndlen = TCP_WBPKTLEN(wrb) - TCP_WBSENT(wrb); if (sndlen > conn->mss) @@ -1048,9 +1050,9 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev, * won't actually happen until the polling cycle completes). */ - devif_iob_send(dev, TCP_WBIOB(wrb), sndlen, - TCP_WBSENT(wrb), tcpip_hdrsize(conn)); - if (dev->d_sndlen == 0) + ret = devif_iob_send(dev, TCP_WBIOB(wrb), sndlen, + TCP_WBSENT(wrb), tcpip_hdrsize(conn)); + if (ret <= 0) { return flags; } diff --git a/net/tcp/tcp_send_unbuffered.c b/net/tcp/tcp_send_unbuffered.c index 829fd2363d..2c40af0d31 100644 --- a/net/tcp/tcp_send_unbuffered.c +++ b/net/tcp/tcp_send_unbuffered.c @@ -126,6 +126,7 @@ static uint16_t tcpsend_eventhandler(FAR struct net_driver_s *dev, { FAR struct send_s *pstate = pvpriv; FAR struct tcp_conn_s *conn; + int ret; DEBUGASSERT(pstate != NULL); @@ -283,8 +284,12 @@ static uint16_t tcpsend_eventhandler(FAR struct net_driver_s *dev, * happen until the polling cycle completes). */ - devif_send(dev, &pstate->snd_buffer[pstate->snd_acked], - sndlen, tcpip_hdrsize(conn)); + ret = devif_send(dev, &pstate->snd_buffer[pstate->snd_acked], + sndlen, tcpip_hdrsize(conn)); + if (ret <= 0) + { + goto end_wait; + } /* Continue waiting */ @@ -365,11 +370,11 @@ static uint16_t tcpsend_eventhandler(FAR struct net_driver_s *dev, * happen until the polling cycle completes). */ - devif_send(dev, &pstate->snd_buffer[pstate->snd_sent], - sndlen, tcpip_hdrsize(conn)); - if (dev->d_sndlen == 0) + ret = devif_send(dev, &pstate->snd_buffer[pstate->snd_sent], + sndlen, tcpip_hdrsize(conn)); + if (ret <= 0) { - return flags; + goto end_wait; } /* Update the amount of data sent (but not necessarily ACKed) */ diff --git a/net/udp/udp_sendto_unbuffered.c b/net/udp/udp_sendto_unbuffered.c index a9318167e3..6d182f1393 100644 --- a/net/udp/udp_sendto_unbuffered.c +++ b/net/udp/udp_sendto_unbuffered.c @@ -194,11 +194,12 @@ static uint16_t sendto_eventhandler(FAR struct net_driver_s *dev, { /* Copy the user data into d_appdata and send it */ - devif_send(dev, pstate->st_buffer, - pstate->st_buflen, udpip_hdrsize(pstate->st_conn)); - if (dev->d_sndlen == 0) + int ret = devif_send(dev, pstate->st_buffer, pstate->st_buflen, + udpip_hdrsize(pstate->st_conn)); + if (ret <= 0) { - return flags; + pstate->st_sndlen = ret; + goto end_wait; } #ifdef NEED_IPDOMAIN_SUPPORT @@ -214,6 +215,8 @@ static uint16_t sendto_eventhandler(FAR struct net_driver_s *dev, pstate->st_sndlen = pstate->st_buflen; } +end_wait: + /* Don't allow any further call backs. */ pstate->st_cb->flags = 0;