From b1980b36b8af492c18a4c53b4639b033efff0101 Mon Sep 17 00:00:00 2001 From: Simon Goldschmidt Date: Fri, 22 Jul 2011 22:05:24 +0200 Subject: [PATCH] fixed bug #31084 (socket API returns always EMSGSIZE on non-blocking sockets if data size > send buffers) -> now lwip_send() sends as much as possible for non-blocking sockets and only returns EWOULDBLOCK if the buffers are full --- CHANGELOG | 5 +++ src/api/api_lib.c | 23 +++++++++-- src/api/api_msg.c | 94 +++++++++++++++++++++--------------------- src/api/sockets.c | 16 +++---- src/include/lwip/api.h | 6 ++- 5 files changed, 81 insertions(+), 63 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2be486e1..66ccf057 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -17,6 +17,11 @@ HISTORY ++ Bugfixes: + 2011-07-22: Simon Goldschmidt + * api_lib.c, api_msg.c, sockets.c, api.h: fixed bug #31084 (socket API returns + always EMSGSIZE on non-blocking sockets if data size > send buffers) -> now + lwip_send() sends as much as possible for non-blocking sockets + 2011-07-22: Simon Goldschmidt * pbuf.c/.h, timers.c: freeing ooseq pbufs when the pbuf pool is empty implemented for NO_SYS==1: when not using sys_check_timeouts(), call PBUF_CHECK_FREE_OOSEQ() diff --git a/src/api/api_lib.c b/src/api/api_lib.c index b1a9e525..3a7ad58c 100644 --- a/src/api/api_lib.c +++ b/src/api/api_lib.c @@ -582,22 +582,30 @@ netconn_send(struct netconn *conn, struct netbuf *buf) * - NETCONN_COPY: data will be copied into memory belonging to the stack * - NETCONN_MORE: for TCP connection, PSH flag will be set on last segment sent * - NETCONN_DONTBLOCK: only write the data if all dat can be written at once + * @param bytes_written pointer to a location that receives the number of written bytes * @return ERR_OK if data was sent, any other err_t on error */ err_t -netconn_write(struct netconn *conn, const void *dataptr, size_t size, u8_t apiflags) +netconn_write_partly(struct netconn *conn, const void *dataptr, size_t size, + u8_t apiflags, size_t *bytes_written) { struct api_msg msg; err_t err; + u8_t dontblock; LWIP_ERROR("netconn_write: invalid conn", (conn != NULL), return ERR_ARG;); LWIP_ERROR("netconn_write: invalid conn->type", (conn->type == NETCONN_TCP), return ERR_VAL;); if (size == 0) { return ERR_OK; } + dontblock = netconn_is_nonblocking(conn) || (apiflags & NETCONN_DONTBLOCK); + if (dontblock && !bytes_written) { + /* This implies netconn_write() cannot be used for non-blocking send, since + it has no way to return the number of bytes written. */ + return ERR_VAL; + } - /* @todo: for non-blocking write, check if 'size' would ever fit into - snd_queue or snd_buf */ + /* non-blocking write sends as much */ msg.function = do_write; msg.msg.conn = conn; msg.msg.msg.w.dataptr = dataptr; @@ -607,6 +615,15 @@ netconn_write(struct netconn *conn, const void *dataptr, size_t size, u8_t apifl but if it is, this is done inside api_msg.c:do_write(), so we can use the non-blocking version here. */ err = TCPIP_APIMSG(&msg); + if ((err == ERR_OK) && (bytes_written != NULL)) { + if (dontblock) { + /* nonblocking write: maybe the data has been sent partly */ + *bytes_written = msg.msg.msg.w.len; + } else { + /* blocking call succeeded: all data has been sent if it */ + *bytes_written = size; + } + } NETCONN_SET_SAFE_ERR(conn, err); return err; diff --git a/src/api/api_msg.c b/src/api/api_msg.c index 448f96dd..f9f709e7 100644 --- a/src/api/api_msg.c +++ b/src/api/api_msg.c @@ -1193,7 +1193,7 @@ do_recv(struct api_msg_msg *msg) static err_t do_writemore(struct netconn *conn) { - err_t err = ERR_OK; + err_t err; void *dataptr; u16_t len, available; u8_t write_finished = 0; @@ -1224,62 +1224,62 @@ do_writemore(struct netconn *conn) if (available < len) { /* don't try to write more than sendbuf */ len = available; + if (dontblock){ + if (!len) { + err = ERR_WOULDBLOCK; + goto err_mem; + } + } else { #if LWIP_TCPIP_CORE_LOCKING - conn->flags |= NETCONN_FLAG_WRITE_DELAYED; + conn->flags |= NETCONN_FLAG_WRITE_DELAYED; #endif - apiflags |= TCP_WRITE_FLAG_MORE; + apiflags |= TCP_WRITE_FLAG_MORE; + } } - if (dontblock && (len < conn->current_msg->msg.w.len)) { - /* failed to send all data at once -> nonblocking write not possible */ - err = ERR_MEM; - } - if (err == ERR_OK) { - LWIP_ASSERT("do_writemore: invalid length!", ((conn->write_offset + len) <= conn->current_msg->msg.w.len)); - err = tcp_write(conn->pcb.tcp, dataptr, len, apiflags); - } - if (dontblock && (err == ERR_MEM)) { - /* nonblocking write failed */ - write_finished = 1; - err = ERR_WOULDBLOCK; - /* let poll_tcp check writable space to mark the pcb - writable again */ - conn->flags |= NETCONN_FLAG_CHECK_WRITESPACE; - /* let select mark this pcb as non-writable. */ - API_EVENT(conn, NETCONN_EVT_SENDMINUS, len); - } else { - /* if OK or memory error, check available space */ - if (((err == ERR_OK) || (err == ERR_MEM)) && - ((tcp_sndbuf(conn->pcb.tcp) <= TCP_SNDLOWAT) || - (tcp_sndqueuelen(conn->pcb.tcp) >= TCP_SNDQUEUELOWAT))) { + LWIP_ASSERT("do_writemore: invalid length!", ((conn->write_offset + len) <= conn->current_msg->msg.w.len)); + err = tcp_write(conn->pcb.tcp, dataptr, len, apiflags); + /* if OK or memory error, check available space */ + if ((err == ERR_OK) || (err == ERR_MEM)) { +err_mem: + if (dontblock && (len < conn->current_msg->msg.w.len)) { + /* non-blocking write did not write everything: mark the pcb non-writable + and let poll_tcp check writable space to mark the pcb writable again */ + API_EVENT(conn, NETCONN_EVT_SENDMINUS, len); + conn->flags |= NETCONN_FLAG_CHECK_WRITESPACE; + } else if ((tcp_sndbuf(conn->pcb.tcp) <= TCP_SNDLOWAT) || + (tcp_sndqueuelen(conn->pcb.tcp) >= TCP_SNDQUEUELOWAT)) { /* The queued byte- or pbuf-count exceeds the configured low-water limit, let select mark this pcb as non-writable. */ API_EVENT(conn, NETCONN_EVT_SENDMINUS, len); } + } - if (err == ERR_OK) { - conn->write_offset += len; - if (conn->write_offset == conn->current_msg->msg.w.len) { - /* everything was written */ - write_finished = 1; - conn->write_offset = 0; - } - tcp_output(conn->pcb.tcp); - } else if (err == ERR_MEM) { - /* If ERR_MEM, we wait for sent_tcp or poll_tcp to be called - we do NOT return to the application thread, since ERR_MEM is - only a temporary error! */ - - /* tcp_write returned ERR_MEM, try tcp_output anyway */ - tcp_output(conn->pcb.tcp); - - #if LWIP_TCPIP_CORE_LOCKING - conn->flags |= NETCONN_FLAG_WRITE_DELAYED; - #endif - } else { - /* On errors != ERR_MEM, we don't try writing any more but return - the error to the application thread. */ + if (err == ERR_OK) { + conn->write_offset += len; + if ((conn->write_offset == conn->current_msg->msg.w.len) || dontblock) { + /* return sent length */ + conn->current_msg->msg.w.len = conn->write_offset; + /* everything was written */ write_finished = 1; + conn->write_offset = 0; } + tcp_output(conn->pcb.tcp); + } else if ((err == ERR_MEM) && !dontblock) { + /* If ERR_MEM, we wait for sent_tcp or poll_tcp to be called + we do NOT return to the application thread, since ERR_MEM is + only a temporary error! */ + + /* tcp_write returned ERR_MEM, try tcp_output anyway */ + tcp_output(conn->pcb.tcp); + +#if LWIP_TCPIP_CORE_LOCKING + conn->flags |= NETCONN_FLAG_WRITE_DELAYED; +#endif + } else { + /* On errors != ERR_MEM, we don't try writing any more but return + the error to the application thread. */ + write_finished = 1; + conn->current_msg->msg.w.len = 0; } if (write_finished) { diff --git a/src/api/sockets.c b/src/api/sockets.c index e36012ce..a9a85c37 100644 --- a/src/api/sockets.c +++ b/src/api/sockets.c @@ -748,6 +748,7 @@ lwip_send(int s, const void *data, size_t size, int flags) struct lwip_sock *sock; err_t err; u8_t write_flags; + size_t written; LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_send(%d, data=%p, size=%"SZT_F", flags=0x%x)\n", s, data, size, flags)); @@ -766,22 +767,15 @@ lwip_send(int s, const void *data, size_t size, int flags) #endif /* (LWIP_UDP || LWIP_RAW) */ } - if ((flags & MSG_DONTWAIT) || netconn_is_nonblocking(sock->conn)) { - if ((size > TCP_SND_BUF) || ((size / TCP_MSS) > TCP_SND_QUEUELEN)) { - /* too much data to ever send nonblocking! */ - sock_set_errno(sock, EMSGSIZE); - return -1; - } - } - write_flags = NETCONN_COPY | ((flags & MSG_MORE) ? NETCONN_MORE : 0) | ((flags & MSG_DONTWAIT) ? NETCONN_DONTBLOCK : 0); - err = netconn_write(sock->conn, data, size, write_flags); + written = 0; + err = netconn_write_partly(sock->conn, data, size, write_flags, &written); - LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_send(%d) err=%d size=%"SZT_F"\n", s, err, size)); + LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_send(%d) err=%d written=%"SZT_F"\n", s, err, written)); sock_set_errno(sock, err_to_errno(err)); - return (err == ERR_OK ? (int)size : -1); + return (err == ERR_OK ? (int)written : -1); } int diff --git a/src/include/lwip/api.h b/src/include/lwip/api.h index 91b9e5d2..6071f6d7 100644 --- a/src/include/lwip/api.h +++ b/src/include/lwip/api.h @@ -230,8 +230,10 @@ void netconn_recved(struct netconn *conn, u32_t length); err_t netconn_sendto(struct netconn *conn, struct netbuf *buf, ip_addr_t *addr, u16_t port); err_t netconn_send(struct netconn *conn, struct netbuf *buf); -err_t netconn_write(struct netconn *conn, const void *dataptr, size_t size, - u8_t apiflags); +err_t netconn_write_partly(struct netconn *conn, const void *dataptr, size_t size, + u8_t apiflags, size_t *bytes_written); +#define netconn_write(conn, dataptr, size, apiflags) \ + netconn_write_partly(conn, dataptr, size, apiflags, NULL) err_t netconn_close(struct netconn *conn); err_t netconn_shutdown(struct netconn *conn, u8_t shut_rx, u8_t shut_tx);