Addressed review comments: error propagation / backpressure

danielinux · danielinux · commit e99e198987e0 · 2026-04-15T17:41:01.000+02:00
diff --git a/src/test/test_wolfguard_loopback.c b/src/test/test_wolfguard_loopback.c
@@ -213,7 +213,7 @@ static void setup_loopback_stacks(uint64_t *now)
     /* ---- Stack A ---- */
     wolfIP_init(&stack_a);
 
-    /* Physical interface (non_ethernet, index 0) */
+    /* Physical interface (non_ethernet, index TEST_PHYS_IF). */
     ll = wolfIP_getdev_ex(&stack_a, TEST_PHYS_IF);
     ll->non_ethernet = 1;
     ll->poll = phys_a_poll;
@@ -236,7 +236,7 @@ static void setup_loopback_stacks(uint64_t *now)
     /* Stack B */
     wolfIP_init(&stack_b);
 
-    /* Physical interface (non_ethernet, index 0) */
+    /* Physical interface (non_ethernet, index TEST_PHYS_IF). */
     ll = wolfIP_getdev_ex(&stack_b, TEST_PHYS_IF);
     ll->non_ethernet = 1;
     ll->poll = phys_b_poll;
diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c
@@ -776,6 +776,9 @@ Suite *wolf_suite(void)
     tcase_add_test(tc_proto, test_regression_loopback_pure_ack_drain_allows_next_send_cycle);
     tcase_add_test(tc_proto, test_regression_loopback_pure_ack_immediate_propagates_eagain_when_queue_full);
     tcase_add_test(tc_proto, test_regression_loopback_udp_tx_backpressure_retries_after_queue_drain);
+    tcase_add_test(tc_proto, test_regression_ll_send_frame_returns_wolfip_error_codes);
+    tcase_add_test(tc_proto, test_regression_loopback_ack_retry_pending_requeued_on_poll);
+    tcase_add_test(tc_proto, test_regression_loopback_queue_full_pure_ack_backpressure_retry);
     tcase_add_test(tc_proto, test_regression_tcp_tx_desc_payload_len_keeps_descriptor_layout_sanity);
     tcase_add_test(tc_proto, test_regression_fast_recovery_cwnd_ssthresh_rfc5681);
     tcase_add_test(tc_proto, test_regression_paws_rejects_stale_timestamp);
diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c
@@ -5076,6 +5076,133 @@ START_TEST(test_regression_loopback_udp_tx_backpressure_retries_after_queue_drai
 }
 END_TEST
 
