From ab92d43bc6cae97aef1d5b6a5fb48343ee4254c0 Mon Sep 17 00:00:00 2001 From: Liu Zhi Fu Date: Sat, 19 Nov 2016 14:57:42 +0800 Subject: [PATCH 1/3] lwip: fix socket memory leak issue 1. Add socket memory leak debug counter 2. Fix TCP PCB leak issue --- components/lwip/api/lwip_debug.c | 21 +++ components/lwip/core/tcp.c | 136 +++++++++--------- components/lwip/core/tcp_out.c | 3 +- .../lwip/include/lwip/lwip/lwip_debug.h | 1 + components/lwip/include/lwip/lwip/memp.h | 4 +- .../lwip/include/lwip/lwip/priv/memp_priv.h | 10 ++ components/lwip/include/lwip/port/lwipopts.h | 1 + 7 files changed, 102 insertions(+), 74 deletions(-) diff --git a/components/lwip/api/lwip_debug.c b/components/lwip/api/lwip_debug.c index d73a23e1a3..08869149a3 100644 --- a/components/lwip/api/lwip_debug.c +++ b/components/lwip/api/lwip_debug.c @@ -18,6 +18,8 @@ #include "lwip/tcp.h" #include "lwip/udp.h" #include "lwip/priv/tcp_priv.h" +#include "lwip/priv/memp_priv.h" +#include "lwip/memp.h" #define DBG_LWIP_IP_SHOW(info, ip) printf("%s type=%d ip=%x\n", (info), (ip).type, (ip).u_addr.ip4.addr) #define DBG_LWIP_IP_PCB_SHOW(pcb) \ @@ -127,3 +129,22 @@ void dbg_lwip_udp_rxtx_show(void) printf("TBC\n"); } +#if (ESP_CNT_DEBUG == 1) + +uint32_t g_lwip_mem_cnt[MEMP_MAX][2]; +extern const struct memp_desc * const memp_pools[MEMP_MAX]; + +void dbg_lwip_cnt_show(void) +{ + int i=0; + + printf("-----lwip memory counter-----\n"); + printf("%6s %8s %8s\n", "index", "alloc", "free"); + for (i=0; inext) { - if (pcb->state == FIN_WAIT_2) { - if ((u32_t) (tcp_ticks - pcb->tmr) >= inactivity) { - inactivity = tcp_ticks - pcb->tmr; - inactive = pcb; - } - } - } - if (inactive != NULL) { - tcp_pcb_remove(&tcp_active_pcbs, inactive); - memp_free(MEMP_TCP_PCB, inactive); - } -} +typedef struct { + u8_t time_wait; + u8_t closing; + u8_t fin_wait2; + u8_t last_ack; + u8_t listen; + u8_t bound; + u8_t total; +}tcp_pcb_num_t; -/** - * Kills the oldest connection that is in LAST_ACK state. - * Called from tcp_alloc() if no more connections are available. - */ -static void tcp_kill_lastack(void) +void tcp_pcb_num_cal(tcp_pcb_num_t *tcp_pcb_num) { - struct tcp_pcb *pcb, *inactive; - u32_t inactivity; - /* Go through the list of LAST_ACK pcbs and get the oldest pcb. */ - inactivity = 0; - inactive = NULL; - for (pcb = tcp_active_pcbs; pcb != NULL; pcb = pcb->next) { - if (pcb->state == LAST_ACK) { - if ((u32_t) (tcp_ticks - pcb->tmr) >= inactivity) { - inactivity = tcp_ticks - pcb->tmr; - inactive = pcb; - } - } - } - if (inactive != NULL) { - tcp_pcb_remove(&tcp_active_pcbs, inactive); - memp_free(MEMP_TCP_PCB, inactive); - } + struct tcp_pcb_listen *listen; + struct tcp_pcb *pcb; + + if (!tcp_pcb_num){ + return; + } + + memset(tcp_pcb_num, 0, sizeof(*tcp_pcb_num)); + for(pcb = tcp_tw_pcbs; pcb != NULL; pcb = pcb->next) { + tcp_pcb_num->total ++; + tcp_pcb_num->time_wait ++; + } + + for (pcb = tcp_active_pcbs; pcb != NULL; pcb = pcb->next){ + tcp_pcb_num->total ++; + if (pcb->state == FIN_WAIT_2){ + tcp_pcb_num->fin_wait2 ++; + } else if (pcb->state == LAST_ACK) { + tcp_pcb_num->last_ack ++; + } else if (pcb->state == CLOSING) { + tcp_pcb_num->closing ++; + } + } + + for (pcb = tcp_listen_pcbs.listen_pcbs; pcb != NULL; pcb = pcb->next){ + tcp_pcb_num->total ++; + tcp_pcb_num->listen ++; + } + + for (pcb = tcp_bound_pcbs; pcb != NULL; pcb = pcb->next){ + tcp_pcb_num->total ++; + tcp_pcb_num->bound ++; + } } #endif - /** * Allocate a new tcp_pcb structure. * @@ -1455,34 +1451,32 @@ tcp_alloc(u8_t prio) u32_t iss; #if ESP_LWIP - /*Kills the oldest connection that is in TIME_WAIT state.*/ - u8_t time_wait_num = 0; - for(pcb = tcp_tw_pcbs; pcb != NULL; pcb = pcb->next) { - time_wait_num ++; + tcp_pcb_num_t tcp_pcb_num; + + tcp_pcb_num_cal(&tcp_pcb_num); + + if (tcp_pcb_num.total >= MEMP_NUM_TCP_PCB){ + if (tcp_pcb_num.time_wait > 0){ + tcp_kill_timewait(); + } else if (tcp_pcb_num.last_ack > 0){ + tcp_kill_state(LAST_ACK); + } else if (tcp_pcb_num.closing > 0){ + tcp_kill_state(CLOSING); + } else if (tcp_pcb_num.fin_wait2 > 0){ + tcp_kill_state(FIN_WAIT_2);//TODO check whether we have issue here????? + } else { + tcp_kill_prio(prio); + } } - if (time_wait_num >= MEMP_NUM_TCP_PCB) - tcp_kill_timewait(); - - /*Kills the oldest connection that is in FIN_WAIT_2 state.*/ - time_wait_num = 0; - for (pcb = tcp_active_pcbs; pcb != NULL; pcb = pcb->next){ - if (pcb->state == FIN_WAIT_2) - time_wait_num ++; + tcp_pcb_num_cal(&tcp_pcb_num); + if (tcp_pcb_num.total >= MEMP_NUM_TCP_PCB){ + LWIP_DEBUGF(TCP_DEBUG, ("tcp_alloc: no available tcp pcb %d %d %d %d %d %d %d\n", + tcp_pcb_num.total, tcp_pcb_num.time_wait, tcp_pcb_num.last_ack, tcp_pcb_num.closing, + tcp_pcb_num.fin_wait2, tcp_pcb_num.listen, tcp_pcb_num.bound)); + return NULL; } - if (time_wait_num >= MEMP_NUM_TCP_PCB) - tcp_kill_finwait2(); - - /*Kills the oldest connection that is in LAST_ACK state.*/ - time_wait_num = 0; - for (pcb = tcp_active_pcbs; pcb != NULL; pcb = pcb->next){ - if (pcb->state == LAST_ACK) - time_wait_num ++; - } - - if (time_wait_num >= MEMP_NUM_TCP_PCB) - tcp_kill_lastack(); #endif pcb = (struct tcp_pcb *)memp_malloc(MEMP_TCP_PCB); diff --git a/components/lwip/core/tcp_out.c b/components/lwip/core/tcp_out.c index 35a8aa145d..fbe879124a 100755 --- a/components/lwip/core/tcp_out.c +++ b/components/lwip/core/tcp_out.c @@ -176,7 +176,8 @@ tcp_create_segment(struct tcp_pcb *pcb, struct pbuf *p, u8_t flags, u32_t seqno, struct tcp_seg *seg; u8_t optlen = LWIP_TCP_OPT_LENGTH(optflags); - if ((seg = (struct tcp_seg *)memp_malloc(MEMP_TCP_SEG)) == NULL) { + seg = (struct tcp_seg *)memp_malloc(MEMP_TCP_SEG); + if (seg == NULL) { LWIP_DEBUGF(TCP_OUTPUT_DEBUG | LWIP_DBG_LEVEL_SERIOUS, ("tcp_create_segment: no memory.\n")); pbuf_free(p); return NULL; diff --git a/components/lwip/include/lwip/lwip/lwip_debug.h b/components/lwip/include/lwip/lwip/lwip_debug.h index abfcd2c1c3..4da520269a 100644 --- a/components/lwip/include/lwip/lwip/lwip_debug.h +++ b/components/lwip/include/lwip/lwip/lwip_debug.h @@ -20,5 +20,6 @@ void dbg_lwip_tcp_pcb_show(void); void dbg_lwip_udp_pcb_show(void); void dbg_lwip_tcp_rxtx_show(void); void dbg_lwip_udp_rxtx_show(void); +void dbg_lwip_mem_cnt_show(void); #endif diff --git a/components/lwip/include/lwip/lwip/memp.h b/components/lwip/include/lwip/lwip/memp.h index d7463f2b33..bc9b021e87 100755 --- a/components/lwip/include/lwip/lwip/memp.h +++ b/components/lwip/include/lwip/lwip/memp.h @@ -71,8 +71,8 @@ extern const struct memp_desc* const memp_pools[MEMP_MAX]; #include "lwip/mem.h" #define memp_init() -#define memp_malloc(type) mem_malloc(memp_pools[type]->size) -#define memp_free(type, mem) mem_free(mem) +#define memp_malloc(type) mem_malloc(memp_pools[type]->size); ESP_CNT_MEM_MALLOC_INC(type) +#define memp_free(type, mem) mem_free(mem); ESP_CNT_MEM_FREE_INC(type) #define LWIP_MEMPOOL_DECLARE(name,num,size,desc) \ const struct memp_desc memp_ ## name = { \ diff --git a/components/lwip/include/lwip/lwip/priv/memp_priv.h b/components/lwip/include/lwip/lwip/priv/memp_priv.h index 34edb9d971..7bfa94d789 100755 --- a/components/lwip/include/lwip/lwip/priv/memp_priv.h +++ b/components/lwip/include/lwip/lwip/priv/memp_priv.h @@ -140,6 +140,16 @@ struct memp_desc { #endif /* MEMP_MEM_MALLOC */ }; +#if (ESP_CNT_DEBUG == 1) +extern uint32_t g_lwip_mem_cnt[MEMP_MAX][2]; +#define ESP_CNT_MEM_MALLOC_INC(type) g_lwip_mem_cnt[type][0]++ +#define ESP_CNT_MEM_FREE_INC(type) g_lwip_mem_cnt[type][1]++ +#else +#define ESP_CNT_MEM_MALLOC_INC(type) +#define ESP_CNT_MEM_FREE_INC(type) +#endif + + #ifdef LWIP_DEBUG #define DECLARE_LWIP_MEMPOOL_DESC(desc) (desc), #else diff --git a/components/lwip/include/lwip/port/lwipopts.h b/components/lwip/include/lwip/port/lwipopts.h index 26bdc3a4e9..c32aca7273 100755 --- a/components/lwip/include/lwip/port/lwipopts.h +++ b/components/lwip/include/lwip/port/lwipopts.h @@ -526,6 +526,7 @@ extern unsigned long os_random(void); #define ESP_IP4_ATON 1 #define ESP_LIGHT_SLEEP 1 #define ESP_L2_TO_L3_COPY CONFIG_L2_TO_L3_COPY +#define ESP_CNT_DEBUG 0 #define TCP_WND_DEFAULT (4*TCP_MSS) #define TCP_SND_BUF_DEFAULT (2*TCP_MSS) From b08113d2b4494120c1c90c32636cf902e015c6b7 Mon Sep 17 00:00:00 2001 From: Liu Zhi Fu Date: Sat, 19 Nov 2016 15:14:37 +0800 Subject: [PATCH 2/3] lwip: fix CI build issue --- components/lwip/core/tcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/lwip/core/tcp.c b/components/lwip/core/tcp.c index 87452f8230..fada62283c 100755 --- a/components/lwip/core/tcp.c +++ b/components/lwip/core/tcp.c @@ -1425,7 +1425,7 @@ void tcp_pcb_num_cal(tcp_pcb_num_t *tcp_pcb_num) } } - for (pcb = tcp_listen_pcbs.listen_pcbs; pcb != NULL; pcb = pcb->next){ + for (listen = tcp_listen_pcbs.listen_pcbs; listen != NULL; listen = listen->next){ tcp_pcb_num->total ++; tcp_pcb_num->listen ++; } From 9cc9710a2725cfae9a7cf0afe36c687033e5de38 Mon Sep 17 00:00:00 2001 From: Liu Zhi Fu Date: Mon, 21 Nov 2016 15:02:36 +0800 Subject: [PATCH 3/3] lwip: update according to review comments 1. Define memp_malloc/memp_free as static inline function 2. When TCP pcb is out of memory, try to kill PCB in FIN_WAIT_1 when necessary --- components/lwip/core/tcp.c | 11 ++++++++--- components/lwip/include/lwip/lwip/memp.h | 21 +++++++++++++++++++-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/components/lwip/core/tcp.c b/components/lwip/core/tcp.c index fada62283c..5be6864356 100755 --- a/components/lwip/core/tcp.c +++ b/components/lwip/core/tcp.c @@ -1394,6 +1394,7 @@ typedef struct { u8_t closing; u8_t fin_wait2; u8_t last_ack; + u8_t fin_wait1; u8_t listen; u8_t bound; u8_t total; @@ -1422,6 +1423,8 @@ void tcp_pcb_num_cal(tcp_pcb_num_t *tcp_pcb_num) tcp_pcb_num->last_ack ++; } else if (pcb->state == CLOSING) { tcp_pcb_num->closing ++; + } else if (pcb->state == FIN_WAIT_1){ + tcp_pcb_num->fin_wait1 ++; } } @@ -1463,7 +1466,9 @@ tcp_alloc(u8_t prio) } else if (tcp_pcb_num.closing > 0){ tcp_kill_state(CLOSING); } else if (tcp_pcb_num.fin_wait2 > 0){ - tcp_kill_state(FIN_WAIT_2);//TODO check whether we have issue here????? + tcp_kill_state(FIN_WAIT_2); + } else if (tcp_pcb_num.fin_wait1 > 0){ + tcp_kill_state(FIN_WAIT_1); } else { tcp_kill_prio(prio); } @@ -1471,9 +1476,9 @@ tcp_alloc(u8_t prio) tcp_pcb_num_cal(&tcp_pcb_num); if (tcp_pcb_num.total >= MEMP_NUM_TCP_PCB){ - LWIP_DEBUGF(TCP_DEBUG, ("tcp_alloc: no available tcp pcb %d %d %d %d %d %d %d\n", + LWIP_DEBUGF(TCP_DEBUG, ("tcp_alloc: no available tcp pcb %d %d %d %d %d %d %d %d\n", tcp_pcb_num.total, tcp_pcb_num.time_wait, tcp_pcb_num.last_ack, tcp_pcb_num.closing, - tcp_pcb_num.fin_wait2, tcp_pcb_num.listen, tcp_pcb_num.bound)); + tcp_pcb_num.fin_wait2, tcp_pcb_num.fin_wait1, tcp_pcb_num.listen, tcp_pcb_num.bound)); return NULL; } diff --git a/components/lwip/include/lwip/lwip/memp.h b/components/lwip/include/lwip/lwip/memp.h index bc9b021e87..fc45d54ad4 100755 --- a/components/lwip/include/lwip/lwip/memp.h +++ b/components/lwip/include/lwip/lwip/memp.h @@ -71,8 +71,25 @@ extern const struct memp_desc* const memp_pools[MEMP_MAX]; #include "lwip/mem.h" #define memp_init() -#define memp_malloc(type) mem_malloc(memp_pools[type]->size); ESP_CNT_MEM_MALLOC_INC(type) -#define memp_free(type, mem) mem_free(mem); ESP_CNT_MEM_FREE_INC(type) +#if ESP_CNT_DEBUG +static inline void* memp_malloc(int type) +{ + ESP_CNT_MEM_MALLOC_INC(type); + return mem_malloc(memp_pools[type]->size); +} + +static inline void memp_free(int type, void *mem) +{ + ESP_CNT_MEM_FREE_INC(type); + mem_free(mem); +} + +//#define memp_malloc(type) mem_malloc(memp_pools[type]->size); ESP_CNT_MEM_MALLOC_INC(type) +//#define memp_free(type, mem) mem_free(mem); ESP_CNT_MEM_FREE_INC(type) +#else +#define memp_malloc(type) mem_malloc(memp_pools[type]->size) +#define memp_free(type, mem) mem_free(mem) +#endif #define LWIP_MEMPOOL_DECLARE(name,num,size,desc) \ const struct memp_desc memp_ ## name = { \