CF_Chunks_Insert drops all data after the chunk list fills, so a lossy Class 2 upload is mostly resent

Open
  • Is something wrong?
#535 0 comments 0 reactions 0 assignees View on GitHub

@SpaceOceanCorp is already working on this.

Since Nov 10, 2026.

  • #536 by @SpaceOceanCorp — open

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
35/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Stale
Tech stack
c
Domain
backend

Research direction

Read cf_chunk.c and the existing CF_ChunkListAdd tests in unit-test/cf_chunk_tests.c. The report identifies the full-list case where a new chunk is past every tracked chunk; check how the insertion and eviction rules handle it. Done means the full-list end case is covered and the existing 126 assertions still pass; pull request #536 is already open for this issue.

Written by the indexing model from the issue text.

Description

Is your feature request related to a problem?
With the default CF_CHANNEL_NUM_RX_CHUNKS_PER_TRANSACTION of CF_NAK_MAX_SEGMENTS, a Class 2 receive
tracks at most that many runs of received data. Once the list is full, CF_Chunks_Insert keeps a new chunk
only if it is larger than the smallest tracked chunk. The first PDU after each new gap is a single PDU, so
it is dropped, and every later contiguous PDU is dropped the same way. All data received after the list
fills goes untracked and is NAKed again.

Measured with cf_chunk.c from main, driven through CF_ChunkListAdd and CF_ChunkList_ComputeGaps:
a 4.2 MB file in 4,228 PDUs, independent loss on every send, NAK gap limit = max_chunks - 1 when full.
Medians over 31 seeds:

Loss RX chunks PDUs sent today NAK rounds today PDUs sent with change NAK rounds with change
5% 58 9,827 8 6,520 4
10% 58 16,742 15 11,940 8
20% 58 28,692 26 26,131 16
5% 128 5,901 4 4,801 3
1% 58, 128, 2048 4,271 1 4,271 1
any 2048 same as today same as today

Describe the solution you'd like. When the list is full and the new chunk lies past every tracked chunk
(i == chunks->count), evict the smallest chunk and keep the new one:

if ((smallest_c->size < chunk->size) || (i == chunks->count))

That chunk is the leading edge of a sequential transfer, so later data extends it, and each extra gap costs one
resend of the smallest run instead of the rest of the file. Chunks inside the tracked range keep today's rule.
The same list type holds TX NAK requests; there the change only swaps which request is dropped when full.

Checks: CF's own unit-test/cf_chunk_tests.c passes unchanged with the change (126 assertions, with ASan and UBSan). A
fuzz of 5.8 million random CF_ChunkListAdd calls (ASan and UBSan) confirmed that the list never records unreceived bytes, stays sorted
and non-overlapping, and stays within max_chunks; the new branch was taken 854,545 times. With the TX list also capped and
NAK PDUs lost, no transfer stalled at 5% to 30% loss. One trade-off: at 30% loss with both lists at 58, there are fewer NAK rounds (38
instead of 52), but 6.5% more PDUs are sent.

Additional context. F´'s CfdpManager is derived from CF and has the same rule; it was reported there with
live measurements in nasa/fprime#6131, which our simulation matches within 0.1%. We can open a PR with the change and a
unit test for the full-list, new-chunk-at-the-end case (not covered today) if this is wanted. CLA: cFS Apps corporate CLA filed.

Dominant language
C
Stars
122
Forks
81
Avg merge
1d 15h
Merged PRs (30d)
4

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from nasa/CF

All issues in nasa/CF

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.