Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02bdf964e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| logger := &logging.DefaultLoggerFactory{Writer: io.Discard, DefaultLogLevel: logging.LogLevelDisabled} | ||
| options := []ice.AgentOption{ | ||
| ice.WithUDPMux(s.mux), ice.WithUDPMuxSrflx(s.mux), |
There was a problem hiding this comment.
When two ICE agents on this socket connect to agents on the same remote process-owned socket, their remote IP:port is identical. Pion's mux maps each remote address to only one connection, and connWorker consults the STUN username only when that address has no existing destination (Pion v4.4.8 implementation). Consequently, the second agent's checks can reach the first agent, or registering the second agent can redirect the first agent's packets, preventing both direct paths from working concurrently. The new test uses only one agent per socket and misses this case. Share one ICE association per remote socket and demultiplex logical peers above it, or provide session-aware packet dispatch instead of passing every agent the same address-keyed mux.
Useful? React with 👍 / 👎.
|
superseded by the replacement cutover tracked in #499 (replacement stack #491). closing this original PR at the author's request so only the replacement review remains active. the original branch, commits, reviews and failure evidence are preserved; no branch is deleted. outstanding integration and validation remain tracked in the replacement, not waived by this closure. |
What
Add shared-socket connectivity discovery around the packet switching owner from #471. Relay and standalone answer STUN on their existing QUIC UDP socket, and peer processes use one explicitly owned socket for relay QUIC, mapping discovery, ICE checks, and direct packets. This foundation feeds the native rewrite whose final integration gate remains #470.
Why
A NAT mapping discovered on one socket cannot establish direct delivery from another. ICE preparation and network changes also must not hold up warm relay traffic or replace the end-to-end QUIC session.
Changes