fix(uavcan): correct TX queue block accounting and report queue pressure - #28395
Merged
dakejahl merged 2 commits intoAug 26, 2026
Merged
dakejahl merged 2 commits into
dakejahl merged 2 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
LimitedPoolAllocator::allocate() incremented used_blocks_ before asking the underlying allocator, so an allocation the pool could not satisfy still spent quota. Nothing ever hands that block back, because deallocate() only runs for a pointer that was returned, so every queue that asked for memory while the pool was empty stays permanently smaller than its configured limit. The pool is shared between the RX side and one TX queue per interface, so it does empty transiently under load -- a DroneCAN node firmware update is enough. The damage accumulates and is only cleared by restarting the node.
`uavcan status` gains, per interface: TX queue peak: 44/84 blocks TX rejected: 80 frames (78 expired, 2 no memory) CanTxQueue already counted rejected frames but tx_queues_ is private and CanIOManager exposed no accessor, so the count was unreachable. It also conflated three causes: a frame handed over with its deadline already passed, a queued frame evicted as expired to make room, and a frame dropped because the queue was at its block limit. Only the last two are memory pressure, and they call for opposite responses, so a single count sends you the wrong way. The peak is what makes the count actionable. The limit is pool_soft / (num_ifaces + 1) + 1, derived from the soft capacity in allocator.hpp, so a peak sitting at the limit says raising that capacity will help and a peak well under it says the frames are ageing out for some other reason and a larger pool will not. Measured on an FMU-v6XRT serving a DroneCAN node firmware update: one node holds 44 of 84 blocks and drops nothing, another pins the limit and drops hundreds. The counts alone did not distinguish those.
Copy link
Copy Markdown
Contributor
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 288 byte (0.01 %)] FILE SIZE VM SIZE
-------------- --------------
+0.0% +288 +0.0% +288 .text
[NEW] +110 [NEW] +110 uavcan::CanIOManager::getTxQueuePerfCounters()
+0.1% +88 +0.1% +88 [section .text]
+8.6% +56 +8.6% +56 UavcanNode::print_info()
+92% +22 +92% +22 uavcan::LimitedPoolAllocator::allocate()
[NEW] +18 [NEW] +18 uavcan::CanTxQueue::registerExpiredFrame()
[NEW] +18 [NEW] +18 uavcan::CanTxQueue::registerOutOfMemoryFrame()
-85.0% +12 -85.0% +12 [6 Others]
+3.8% +8 +3.8% +8 uavcan::CanIOManager::CanIOManager()
+2.0% +4 +2.0% +4 UavcanNode::publish_can_interface_statuses()
+33% +2 +33% +2 uavcan::Dispatcher::hasServer()
+33% +2 +33% +2 uavcan::Dispatcher::hasSubscriber()
-0.2% -2 -0.2% -2 matrix::Matrix<>::operator=()
-0.5% -2 -0.5% -2 uavcan::CanIOManager::send()
-1.0% -4 -1.0% -4 EKFGSF_yaw::fuseVelocity()
-1.1% -4 -1.1% -4 UavcanNode::cb_getset()
-3.4% -4 -3.4% -4 UavcanNode::cb_restart()
-4.5% -4 -4.5% -4 UavcanNode::param_opcode()
-1.4% -4 -1.4% -4 UavcanNode::publish_node_statuses()
-0.8% -8 -0.8% -8 UavcanNode::UavcanNode()
-1.4% -8 -1.4% -8 UavcanNode::~UavcanNode()
-0.7% -12 -0.7% -12 UavcanNode::Run()
+0.0% +67 [ = ] 0 .debug_abbrev
+0.0% +24 [ = ] 0 .debug_aranges
+0.0% +92 [ = ] 0 .debug_frame
+0.0% +14.3Ki [ = ] 0 .debug_info
+0.0% +509 [ = ] 0 .debug_line
+25% +1 [ = ] 0 [Unmapped]
+0.0% +508 [ = ] 0 [section .debug_line]
+0.0% +345 [ = ] 0 .debug_loclists
+0.0% +91 [ = ] 0 .debug_rnglists
+0.0% +896 [ = ] 0 .debug_str
+0.8% +2 [ = ] 0 .shstrtab
+0.0% +150 [ = ] 0 .strtab
-39.0% -16 [ = ] 0 __nxsem_clockwait_veneer
+80% +16 [ = ] 0 __sched_lock_veneer
[NEW] +52 [ = ] 0 uavcan::CanIOManager::getTxQueuePerfCounters()
[NEW] +47 [ = ] 0 uavcan::CanTxQueue::registerExpiredFrame()
[NEW] +51 [ = ] 0 uavcan::CanTxQueue::registerOutOfMemoryFrame()
+0.0% +96 [ = ] 0 .symtab
+100% +32 [ = ] 0 EKFGSF_yaw::fuseVelocity()
+20% +16 [ = ] 0 UavcanNode::print_params()
-50.0% -16 [ = ] 0 UavcanNode::update_params()
-0.5% -64 [ = ] 0 [section .symtab]
-40.0% -32 [ = ] 0 __nxsem_clockwait_veneer
+67% +32 [ = ] 0 __sched_lock_veneer
[NEW] +32 [ = ] 0 uavcan::CanIOManager::getTxQueuePerfCounters()
+100% +16 [ = ] 0 uavcan::CanIOManager::send()
+50% +16 [ = ] 0 uavcan::CanTxQueue::push()
[NEW] +32 [ = ] 0 uavcan::CanTxQueue::registerExpiredFrame()
[NEW] +32 [ = ] 0 uavcan::CanTxQueue::registerOutOfMemoryFrame()
-2.9% -288 [ = ] 0 [Unmapped]
+0.0% +16.5Ki +0.0% +288 TOTAL
FILE SIZE VM SIZE
-------------- --------------
+0.0% +296 +0.0% +296 .text
[NEW] +110 [NEW] +110 uavcan::CanIOManager::getTxQueuePerfCounters()
+0.1% +88 +0.1% +88 [section .text]
+8.6% +56 +8.6% +56 UavcanNode::print_info()
+92% +22 +92% +22 uavcan::LimitedPoolAllocator::allocate()
[NEW] +18 [NEW] +18 uavcan::CanTxQueue::registerExpiredFrame()
[NEW] +18 [NEW] +18 uavcan::CanTxQueue::registerOutOfMemoryFrame()
-83.9% +10 -83.9% +10 [5 Others]
+3.8% +8 +3.8% +8 uavcan::CanIOManager::CanIOManager()
+2.0% +4 +2.0% +4 UavcanNode::publish_can_interface_statuses()
+0.2% +2 +0.2% +2 matrix::Matrix<>::operator=()
+33% +2 +33% +2 uavcan::Dispatcher::hasServer()
+33% +2 +33% +2 uavcan::Dispatcher::hasSubscriber()
+11% +2 +11% +2 uavcan::Dispatcher::registerMessageListener()
-0.5% -2 -0.5% -2 uavcan::CanIOManager::send()
-1.1% -4 -1.1% -4 UavcanNode::cb_getset()
-3.4% -4 -3.4% -4 UavcanNode::cb_restart()
-4.5% -4 -4.5% -4 UavcanNode::param_opcode()
-1.4% -4 -1.4% -4 UavcanNode::publish_node_statuses()
-0.8% -8 -0.8% -8 UavcanNode::UavcanNode()
-1.4% -8 -1.4% -8 UavcanNode::~UavcanNode()
-0.7% -12 -0.7% -12 UavcanNode::Run()
+0.0% +67 [ = ] 0 .debug_abbrev
+0.0% +24 [ = ] 0 .debug_aranges
+0.0% +92 [ = ] 0 .debug_frame
+0.1% +14.3Ki [ = ] 0 .debug_info
+0.0% +511 [ = ] 0 .debug_line
+150% +3 [ = ] 0 [Unmapped]
+0.0% +508 [ = ] 0 [section .debug_line]
+0.0% +352 [ = ] 0 .debug_loclists
+0.0% +92 [ = ] 0 .debug_rnglists
+100% +1 [ = ] 0 [Unmapped]
+0.0% +91 [ = ] 0 [section .debug_rnglists]
+0.0% +896 [ = ] 0 .debug_str
-0.8% -2 [ = ] 0 .shstrtab
+0.0% +150 [ = ] 0 .strtab
[NEW] +52 [ = ] 0 uavcan::CanIOManager::getTxQueuePerfCounters()
[NEW] +47 [ = ] 0 uavcan::CanTxQueue::registerExpiredFrame()
[NEW] +51 [ = ] 0 uavcan::CanTxQueue::registerOutOfMemoryFrame()
+0.0% +96 [ = ] 0 .symtab
+20% +16 [ = ] 0 UavcanNode::print_params()
-50.0% -16 [ = ] 0 UavcanNode::update_params()
-0.3% -32 [ = ] 0 [section .symtab]
[NEW] +32 [ = ] 0 uavcan::CanIOManager::getTxQueuePerfCounters()
+100% +16 [ = ] 0 uavcan::CanIOManager::send()
+50% +16 [ = ] 0 uavcan::CanTxQueue::push()
[NEW] +32 [ = ] 0 uavcan::CanTxQueue::registerExpiredFrame()
[NEW] +32 [ = ] 0 uavcan::CanTxQueue::registerOutOfMemoryFrame()
-5.2% -296 [ = ] 0 [Unmapped]
+0.0% +16.5Ki +0.0% +296 TOTAL
Updated: 2026-08-26T21:21:14 |
Sorry, something went wrong.
mrpollo
pushed a commit
that referenced
this pull request
Sep 10, 2026
…ure (#28395) * fix(uavcan): count a TX queue block only once the pool provides it LimitedPoolAllocator::allocate() incremented used_blocks_ before asking the underlying allocator, so an allocation the pool could not satisfy still spent quota. Nothing ever hands that block back, because deallocate() only runs for a pointer that was returned, so every queue that asked for memory while the pool was empty stays permanently smaller than its configured limit. The pool is shared between the RX side and one TX queue per interface, so it does empty transiently under load -- a DroneCAN node firmware update is enough. The damage accumulates and is only cleared by restarting the node. * feat(uavcan): report TX queue depth and why frames were dropped `uavcan status` gains, per interface: TX queue peak: 44/84 blocks TX rejected: 80 frames (78 expired, 2 no memory) CanTxQueue already counted rejected frames but tx_queues_ is private and CanIOManager exposed no accessor, so the count was unreachable. It also conflated three causes: a frame handed over with its deadline already passed, a queued frame evicted as expired to make room, and a frame dropped because the queue was at its block limit. Only the last two are memory pressure, and they call for opposite responses, so a single count sends you the wrong way. The peak is what makes the count actionable. The limit is pool_soft / (num_ifaces + 1) + 1, derived from the soft capacity in allocator.hpp, so a peak sitting at the limit says raising that capacity will help and a peak well under it says the frames are ageing out for some other reason and a larger pool will not. Measured on an FMU-v6XRT serving a DroneCAN node firmware update: one node holds 44 of 84 blocks and drops nothing, another pins the limit and drops hundreds. The counts alone did not distinguish those. (cherry picked from commit 1888206) Assisted-by: GitHub Copilot:gpt-6-astra
mrpollo
pushed a commit
that referenced
this pull request
Sep 10, 2026
peek() discards entries whose deadline has passed before returning the head of the queue, and counted them with a bare registerRejectedFrame(). That is the fourth drop path; the reported split covers three. The frames landed in the total and in neither bucket, so `uavcan status` printed lines like TX rejected: 64 frames (0 expired, 0 no memory) which reads as "no drops worth caring about" when 64 frames of in-flight transfers had just been discarded. Serving a DroneCAN node firmware update hits this path on every interface. The total stays a separate counter rather than the sum of the two buckets: it is what makes an unclassified path visible at all, which is how this one was found. (cherry picked from commit c2cf9be) Assisted-by: GitHub Copilot:gpt-6-astra Backport-note: Companion to #28395: account for expired frames in the new TX-drop diagnostics.
mrpollo
pushed a commit
that referenced
this pull request
Sep 10, 2026
…ure (#28395) * fix(uavcan): count a TX queue block only once the pool provides it LimitedPoolAllocator::allocate() incremented used_blocks_ before asking the underlying allocator, so an allocation the pool could not satisfy still spent quota. Nothing ever hands that block back, because deallocate() only runs for a pointer that was returned, so every queue that asked for memory while the pool was empty stays permanently smaller than its configured limit. The pool is shared between the RX side and one TX queue per interface, so it does empty transiently under load -- a DroneCAN node firmware update is enough. The damage accumulates and is only cleared by restarting the node. * feat(uavcan): report TX queue depth and why frames were dropped `uavcan status` gains, per interface: TX queue peak: 44/84 blocks TX rejected: 80 frames (78 expired, 2 no memory) CanTxQueue already counted rejected frames but tx_queues_ is private and CanIOManager exposed no accessor, so the count was unreachable. It also conflated three causes: a frame handed over with its deadline already passed, a queued frame evicted as expired to make room, and a frame dropped because the queue was at its block limit. Only the last two are memory pressure, and they call for opposite responses, so a single count sends you the wrong way. The peak is what makes the count actionable. The limit is pool_soft / (num_ifaces + 1) + 1, derived from the soft capacity in allocator.hpp, so a peak sitting at the limit says raising that capacity will help and a peak well under it says the frames are ageing out for some other reason and a larger pool will not. Measured on an FMU-v6XRT serving a DroneCAN node firmware update: one node holds 44 of 84 blocks and drops nothing, another pins the limit and drops hundreds. The counts alone did not distinguish those. (cherry picked from commit 1888206) Assisted-by: GitHub Copilot:gpt-6-astra
mrpollo
pushed a commit
that referenced
this pull request
Sep 10, 2026
peek() discards entries whose deadline has passed before returning the head of the queue, and counted them with a bare registerRejectedFrame(). That is the fourth drop path; the reported split covers three. The frames landed in the total and in neither bucket, so `uavcan status` printed lines like TX rejected: 64 frames (0 expired, 0 no memory) which reads as "no drops worth caring about" when 64 frames of in-flight transfers had just been discarded. Serving a DroneCAN node firmware update hits this path on every interface. The total stays a separate counter rather than the sum of the two buckets: it is what makes an unclassified path visible at all, which is how this one was found. (cherry picked from commit c2cf9be) Assisted-by: GitHub Copilot:gpt-6-astra Backport-note: Companion to #28395: account for expired frames in the new TX-drop diagnostics.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two changes to the DroneCAN TX path: a fix for a block-accounting bug that permanently shrinks a TX queue, and per-interface queue diagnostics in uavcan status.
Problem
LimitedPoolAllocator::allocate() incremented used_blocks_ before asking the underlying allocator for the block. When the shared pool could not satisfy the request, the quota was spent on a block that was never returned, and nothing gives it back — deallocate() only runs for a pointer that was handed out. Every queue that asked for memory while the pool was empty stays permanently smaller than its configured limit, and the loss accumulates until the node restarts. The pool is shared between the RX side and one TX queue per interface, so it does empty transiently under load; serving a node firmware update is enough to do it.
Separately, there was no way to see any of this from a running system. CanTxQueue counts rejected frames, but tx_queues_ is private and CanIOManager exposed no accessor. The count also conflates three distinct causes — a frame arriving with its deadline already passed, a queued frame evicted as expired to make room, and a frame dropped with the queue at its block limit. Only the latter two are memory pressure, and the responses are opposite, so one number points the wrong way as often as not.
Solution
Count the block only after the allocator returns one.
Add CanIOManager::getTxQueuePerfCounters() and split the reject count, so uavcan status reports per interface:
The peak against the limit is what makes the count actionable. The limit is pool_soft / (num_ifaces + 1) + 1, derived from CapacitySoftLimit in allocator.hpp, so a peak sitting at the limit says raising that capacity will help, and a peak well under it says frames are ageing out for another reason and a larger pool will not.
Measured on an FMU-v6XRT serving DroneCAN node firmware updates over two buses: one node holds 44 of 84 blocks and drops nothing, while another pins the limit and drops hundreds per update. The raw counts did not distinguish those, and quadrupling CapacitySoftLimit moved neither node's download time — the second node simply refilled the larger queue (84/84, then 167/167, then 334/334).