+START_TEST(test_regression_ll_send_frame_returns_wolfip_error_codes)
+{
+    struct wolfIP s;
+    struct wolfIP_ll_dev *ll;
+    uint8_t frame[ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN] = {0};
+
+    wolfIP_init(&s);
+    mock_link_init(&s);
+    ll = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF);
+    ck_assert_ptr_nonnull(ll);
+
+    ck_assert_int_eq(
+        wolfIP_ll_send_frame(NULL, TEST_PRIMARY_IF, frame, sizeof(frame)),
+        -WOLFIP_EINVAL);
+    ck_assert_int_eq(
+        wolfIP_ll_send_frame(&s, WOLFIP_MAX_INTERFACES, frame, sizeof(frame)),
+        -WOLFIP_EINVAL);
+
+    ll->send = NULL;
+    ck_assert_int_eq(
+        wolfIP_ll_send_frame(&s, TEST_PRIMARY_IF, frame, sizeof(frame)),
+        -WOLFIP_EINVAL);
+
+    ll->send = mock_send;
+    ck_assert_int_eq(
+        wolfIP_ll_send_frame(&s, TEST_PRIMARY_IF, frame, LINK_MTU + 1U),
+        -WOLFIP_EINVAL);
+
+    ll->non_ethernet = 1;
+    ck_assert_int_eq(
+        wolfIP_ll_send_frame(&s, TEST_PRIMARY_IF, frame, ETH_HEADER_LEN),
+        -WOLFIP_EINVAL);
+}
+END_TEST
+
+START_TEST(test_regression_loopback_ack_retry_pending_requeued_on_poll)
+{
+    struct wolfIP s;
+    struct tsocket *ts;
+    struct wolfIP_ll_dev *loop;
+    uint8_t frame[16] = {0};
+    uint8_t rx[IP_MTU_MAX];
+    unsigned int i;
+    uint64_t now = 300;
+
+    wolfIP_init(&s);
+    loop = wolfIP_getdev_ex(&s, TEST_LOOPBACK_IF);
+    ck_assert_ptr_nonnull(loop);
+
+    for (i = 0; i < WOLFIP_LOOPBACK_QUEUE_DEPTH; i++) {
+        ck_assert_int_eq(wolfIP_loopback_send(loop, frame, sizeof(frame)),
+            (int)sizeof(frame));
+    }
+
+    ts = &s.tcpsockets[0];
+    memset(ts, 0, sizeof(*ts));
+    ts->proto = WI_IPPROTO_TCP;
+    ts->S = &s;
+    ts->if_idx = TEST_LOOPBACK_IF;
+    ts->sock.tcp.state = TCP_ESTABLISHED;
+    ts->sock.tcp.ack = 100;
+    ts->sock.tcp.seq = 1000;
+    ts->src_port = 1234;
+    ts->dst_port = 4321;
+    ts->local_ip = 0x7F000001U;
+    ts->remote_ip = 0x7F000001U;
+
+    tcp_send_ack(ts);
+    ck_assert_uint_eq(ts->sock.tcp.ack_retry_pending, 1U);
+
+    ck_assert_int_gt(loop->poll(loop, rx, sizeof(rx)), 0);
+
+    loop->poll = mock_poll;
+    (void)wolfIP_poll(&s, now);
+
+    ck_assert_uint_eq(ts->sock.tcp.ack_retry_pending, 0U);
+    ck_assert_uint_eq(s.loopback_count, WOLFIP_LOOPBACK_QUEUE_DEPTH);
+}
+END_TEST
+
+START_TEST(test_regression_loopback_queue_full_pure_ack_backpressure_retry)
+{
+    struct wolfIP s;
+    struct tsocket *ts;
+    struct wolfIP_ll_dev *loop;
+    uint8_t frame[16] = {0};
+    uint8_t rx[IP_MTU_MAX];
+    unsigned int i;
+    uint64_t now = 350;
+
+    wolfIP_init(&s);
+    loop = wolfIP_getdev_ex(&s, TEST_LOOPBACK_IF);
+    ck_assert_ptr_nonnull(loop);
+
+    for (i = 0; i < WOLFIP_LOOPBACK_QUEUE_DEPTH; i++) {
+        ck_assert_int_eq(wolfIP_loopback_send(loop, frame, sizeof(frame)),
+            (int)sizeof(frame));
+    }
+
+    ts = &s.tcpsockets[1];
+    memset(ts, 0, sizeof(*ts));
+    ts->proto = WI_IPPROTO_TCP;
+    ts->S = &s;
+    ts->if_idx = TEST_LOOPBACK_IF;
+    ts->sock.tcp.state = TCP_ESTABLISHED;
+    ts->sock.tcp.ack = 200;
+    ts->sock.tcp.seq = 300;
+    ts->src_port = 5000;
+    ts->dst_port = 5001;
+    ts->local_ip = 0x7F000001U;
+    ts->remote_ip = 0x7F000001U;
+
+    /* Queue full -> pure ACK send must backpressure and mark retry pending. */
+    tcp_send_ack(ts);
+    ck_assert_uint_eq(ts->sock.tcp.ack_retry_pending, 1U);
+    ck_assert_uint_eq(s.loopback_count, WOLFIP_LOOPBACK_QUEUE_DEPTH);
+
+    /* Free one slot, then poll should retry and enqueue the pending ACK. */
+    ck_assert_int_gt(loop->poll(loop, rx, sizeof(rx)), 0);
+    loop->poll = mock_poll;
+    (void)wolfIP_poll(&s, now);
+
+    ck_assert_uint_eq(ts->sock.tcp.ack_retry_pending, 0U);
+    ck_assert_uint_eq(s.loopback_count, WOLFIP_LOOPBACK_QUEUE_DEPTH);
+}
+END_TEST
+
 START_TEST(test_regression_tcp_tx_desc_payload_len_keeps_descriptor_layout_sanity)
 {
     struct wolfIP s;
diff --git a/src/wolfip.c b/src/wolfip.c
@@ -1084,6 +1084,7 @@ struct tcpsocket {
     uint8_t ctrl_rto_active;
     uint8_t fin_wait_2_timeout_active;
     uint8_t is_listener;
+    uint8_t ack_retry_pending;
     ip4 local_ip, remote_ip;
     uint32_t peer_rwnd;
     uint16_t peer_mss;
@@ -1483,13 +1484,13 @@ static inline int wolfIP_ll_send_frame(struct wolfIP *s, unsigned int if_idx,
     uint32_t frame_mtu;
 
     if (!ll || !ll->send)
-        return -1;
+        return -WOLFIP_EINVAL;
     frame_mtu = wolfIP_ll_frame_mtu(ll);
     if (len > frame_mtu)
-        return -1;
+        return -WOLFIP_EINVAL;
     if (ll->non_ethernet) {
         if (len <= ETH_HEADER_LEN)
-            return -1;
+            return -WOLFIP_EINVAL;
         return ll->send(ll, (uint8_t *)buf + ETH_HEADER_LEN, len - ETH_HEADER_LEN);
     }
     return ll->send(ll, buf, len);
@@ -2681,7 +2682,14 @@ static int tcp_send_empty(struct tsocket *t, uint8_t flags)
 
 static void tcp_send_ack(struct tsocket *t)
 {
-    (void)tcp_send_empty(t, TCP_FLAG_ACK);
+    int ret = tcp_send_empty(t, TCP_FLAG_ACK);
+
+    if (!t)
+        return;
+    if (ret == -WOLFIP_EAGAIN)
+        t->sock.tcp.ack_retry_pending = 1;
+    else if (ret >= 0)
+        t->sock.tcp.ack_retry_pending = 0;
 }
 
 static void tcp_send_reset_reply(struct wolfIP *s, unsigned int if_idx,
@@ -7600,6 +7608,13 @@ int wolfIP_poll(struct wolfIP *s, uint64_t now)
         struct pkt_desc *desc;
         struct wolfIP_tcp_seg *tcp;
         tcp_resync_inflight(s, ts, now);
+        if (ts->sock.tcp.ack_retry_pending) {
+            int ack_ret = tcp_send_empty(ts, TCP_FLAG_ACK);
+            if (ack_ret == -WOLFIP_EAGAIN)
+                ts->sock.tcp.ack_retry_pending = 1;
+            else if (ack_ret >= 0)
+                ts->sock.tcp.ack_retry_pending = 0;
+        }
         in_flight = ts->sock.tcp.bytes_in_flight;
         if (ts->sock.tcp.persist_active && (ts->sock.tcp.peer_rwnd > 0 ||
                 !tcp_has_pending_unsent_payload(ts)))
