Repository navigation
Conversation
lwIP hands a PPP frame to the output callback in 256 byte chunks and the callback checked uart space per chunk, so a frame larger than one chunk could be partly written and then abandoned. That put a bad frame on the wire and left the packet queued in lwIP. On a busy link a TCP segment could never get through, and with lwIP retrying it on every send() the link filled with fragments and the port thread burnt CPU at high priority, starving lower priority threads such as scripting. Stage the chunks and write the frame once it is complete. A frame that does not fit waits for uart space instead of being dropped, so large frames are not starved by small ones.
When a TCP port could not send, the port thread retried every 100us and lwIP re-ran tcp_output on each attempt. Wait for send space instead. Also discard queued output when a connection closes so it is not sent to the next client.
|
Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting. Reviewed at head Thanks, the PPP side looks good. We traced it through lwIP and ran host harnesses. Only complete frames reach the UART, refused segments stay on lwIP's unsent queue and are retried, and LCP still gets past a pending data frame. CubeOrangePlus builds and Plane.PPPPeriph passes at this head. We couldn't reproduce the saturated-link stall itself at runtime, so that part is checked from source only. Two small points, neither blocking: The second commit says it discards queued output on close so it isn't sent to the next client. That's true for bytes queued before the close (https://github.com/ArduPilot/ardupilot/pull/34803/files#diff-05c582baf2eb8e5bae9973bc176d02c3534f962f10fe9a5379e5cdcd92a6e412R482). But Port::_write() and txspace() don't check for a socket, so GCS output refills the buffer straight away. In a Rover SITL probe with a 10 s gap between clients, the new client's first SYSTEM_TIME was 10.9 s old, the same as on master. Clearing writebuffer when a connection is accepted or connected, or dropping writes while sock is null, would make the message hold. If the UART TX buffer comes up smaller than 8192 because set_size_best() fell back under memory pressure (down to 2592), a heavily escaped frame bigger than that can never pass the check in write_frame() (https://github.com/ArduPilot/ardupilot/pull/34803/files#diff-0b4fc8ce0bccb1fee6585c8d8890302caf17972cf55174734fab4a05d1f07d11R156). tx.pending then stays set and blocks all data while LCP keeps the link alive. A host harness with the extracted code reproduces this. It's unlikely with TCP_MSS 1024, but dropping a staged frame that can't fit the real buffer, or one that has been pending too long, would cover it cheaply. CI was still running when we checked (17 jobs in progress, none failed). |
Global Size Summary (Total Flash delta in bytes)Built from 00131c7 rebased onto afeb9c4
|
2 participants
Summary
Fix TCP sessions over PPP stalling permanently when the PPP link is near saturation, and the port thread spinning while they are stuck, which starved lower priority threads (Lua) in a flight log.
This is the network side of #34769
Classification & Testing (check all that apply and add your own)
Checked by a human programmer
Non-functional change
No-binary change
Infrastructure change (e.g. unit tests, helper scripts)
Automated test(s) verify changes (e.g. unit test, autotest)
Tested manually, description below (e.g. SITL)
Tested on hardware
Logs attached
Logs available on request
Rover.NetworkingWebServerPPP and Plane.PPPPeriph autotests pass
Reproduced in Renode emulation of an H7 flight controller with PPP to a DroneNet periph, with the PPP link saturated by log streaming and a MAVLink TCP client on a second network port. Before: the TCP session never received a heartbeat and the main loop had gaps of up to 2.2 s. After: the session stayed healthy, all parameters were fetched over MAVFTP, and the largest loop gap was 369 ms
Description