Skip to content

Enhance canardTxPush to support scattered transfer payload buffers. #223

@serges147

Description

@serges147

See OpenCyphal-Garage/libcyphal#343 (comment), but note that we have since altered the design such that the TX pipeline allows exactly one copy, which happens when the user data is copied into individual frame payloads. This is avoidable but the complexity increases dramatically at this point because the application would be required to keep its data alive and unmodified until the TX frames have cleared the queue, which requires reference counting, and the application would likely be required to copy its data into a separate storage before transmitting it in this way anyway, which defeats the purpose of the optimization.

So the input data may be scattered, but it is borrowed rather than moved, as we copy it into small contiguous per-frame chunks. The affected functions are txPushSingleFrame and txGenerateMultiFrameChain -- currently they rely on simple contiguous pointer arithmetics, which will need to be replaced with a stateful iterator struct with a helper function for extracting the data from a scattered buffer step by step:

CANARD_PRIVATE int32_t txPushSingleFrame(struct CanardTxQueue* const que,
const struct CanardInstance* const ins,
const CanardMicrosecond deadline_usec,
const uint32_t can_id,
const CanardTransferID transfer_id,
const struct CanardPayload payload)
{
CANARD_ASSERT((que != NULL) && (ins != NULL));
CANARD_ASSERT((payload.data != NULL) || (payload.size == 0));
const size_t frame_payload_size = txRoundFramePayloadSizeUp(payload.size + 1U);
CANARD_ASSERT(frame_payload_size > payload.size);
const size_t padding_size = frame_payload_size - payload.size - 1U;
CANARD_ASSERT((padding_size + payload.size + 1U) == frame_payload_size);
int32_t out = 0;
struct CanardTxQueueItem* const tqi =
(que->size < que->capacity) ? txAllocateQueueItem(que, ins, can_id, deadline_usec, frame_payload_size) : NULL;
if (tqi != NULL)
{
if (payload.size > 0U) // The check is needed to avoid calling memcpy() with a NULL pointer, it's an UB.
{
CANARD_ASSERT(payload.data != NULL);
// Clang-Tidy raises an error recommending the use of memcpy_s() instead.
// We ignore it because the safe functions are poorly supported; reliance on them may limit the portability.
(void) memcpy(tqi->frame.payload.data, payload.data, payload.size); // NOLINT
}
// Clang-Tidy raises an error recommending the use of memset_s() instead.
// We ignore it because the safe functions are poorly supported; reliance on them may limit the portability.
uint8_t* const frame_bytes = tqi->frame.payload.data;
(void) memset(frame_bytes + payload.size, PADDING_BYTE_VALUE, padding_size); // NOLINT
*(frame_bytes + (frame_payload_size - 1U)) = txMakeTailByte(true, true, true, transfer_id);
// Insert the newly created TX item into the priority queue.
const struct CanardTreeNode* const priority_queue_res =
cavlSearch(&que->priority_root, &tqi->priority_base, &txAVLPriorityPredicate, &avlTrivialFactory);
(void) priority_queue_res;
CANARD_ASSERT(priority_queue_res == &tqi->priority_base);
// Insert the newly created TX item into the deadline queue.
const struct CanardTreeNode* const deadline_queue_res =
cavlSearch(&que->deadline_root, &tqi->deadline_base, &txAVLDeadlinePredicate, &avlTrivialFactory);
(void) deadline_queue_res;
CANARD_ASSERT(deadline_queue_res == &tqi->deadline_base);
que->size++;
CANARD_ASSERT(que->size <= que->capacity);
out = 1; // One frame enqueued.
}
else
{
out = -CANARD_ERROR_OUT_OF_MEMORY;
}
CANARD_ASSERT((out < 0) || (out == 1));
return out;
}

