From 8290c3b8f2adaf82aa45ec992b87f16205f2689b Mon Sep 17 00:00:00 2001 From: Abhik Roy Date: Tue, 11 Apr 2023 20:37:54 +1000 Subject: [PATCH] lwip/dhcp: Fixed ondemand fine timers leakage bug. Fixed ondemand fine timers bug, that allowed only one dhcp client to run at a time without issue. --- src/core/ipv4/dhcp.c | 76 ++++++++++++++++++++++---------------- src/include/lwip/dhcp.h | 8 +++- test/unit/dhcp/test_dhcp.c | 6 ++- 3 files changed, 55 insertions(+), 35 deletions(-) diff --git a/src/core/ipv4/dhcp.c b/src/core/ipv4/dhcp.c index 39a2aca6..60bd0ef2 100644 --- a/src/core/ipv4/dhcp.c +++ b/src/core/ipv4/dhcp.c @@ -85,16 +85,15 @@ #if ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND #include #include "lwip/timeouts.h" -static bool is_tmr_start = false; -#define ESP_LWIP_DHCP_FINE_TIMER_START_ONCE() if (!is_tmr_start) { \ - sys_timeout(DHCP_FINE_TIMER_MSECS, dhcp_fine_timeout_cb, NULL); \ - is_tmr_start = true; } -#define ESP_LWIP_DHCP_FINE_CLOSE() if (is_tmr_start) { \ - sys_untimeout(dhcp_fine_timeout_cb, NULL); \ - is_tmr_start = false; } +#define ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(netif, dhcp) if(!dhcp->fine_timer_enabled) { \ + sys_timeout(DHCP_FINE_TIMER_MSECS, dhcp_fine_timeout_cb, (void *)netif); \ + dhcp->fine_timer_enabled = true;} +#define ESP_LWIP_DHCP_FINE_CLOSE(netif, dhcp) if(dhcp->fine_timer_enabled) { \ + sys_untimeout(dhcp_fine_timeout_cb, (void *)netif); \ + dhcp->fine_timer_enabled = false;} #else -#define ESP_LWIP_DHCP_FINE_TIMER_START_ONCE() -#define ESP_LWIP_DHCP_FINE_CLOSE() +#define ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(netif, dhcp) +#define ESP_LWIP_DHCP_FINE_CLOSE(netif, dhcp) #endif /* ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND */ #ifndef DHCP_DEFINE_CUSTOM_TIMEOUTS @@ -323,8 +322,7 @@ dhcp_dec_pcb_refcount(void) #if ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND void dhcp_fine_timeout_cb(void *arg) { - LWIP_UNUSED_ARG(arg); - dhcp_fine_tmr(); + dhcp_fine_tmr((struct netif *)arg); } #endif @@ -397,7 +395,7 @@ dhcp_check(struct netif *netif) msecs = 500; dhcp->request_timeout = (u16_t)((msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS); LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_check(): set request timeout %"U16_F" msecs\n", msecs)); - ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(); + ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(netif, dhcp); } #endif /* DHCP_DOES_ARP_CHECK */ @@ -518,7 +516,7 @@ dhcp_select(struct netif *netif) msecs = DHCP_REQUEST_TIMEOUT_SEQUENCE(DHCP_STATE_REQUESTING, dhcp->tries); dhcp->request_timeout = (u16_t)((msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS); LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_STATE, ("dhcp_select(): set request timeout %"U16_F" msecs\n", msecs)); - ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(); + ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(netif, dhcp); return result; } @@ -584,15 +582,27 @@ dhcp_coarse_tmr(void) * This timer checks whether an outstanding DHCP request is timed out. */ void +#if ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND +dhcp_fine_tmr(struct netif *netif) +#else dhcp_fine_tmr(void) +#endif { - struct netif *netif; + struct dhcp *dhcp; #if ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND bool tmr_restart = false; -#endif /* ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND */ + + if (netif == NULL) { + return; + } +#else + struct netif *netif; + /* loop through netif's */ NETIF_FOREACH(netif) { - struct dhcp *dhcp = netif_dhcp_data(netif); +#endif /* ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND */ + + dhcp = netif_dhcp_data(netif); /* only act on DHCP configured interfaces */ if (dhcp != NULL) { /* timer is active (non zero), and is about to trigger now */ @@ -612,15 +622,19 @@ dhcp_fine_tmr(void) #endif } #if ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND - if (tmr_restart) { - sys_timeout(DHCP_FINE_TIMER_MSECS, dhcp_fine_timeout_cb, NULL); - } else { - sys_untimeout(dhcp_fine_timeout_cb, NULL); - is_tmr_start = false; - } + if (tmr_restart) { + if (dhcp->fine_timer_enabled == true) { + sys_timeout(DHCP_FINE_TIMER_MSECS, dhcp_fine_timeout_cb, (void *)netif); + } + } else { + sys_untimeout(dhcp_fine_timeout_cb, (void *)netif); + dhcp->fine_timer_enabled = false; + } #endif } +#if !ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND } +#endif } /** @@ -1002,8 +1016,7 @@ dhcp_start(struct netif *netif) return ERR_OK; } #endif - - ESP_LWIP_DHCP_FINE_CLOSE(); + ESP_LWIP_DHCP_FINE_CLOSE(netif, dhcp); /* (re)start the DHCP negotiation */ result = dhcp_discover(netif); if (result != ERR_OK) { @@ -1190,7 +1203,7 @@ dhcp_decline(struct netif *netif) msecs = 10 * 1000; dhcp->request_timeout = (u16_t)((msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS); LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE, ("dhcp_decline(): set request timeout %"U16_F" msecs\n", msecs)); - ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(); + ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(netif, dhcp); return result; } #endif /* DHCP_DOES_ARP_CHECK */ @@ -1267,7 +1280,7 @@ dhcp_discover(struct netif *netif) msecs = DHCP_REQUEST_TIMEOUT_SEQUENCE(DHCP_STATE_SELECTING, dhcp->tries); dhcp->request_timeout = (u16_t)((msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS); LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_discover(): set request timeout %"U16_F" msecs\n", msecs)); - ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(); + ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(netif, dhcp); return result; } @@ -1406,8 +1419,8 @@ dhcp_bind(struct netif *netif) /* netif is now bound to DHCP leased address - set this before assigning the address to ensure the callback can use dhcp_supplied_address() */ dhcp_set_state(dhcp, DHCP_STATE_BOUND); - ESP_LWIP_DHCP_FINE_CLOSE(); - LWIP_DEBUGF(ESP_DHCP_DEBUG | LWIP_DBG_STATE, ("dhcp_bind(): dhcp state is BOUND\n")); + ESP_LWIP_DHCP_FINE_CLOSE(netif, dhcp); + LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_STATE, ("dhcp_bind(): dhcp state is BOUND\n")); netif_set_addr(netif, &dhcp->offered_ip_addr, &sn_mask, &gw_addr); /* interface is used by routing now that an address is set */ @@ -1488,7 +1501,7 @@ dhcp_renew(struct netif *netif) msecs = (u16_t)(dhcp->tries < 10 ? dhcp->tries * 2000 : 20 * 1000); dhcp->request_timeout = (u16_t)((msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS); LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_renew(): set request timeout %"U16_F" msecs\n", msecs)); - ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(); + ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(netif, dhcp); return result; } @@ -1551,7 +1564,7 @@ dhcp_rebind(struct netif *netif) msecs = (u16_t)(dhcp->tries < 10 ? dhcp->tries * 1000 : 10 * 1000); dhcp->request_timeout = (u16_t)((msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS); LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_rebind(): set request timeout %"U16_F" msecs\n", msecs)); - ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(); + ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(netif, dhcp); return result; } @@ -1617,7 +1630,7 @@ dhcp_reboot(struct netif *netif) msecs = (u16_t)(dhcp->tries < 10 ? dhcp->tries * 1000 : 10 * 1000); dhcp->request_timeout = (u16_t)((msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS); LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_reboot(): set request timeout %"U16_F" msecs\n", msecs)); - ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(); + ESP_LWIP_DHCP_FINE_TIMER_START_ONCE(netif, dhcp); return result; } @@ -1705,6 +1718,7 @@ dhcp_release_and_stop(struct netif *netif) dhcp->autoip_coop_state = DHCP_AUTOIP_COOP_STATE_OFF; } #endif /* LWIP_DHCP_AUTOIP_COOP */ + ESP_LWIP_DHCP_FINE_CLOSE(netif, dhcp); dhcp_set_state(dhcp, DHCP_STATE_OFF); LWIP_DEBUGF(ESP_DHCP_DEBUG | LWIP_DBG_STATE, ("dhcp_release_and_stop(): dhcp state is OFF\n")); diff --git a/src/include/lwip/dhcp.h b/src/include/lwip/dhcp.h index bea1354c..1b842968 100644 --- a/src/include/lwip/dhcp.h +++ b/src/include/lwip/dhcp.h @@ -80,7 +80,9 @@ struct dhcp u8_t autoip_coop_state; #endif u8_t subnet_mask_given; - +#if ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND + u8_t fine_timer_enabled; +#endif u16_t request_timeout; /* #ticks with period DHCP_FINE_TIMER_SECS for request timeout */ #if ESP_DHCP u32_t t1_timeout; /* #ticks with period DHCP_COARSE_TIMER_SECS for renewal time */ @@ -148,8 +150,10 @@ u8_t dhcp_supplied_address(const struct netif *netif); /* to be called every minute */ void dhcp_coarse_tmr(void); /* to be called every half second */ +#if !ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND void dhcp_fine_tmr(void); -#if ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND +#else +void dhcp_fine_tmr(struct netif *netif); void dhcp_fine_timeout_cb(void *arg); #endif diff --git a/test/unit/dhcp/test_dhcp.c b/test/unit/dhcp/test_dhcp.c index 0e5c7e12..fa95bc8d 100644 --- a/test/unit/dhcp/test_dhcp.c +++ b/test/unit/dhcp/test_dhcp.c @@ -135,9 +135,11 @@ static void tick_lwip(void) tick++; if (tick % 5 == 0) { #if ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND - sys_untimeout(dhcp_fine_timeout_cb, NULL); -#endif + sys_untimeout(dhcp_fine_timeout_cb, (void *)&net_test); + dhcp_fine_tmr(&net_test); +#else dhcp_fine_tmr(); +#endif } #if ESP_DHCP if (tick % 10 == 0) {