From 306f2203fa0bdbd48e9bd4d95ef21dfb70009f83 Mon Sep 17 00:00:00 2001 From: goldsimon Date: Tue, 9 Feb 2010 20:23:39 +0000 Subject: [PATCH] Fixed bug #22110 (recv() makes receive window update for data that wasn't received by application); added function-like macros to correctly access/change conn->recv_timeout and conn->recv_bufsize --- CHANGELOG | 6 +++++ src/api/api_lib.c | 51 ++++++++++++++++++++++++++++++-------- src/api/api_msg.c | 7 +++++- src/api/sockets.c | 22 +++++++++++++--- src/include/lwip/api.h | 24 +++++++++++++++++- src/include/lwip/api_msg.h | 2 +- 6 files changed, 94 insertions(+), 18 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index aebe14a8..087aa1df 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -103,6 +103,12 @@ HISTORY ++ Bugfixes: + 2010-02-09: Simon Goldschmidt + * api_lib.c, api_msg.c, sockets.c, api.h, api_msg.h: Fixed bug #22110 + (recv() makes receive window update for data that wasn't received by + application) + + 2010-02-09: Simon Goldschmidt/Stephane Lesage * sockets.c: Fixed bug #28853 (lwip_recvfrom() returns 0 on receive time-out or any netconn_recv() error) diff --git a/src/api/api_lib.c b/src/api/api_lib.c index 6e800cc5..861ee1bd 100644 --- a/src/api/api_lib.c +++ b/src/api/api_lib.c @@ -388,18 +388,20 @@ netconn_recv(struct netconn *conn, struct netbuf **new_buf) buf->port = 0; buf->addr = NULL; - /* Let the stack know that we have taken the data. */ - /* TODO: Speedup: Don't block and wait for the answer here - (to prevent multiple thread-switches). */ - msg.function = do_recv; - msg.msg.conn = conn; - if (buf != NULL) { - msg.msg.msg.r.len = buf->p->tot_len; - } else { - msg.msg.msg.r.len = 1; + if (!netconn_get_noautorecved(conn) || (buf == NULL)) { + /* Let the stack know that we have taken the data. */ + /* TODO: Speedup: Don't block and wait for the answer here + (to prevent multiple thread-switches). */ + msg.function = do_recv; + msg.msg.conn = conn; + if (buf != NULL) { + msg.msg.msg.r.len = buf->p->tot_len; + } else { + msg.msg.msg.r.len = 1; + } + /* don't care for the return value of do_recv */ + TCPIP_APIMSG(&msg); } - /* don't care for the return value of do_recv */ - TCPIP_APIMSG(&msg); #endif /* LWIP_TCP */ } else { #if (LWIP_UDP || LWIP_RAW) @@ -427,6 +429,33 @@ netconn_recv(struct netconn *conn, struct netbuf **new_buf) return ERR_OK; } +/** + * TCP: update the receive window: by calling this, the application + * tells the stack that it has processed data and is able to accept + * new data. + * ATTENTION: use with care, this is mainly used for sockets! + * Can only be used when calling netconn_set_noautorecved(conn, 1) before. + * + * @param conn the netconn for which to update the receive window + * @param length amount of data processed (ATTENTION: this must be accurate!) + */ +void +netconn_recved(struct netconn *conn, u32_t length) +{ + if ((conn != NULL) && (conn->type == NETCONN_TCP) && + (netconn_get_noautorecved(conn))) { + struct api_msg msg; + /* Let the stack know that we have taken the data. */ + /* TODO: Speedup: Don't block and wait for the answer here + (to prevent multiple thread-switches). */ + msg.function = do_recv; + msg.msg.conn = conn; + msg.msg.msg.r.len = length; + /* don't care for the return value of do_recv */ + TCPIP_APIMSG(&msg); + } +} + /** * Send data (in form of a netbuf) to a specific remote IP address and port. * Only to be used for UDP and RAW netconns (not TCP). diff --git a/src/api/api_msg.c b/src/api/api_msg.c index 23c05e89..f7f19c5b 100644 --- a/src/api/api_msg.c +++ b/src/api/api_msg.c @@ -1102,7 +1102,12 @@ do_recv(struct api_msg_msg *msg) } else #endif /* TCP_LISTEN_BACKLOG */ { - tcp_recved(msg->conn->pcb.tcp, msg->msg.r.len); + u32_t remaining = msg->msg.r.len; + do { + u16_t recved = (remaining > 0xffff) ? 0xffff : (u16_t)remaining; + tcp_recved(msg->conn->pcb.tcp, recved); + remaining -= recved; + }while(remaining != 0); } } } diff --git a/src/api/sockets.c b/src/api/sockets.c index de551990..7ff83bee 100644 --- a/src/api/sockets.c +++ b/src/api/sockets.c @@ -304,6 +304,8 @@ lwip_accept(int s, struct sockaddr *addr, socklen_t *addrlen) return -1; } LWIP_ASSERT("newconn != NULL", newconn != NULL); + /* Prevent automatic window updates, we do this on our own! */ + netconn_set_noautorecved(newconn, 1); /* get the IP address and port of the remote host */ err = netconn_peer(newconn, &naddr, &port); @@ -519,6 +521,8 @@ lwip_recvfrom(int s, void *mem, size_t len, int flags, if (((flags & MSG_DONTWAIT) || netconn_is_nonblocking(sock->conn)) && (sock->rcvevent <= 0)) { if (off > 0) { + /* update receive window */ + netconn_recved(sock->conn, (u32_t)off); /* already received data, return that */ sock_set_errno(sock, 0); return off; @@ -536,6 +540,8 @@ lwip_recvfrom(int s, void *mem, size_t len, int flags, if (err != ERR_OK) { if (off > 0) { + /* update receive window */ + netconn_recved(sock->conn, (u32_t)off); /* already received data, return that */ sock_set_errno(sock, 0); return off; @@ -649,6 +655,10 @@ lwip_recvfrom(int s, void *mem, size_t len, int flags, } } while (!done); + if (off > 0) { + /* update receive window */ + netconn_recved(sock->conn, (u32_t)off); + } sock_set_errno(sock, 0); return off; } @@ -819,6 +829,10 @@ lwip_socket(int domain, int type, int protocol) conn = netconn_new_with_callback(NETCONN_TCP, event_callback); LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_socket(%s, SOCK_STREAM, %d) = ", domain == PF_INET ? "PF_INET" : "UNKNOWN", protocol)); + if (conn != NULL) { + /* Prevent automatic window updates, we do this on our own! */ + netconn_set_noautorecved(conn, 1); + } break; default: LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_socket(%d, %d/UNKNOWN, %d) = -1\n", @@ -1488,12 +1502,12 @@ lwip_getsockopt_internal(void *arg) #if LWIP_SO_RCVTIMEO case SO_RCVTIMEO: - *(int *)optval = sock->conn->recv_timeout; + *(int *)optval = netconn_get_recvtimeout(sock->conn); break; #endif /* LWIP_SO_RCVTIMEO */ #if LWIP_SO_RCVBUF case SO_RCVBUF: - *(int *)optval = sock->conn->recv_bufsize; + *(int *)optval = netconn_get_recvbufsize(sock->conn); break; #endif /* LWIP_SO_RCVBUF */ #if LWIP_UDP @@ -1833,12 +1847,12 @@ lwip_setsockopt_internal(void *arg) break; #if LWIP_SO_RCVTIMEO case SO_RCVTIMEO: - sock->conn->recv_timeout = ( *(int*)optval ); + netconn_set_recvtimeout(sock->conn, *(int*)optval); break; #endif /* LWIP_SO_RCVTIMEO */ #if LWIP_SO_RCVBUF case SO_RCVBUF: - sock->conn->recv_bufsize = ( *(int*)optval ); + netconn_set_recvbufsize(sock->conn, *(int*)optval); break; #endif /* LWIP_SO_RCVBUF */ #if LWIP_UDP diff --git a/src/include/lwip/api.h b/src/include/lwip/api.h index 13771d61..cc2fb288 100644 --- a/src/include/lwip/api.h +++ b/src/include/lwip/api.h @@ -69,7 +69,7 @@ extern "C" { #define NETCONN_FLAG_IN_NONBLOCKING_CONNECT 0x04 /** If this is set, a TCP netconn must call netconn_recved() to update the TCP receive window (done automatically if not set). */ -#define NETCONN_FLAG_UPDATE_TCPWIN_MANUALLY 0x08 +#define NETCONN_FLAG_NO_AUTO_RECVED 0x08 /* Helpers to process several netconn_types by the same code */ @@ -214,6 +214,7 @@ err_t netconn_listen_with_backlog(struct netconn *conn, u8_t backlog); #define netconn_listen(conn) netconn_listen_with_backlog(conn, TCP_DEFAULT_LISTEN_BACKLOG) err_t netconn_accept(struct netconn *conn, struct netconn **new_conn); err_t netconn_recv(struct netconn *conn, struct netbuf **new_buf); +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); @@ -240,6 +241,27 @@ err_t netconn_gethostbyname(const char *name, ip_addr_t *addr); /** Get the blocking status of netconn calls (@todo: write/send is missing) */ #define netconn_is_nonblocking(conn) (((conn)->flags & NETCONN_FLAG_NON_BLOCKING) != 0) +/** TCP: Set the no-auto-recved status of netconn calls (see NETCONN_FLAG_NO_AUTO_RECVED) */ +#define netconn_set_noautorecved(conn, val) do { if(val) { \ + (conn)->flags |= NETCONN_FLAG_NO_AUTO_RECVED; \ +} else { \ + (conn)->flags &= ~ NETCONN_FLAG_NO_AUTO_RECVED; }} while(0) +/** TCP: Get the no-auto-recved status of netconn calls (see NETCONN_FLAG_NO_AUTO_RECVED) */ +#define netconn_get_noautorecved(conn) (((conn)->flags & NETCONN_FLAG_NO_AUTO_RECVED) != 0) + +#if LWIP_SO_RCVTIMEO +/** Set the receive timeout in miliseconds */ +#define netconn_set_recvtimeout(conn, timeout) ((conn)->recv_timeout = (timeout)) +/** Get the receive timeout in miliseconds */ +#define netconn_get_recvtimeout(conn) ((conn)->recv_timeout) +#endif /* LWIP_SO_RCVTIMEO */ +#if LWIP_SO_RCVBUF +/** Set the receive buffer in bytes */ +#define netconn_set_recvbufsize(conn, recvbufsize) ((conn)->recv_bufsize = (recvbufsize)) +/** Get the receive buffer in bytes */ +#define netconn_get_recvbufsize(conn) ((conn)->recv_bufsize) +#endif /* LWIP_SO_RCVBUF*/ + #ifdef __cplusplus } #endif diff --git a/src/include/lwip/api_msg.h b/src/include/lwip/api_msg.h index 942e9398..4de7a488 100644 --- a/src/include/lwip/api_msg.h +++ b/src/include/lwip/api_msg.h @@ -87,7 +87,7 @@ struct api_msg_msg { } w; /** used for do_recv */ struct { - u16_t len; + u32_t len; } r; #if LWIP_IGMP /** used for do_join_leave_group */