AP_Networking: accept UDP client replies from a different source port - #34227
Open
timtuxworth wants to merge 1 commit into
Open
AP_Networking: accept UDP client replies from a different source port#34227timtuxworth wants to merge 1 commit into
timtuxworth wants to merge 1 commit into
Conversation
NET_Pn set to UDP_CLIENT connect()'d its socket to the configured destination, which makes the kernel filter incoming packets to match that exact source address *and port*. That's fine for the common case of a device that replies from the same socket it was queried on, but some devices (confirmed against a real Topotek KHP415 gimbal) reply from a different, fixed source port instead - every reply was silently dropped before ArduPilot's own code ever saw it, regardless of NET_Pn config being otherwise correct. For a unicast destination, stop calling connect() and use sendto()/an unconnected recv() instead, checking the source IP ourselves (but not the port) before accepting a packet. Broadcast and multicast destinations keep the original connect()-based path unchanged, since connect() also does necessary setup for them (joining the multicast group via IP_ADD_MEMBERSHIP) unrelated to this fix, and neither is a point-to-point relationship that could hit this problem in the first place. Verified against the real KHP415 (which replies from a fixed but different port than it's queried on) by hand-crafting its wire protocol and sending it from an unconnected socket. Regression-tested against the existing TestLogDownloadMAVProxyNetwork suite (unicast/ multicast/broadcast UDP client, UDP server, TCP client/server) and the full AP_Mount network autotest suite - no regressions.
9 tasks
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
Fix
AP_NetworkingUDP client sockets silently dropping replies from devices that reply on a different source port than the one they were queried on.Classification & Testing (check all that apply and add your own)
Regression-tested against the existing
TestLogDownloadMAVProxyNetworkautotest (covers unicast/multicast/broadcast UDP client, UDP server, and TCP client/server) and the fullAP_Mountnetwork autotest suite (MountSkyDroidNetwork,MountTopotekNetwork) — no regressions. The root cause was confirmed manually by hand-crafting the real device's wire protocol against an unconnected socket, and the fix was validated on real hardware: a Cube-based flight controller connected to a Topotek KHP415 gimbal overNET_Pnas a network mount.Description
NET_Pnset toUDP_CLIENTcallsconnect()on its socket, which makes the kernel filter incoming packets by source IP and port. That's fine for a device that replies from the same socket it was queried on, but some devices reply from a different, fixed source port instead — every reply is silently dropped before ArduPilot's own code ever sees it, even though theNET_Pnconfiguration is otherwise correct and matches the documented setup exactly. This was found via a real Topotek KHP415 gimbal connected as a network mount, which consistently reported "Mount: not healthy" for this reason.This PR skips
connect()for unicastUDP_CLIENTdestinations and usessendto()/an unconnectedrecv()instead, checking only the source IP (not the port) before accepting a reply. Broadcast and multicast destinations are left on the originalconnect()-based path unchanged, sinceconnect()also performs multicast-group-join setup (IP_ADD_MEMBERSHIP) for them that's unrelated to this fix, and neither is a point-to-point relationship that could hit this failure mode in the first place.This class of bug is structurally invisible to SITL, since simulated network devices always reply from the same bound socket they were queried on — port symmetry is guaranteed by construction there.
This PR was developed with AI assistance (Claude).