From 1c7253726fcbab09917f143f0b703efbd2df55c3 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 3 Jun 2021 09:20:45 +0200 Subject: [PATCH] udhcpc[6]: when renewing, send 1 packet (not 3), on failure go back to BOUND This restores old behavior where we slept for 1/2 of lease, then tried renewing, thel slept for 1/4 and tried again, etc. But now we will NOT be listening to all packets for 1/2 of lease time, processing (rejecting) everyone else's DHCP traffic. We'll go back to bound state, where we have no listening socket at all. function old new delta udhcpc6_main 2600 2655 +55 udhcpc_main 2608 2625 +17 .rodata 103250 103249 -1 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 2/1 up/down: 72/-1) Total: 71 bytes Signed-off-by: Denys Vlasenko --- networking/udhcp/d6_dhcpc.c | 43 +++++++++++++++++------------ networking/udhcp/dhcpc.c | 55 +++++++++++++++++++------------------ 2 files changed, 54 insertions(+), 44 deletions(-) diff --git a/networking/udhcp/d6_dhcpc.c b/networking/udhcp/d6_dhcpc.c index 555446602..276ceca3c 100644 --- a/networking/udhcp/d6_dhcpc.c +++ b/networking/udhcp/d6_dhcpc.c @@ -1420,7 +1420,7 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) retval = 1; goto ret; } - /* wait before trying again */ + /* Wait before trying again */ timeout = tryagain_timeout; packet_num = 0; continue; @@ -1442,14 +1442,14 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) /* 1/2 lease passed, enter renewing state */ client_data.state = RENEWING; client_data.first_secs = 0; /* make secs field count from 0 */ - change_listen_mode(LISTEN_KERNEL); + got_SIGUSR1: log1s("entering renew state"); + change_listen_mode(LISTEN_KERNEL); /* fall right through */ - case RENEW_REQUESTED: /* manual (SIGUSR1) renew */ - case_RENEW_REQUESTED: + case RENEW_REQUESTED: /* in manual (SIGUSR1) renew */ case RENEWING: - if (packet_num < 3) { - /* send an unicast renew request */ + if (packet_num == 0) { + /* Send an unicast renew request */ /* Sometimes observed to fail (EADDRNOTAVAIL) to bind * a new UDP socket for sending inside send_renew. * I hazard to guess existing listening socket @@ -1463,13 +1463,18 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) else send_d6_renew(xid, &srv6_buf, requested_ipv6); timeout = discover_timeout; - /* ^^^ used to be = lease_remaining / 2 - WAY too long */ packet_num++; continue; + } /* else: we had sent one packet, but got no reply */ + log1s("no response to renew"); + if (lease_remaining > 30) { + /* Some lease time remains, try to renew later */ + change_listen_mode(LISTEN_NONE); + goto BOUND_for_half_lease; } - /* Timed out, enter rebinding state */ - log1s("entering rebinding state"); + /* Enter rebinding state */ client_data.state = REBINDING; + log1s("entering rebinding state"); /* Switch to bcast receive */ change_listen_mode(LISTEN_RAW); packet_num = 0; @@ -1509,8 +1514,14 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) /* Is it a signal? */ switch (udhcp_sp_read()) { case SIGUSR1: + if (client_data.state <= REQUESTING) + /* Initial negotiations in progress, do not disturb */ + break; + + if (lease_remaining > 30) /* if renew fails, do not go back to BOUND */ + lease_remaining = 30; client_data.first_secs = 0; /* make secs field count from 0 */ - bb_simple_info_msg("performing DHCP renew"); + packet_num = 0; switch (client_data.state) { /* Try to renew/rebind */ @@ -1519,21 +1530,19 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) case REBINDING: change_listen_mode(LISTEN_KERNEL); client_data.state = RENEW_REQUESTED; - goto case_RENEW_REQUESTED; + goto got_SIGUSR1; - /* Start things over */ - case RENEW_REQUESTED: /* two or more SIGUSR1 received */ + /* Two SIGUSR1 received, start things over */ + case RENEW_REQUESTED: change_listen_mode(LISTEN_NONE); d6_run_script_no_option("deconfig"); + /* Wake from SIGUSR2-induced deconfigured state */ default: - /* case REQUESTING: */ /* case RELEASED: */ - /* case INIT_SELECTING: */ change_listen_mode(LISTEN_NONE); } client_data.state = INIT_SELECTING; - packet_num = 0; /* Kill any timeouts, user wants this to hurry along */ timeout = 0; continue; @@ -1830,7 +1839,7 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) } #endif -// BOUND_for_half_lease: + BOUND_for_half_lease: timeout = (unsigned)lease_remaining / 2; client_data.state = BOUND; packet_num = 0; diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c index f441976dd..e0bddcdc9 100644 --- a/networking/udhcp/dhcpc.c +++ b/networking/udhcp/dhcpc.c @@ -1480,7 +1480,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) retval = 1; goto ret; } - /* wait before trying again */ + /* Wait before trying again */ timeout = tryagain_timeout; packet_num = 0; continue; @@ -1502,14 +1502,14 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) /* 1/2 lease passed, enter renewing state */ client_data.state = RENEWING; client_data.first_secs = 0; /* make secs field count from 0 */ - change_listen_mode(LISTEN_KERNEL); + got_SIGUSR1: log1s("entering renew state"); + change_listen_mode(LISTEN_KERNEL); /* fall right through */ - case RENEW_REQUESTED: /* manual (SIGUSR1) renew */ - case_RENEW_REQUESTED: + case RENEW_REQUESTED: /* in manual (SIGUSR1) renew */ case RENEWING: - if (packet_num < 3) { - /* send an unicast renew request */ + if (packet_num == 0) { + /* Send an unicast renew request */ /* Sometimes observed to fail (EADDRNOTAVAIL) to bind * a new UDP socket for sending inside send_renew. * I hazard to guess existing listening socket @@ -1520,7 +1520,6 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) */ if (send_renew(xid, server_addr, requested_ip) >= 0) { timeout = discover_timeout; - /* ^^^ used to be = lease_remaining / 2 - WAY too long */ packet_num++; continue; } @@ -1529,15 +1528,16 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) * which gave us bogus server ID 1.1.1.1 * which wasn't reachable (and probably did not exist). */ + } /* else: we had sent one packet, but got no reply */ + log1s("no response to renew"); + if (lease_remaining > 30) { + /* Some lease time remains, try to renew later */ + change_listen_mode(LISTEN_NONE); + goto BOUND_for_half_lease; } -//TODO: if 3 renew's failed (no reply) but remaining lease is large enough, -//it might make sense to go back to BOUND and try later? a-la -// if (lease_remaining > 30) change_listen_mode(LISTEN_NONE) + goto BOUND_for_half_lease; -//If we do that, "packet_num < 3" test below might be superfluous -//(lease_remaining will run out anyway) - /* Timed out or error, enter rebinding state */ - log1s("entering rebinding state"); + /* Enter rebinding state */ client_data.state = REBINDING; + log1s("entering rebinding state"); /* Switch to bcast receive */ change_listen_mode(LISTEN_RAW); packet_num = 0; @@ -1575,31 +1575,34 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) /* Is it a signal? */ switch (udhcp_sp_read()) { case SIGUSR1: + if (client_data.state <= REQUESTING) + /* Initial negotiations in progress, do not disturb */ + break; + + if (lease_remaining > 30) /* if renew fails, do not go back to BOUND */ + lease_remaining = 30; client_data.first_secs = 0; /* make secs field count from 0 */ - bb_simple_info_msg("performing DHCP renew"); + packet_num = 0; switch (client_data.state) { /* Try to renew/rebind */ case BOUND: case RENEWING: case REBINDING: - change_listen_mode(LISTEN_KERNEL); client_data.state = RENEW_REQUESTED; - goto case_RENEW_REQUESTED; + goto got_SIGUSR1; - /* Start things over */ - case RENEW_REQUESTED: /* two or more SIGUSR1 received */ + /* Two SIGUSR1 received, start things over */ + case RENEW_REQUESTED: change_listen_mode(LISTEN_NONE); udhcp_run_script(NULL, "deconfig"); + /* Wake from SIGUSR2-induced deconfigured state */ default: - /* case REQUESTING: */ /* case RELEASED: */ - /* case INIT_SELECTING: */ change_listen_mode(LISTEN_NONE); } client_data.state = INIT_SELECTING; - packet_num = 0; /* Kill any timeouts, user wants this to hurry along */ timeout = 0; continue; @@ -1723,10 +1726,9 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) strcpy(server_str, inet_ntoa(temp_addr)); temp_addr.s_addr = packet.yiaddr; + lease_remaining = 60 * 60; temp = udhcp_get_option32(&packet, DHCP_LEASE_TIME); - if (!temp) { - lease_remaining = 60 * 60; - } else { + if (temp) { uint32_t lease; /* it IS unaligned sometimes, don't "optimize" */ move_from_unaligned32(lease, temp); @@ -1798,8 +1800,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) opt = ((opt & ~OPT_b) | OPT_f); } #endif - -// BOUND_for_half_lease: + BOUND_for_half_lease: timeout = (unsigned)lease_remaining / 2; client_data.state = BOUND; /* make future renew packets use different xid */