Addressed copilot's review comments

Author: danielinux

src/test/unit/unit.c

@@ -313,6 +313,7 @@ Suite *wolf_suite(void)
     tcase_add_test(tc_utils, test_poll_icmp_send_on_arp_hit);
     tcase_add_test(tc_utils, test_poll_icmp_send_on_arp_miss_requests_arp_and_retains_queue);
     tcase_add_test(tc_utils, test_dhcp_timer_cb_paths);
+    tcase_add_test(tc_utils, test_dhcp_timer_cb_send_failure_does_not_consume_retry_budget);
     tcase_add_test(tc_utils, test_dhcp_client_init_and_bound);
     tcase_add_test(tc_utils, test_dhcp_send_request_renewing_sets_ciaddr_and_rebind_deadline);
     tcase_add_test(tc_utils, test_dhcp_send_request_rebinding_broadcasts_to_lease_expiry);

src/test/unit/unit_tests_dns_dhcp.c

@@ -952,7 +952,7 @@ START_TEST(test_dhcp_send_discover_send_failure_retries_next_tick)
 
     ck_assert_int_eq(dhcp_send_discover(&s), -WOLFIP_EAGAIN);
     ck_assert_ptr_eq(fifo_peek(&ts->sock.udp.txbuf), NULL);
-    ck_assert_int_eq(s.dhcp_state, DHCP_OFF);
+    ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT);
     ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer), s.last_tick + 1U);
 }
 END_TEST
@@ -3516,6 +3516,38 @@ START_TEST(test_dhcp_timer_cb_paths)
 }
 END_TEST
 
+START_TEST(test_dhcp_timer_cb_send_failure_does_not_consume_retry_budget)
+{
+    struct wolfIP s;
+    struct tsocket *ts;
+    uint8_t tiny[2];
+
+    wolfIP_init(&s);
+    mock_link_init(&s);
+    s.dhcp_xid = 1U;
+    s.last_tick = 1000U;
+
+    s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP);
+    ck_assert_int_gt(s.dhcp_udp_sd, 0);
+    ts = &s.udpsockets[SOCKET_UNMARK(s.dhcp_udp_sd)];
+    fifo_init(&ts->sock.udp.txbuf, tiny, sizeof(tiny));
+
+    s.dhcp_state = DHCP_DISCOVER_SENT;
+    s.dhcp_timeout_count = 0;
+    dhcp_timer_cb(&s);
+    ck_assert_int_eq(s.dhcp_timeout_count, 0);
+    ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT);
+    ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer), s.last_tick + 1U);
+
+    s.dhcp_state = DHCP_REQUEST_SENT;
+    s.dhcp_timeout_count = 0;
+    dhcp_timer_cb(&s);
+    ck_assert_int_eq(s.dhcp_timeout_count, 0);
+    ck_assert_int_eq(s.dhcp_state, DHCP_REQUEST_SENT);
+    ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer), s.last_tick + 1U);
+}
+END_TEST
+
 START_TEST(test_dhcp_client_init_and_bound)
 {
     struct wolfIP s;

src/wolfip.c

@@ -5221,22 +5221,25 @@ static void dhcp_schedule_lease_timer(struct wolfIP *s,
 static void dhcp_timer_cb(void *arg)
 {
     struct wolfIP *s = (struct wolfIP *)arg;
+    int ret;
     LOG("dhcp timeout\n");
     if (!s)
         return;
     s->dhcp_timer = NO_TIMER;
     switch(s->dhcp_state) {
         case DHCP_DISCOVER_SENT:
             if (s->dhcp_timeout_count < DHCP_DISCOVER_RETRIES) {
-                dhcp_send_discover(s);
-                s->dhcp_timeout_count++;
+                ret = dhcp_send_discover(s);
+                if (ret >= 0)
+                    s->dhcp_timeout_count++;
             } else
                 s->dhcp_state = DHCP_OFF;
             break;
         case DHCP_REQUEST_SENT:
             if (s->dhcp_timeout_count < DHCP_REQUEST_RETRIES) {
-                dhcp_send_request(s);
-                s->dhcp_timeout_count++;
+                ret = dhcp_send_request(s);
+                if (ret >= 0)
+                    s->dhcp_timeout_count++;
             } else
                 s->dhcp_state = DHCP_OFF;
             break;
@@ -5247,24 +5250,27 @@ static void dhcp_timer_cb(void *arg)
             }
             s->dhcp_state = DHCP_RENEWING;
             s->dhcp_timeout_count = 0;
-            dhcp_send_request(s);
-            s->dhcp_timeout_count++;
+            ret = dhcp_send_request(s);
+            if (ret >= 0)
+                s->dhcp_timeout_count++;
             break;
         case DHCP_RENEWING:
             if (s->dhcp_rebind_at != 0 && s->last_tick >= s->dhcp_rebind_at) {
                 s->dhcp_state = DHCP_REBINDING;
                 s->dhcp_timeout_count = 0;
             }
-            dhcp_send_request(s);
-            s->dhcp_timeout_count++;
+            ret = dhcp_send_request(s);
+            if (ret >= 0)
+                s->dhcp_timeout_count++;
             break;
         case DHCP_REBINDING:
             if (s->dhcp_lease_expires != 0 && s->last_tick >= s->dhcp_lease_expires) {
                 s->dhcp_state = DHCP_OFF;
                 break;
             }
-            dhcp_send_request(s);
-            s->dhcp_timeout_count++;
+            ret = dhcp_send_request(s);
+            if (ret >= 0)
+                s->dhcp_timeout_count++;
             break;
         default:
             break;
@@ -5738,8 +5744,10 @@ static int dhcp_send_discover(struct wolfIP *s)
     ret = wolfIP_sock_sendto(s, s->dhcp_udp_sd, &disc, DHCP_HEADER_LEN + opt_sz, 0,
             (struct wolfIP_sockaddr *)&sin, sizeof(struct wolfIP_sockaddr_in));
     if (ret < 0) {
-        /* Retry on the next tick after local backpressure instead of
+        /* Enter discover-sent before retrying so dhcp_timer_cb() continues
+         * DHCP startup after local backpressure. Retry on the next tick instead of
          * waiting a full discover timeout for a packet that never queued. */
+        s->dhcp_state = DHCP_DISCOVER_SENT;
         dhcp_schedule_timer_at(s, retry_at);
         return ret;
     }