Skip to content

Commit

Permalink
lwip/dhcp: Fixed ondemand fine timers bug, that allowed only one dhcp
Browse files Browse the repository at this point in the history
client to run at a time without issue.
  • Loading branch information
espressif-abhikroy committed Jan 20, 2023
1 parent 10197b2 commit 86df9f4
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 34 deletions.
72 changes: 41 additions & 31 deletions src/core/ipv4/dhcp.c
Expand Up @@ -85,16 +85,15 @@

#if ESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND
#include <stdbool.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 */

#ifdef LWIP_HOOK_FILENAME
Expand Down Expand Up @@ -314,8 +313,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

Expand Down Expand Up @@ -380,7 +378,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 */

Expand Down Expand Up @@ -481,7 +479,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;
}

Expand Down Expand Up @@ -527,18 +525,28 @@ dhcp_coarse_tmr(void)
* A DHCP server is expected to respond within a short period of time.
* 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 */
Expand All @@ -558,15 +566,17 @@ 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) {
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
}

/**
Expand Down Expand Up @@ -888,7 +898,7 @@ dhcp_start(struct netif *netif)
return result;
}
#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) {
Expand Down Expand Up @@ -1067,7 +1077,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 */
Expand Down Expand Up @@ -1134,7 +1144,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;
}

Expand Down Expand Up @@ -1214,7 +1224,7 @@ 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();
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 */
Expand Down Expand Up @@ -1274,7 +1284,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;
}

Expand Down Expand Up @@ -1329,7 +1339,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;
}

Expand Down Expand Up @@ -1387,7 +1397,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;
}

Expand Down
7 changes: 6 additions & 1 deletion src/include/lwip/dhcp.h
Expand Up @@ -82,6 +82,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 */
u32_t t1_timeout; /* #ticks with period DHCP_COARSE_TIMER_SECS for renewal time */
u32_t t2_timeout; /* #ticks with period DHCP_COARSE_TIMER_SECS for rebind time */
Expand Down Expand Up @@ -122,8 +125,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

Expand Down
6 changes: 4 additions & 2 deletions test/unit/dhcp/test_dhcp.c
Expand Up @@ -142,9 +142,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_LWIP
if (tick % 10 == 0) {
Expand Down

0 comments on commit 86df9f4

Please sign in to comment.