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 <vda.linux@googlemail.com>

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 */