diff --git a/AGENTS.md b/AGENTS.md index 8e77306..a8899de 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,7 @@ Please read `README.md` for general information about LibUDPard, and `CONTRIBUTING.md` for development-related notes. -Keep the code and comments very brief. Be sure every significant code block is preceded with a brief comment. +DO NOT COMMENT THE CODE unless comments add critical information that is impossible to infer from reading the code (design rationale, gotchas, etc), in which case extremely terse comments are allowed. If you need a build directory, create one in the project root named with a `build` prefix; you can also use existing build directories if you prefer so, diff --git a/libudpard/udpard.c b/libudpard/udpard.c index 75a1d22..b23bf44 100644 --- a/libudpard/udpard.c +++ b/libudpard/udpard.c @@ -481,7 +481,7 @@ static byte_t* header_serialize(byte_t* const buffer, const uint32_t frame_payload_offset, const uint32_t prefix_crc) { - assert(meta.priority < 8U); + UDPARD_ASSERT(meta.priority < 8U); byte_t* ptr = buffer; *ptr++ = (byte_t)(HEADER_VERSION | (meta.priority << 5U)); *ptr++ = 0; @@ -643,13 +643,21 @@ static void tx_transfer_retire(udpard_tx_t* const tx, tx_transfer_t* const tr) /// The heuristics are subject to review and improvement. static tx_transfer_t* tx_sacrifice(udpard_tx_t* const tx) { return LIST_TAIL(tx->agewise, tx_transfer_t, agewise); } +/// Zero if the count already reached the limit, which can happen if the limit was lowered at runtime. +static size_t tx_queue_vacancy(const udpard_tx_t* const tx) +{ + return (tx->enqueued_frames_count < tx->enqueued_frames_limit) + ? (tx->enqueued_frames_limit - tx->enqueued_frames_count) + : 0U; +} + /// True on success, false if not possible to reclaim enough space. static bool tx_ensure_queue_space(udpard_tx_t* const tx, const size_t total_frames_needed) { if (total_frames_needed > tx->enqueued_frames_limit) { return false; // not gonna happen } - while (total_frames_needed > (tx->enqueued_frames_limit - tx->enqueued_frames_count)) { + while (total_frames_needed > tx_queue_vacancy(tx)) { tx_transfer_t* const tr = tx_sacrifice(tx); if (tr == NULL) { break; // We may have no transfers anymore but the NIC TX driver could still be holding some frames. @@ -657,7 +665,7 @@ static bool tx_ensure_queue_space(udpard_tx_t* const tx, const size_t total_fram tx_transfer_retire(tx, tr); tx->errors_sacrifice++; } - return total_frames_needed <= (tx->enqueued_frames_limit - tx->enqueued_frames_count); + return total_frames_needed <= tx_queue_vacancy(tx); } static int32_t tx_cavl_compare_deadline(const void* const user, const udpard_tree_t* const node) @@ -792,9 +800,11 @@ static bool tx_push(udpard_tx_t* const tx, UDPARD_ASSERT(now <= deadline); UDPARD_ASSERT(tx != NULL); - const uint16_t iface_bitmap = valid_ep_bitmap(endpoints); + uint16_t iface_bitmap = valid_ep_bitmap(endpoints); UDPARD_ASSERT((iface_bitmap & UDPARD_IFACE_BITMAP_ALL) != 0); UDPARD_ASSERT((iface_bitmap & UDPARD_IFACE_BITMAP_ALL) == iface_bitmap); + iface_bitmap &= tx->iface_bitmap; + UDPARD_ASSERT(iface_bitmap != 0U); // Purge expired transfers before accepting a new one to make room in the queue. tx_purge_expired_transfers(tx, now); @@ -894,11 +904,12 @@ bool udpard_tx_new(udpard_tx_t* const self, const uint64_t local_uid, const uint64_t unicast_transfer_id_seed, const size_t enqueued_frames_limit, + const uint16_t iface_bitmap, const udpard_tx_mem_resources_t memory, const udpard_tx_vtable_t* const vtable) { - const bool ok = (NULL != self) && (local_uid != 0) && tx_validate_mem_resources(memory) && (vtable != NULL) && - (vtable->eject != NULL); + const bool ok = (NULL != self) && (local_uid != 0) && ((iface_bitmap & UDPARD_IFACE_BITMAP_ALL) == iface_bitmap) && + tx_validate_mem_resources(memory) && (vtable != NULL) && (vtable->eject != NULL); if (ok) { mem_zero(sizeof(*self), self); self->vtable = vtable; @@ -906,6 +917,7 @@ bool udpard_tx_new(udpard_tx_t* const self, self->unicast_transfer_id = unicast_transfer_id_seed + local_uid; // extra entropy self->enqueued_frames_limit = enqueued_frames_limit; self->enqueued_frames_count = 0; + self->iface_bitmap = iface_bitmap; self->memory = memory; self->index_deadline = NULL; self->agewise = (udpard_list_t){ NULL, NULL }; @@ -931,9 +943,11 @@ bool udpard_tx_push(udpard_tx_t* const self, const udpard_bytes_scattered_t payload, void* const user) { + // Only the head payload fragment is validated; inner fragments of the caller-owned chain are not checked. bool ok = (self != NULL) && (deadline >= now) && (now >= 0) && (self->local_uid != 0) && - ((iface_bitmap & UDPARD_IFACE_BITMAP_ALL) != 0) && (priority < UDPARD_PRIORITY_COUNT) && - udpard_is_valid_endpoint(endpoint) && ((payload.bytes.data != NULL) || (payload.bytes.size == 0U)); + ((iface_bitmap & UDPARD_IFACE_BITMAP_ALL & self->iface_bitmap) != 0) && + (priority < UDPARD_PRIORITY_COUNT) && udpard_is_valid_endpoint(endpoint) && + ((payload.bytes.data != NULL) || (payload.bytes.size == 0U)); if (ok) { const meta_t meta = { .priority = priority, @@ -960,8 +974,9 @@ bool udpard_tx_push_unicast(udpard_tx_t* const self, const udpard_bytes_scattered_t payload, void* const user) { + // Only the head payload fragment is validated; inner fragments of the caller-owned chain are not checked. bool ok = (self != NULL) && (deadline >= now) && (now >= 0) && (self->local_uid != 0) && - (valid_ep_bitmap(endpoints) != 0) && (priority < UDPARD_PRIORITY_COUNT) && + ((valid_ep_bitmap(endpoints) & self->iface_bitmap) != 0) && (priority < UDPARD_PRIORITY_COUNT) && ((payload.bytes.data != NULL) || (payload.bytes.size == 0U)); if (ok) { const meta_t meta = { @@ -1093,6 +1108,8 @@ void udpard_tx_free(udpard_tx_t* const self) while (self->agewise.tail != NULL) { tx_transfer_retire(self, LIST_TAIL(self->agewise, tx_transfer_t, agewise)); } + // Datagram references retained via udpard_tx_refcount_inc() must be released before discarding. + UDPARD_ASSERT(self->enqueued_frames_count == 0U); } } @@ -1567,7 +1584,6 @@ static void rx_session_eject(rx_session_t* const self, udpard_rx_t* const rx, rx self->history_current = (self->history_current + 1U) % RX_TRANSFER_HISTORY_COUNT; self->history[self->history_current] = slot->transfer_id; - // Construct the arguments and invoke the callback. const udpard_rx_transfer_t transfer = { .timestamp = slot->ts_min, .priority = slot->priority, @@ -1577,11 +1593,13 @@ static void rx_session_eject(rx_session_t* const self, udpard_rx_t* const rx, rx .payload_size_wire = slot->total_size, .payload = (udpard_fragment_t*)slot->fragments, }; - self->port->vtable->on_message(rx, self->port, transfer); - // Finally, destroy the slot to reclaim memory. + // Destroy the slot before the callback: a re-entrant udpard_rx_port_push() could otherwise evict and + // free this slot, whose fragments are now owned by the application. slot->fragments = NULL; // Transfer ownership to the application. rx_slot_destroy(slot_ref, self->port->memory.fragment, self->port->memory.slot); + + self->port->vtable->on_message(rx, self->port, transfer); } /// Finds an existing in-progress slot with the specified transfer-ID, or allocates a new one. Returns NULL on OOM. @@ -1771,10 +1789,12 @@ void udpard_rx_new(udpard_rx_t* const self) void udpard_rx_poll(udpard_rx_t* const self, const udpard_us_t now) { - // Retire at most one per poll to avoid burstiness. - rx_session_t* const ses = LIST_TAIL(self->list_session_by_animation, rx_session_t, list_by_animation); - if ((ses != NULL) && (now >= (ses->last_animated_ts + SESSION_LIFETIME))) { - rx_session_free(ses, &self->list_session_by_animation); + if (self != NULL) { + // Retire at most one per poll to avoid burstiness. + rx_session_t* const ses = LIST_TAIL(self->list_session_by_animation, rx_session_t, list_by_animation); + if ((ses != NULL) && (now >= (ses->last_animated_ts + SESSION_LIFETIME))) { + rx_session_free(ses, &self->list_session_by_animation); + } } } diff --git a/libudpard/udpard.h b/libudpard/udpard.h index 61e737f..bfef5f8 100644 --- a/libudpard/udpard.h +++ b/libudpard/udpard.h @@ -300,7 +300,6 @@ typedef struct udpard_tx_mem_resources_t } udpard_tx_mem_resources_t; /// Request to transmit a UDP datagram over the specified interface. -/// Which interface indexes are available is determined by the user when pushing the transfer. /// If Berkeley sockets or similar API is used, the application should use a dedicated socket per redundant interface. typedef struct udpard_tx_ejection_t { @@ -309,7 +308,7 @@ typedef struct udpard_tx_ejection_t /// Specifies when the frame should be considered expired and dropped if not yet transmitted by then; /// it is optional to use depending on the implementation of the NIC driver (most traditional drivers ignore it). - /// The library guarantees that now >= deadline at the time of ejection -- expired frames are purged beforehand. + /// The library guarantees that now <= deadline at the time of ejection -- expired frames are purged beforehand. udpard_us_t deadline; udpard_udpip_ep_t destination; @@ -356,6 +355,9 @@ struct udpard_tx_t /// able to avoid frame duplication and instead reuse each frame across all interfaces. size_t mtu[UDPARD_IFACE_COUNT_MAX]; + /// Transfers are enqueued only on this subset of UDPARD_IFACE_BITMAP_ALL (applied at push); zero = listen-only. + uint16_t iface_bitmap; + /// Optional user-managed mapping from the Cyphal priority level in [0,7] (highest priority at index 0) /// to the IP DSCP field value for use by the application when transmitting. By default, all entries are zero. uint_least8_t dscp_value_per_priority[UDPARD_PRIORITY_COUNT]; @@ -403,11 +405,14 @@ struct udpard_tx_t /// If the limit is reached, the library will apply heuristics to sacrifice some older transfers to make room /// for the new one. This behavior allows the library to make progress even when some interfaces are stalled. /// +/// iface_bitmap must be a subset of UDPARD_IFACE_BITMAP_ALL (zero = listen-only); see its field docs. +/// /// True on success, false if any of the arguments are invalid. bool udpard_tx_new(udpard_tx_t* const self, const uint64_t local_uid, const uint64_t unicast_transfer_id_seed, const size_t enqueued_frames_limit, + const uint16_t iface_bitmap, const udpard_tx_mem_resources_t memory, const udpard_tx_vtable_t* const vtable); @@ -425,7 +430,7 @@ bool udpard_tx_new(udpard_tx_t* const self, /// Excess most significant bits are ignored. /// Related thread on random transfer-ID init: https://forum.opencyphal.org/t/improve-the-transfer-id-timeout/2375 /// -/// The enqueued transfer will be emitted over all interfaces specified in the iface_bitmap. +/// The transfer is emitted over iface_bitmap masked by udpard_tx_t.iface_bitmap; an empty result returns false. /// /// The user context value is carried through to the callbacks. /// @@ -448,6 +453,7 @@ bool udpard_tx_push(udpard_tx_t* const self, /// This is a specialization of the general push function for unicast transfers. /// The transfer-ID counter is managed automatically. /// Endpoints may be empty (zero) for some ifaces, in which case no transmission over those ifaces will be attempted. +/// The iface set is also masked by udpard_tx_t.iface_bitmap. bool udpard_tx_push_unicast(udpard_tx_t* const self, const udpard_us_t now, const udpard_us_t deadline, @@ -466,7 +472,7 @@ void udpard_tx_poll(udpard_tx_t* const self, const udpard_us_t now, const uint16 /// Returns a bitmap of interfaces that have pending transmissions. This is useful for IO multiplexing loops. /// Zero indicates that there are no pending transmissions. -/// Which interfaces are usable is defined by the remote endpoints provided when pushing transfers. +/// Which interfaces can carry transfers is set at push time by the remote endpoints and udpard_tx_t.iface_bitmap. uint16_t udpard_tx_pending_ifaces(const udpard_tx_t* const self); /// When a datagram is ejected and the application opts to keep it, these functions must be used to manage the @@ -475,6 +481,7 @@ void udpard_tx_refcount_inc(const udpard_bytes_t tx_payload_view); void udpard_tx_refcount_dec(const udpard_bytes_t tx_payload_view); /// Drops all enqueued items; afterward, the instance is safe to discard. +/// Any references retained via udpard_tx_refcount_inc() must be released beforehand. void udpard_tx_free(udpard_tx_t* const self); // ===================================================================================================================== diff --git a/tests/src/test_e2e_api.cpp b/tests/src/test_e2e_api.cpp index 9e8b015..0200f2a 100644 --- a/tests/src/test_e2e_api.cpp +++ b/tests/src/test_e2e_api.cpp @@ -92,7 +92,7 @@ void test_subject_roundtrip() res = instrumented_allocator_make_resource(&tx_alloc_payload); } udpard_tx_t tx{}; - TEST_ASSERT_TRUE(udpard_tx_new(&tx, 0x1010101010101010ULL, 123U, 32U, tx_mem, &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, 0x1010101010101010ULL, 123U, 32U, UDPARD_IFACE_BITMAP_ALL, tx_mem, &tx_vtable)); tx.mtu[0] = 256U; tx.mtu[1] = 256U; tx.mtu[2] = 256U; diff --git a/tests/src/test_e2e_edge.cpp b/tests/src/test_e2e_edge.cpp index 721c349..233d9b9 100644 --- a/tests/src/test_e2e_edge.cpp +++ b/tests/src/test_e2e_edge.cpp @@ -117,8 +117,13 @@ void test_zero_payload_transfer() udpard_tx_t tx{}; std::vector frames; - TEST_ASSERT_TRUE(udpard_tx_new( - &tx, 0x1111222233334444ULL, 123U, 8U, make_tx_mem(tx_alloc_transfer, tx_alloc_payload), &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, + 0x1111222233334444ULL, + 123U, + 8U, + UDPARD_IFACE_BITMAP_ALL, + make_tx_mem(tx_alloc_transfer, tx_alloc_payload), + &tx_vtable)); tx.mtu[0] = 128U; tx.mtu[1] = 128U; tx.mtu[2] = 128U; @@ -182,8 +187,13 @@ void test_out_of_order_multiframe_reassembly() udpard_tx_t tx{}; std::vector frames; - TEST_ASSERT_TRUE(udpard_tx_new( - &tx, 0xAAAABBBBCCCCDDDDULL, 321U, 32U, make_tx_mem(tx_alloc_transfer, tx_alloc_payload), &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, + 0xAAAABBBBCCCCDDDDULL, + 321U, + 32U, + UDPARD_IFACE_BITMAP_ALL, + make_tx_mem(tx_alloc_transfer, tx_alloc_payload), + &tx_vtable)); tx.mtu[0] = 96U; tx.mtu[1] = 96U; tx.mtu[2] = 96U; @@ -259,8 +269,13 @@ void test_stateless_single_frame_acceptance() udpard_tx_t tx{}; std::vector frames; - TEST_ASSERT_TRUE(udpard_tx_new( - &tx, 0x1234123412341234ULL, 777U, 8U, make_tx_mem(tx_alloc_transfer, tx_alloc_payload), &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, + 0x1234123412341234ULL, + 777U, + 8U, + UDPARD_IFACE_BITMAP_ALL, + make_tx_mem(tx_alloc_transfer, tx_alloc_payload), + &tx_vtable)); tx.mtu[0] = 128U; tx.mtu[1] = 128U; tx.mtu[2] = 128U; @@ -325,8 +340,13 @@ void test_stateless_multiframe_first_frame_handling(const std::size_t extent, co udpard_tx_t tx{}; std::vector frames; - TEST_ASSERT_TRUE(udpard_tx_new( - &tx, 0x5555666677778888ULL, 999U, 16U, make_tx_mem(tx_alloc_transfer, tx_alloc_payload), &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, + 0x5555666677778888ULL, + 999U, + 16U, + UDPARD_IFACE_BITMAP_ALL, + make_tx_mem(tx_alloc_transfer, tx_alloc_payload), + &tx_vtable)); tx.mtu[0] = 128U; tx.mtu[1] = 128U; tx.mtu[2] = 128U; diff --git a/tests/src/test_e2e_random.cpp b/tests/src/test_e2e_random.cpp index eb3cc12..514f4ac 100644 --- a/tests/src/test_e2e_random.cpp +++ b/tests/src/test_e2e_random.cpp @@ -118,8 +118,13 @@ void test_randomized_deduplication() instrumented_allocator_new(&tx_alloc_payload); udpard_tx_t tx{}; std::vector frames; - TEST_ASSERT_TRUE(udpard_tx_new( - &tx, 0x1010101010101010ULL, 123U, 512U, make_tx_mem(tx_alloc_transfer, tx_alloc_payload), &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, + 0x1010101010101010ULL, + 123U, + 512U, + UDPARD_IFACE_BITMAP_ALL, + make_tx_mem(tx_alloc_transfer, tx_alloc_payload), + &tx_vtable)); tx.mtu[0] = 192U; tx.mtu[1] = 192U; tx.mtu[2] = 192U; diff --git a/tests/src/test_e2e_responses.cpp b/tests/src/test_e2e_responses.cpp index 78b1dfe..755497e 100644 --- a/tests/src/test_e2e_responses.cpp +++ b/tests/src/test_e2e_responses.cpp @@ -108,8 +108,13 @@ void test_unicast_response_roundtrip() instrumented_allocator_new(&b_tx_payload); udpard_tx_t b_tx{}; std::vector b_frames; - TEST_ASSERT_TRUE( - udpard_tx_new(&b_tx, 0xBBBBBBBBBBBBBBBBULL, 10U, 16U, make_tx_mem(b_tx_transfer, b_tx_payload), &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&b_tx, + 0xBBBBBBBBBBBBBBBBULL, + 10U, + 16U, + UDPARD_IFACE_BITMAP_ALL, + make_tx_mem(b_tx_transfer, b_tx_payload), + &tx_vtable)); b_tx.mtu[0] = 256U; b_tx.mtu[1] = 256U; b_tx.mtu[2] = 256U; diff --git a/tests/src/test_integration_sockets.cpp b/tests/src/test_integration_sockets.cpp index a617b12..2e053eb 100644 --- a/tests/src/test_integration_sockets.cpp +++ b/tests/src/test_integration_sockets.cpp @@ -114,8 +114,13 @@ void test_reordered_multiframe_delivery() instrumented_allocator_new(&tx_alloc_payload); udpard_tx_t tx{}; std::vector frames; - TEST_ASSERT_TRUE( - udpard_tx_new(&tx, 0xAAAAAAAABBBBBBBBULL, 1U, 32U, make_tx_mem(tx_alloc_transfer, tx_alloc_payload), &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, + 0xAAAAAAAABBBBBBBBULL, + 1U, + 32U, + UDPARD_IFACE_BITMAP_ALL, + make_tx_mem(tx_alloc_transfer, tx_alloc_payload), + &tx_vtable)); tx.mtu[0] = 96U; tx.mtu[1] = 96U; tx.mtu[2] = 96U; @@ -198,10 +203,20 @@ void test_two_publishers() udpard_tx_t b_tx{}; std::vector a_frames; std::vector b_frames; - TEST_ASSERT_TRUE( - udpard_tx_new(&a_tx, 0x1111111111111111ULL, 2U, 16U, make_tx_mem(a_tx_transfer, a_tx_payload), &tx_vtable)); - TEST_ASSERT_TRUE( - udpard_tx_new(&b_tx, 0x2222222222222222ULL, 3U, 16U, make_tx_mem(b_tx_transfer, b_tx_payload), &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&a_tx, + 0x1111111111111111ULL, + 2U, + 16U, + UDPARD_IFACE_BITMAP_ALL, + make_tx_mem(a_tx_transfer, a_tx_payload), + &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&b_tx, + 0x2222222222222222ULL, + 3U, + 16U, + UDPARD_IFACE_BITMAP_ALL, + make_tx_mem(b_tx_transfer, b_tx_payload), + &tx_vtable)); a_tx.mtu[0] = 128U; a_tx.mtu[1] = 128U; a_tx.mtu[2] = 128U; diff --git a/tests/src/test_intrusive_guards.c b/tests/src/test_intrusive_guards.c index 408942c..0360468 100644 --- a/tests/src/test_intrusive_guards.c +++ b/tests/src/test_intrusive_guards.c @@ -82,10 +82,10 @@ static void test_tx_new_guards(void) // Validate constructor argument checks. udpard_tx_t tx = { 0 }; - TEST_ASSERT_FALSE(udpard_tx_new(NULL, 1U, 1U, 1U, mem_ok, &tx_vtable)); - TEST_ASSERT_FALSE(udpard_tx_new(&tx, 0U, 1U, 1U, mem_ok, &tx_vtable)); - TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 1U, mem_ok, NULL)); - TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 1U, mem_ok, &tx_vtable_null_eject)); + TEST_ASSERT_FALSE(udpard_tx_new(NULL, 1U, 1U, 1U, UDPARD_IFACE_BITMAP_ALL, mem_ok, &tx_vtable)); + TEST_ASSERT_FALSE(udpard_tx_new(&tx, 0U, 1U, 1U, UDPARD_IFACE_BITMAP_ALL, mem_ok, &tx_vtable)); + TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 1U, UDPARD_IFACE_BITMAP_ALL, mem_ok, NULL)); + TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 1U, UDPARD_IFACE_BITMAP_ALL, mem_ok, &tx_vtable_null_eject)); // Reject invalid payload memory resources. const udpard_mem_vtable_t bad_alloc = { .base = { .free = free_heap }, .alloc = NULL }; @@ -93,9 +93,14 @@ static void test_tx_new_guards(void) .transfer = make_mem(transfer_pool), .payload = { make_mem(payload_pool), { .vtable = &bad_alloc, .context = NULL }, make_mem(payload_pool) }, }; - TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 1U, mem_bad_payload, &tx_vtable)); + TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 1U, UDPARD_IFACE_BITMAP_ALL, mem_bad_payload, &tx_vtable)); - TEST_ASSERT_TRUE(udpard_tx_new(&tx, 1U, 1U, 4U, mem_ok, &tx_vtable)); + TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 4U, (uint16_t)(1U << UDPARD_IFACE_COUNT_MAX), mem_ok, &tx_vtable)); + TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 4U, 0xFFFFU, mem_ok, &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, 1U, 1U, 4U, 0U, mem_ok, &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, 1U, 1U, 4U, 1U, mem_ok, &tx_vtable)); + + TEST_ASSERT_TRUE(udpard_tx_new(&tx, 1U, 1U, 4U, UDPARD_IFACE_BITMAP_ALL, mem_ok, &tx_vtable)); udpard_tx_free(&tx); } @@ -109,7 +114,7 @@ static void test_tx_push_guards(void) .payload = { make_mem(payload_pool), make_mem(payload_pool), make_mem(payload_pool) }, }; udpard_tx_t tx = { 0 }; - TEST_ASSERT_TRUE(udpard_tx_new(&tx, 1U, 1U, 4U, mem_ok, &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, 1U, 1U, 4U, UDPARD_IFACE_BITMAP_ALL, mem_ok, &tx_vtable)); // Validate argument checks for subject push. const udpard_bytes_scattered_t empty_payload = make_scattered("", 0U); @@ -148,7 +153,7 @@ static void test_tx_push_unicast_guards(void) .payload = { make_mem(payload_pool), make_mem(payload_pool), make_mem(payload_pool) }, }; udpard_tx_t tx = { 0 }; - TEST_ASSERT_TRUE(udpard_tx_new(&tx, 2U, 2U, 4U, mem_ok, &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, 2U, 2U, 4U, UDPARD_IFACE_BITMAP_ALL, mem_ok, &tx_vtable)); // Validate argument checks for unicast push. const udpard_bytes_scattered_t empty_payload = make_scattered("", 0U); @@ -180,10 +185,12 @@ static void test_tx_poll_and_free_guards(void) .payload = { make_mem(payload_pool), make_mem(payload_pool), make_mem(payload_pool) }, }; udpard_tx_t tx = { 0 }; - TEST_ASSERT_TRUE(udpard_tx_new(&tx, 10U, 11U, 4U, mem_ok, &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, 10U, 11U, 4U, UDPARD_IFACE_BITMAP_ALL, mem_ok, &tx_vtable)); udpard_tx_poll(NULL, 0, UDPARD_IFACE_BITMAP_ALL); udpard_tx_poll(&tx, -1, UDPARD_IFACE_BITMAP_ALL); TEST_ASSERT_EQUAL_UINT16(0U, udpard_tx_pending_ifaces(NULL)); + udpard_rx_poll(NULL, 0); + udpard_rx_poll(NULL, 1000); udpard_tx_free(&tx); udpard_tx_free(NULL); } diff --git a/tests/src/test_intrusive_rx.c b/tests/src/test_intrusive_rx.c index 66dc0a6..9e8403a 100644 --- a/tests/src/test_intrusive_rx.c +++ b/tests/src/test_intrusive_rx.c @@ -805,6 +805,84 @@ static void test_rx_fragment_tree_update_paths(void) rx_mem_fixture_fini(&fx); } +typedef struct +{ + udpard_mem_t dgram_mem; + udpard_deleter_t dgram_del; + udpard_udpip_ep_t src; + uint64_t uid; + size_t count; + uint64_t last_tid; + bool reentered; +} reentr_state_t; + +// On first delivery, re-enters udpard_rx_port_push() while all slots are occupied -- the pre-fix UAF trigger. +static void on_message_reentrant(udpard_rx_t* const rx, + udpard_rx_port_t* const port, + const udpard_rx_transfer_t transfer) +{ + reentr_state_t* const st = (reentr_state_t*)rx->user; + TEST_ASSERT_NOT_NULL(st); + st->count++; + st->last_tid = transfer.transfer_id; + if (!st->reentered) { + st->reentered = true; + const byte_t pl[8] = { 1U, 2U, 3U, 4U, 5U, 6U, 7U, 8U }; + udpard_bytes_mut_t d = + make_first_frame_datagram(st->dgram_mem, udpard_prio_nominal, 200U, st->uid, 4000U, pl, sizeof(pl)); + (void)udpard_rx_port_push(rx, port, 2000, st->src, d, st->dgram_del, 0U); + } + // Touch the delivered payload; pre-fix it was freed by the re-entrant push. + if (transfer.payload_size_stored > 0U) { + const udpard_fragment_t* cursor = transfer.payload; + byte_t tmp[16] = { 0 }; + TEST_ASSERT_EQUAL_size_t(transfer.payload_size_stored, + udpard_fragment_gather(&cursor, 0, transfer.payload_size_stored, tmp)); + } + udpard_fragment_free_all(transfer.payload, udpard_make_deleter(port->memory.fragment)); +} +static const udpard_rx_port_vtable_t callbacks_reentrant = { .on_message = on_message_reentrant }; + +static void test_rx_reentrant_push_in_callback(void) +{ + // Complete a transfer whose callback re-enters the RX push while all slots are occupied. + rx_mem_fixture_t fx; + rx_mem_fixture_init(&fx); + udpard_rx_t rx; + udpard_rx_new(&rx); + udpard_rx_port_t port; + TEST_ASSERT_TRUE(udpard_rx_port_new(&port, 65536U, fx.rx_mem, &callbacks_reentrant)); + const udpard_udpip_ep_t src = { .ip = 0x0A000001U, .port = 7000U }; + const uint64_t uid = 0xABCDEFU; + reentr_state_t st = { .dgram_mem = fx.dgram_mem, + .dgram_del = fx.dgram_del, + .src = src, + .uid = uid, + .count = 0U, + .last_tid = 0U, + .reentered = false }; + rx.user = &st; + + // Fill all but one slot with incomplete transfers at a large timestamp. + const byte_t pl[8] = { 9U, 8U, 7U, 6U, 5U, 4U, 3U, 2U }; + for (uint64_t tid = 1U; tid < RX_SLOT_COUNT; tid++) { + udpard_bytes_mut_t d = + make_first_frame_datagram(fx.dgram_mem, udpard_prio_nominal, tid, uid, 4000U, pl, sizeof(pl)); + TEST_ASSERT_TRUE(udpard_rx_port_push(&rx, &port, 1000, src, d, fx.dgram_del, 0U)); + } + // Complete a transfer at the smallest timestamp so it becomes the eviction victim during the re-entry. + udpard_bytes_mut_t done = + make_first_frame_datagram(fx.dgram_mem, udpard_prio_nominal, 100U, uid, sizeof(pl), pl, sizeof(pl)); + TEST_ASSERT_TRUE(udpard_rx_port_push(&rx, &port, 1, src, done, fx.dgram_del, 0U)); + + TEST_ASSERT_EQUAL_size_t(1U, st.count); + TEST_ASSERT_EQUAL_UINT64(100U, st.last_tid); + TEST_ASSERT_TRUE(st.reentered); + + udpard_rx_port_free(&rx, &port); + rx_mem_fixture_fini(&fx); +} + void setUp(void) {} void tearDown(void) {} @@ -825,5 +903,6 @@ int main(void) RUN_TEST(test_rx_session_update_failure_paths); RUN_TEST(test_rx_port_free_with_incomplete_transfer); RUN_TEST(test_rx_fragment_tree_update_paths); + RUN_TEST(test_rx_reentrant_push_in_callback); return UNITY_END(); } diff --git a/tests/src/test_intrusive_tx.c b/tests/src/test_intrusive_tx.c index f15e2aa..6f4d71b 100644 --- a/tests/src/test_intrusive_tx.c +++ b/tests/src/test_intrusive_tx.c @@ -73,7 +73,8 @@ static void fixture_init(tx_fixture_t* const self, const size_t queue_limit, con self->mem.payload[i] = instrumented_allocator_make_resource(&self->payload_alloc); } self->eject = (eject_state_t){ .allow = allow_eject, .retain_first = false, .count = 0U, .held = { 0 } }; - TEST_ASSERT_TRUE(udpard_tx_new(&self->tx, 0x1122334455667788ULL, 123U, queue_limit, self->mem, &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new( + &self->tx, 0x1122334455667788ULL, 123U, queue_limit, UDPARD_IFACE_BITMAP_ALL, self->mem, &tx_vtable)); for (size_t i = 0; i < UDPARD_IFACE_COUNT_MAX; i++) { self->tx.mtu[i] = mtu; } @@ -271,11 +272,12 @@ static void test_tx_refcount_retention(void) udpard_tx_refcount_inc((udpard_bytes_t){ 0 }); udpard_tx_refcount_dec((udpard_bytes_t){ 0 }); - udpard_tx_free(&fx.tx); - TEST_ASSERT_EQUAL_size_t(0U, fx.transfer_alloc.allocated_fragments); + // Release the retained reference before udpard_tx_free(), which asserts none remain. TEST_ASSERT_EQUAL_size_t(1U, fx.payload_alloc.allocated_fragments); udpard_tx_refcount_dec(fx.eject.held); TEST_ASSERT_EQUAL_size_t(0U, fx.payload_alloc.allocated_fragments); + udpard_tx_free(&fx.tx); + TEST_ASSERT_EQUAL_size_t(0U, fx.transfer_alloc.allocated_fragments); instrumented_allocator_reset(&fx.transfer_alloc); instrumented_allocator_reset(&fx.payload_alloc); } @@ -291,7 +293,7 @@ static void test_tx_validate_and_compare_deadlines(void) .payload = { valid, valid, valid }, }; udpard_tx_t tx = { 0 }; - TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 1U, memory, &tx_vtable)); + TEST_ASSERT_FALSE(udpard_tx_new(&tx, 1U, 1U, 1U, UDPARD_IFACE_BITMAP_ALL, memory, &tx_vtable)); tx_transfer_t early = { 0 }; tx_transfer_t late = { 0 }; @@ -393,7 +395,7 @@ static void test_tx_sharing_branches(void) instrumented_allocator_make_resource(&alloc_p2) }, }; udpard_tx_t tx = { 0 }; - TEST_ASSERT_TRUE(udpard_tx_new(&tx, 0x0102030405060708ULL, 42U, 8U, tx_mem, &tx_vtable)); + TEST_ASSERT_TRUE(udpard_tx_new(&tx, 0x0102030405060708ULL, 42U, 8U, UDPARD_IFACE_BITMAP_ALL, tx_mem, &tx_vtable)); tx.user = NULL; tx.mtu[0] = tx.mtu[1] = tx.mtu[2] = 128U; const byte_t p[] = { 0xABU }; @@ -420,6 +422,156 @@ static void test_tx_sharing_branches(void) instrumented_allocator_reset(&alloc_b); } +// Retains a reference to every ejected frame (via tx->user) to keep enqueued_frames_count pinned. +typedef struct +{ + udpard_bytes_t held[16]; + size_t count; +} retain_all_state_t; +static bool eject_retain_all(udpard_tx_t* const tx, udpard_tx_ejection_t* const ejection) +{ + retain_all_state_t* const st = (retain_all_state_t*)tx->user; + TEST_ASSERT_NOT_NULL(st); + if (st->count < (sizeof(st->held) / sizeof(st->held[0]))) { + st->held[st->count++] = ejection->datagram; + udpard_tx_refcount_inc(ejection->datagram); + } + return true; +} +static const udpard_tx_vtable_t tx_vtable_retain_all = { .eject = eject_retain_all }; + +static void test_tx_queue_limit_lowered_below_count(void) +{ + // Lowering the frame limit below the in-flight count must not underflow the vacancy check. + retain_all_state_t held = { 0 }; + tx_fixture_t fx = { 0 }; + fixture_init(&fx, 8U, 128U, true); + fx.tx.vtable = &tx_vtable_retain_all; + fx.tx.user = &held; + byte_t data[2000] = { 0 }; + TEST_ASSERT_TRUE(udpard_tx_push(&fx.tx, + 0, + 10000, + 1U, + udpard_prio_nominal, + 1U, + udpard_make_subject_endpoint(700U), + make_scattered(data, sizeof(data)), + NULL)); + udpard_tx_poll(&fx.tx, 1, 1U); + const size_t pinned = fx.tx.enqueued_frames_count; + TEST_ASSERT_TRUE(pinned > 2U); + TEST_ASSERT_EQUAL_size_t(pinned, held.count); + + fx.tx.enqueued_frames_limit = 2U; + const byte_t small[] = { 0xABU }; + TEST_ASSERT_FALSE(udpard_tx_push(&fx.tx, + 2, + 10000, + 1U, + udpard_prio_nominal, + 2U, + udpard_make_subject_endpoint(701U), + make_scattered(small, sizeof(small)), + NULL)); + TEST_ASSERT_EQUAL_UINT64(1U, fx.tx.errors_capacity); + TEST_ASSERT_EQUAL_size_t(pinned, fx.tx.enqueued_frames_count); + + for (size_t i = 0; i < held.count; i++) { + udpard_tx_refcount_dec(held.held[i]); + } + TEST_ASSERT_EQUAL_size_t(0U, fx.tx.enqueued_frames_count); + fixture_fini(&fx); +} + +static void test_tx_iface_available_mask(void) +{ + tx_fixture_t fx = { 0 }; + fixture_init(&fx, 8U, 128U, true); + fx.tx.iface_bitmap = 1U << 0U; + const byte_t data[] = { 1, 2, 3, 4, 5, 6 }; + const udpard_udpip_ep_t subject = udpard_make_subject_endpoint(321U); + const udpard_bytes_scattered_t payload = make_scattered(data, sizeof(data)); + TEST_ASSERT_TRUE(udpard_tx_push( + &fx.tx, 0, 10000, UDPARD_IFACE_BITMAP_ALL, udpard_prio_fast, 0x0000AABBCCDDEEFFULL, subject, payload, NULL)); + TEST_ASSERT_EQUAL_UINT16(1U << 0U, udpard_tx_pending_ifaces(&fx.tx)); + + udpard_udpip_ep_t eps[UDPARD_IFACE_COUNT_MAX] = { 0 }; + eps[0] = (udpard_udpip_ep_t){ .ip = 0x0A000001U, .port = 8001U }; + eps[2] = (udpard_udpip_ep_t){ .ip = 0x0A000003U, .port = 8003U }; + TEST_ASSERT_TRUE(udpard_tx_push_unicast(&fx.tx, 0, 10000, udpard_prio_nominal, eps, payload, NULL)); + TEST_ASSERT_EQUAL_UINT16(1U << 0U, udpard_tx_pending_ifaces(&fx.tx)); + + const size_t count = fx.tx.enqueued_frames_count; + TEST_ASSERT_FALSE( + udpard_tx_push(&fx.tx, 0, 10000, 1U << 1U, udpard_prio_fast, 0x1122334455667788ULL, subject, payload, NULL)); + udpard_udpip_ep_t eps_far[UDPARD_IFACE_COUNT_MAX] = { 0 }; + eps_far[2] = (udpard_udpip_ep_t){ .ip = 0x0A000003U, .port = 8003U }; + TEST_ASSERT_FALSE(udpard_tx_push_unicast(&fx.tx, 0, 10000, udpard_prio_nominal, eps_far, payload, NULL)); + TEST_ASSERT_EQUAL_UINT16(1U << 0U, udpard_tx_pending_ifaces(&fx.tx)); + TEST_ASSERT_EQUAL_size_t(count, fx.tx.enqueued_frames_count); + TEST_ASSERT_EQUAL_UINT64( + 0U, fx.tx.errors_capacity + fx.tx.errors_oom + fx.tx.errors_sacrifice + fx.tx.errors_expiration); + + fixture_fini(&fx); +} + +static void test_tx_listen_only(void) +{ + tx_fixture_t fx = { 0 }; + fixture_init(&fx, 8U, 128U, true); + fx.tx.iface_bitmap = 0U; + const byte_t data[] = { 1, 2, 3 }; + const udpard_bytes_scattered_t payload = make_scattered(data, sizeof(data)); + TEST_ASSERT_FALSE(udpard_tx_push(&fx.tx, + 0, + 10000, + UDPARD_IFACE_BITMAP_ALL, + udpard_prio_fast, + 1U, + udpard_make_subject_endpoint(321U), + payload, + NULL)); + udpard_udpip_ep_t eps[UDPARD_IFACE_COUNT_MAX] = { 0 }; + eps[0] = (udpard_udpip_ep_t){ .ip = 0x0A000001U, .port = 8001U }; + TEST_ASSERT_FALSE(udpard_tx_push_unicast(&fx.tx, 0, 10000, udpard_prio_nominal, eps, payload, NULL)); + TEST_ASSERT_EQUAL_UINT16(0U, udpard_tx_pending_ifaces(&fx.tx)); + TEST_ASSERT_EQUAL_size_t(0U, fx.tx.enqueued_frames_count); + fixture_fini(&fx); +} + +static void test_tx_iface_bitmap_extra_bits(void) +{ + tx_fixture_t fx = { 0 }; + fixture_init(&fx, 8U, 128U, true); + const byte_t data[] = { 1, 2, 3 }; + const udpard_udpip_ep_t subject = udpard_make_subject_endpoint(321U); + const udpard_bytes_scattered_t payload = make_scattered(data, sizeof(data)); + + fx.tx.iface_bitmap = 1U << 0U; + TEST_ASSERT_TRUE(udpard_tx_push(&fx.tx, + 0, + 10000, + (uint16_t)((1U << 0U) | (1U << UDPARD_IFACE_COUNT_MAX)), + udpard_prio_fast, + 1U, + subject, + payload, + NULL)); + TEST_ASSERT_EQUAL_UINT16(1U << 0U, udpard_tx_pending_ifaces(&fx.tx)); + const size_t count = fx.tx.enqueued_frames_count; + + fx.tx.iface_bitmap = (uint16_t)(1U << UDPARD_IFACE_COUNT_MAX); + TEST_ASSERT_FALSE(udpard_tx_push( + &fx.tx, 0, 10000, (uint16_t)(1U << UDPARD_IFACE_COUNT_MAX), udpard_prio_fast, 2U, subject, payload, NULL)); + udpard_udpip_ep_t eps[UDPARD_IFACE_COUNT_MAX] = { 0 }; + eps[0] = (udpard_udpip_ep_t){ .ip = 0x0A000001U, .port = 8001U }; + TEST_ASSERT_FALSE(udpard_tx_push_unicast(&fx.tx, 0, 10000, udpard_prio_nominal, eps, payload, NULL)); + TEST_ASSERT_EQUAL_UINT16(1U << 0U, udpard_tx_pending_ifaces(&fx.tx)); + TEST_ASSERT_EQUAL_size_t(count, fx.tx.enqueued_frames_count); + fixture_fini(&fx); +} + void setUp(void) {} void tearDown(void) {} @@ -438,5 +590,9 @@ int main(void) RUN_TEST(test_tx_transfer_alloc_oom); RUN_TEST(test_tx_eject_stall); RUN_TEST(test_tx_sharing_branches); + RUN_TEST(test_tx_queue_limit_lowered_below_count); + RUN_TEST(test_tx_iface_available_mask); + RUN_TEST(test_tx_listen_only); + RUN_TEST(test_tx_iface_bitmap_extra_bits); return UNITY_END(); }