From 49c236d6130e3e31b177dd08754555e872717542 Mon Sep 17 00:00:00 2001 From: Liu Zhi Fu Date: Fri, 29 Jun 2018 13:40:20 +0800 Subject: [PATCH] lwip: fix the assertion in tcp_pcb_purge() Fix the assertion in tcp_pcb_purge(). --- components/lwip/api/api_lib.c | 6 +- components/lwip/api/api_msg.c | 43 +++++-- components/lwip/core/tcp.c | 117 +++++++++++++----- components/lwip/core/tcp_in.c | 17 ++- .../lwip/include/lwip/lwip/priv/api_msg.h | 3 + components/lwip/include/lwip/lwip/tcp.h | 20 ++- 6 files changed, 149 insertions(+), 57 deletions(-) diff --git a/components/lwip/api/api_lib.c b/components/lwip/api/api_lib.c index 42d80a1ee..92ea4dced 100644 --- a/components/lwip/api/api_lib.c +++ b/components/lwip/api/api_lib.c @@ -407,9 +407,9 @@ netconn_accept(struct netconn *conn, struct netconn **new_conn) #if TCP_LISTEN_BACKLOG /* Let the stack know that we have accepted the connection. */ API_MSG_VAR_ALLOC_DONTFAIL(msg); - API_MSG_VAR_REF(msg).msg.conn = conn; - /* don't care for the return value of lwip_netconn_do_recv */ - TCPIP_APIMSG_NOERR(&API_MSG_VAR_REF(msg), lwip_netconn_do_recv); + API_MSG_VAR_REF(msg).msg.conn = newconn; + /* don't care for the return value of lwip_netconn_do_accepted */ + TCPIP_APIMSG_NOERR(&API_MSG_VAR_REF(msg), lwip_netconn_do_accepted); API_MSG_VAR_FREE(msg); #endif /* TCP_LISTEN_BACKLOG */ diff --git a/components/lwip/api/api_msg.c b/components/lwip/api/api_msg.c index 32a375419..531bca235 100644 --- a/components/lwip/api/api_msg.c +++ b/components/lwip/api/api_msg.c @@ -536,6 +536,9 @@ accept_function(void *arg, struct tcp_pcb *newpcb, err_t err) to the application thread */ newconn->last_err = err; + /* handle backlog counter */ + tcp_backlog_delayed(newpcb); + if (sys_mbox_trypost(&conn->acceptmbox, newconn) != ERR_OK) { ESP_STATS_DROP_INC(esp.acceptmbox_post_fail); /* When returning != ERR_OK, the pcb is aborted in tcp_process(), @@ -1503,24 +1506,38 @@ lwip_netconn_do_recv(void *m) msg->err = ERR_OK; if (msg->conn->pcb.tcp != NULL) { if (NETCONNTYPE_GROUP(msg->conn->type) == NETCONN_TCP) { -#if TCP_LISTEN_BACKLOG - if (msg->conn->pcb.tcp->state == LISTEN) { - tcp_accepted(msg->conn->pcb.tcp); - } else -#endif /* TCP_LISTEN_BACKLOG */ - { - 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); - } + 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); } } TCPIP_APIMSG_ACK(msg); } +#if TCP_LISTEN_BACKLOG +/** Indicate that a TCP pcb has been accepted + * Called from netconn_accept + * + * @param m the api_msg pointing to the connection + */ +void +lwip_netconn_do_accepted(void *m) +{ + struct api_msg_msg *msg = (struct api_msg_msg *)m; + + msg->err = ERR_OK; + if (msg->conn->pcb.tcp != NULL) { + if (NETCONNTYPE_GROUP(msg->conn->type) == NETCONN_TCP) { + tcp_backlog_accepted(msg->conn->pcb.tcp); + } + } + TCPIP_APIMSG_ACK(msg); +} +#endif /* TCP_LISTEN_BACKLOG */ + /** * See if more data needs to be written from a previous call to netconn_write. * Called initially from lwip_netconn_do_write. If the first call can't send all data diff --git a/components/lwip/core/tcp.c b/components/lwip/core/tcp.c index 96e35d0df..596f3d434 100644 --- a/components/lwip/core/tcp.c +++ b/components/lwip/core/tcp.c @@ -149,6 +149,22 @@ tcp_tmr(void) } } +#if LWIP_CALLBACK_API || TCP_LISTEN_BACKLOG +/** Called when a listen pcb is closed. Iterates one pcb list and removes the + * closed listener pcb from pcb->listener if matching. + */ +static void +tcp_remove_listener(struct tcp_pcb *list, struct tcp_pcb_listen *lpcb) +{ + struct tcp_pcb *pcb; + for (pcb = list; pcb != NULL; pcb = pcb->next) { + if (pcb->listener == lpcb) { + pcb->listener = NULL; + } + } +} +#endif + void tcp_set_fin_wait_1(struct tcp_pcb *pcb) { @@ -158,6 +174,70 @@ tcp_set_fin_wait_1(struct tcp_pcb *pcb) #endif } +/** Called when a listen pcb is closed. Iterates all pcb lists and removes the + * closed listener pcb from pcb->listener if matching. + */ +static void +tcp_listen_closed(struct tcp_pcb *pcb) +{ +#if LWIP_CALLBACK_API || TCP_LISTEN_BACKLOG + size_t i; + LWIP_ASSERT("pcb != NULL", pcb != NULL); + LWIP_ASSERT("pcb->state == LISTEN", pcb->state == LISTEN); + for (i = 1; i < LWIP_ARRAYSIZE(tcp_pcb_lists); i++) { + tcp_remove_listener(*tcp_pcb_lists[i], (struct tcp_pcb_listen *)pcb); + } +#endif + LWIP_UNUSED_ARG(pcb); +} + +#if TCP_LISTEN_BACKLOG +/** @ingroup tcp_raw + * Delay accepting a connection in respect to the listen backlog: + * the number of outstanding connections is increased until + * tcp_backlog_accepted() is called. + * + * ATTENTION: the caller is responsible for calling tcp_backlog_accepted() + * or else the backlog feature will get out of sync! + * + * @param pcb the connection pcb which is not fully accepted yet + */ +void +tcp_backlog_delayed(struct tcp_pcb *pcb) +{ + LWIP_ASSERT("pcb != NULL", pcb != NULL); + if ((pcb->flags & TF_BACKLOGPEND) == 0) { + if (pcb->listener != NULL) { + pcb->listener->accepts_pending++; + LWIP_ASSERT("accepts_pending != 0", pcb->listener->accepts_pending != 0); + pcb->flags |= TF_BACKLOGPEND; + } + } +} + +/** @ingroup tcp_raw + * A delayed-accept a connection is accepted (or closed/aborted): decreases + * the number of outstanding connections after calling tcp_backlog_delayed(). + * + * ATTENTION: the caller is responsible for calling tcp_backlog_accepted() + * or else the backlog feature will get out of sync! + * + * @param pcb the connection pcb which is now fully accepted (or closed/aborted) + */ +void +tcp_backlog_accepted(struct tcp_pcb *pcb) +{ + LWIP_ASSERT("pcb != NULL", pcb != NULL); + if ((pcb->flags & TF_BACKLOGPEND) != 0) { + if (pcb->listener != NULL) { + LWIP_ASSERT("accepts_pending != 0", pcb->listener->accepts_pending != 0); + pcb->listener->accepts_pending--; + pcb->flags &= ~TF_BACKLOGPEND; + } + } +} +#endif /* TCP_LISTEN_BACKLOG */ + /** * Closes the TX side of a connection held by the PCB. * For tcp_close(), a RST is sent if the application didn't receive all data @@ -227,6 +307,7 @@ tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data) break; case LISTEN: err = ERR_OK; + tcp_listen_closed(pcb); tcp_pcb_remove(&tcp_listen_pcbs.pcbs, pcb); memp_free(MEMP_TCP_PCB_LISTEN, pcb); pcb = NULL; @@ -241,6 +322,7 @@ tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data) case SYN_RCVD: err = tcp_send_fin(pcb); if (err == ERR_OK) { + tcp_backlog_accepted(pcb); MIB2_STATS_INC(mib2.tcpattemptfails); tcp_set_fin_wait_1(pcb); } @@ -408,6 +490,7 @@ tcp_abandon(struct tcp_pcb *pcb, int reset) pcb->ooseq = NULL; } #endif /* TCP_QUEUE_OOSEQ */ + tcp_backlog_accepted(pcb); if (send_rst) { LWIP_DEBUGF(TCP_RST_DEBUG, ("tcp_abandon: sending RST\n")); tcp_rst(seqno, ackno, &pcb->local_ip, &pcb->remote_ip, local_port, pcb->remote_port); @@ -1747,39 +1830,7 @@ tcp_pcb_purge(struct tcp_pcb *pcb) LWIP_DEBUGF(TCP_DEBUG, ("tcp_pcb_purge\n")); -#if TCP_LISTEN_BACKLOG - if (pcb->state == SYN_RCVD) { - /* Need to find the corresponding listen_pcb and decrease its accepts_pending */ - struct tcp_pcb_listen *lpcb; - - /* - * The official LWIP will assert the system if tcp_listen_pcbs.listen_pcbs is NULL, it's a bug. - * - * Considering following scenario: - * 1. On TCP server side, one of TCP pcb is in SYNC_RECV state and is waiting for TCP ACK from TCP client. - * 2. The TCP server is closed by application, which causes the tcp_listen_pcbs.listen_pcbs to become NULL. - * 3. When SYNC_RECV state timeout (20s by default), tcp_pcb_purge() is called in tcp_slowtmr(), it asserts - * the system. - * This is a normal scenario, should NOT assert the system. So just remove it. - * - */ - if (tcp_listen_pcbs.listen_pcbs) { - for (lpcb = tcp_listen_pcbs.listen_pcbs; lpcb != NULL; lpcb = lpcb->next) { - if ((lpcb->local_port == pcb->local_port) && - (IP_IS_V6_VAL(pcb->local_ip) == IP_IS_V6_VAL(lpcb->local_ip)) && - (ip_addr_isany(&lpcb->local_ip) || - ip_addr_cmp(&pcb->local_ip, &lpcb->local_ip))) { - /* port and address of the listen pcb match the timed-out pcb */ - LWIP_ASSERT("tcp_pcb_purge: listen pcb does not have accepts pending", - lpcb->accepts_pending > 0); - lpcb->accepts_pending--; - break; - } - } - } - } -#endif /* TCP_LISTEN_BACKLOG */ - + tcp_backlog_accepted(pcb); if (pcb->refused_data != NULL) { LWIP_DEBUGF(TCP_DEBUG, ("tcp_pcb_purge: data left on ->refused_data\n")); diff --git a/components/lwip/core/tcp_in.c b/components/lwip/core/tcp_in.c index 23dc5ae1c..a6f294629 100644 --- a/components/lwip/core/tcp_in.c +++ b/components/lwip/core/tcp_in.c @@ -568,6 +568,7 @@ tcp_listen_input(struct tcp_pcb_listen *pcb) } #if TCP_LISTEN_BACKLOG pcb->accepts_pending++; + npcb->flags |= TF_BACKLOGPEND; #endif /* TCP_LISTEN_BACKLOG */ /* Set up the new PCB. */ ip_addr_copy(npcb->local_ip, *ip_current_dest_addr()); @@ -582,6 +583,10 @@ tcp_listen_input(struct tcp_pcb_listen *pcb) #if LWIP_CALLBACK_API npcb->accept = pcb->accept; #endif /* LWIP_CALLBACK_API */ + +#if LWIP_CALLBACK_API || TCP_LISTEN_BACKLOG + npcb->listener = pcb; +#endif /* inherit socket options */ npcb->so_options = pcb->so_options & SOF_INHERITED; /* Register the new PCB so that we can begin receiving segments @@ -785,11 +790,19 @@ tcp_process(struct tcp_pcb *pcb) if (TCP_SEQ_BETWEEN(ackno, pcb->lastack+1, pcb->snd_nxt)) { pcb->state = ESTABLISHED; LWIP_DEBUGF(TCP_DEBUG, ("TCP connection established %"U16_F" -> %"U16_F".\n", inseg.tcphdr->src, inseg.tcphdr->dest)); +#if LWIP_CALLBACK_API || TCP_LISTEN_BACKLOG #if LWIP_CALLBACK_API LWIP_ASSERT("pcb->accept != NULL", pcb->accept != NULL); #endif - /* Call the accept function. */ - TCP_EVENT_ACCEPT(pcb, ERR_OK, err); + + if (pcb->listener == NULL) { + err = ERR_VAL; + } else { +#endif + tcp_backlog_accepted(pcb); + /* Call the accept function. */ + TCP_EVENT_ACCEPT(pcb, ERR_OK, err); + } if (err != ERR_OK) { /* If the accept function returns with an error, we abort * the connection. */ diff --git a/components/lwip/include/lwip/lwip/priv/api_msg.h b/components/lwip/include/lwip/lwip/priv/api_msg.h index 02d191a53..a786b2ee4 100644 --- a/components/lwip/include/lwip/lwip/priv/api_msg.h +++ b/components/lwip/include/lwip/lwip/priv/api_msg.h @@ -232,6 +232,9 @@ void lwip_netconn_do_disconnect (void *m); void lwip_netconn_do_listen (void *m); void lwip_netconn_do_send (void *m); void lwip_netconn_do_recv (void *m); +#if TCP_LISTEN_BACKLOG +void lwip_netconn_do_accepted (void *m); +#endif void lwip_netconn_do_write (void *m); void lwip_netconn_do_getaddr (void *m); void lwip_netconn_do_close (void *m); diff --git a/components/lwip/include/lwip/lwip/tcp.h b/components/lwip/include/lwip/lwip/tcp.h index f7a46b2e8..269b4e3d3 100644 --- a/components/lwip/include/lwip/lwip/tcp.h +++ b/components/lwip/include/lwip/lwip/tcp.h @@ -138,7 +138,7 @@ typedef u16_t tcpflags_t; #define TCPWND16(x) (x) #define TCP_WND_MAX(pcb) TCP_WND(pcb) typedef u16_t tcpwnd_size_t; -typedef u8_t tcpflags_t; +typedef u16_t tcpflags_t; #endif enum tcp_state { @@ -203,6 +203,9 @@ struct tcp_pcb { #define TF_NAGLEMEMERR 0x80U /* nagle enabled, memerr, try to output to prevent delayed ACK to happen */ #if LWIP_WND_SCALE #define TF_WND_SCALE 0x0100U /* Window Scale option enabled */ +#endif +#if TCP_LISTEN_BACKLOG +#define TF_BACKLOGPEND 0x0200U /* If this is set, a connection pcb has increased the backlog on its listener */ #endif /* the rest of the fields are in host byte order @@ -311,6 +314,10 @@ struct tcp_pcb { u8_t rcv_scale; #endif +#if TCP_LISTEN_BACKLOG + struct tcp_pcb_listen *listener; +#endif + #if ESP_STATS_TCP #define ESP_STATS_TCP_ARRAY_SIZE 20 u16_t retry_cnt[TCP_MAXRTX]; @@ -383,16 +390,17 @@ void tcp_err (struct tcp_pcb *pcb, tcp_err_fn err); #define tcp_nagle_disabled(pcb) (((pcb)->flags & TF_NODELAY) != 0) #if TCP_LISTEN_BACKLOG -#define tcp_accepted(pcb) do { \ - LWIP_ASSERT("pcb->state == LISTEN (called for wrong pcb?)", pcb->state == LISTEN); \ - (((struct tcp_pcb_listen *)(pcb))->accepts_pending--); } while(0) #define tcp_backlog_set(pcb, new_backlog) do { \ LWIP_ASSERT("pcb->state == LISTEN (called for wrong pcb?)", (pcb)->state == LISTEN); \ ((struct tcp_pcb_listen *)(pcb))->backlog = ((new_backlog) ? (new_backlog) : 1); } while(0) +void tcp_backlog_delayed(struct tcp_pcb* pcb); +void tcp_backlog_accepted(struct tcp_pcb* pcb); #else /* TCP_LISTEN_BACKLOG */ -#define tcp_accepted(pcb) LWIP_ASSERT("pcb->state == LISTEN (called for wrong pcb?)", \ - (pcb)->state == LISTEN) +#define tcp_backlog_set(pcb, new_backlog) +#define tcp_backlog_delayed(pcb) +#define tcp_backlog_accepted(pcb) #endif /* TCP_LISTEN_BACKLOG */ +#define tcp_accepted(pcb) do { LWIP_UNUSED_ARG(pcb); } while(0) /* compatibility define, not needed any more */ void tcp_recved (struct tcp_pcb *pcb, u16_t len); err_t tcp_bind (struct tcp_pcb *pcb, const ip_addr_t *ipaddr,