Skip to content

Commit d52e640

Browse files
fixes
1 parent 92deebf commit d52e640

File tree

1 file changed

+12
-18
lines changed

1 file changed

+12
-18
lines changed

libcanard/canard.c

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,9 +1264,9 @@ typedef struct
12641264
rx_slot_t* slots[CANARD_PRIO_COUNT]; // Indexed by priority level to allow preemption.
12651265
canard_subscription_t* owner;
12661266
byte_t node_id;
1267-
byte_t last_admitted_transfer_id : CANARD_TRANSFER_ID_BITS;
1268-
byte_t last_admitted_priority : CANARD_PRIO_BITS;
1269-
byte_t iface_index : IFACE_INDEX_BITS;
1267+
byte_t last_admitted_transfer_id;
1268+
byte_t last_admitted_priority;
1269+
byte_t iface_index;
12701270
} rx_session_t;
12711271
static_assert((sizeof(void*) > 4) || (sizeof(rx_session_t) <= 120), "too large");
12721272

@@ -1366,8 +1366,8 @@ static bool rx_session_solve_admission(const rx_session_t* const ses,
13661366
}
13671367
// Duplicate start frames do not require special treatment because a duplicate frame can only follow the original
13681368
// without any frames belonging to the same transfer in between (see the assumptions). If we get a duplicate start,
1369-
// with a nonzero TID timeout it will be rejected as not-new; even if the timeout is zero, accepting the duplicate
1370-
// will simply restart the slot without loss of information since there were no other frames in between.
1369+
// with a nonzero TID timeout it will be rejected as not-new. Zero transfer-ID timeout means the application
1370+
// accepts duplicates; with zero timeout duplicate rejection is not possible given this protocol design.
13711371
//
13721372
// The original design had a special case that enabled admittance of a transfer with the next transfer-ID from a
13731373
// different interface if there is no pending reassembly in progress; see details here and the original v4 code:
@@ -1390,20 +1390,14 @@ static bool rx_session_solve_admission(const rx_session_t* const ses,
13901390
// P=low | 0 | <-- arrived late, rejected as duplicate!
13911391
// P=high | 1 2 3 0 |
13921392
//
1393-
// One solution is to unroll the transfer-ID sequence locally to track wraparounds:
1393+
// One solution is to track the last admitted priority along with the transfer-ID:
1394+
// preemption alters the admitted priority, which prevents false rejection.
13941395
//
1395-
// |RECEIVER (unrolling) |
1396-
// P=low | 5 | <-- thanks to unrolling we observe this as not a duplicate
1397-
// P=high | 1 2 3 4 | <-- unrolled
1398-
//
1399-
// Another solution that is much simpler but yields identical behavior is to track the last admitted priority
1400-
// along with the transfer-ID: preemption alters the admitted priority, which prevents false rejection.
1401-
//
1402-
// Regardless of the approach, there is one critical edge case: if a low-priority frame is duplicated (due to the
1403-
// CAN ACK glitch effect) AND at least one higher-priority frame is admitted between the original frame and its
1404-
// duplicate, then the duplicate will be accepted as a new transfer.
1396+
// There is one critical edge case: if a low-priority frame is duplicated (due to the CAN ACK glitch effect)
1397+
// AND at least one higher-priority frame is admitted between the original frame and its duplicate,
1398+
// then the duplicate will be accepted as a new transfer.
14051399
// Example: low-prio tid=1, high-prio preemption tid=2, high-prio tid=2...X, low-prio duplicate tid=1.
1406-
// In general, the problem of duplicate detection under these conditions appears undecidable.
1400+
// In general, the problem of duplicate detection under these conditions is believed to be undecidable.
14071401
// We inherit the CAN PHY design limitation here by accepting the risk of spurious duplication, recognizing that
14081402
// it is very low, as it requires both unlikely events to occur simultaneously: CAN ACK glitch and a high-priority
14091403
// preemption exactly between the original transmission and its duplicate.
@@ -1425,7 +1419,7 @@ static bool rx_session_update(canard_subscription_t* const sub,
14251419

14261420
// This is a start frame, but before we allocate new state for it, we must ensure that it is of the correct version.
14271421
// Wrong protocol version must be rejected as early as possible to avoid wasting memory on unused states.
1428-
// The protocol version is only acceptable on start-of-transfer frames, but new sessions can only be created
1422+
// The protocol version is only visible on start-of-transfer frames, but new sessions can only be created
14291423
// on start frames, so this is robust.
14301424
if (frame->start && (frame->toggle != kind_is_v1(sub->kind))) {
14311425
return true; // Wrong protocol version is not a failure.

0 commit comments

Comments
 (0)