CANARD_PRIVATE TxChain txGenerateMultiFrameChain(struct CanardTxQueue* const que,
const struct CanardInstance* const ins,
const size_t presentation_layer_mtu,
const CanardMicrosecond deadline_usec,
const uint32_t can_id,
const CanardTransferID transfer_id,
const struct CanardPayload payload)
{
CANARD_ASSERT(que != NULL);
CANARD_ASSERT(presentation_layer_mtu > 0U);
CANARD_ASSERT(payload.size > presentation_layer_mtu); // Otherwise, a single-frame transfer should be used.
CANARD_ASSERT(payload.data != NULL);
TxChain out = {.head = NULL, .tail = NULL, .size = 0U};
const size_t payload_size_with_crc = payload.size + CRC_SIZE_BYTES;
size_t offset = 0U;
TransferCRC crc = crcAdd(CRC_INITIAL, payload);
bool toggle = INITIAL_TOGGLE_STATE;
const uint8_t* payload_ptr = (const uint8_t*) payload.data;
while (offset < payload_size_with_crc)
{
out.size++;
const size_t frame_payload_size_with_tail =
((payload_size_with_crc - offset) < presentation_layer_mtu)
? txRoundFramePayloadSizeUp((payload_size_with_crc - offset) + 1U) // Padding in the last frame only.
: (presentation_layer_mtu + 1U);
struct CanardTxQueueItem* const tqi =
txAllocateQueueItem(que, ins, can_id, deadline_usec, frame_payload_size_with_tail);
if (NULL == out.head)
{
out.head = tqi;
}
else
{
// C std, 6.7.2.1.15: A pointer to a structure object <...> points to its initial member, and vice versa.
// Can't just read tqi->base because tqi may be NULL; https://github.com/OpenCyphal/libcanard/issues/203.
out.tail->next_in_transfer = tqi;
}
out.tail = tqi;
if (NULL == out.tail)
{
break;
}
// Copy the payload into the frame.
uint8_t* const frame_bytes = tqi->frame.payload.data;
const size_t frame_payload_size = frame_payload_size_with_tail - 1U;
size_t frame_offset = 0U;
if (offset < payload.size)
{
size_t move_size = payload.size - offset;
if (move_size > frame_payload_size)
{
move_size = frame_payload_size;
}
// Clang-Tidy raises an error recommending the use of memcpy_s() instead.
// We ignore it because the safe functions are poorly supported; reliance on them may limit the portability.
// SonarQube incorrectly detects a buffer overflow here.
(void) memcpy(frame_bytes, payload_ptr, move_size); // NOLINT NOSONAR
frame_offset = frame_offset + move_size;
offset += move_size;
payload_ptr += move_size;
}
// Handle the last frame of the transfer: it is special because it also contains padding and CRC.
if (offset >= payload.size)
{
// Insert padding -- only in the last frame. Don't forget to include padding into the CRC.
while ((frame_offset + CRC_SIZE_BYTES) < frame_payload_size)
{
frame_bytes[frame_offset] = PADDING_BYTE_VALUE;
++frame_offset;
crc = crcAddByte(crc, PADDING_BYTE_VALUE);
}
// Insert the CRC.
if ((frame_offset < frame_payload_size) && (offset == payload.size))
{
// SonarQube incorrectly detects a buffer overflow here.
frame_bytes[frame_offset] = (uint8_t) (crc >> BITS_PER_BYTE); // NOSONAR
++frame_offset;
++offset;
}
if ((frame_offset < frame_payload_size) && (offset > payload.size))
{
frame_bytes[frame_offset] = (uint8_t) (crc & BYTE_MAX);
++frame_offset;
++offset;
}
}
// Finalize the frame.
CANARD_ASSERT((frame_offset + 1U) == out.tail->frame.payload.size);
// SonarQube incorrectly detects a buffer overflow here.
frame_bytes[frame_offset] = // NOSONAR
txMakeTailByte(out.head == out.tail, offset >= payload_size_with_crc, toggle, transfer_id);
toggle = !toggle;
}
return out;
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions