extmod/modlwip: Poll the stack while waiting for TCP send memory. - #19627
Open
kwagyeman wants to merge 1 commit into
Open
extmod/modlwip: Poll the stack while waiting for TCP send memory.#19627kwagyeman wants to merge 1 commit into
kwagyeman wants to merge 1 commit into
Conversation
Contributor
Author
|
@dpgeorge - Found a large issue with TCP performance loss that affects all boards. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19627 +/- ##
=======================================
Coverage 98.58% 98.58%
=======================================
Files 182 182
Lines 23322 23322
Branches 5 5
=======================================
Hits 22993 22993
Misses 328 328
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Code size report: |
dpgeorge
reviewed
Aug 17, 2026
| // instead of sleeping blindly: a flat 50ms delay caps throughput at | ||
| // write_size/50ms whenever the heap is smaller than the send buffer. | ||
| mp_hal_delay_ms(1); | ||
| poll_sockets(); |
Member
There was a problem hiding this comment.
I think this can just be poll_sockets(), and remove the mp_hal_delay_ms(1). Because poll_sockets() already does a 1ms delay.
Also, no need for the comment. The 50ms no longer means anything.
But, the for loop itself needs updating to use mp_hal_ticks_ms() to timeout after 10s (similar to the other waiting loop above).
kwagyeman
force-pushed
the
kwabena/modlwip_send_retry
branch
from
August 17, 2026 23:11
df1add1 to
689b2e6
Compare
dpgeorge
reviewed
Aug 25, 2026
| if (err != ERR_OK) { | ||
| break; | ||
| } | ||
| if (mp_hal_ticks_ms() - write_start > 10000) { |
Member
There was a problem hiding this comment.
I suggest making the constant explicitly unsigned via 10000U, just to be on the safe side with the wrap around logic.
When tcp_write() returns ERR_MEM, lwip_tcp_send() retried on a flat 50ms delay. During that sleep nothing drives lwIP forward, so the condition the retry is waiting for (ACKs arriving and freeing queued segments) is only resolved by background polling, and every write that hits this path is stalled for a fixed 50ms. On any configuration whose lwIP heap is small relative to TCP_SND_BUF this becomes the dominant cost and caps TCP transmit throughput at roughly write_size per 50ms, with the socket reporting no error at all. Poll the stack via poll_sockets() (which includes a 1ms wait) instead of sleeping blindly, and time the retry loop out after 10 seconds using mp_hal_ticks_ms(), matching the sndbuf waiting loop above. Measured on an OpenMV RT1060 (CYW4343W WiFi, MEM_SIZE smaller than TCP_SND_BUF): sustained TCP transmit from a Python socket benchmark sat at a constant 1.3Mbit/s -- exactly its 8KB writes paced by the 50ms sleep. With this change the same benchmark reaches 14.4Mbit/s, and configurations with adequately sized heaps are unaffected. Signed-off-by: Kwabena W. Agyeman <kwagyeman@live.com>
kwagyeman
force-pushed
the
kwabena/modlwip_send_retry
branch
from
August 25, 2026 17:10
689b2e6 to
f928098
Compare
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
2 participants
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
When
tcp_write()returnsERR_MEM,lwip_tcp_send()retries on a flat50ms delay:
Nothing drives lwIP forward during that sleep, so the condition the retry
is waiting for — ACKs arriving and freeing queued segments — is only
resolved by background polling. Every write that takes this path stalls
for the full 50ms, and on any configuration whose lwIP heap is small
relative to
TCP_SND_BUFthis becomes the steady state: TCP transmitthroughput is silently capped at roughly write_size per 50ms, with no
error reported to the caller.
Replace the blind sleep with a 1ms delay plus
poll_sockets()— the samehelper the sndbuf wait loop earlier in the function already uses.
Measured on an OpenMV RT1060 (CYW4343W WiFi, 2.4GHz) with a configuration
whose heap is smaller than the send buffer (MEM_SIZE=10K,
TCP_SND_BUF=8*MSS), sustained TCP transmit from a Python socket benchmark
using 8KB writes:
The 1.3 Mbit/s figure is exactly 8KB per 50ms — the sleep, not the
network, sets the throughput. Configurations whose heap comfortably covers
the send buffer never enter this path and are unaffected (verified: an
adequately-provisioned config on the same board measures identically
before and after this change).
Testing
Runtime tested on an OpenMV RT1060 (CYW4343W WiFi) in both the
heap-constrained configuration above (numbers in the table) and an
adequately-provisioned configuration (no regression, path not taken).
TCP receive and UDP in both directions are unaffected in both
configurations.
Build-tested: NUCLEO_F767ZI, OPENMV_RT1060.
Generative AI
I used generative AI tools when creating this PR, but a human has checked
the code and is responsible for the code and the description above.