Improve LAN direct-path discovery and connection reliability - #853
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughAdds relay-less LAN peer bootstrapping to ChangesLAN Direct-Path Bootstrapping
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over Runtime,Node: Startup
Runtime->>Runtime: effective_quic_bind_ip() → detect_primary_lan_ipv4()
Runtime->>Node: start(bind_ip)
Runtime->>spawn_mdns_reverse_dial: spawn(options, node)
spawn_mdns_reverse_dial->>publish_lan_loop: spawn (ep_addr in TXT)
spawn_mdns_reverse_dial->>mdns_reverse_dial: run_loop spawn
spawn_mdns_reverse_dial->>lan_beacon: spawn (multicast listener)
end
rect rgba(144, 238, 144, 0.5)
Note over lan_beacon,Node: Beacon path
lan_beacon->>lan_beacon: emit_beacon (multicast + unicast to peer LAN IPs)
lan_beacon->>lan_beacon: handle_beacon (parse, validate, mesh-id check)
lan_beacon->>Node: dial_peer_addr(EndpointAddr)
end
rect rgba(255, 218, 185, 0.5)
Note over mdns_reverse_dial,Node: mDNS reverse-dial path
mdns_reverse_dial->>discover_lan_on_interface: browse (interface-pinned)
discover_lan_on_interface-->>mdns_reverse_dial: LanDiscoveredMesh[].endpoint_addr
mdns_reverse_dial->>Node: dial_peer_addr(EndpointAddr)
end
rect rgba(255, 182, 193, 0.5)
Note over Runtime,lan_bootstrap_tasks: Shutdown
Runtime->>lan_bootstrap_tasks: abort()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
crates/mesh-llm-host-runtime/src/network/lan_beacon.rs (3)
128-161: 💤 Low value
spawn_blockingpanics are silently discarded.The
.ok()on line 159 silently drops any panic from the blocking task. While this is acceptable for best-effort emission, consider at least logging join errors for debugging multi-homed host issues:- tokio::task::spawn_blocking(move || emit_blocking(mcast, &peers, &payload)) - .await - .ok(); + if let Err(err) = tokio::task::spawn_blocking(move || emit_blocking(mcast, &peers, &payload)).await { + tracing::trace!("LAN beacon spawn_blocking failed: {err}"); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/mesh-llm-host-runtime/src/network/lan_beacon.rs` around lines 128 - 161, The spawn_blocking task result is being silently discarded with .ok() at the end of the emit_beacon function, which hides any errors or panics from the emit_blocking closure. Replace the .ok() call with proper error handling that logs failures using an appropriate logging mechanism (such as the pattern used elsewhere in the codebase for debug or warn level logging), while still returning Ok(()) from emit_beacon to maintain the best-effort emission behavior. This will provide visibility into multi-homed host issues during debugging without changing the overall function semantics.
32-46: 💤 Low valueNote:
BEACON_GROUPreuses the mDNS multicast address.224.0.0.251 is the IANA-assigned mDNS multicast address. While using a different port (47654 vs 5353) makes this technically distinct, some network equipment or firewalls may treat this address specially for mDNS. This appears intentional based on the comment about link-local routing, but worth documenting explicitly that mDNS's address is being reused (which may actually be beneficial for firewall traversal).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/mesh-llm-host-runtime/src/network/lan_beacon.rs` around lines 32 - 46, The comment for the BEACON_GROUP constant does not explicitly document that 224.0.0.251 is the IANA-assigned mDNS multicast address. Update the comment block above the BEACON_GROUP definition to explicitly mention that this multicast address is the same as mDNS's address (224.0.0.251), and clarify that while a different port (47654 versus mDNS's 5353) ensures technical distinctness, the intentional address reuse is a deliberate design choice that may benefit firewall traversal.
163-189: 💤 Low valueSocket-per-send is intentional but costly.
Creating a new socket for every multicast and unicast send is documented as necessary to avoid
EHOSTUNREACHon multi-homed macOS hosts. This is fine for correctness, but creates significant overhead at the 5-second beacon interval with multiple peers.If performance becomes a concern, consider caching the send socket and only recreating on send failure. For now this is acceptable given the low beacon frequency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/mesh-llm-host-runtime/src/network/lan_beacon.rs` around lines 163 - 189, The review comment notes that creating a new socket for every multicast and unicast send in the emit_blocking, send_multicast, and send_unicast functions is intentional for correctness on multi-homed macOS hosts but has performance implications. While acceptable for now given the low beacon frequency, add comprehensive code comments explaining this deliberate design trade-off and document the potential optimization approach of caching the send socket and only recreating on send failure for future reference when performance becomes a concern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/mesh-llm-host-runtime/src/mesh/mod.rs`:
- Around line 4851-4865: The function known_peer_lan_ipv4 currently returns all
IPv4 candidates from peer gossip without filtering, which causes public and STUN
addresses to be included in the beacon results. This allows mesh metadata and
beacon traffic to be sent off-LAN. Modify the function to filter the results
using is_private_lan_ipv4 to only include RFC1918 private LAN addresses before
adding them to the output vector. The same filtering should also be applied to
the similar helper function at lines 5008-5021.
- Around line 507-522: The first_private_lan_interface_ipv4 function currently
filters for any RFC1918 private address but incorrectly includes Docker and CNI
bridge addresses (such as 172.* ranges) which are host-local and reused across
machines. This causes QUIC to pin to docker0/cni0 instead of the actual LAN NIC,
breaking direct dialing. Add an additional filter in the chain after the
is_private_lan_ipv4 check to explicitly exclude container bridge address ranges
(such as the 172.17.0.0/16 range used by Docker and similar ranges used by other
container runtimes). This filter should reject these ranges while still
accepting genuine LAN interfaces.
- Around line 447-522: Create a semantically-named submodule for LAN bootstrap
functionality to address the excessive size and mixed responsibilities in
mesh/mod.rs. Move the LAN IP detection functions (detect_primary_lan_ipv4,
is_private_lan_ipv4, default_route_source_ipv4,
first_private_lan_interface_ipv4) from lines 447-522 and their associated tests
to a new module file (e.g.,
crates/mesh-llm-host-runtime/src/mesh/lan_bootstrap.rs). Similarly, move the
join-target tracking code and tests from lines 642-686, 2405-2408, and 4821-5021
into appropriately named submodules or consolidate them based on semantic
coherence. Update all references in mesh/mod.rs to use the new module paths, and
declare these new modules in mesh/mod.rs via mod statements so they remain
accessible to the rest of the codebase.
- Around line 4829-4835: The dial_peer_addr method calls connect_to_peer which
returns early for peers still in the dead_peers collection, preventing freshly
advertised EndpointAddrs from being retried within the 5-minute dead peer TTL.
Before calling connect_to_peer in dial_peer_addr, clear the dead peer gate by
removing the peer from the dead_peers collection (or calling the appropriate
override method) just like join and join_with_retry already do, so that freshly
advertised peers can bypass the dead peer TTL check.
In `@crates/mesh-llm-host-runtime/src/mesh/tests.rs`:
- Around line 3156-3192: The test
`remember_join_target_updates_address_on_peer_rebind` is being added to an
already oversized tests.rs file, which violates the coding guideline to split
files that exceed 1,000 lines and contain multiple separable responsibilities.
Extract this test into a new semantically named test module (such as
`join_target_tests` or `dial_target_tests`) that groups join-target and
dial-target behavior concerns together. Move the entire test function and any
shared test utilities it depends on into this new module, then remove it from
the root test file to improve navigation and ownership clarity.
In `@crates/mesh-llm-host-runtime/src/runtime/mod.rs`:
- Around line 6064-6084: Extract the QUIC bind IP selection and LAN bootstrap
subsystem from runtime/mod.rs by moving the effective_quic_bind_ip function
(shown at lines 6064-6084) and the related LAN bootstrap orchestration code
(referenced at lines 7962-8032, including spawn_mdns_reverse_dial and associated
task/publisher/reverse-dial/beacon wiring) into a new semantically named module
under crates/mesh-llm-host-runtime/src/network/. Update all imports and
references in runtime/mod.rs to point to the new module, and consider renaming
spawn_mdns_reverse_dial to better reflect its expanded responsibilities once
extracted from the runtime context.
- Around line 7967-7977: The `LanBootstrapTasks` struct with its `abort()`
method only cancels handles when `shutdown_run_auto_runtime()` is called, but
early returns or drops from error paths (like those after
`build_run_auto_node_setup()` or `RunAutoModelSelection::Shutdown` early return)
leave mDNS/beacon tasks running with active sockets. Implement the `Drop` trait
for `LanBootstrapTasks` to automatically call `abort()` whenever the struct is
dropped, ensuring handles are properly cleaned up on every exit path regardless
of how the scope is exited.
---
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/network/lan_beacon.rs`:
- Around line 128-161: The spawn_blocking task result is being silently
discarded with .ok() at the end of the emit_beacon function, which hides any
errors or panics from the emit_blocking closure. Replace the .ok() call with
proper error handling that logs failures using an appropriate logging mechanism
(such as the pattern used elsewhere in the codebase for debug or warn level
logging), while still returning Ok(()) from emit_beacon to maintain the
best-effort emission behavior. This will provide visibility into multi-homed
host issues during debugging without changing the overall function semantics.
- Around line 32-46: The comment for the BEACON_GROUP constant does not
explicitly document that 224.0.0.251 is the IANA-assigned mDNS multicast
address. Update the comment block above the BEACON_GROUP definition to
explicitly mention that this multicast address is the same as mDNS's address
(224.0.0.251), and clarify that while a different port (47654 versus mDNS's
5353) ensures technical distinctness, the intentional address reuse is a
deliberate design choice that may benefit firewall traversal.
- Around line 163-189: The review comment notes that creating a new socket for
every multicast and unicast send in the emit_blocking, send_multicast, and
send_unicast functions is intentional for correctness on multi-homed macOS hosts
but has performance implications. While acceptable for now given the low beacon
frequency, add comprehensive code comments explaining this deliberate design
trade-off and document the potential optimization approach of caching the send
socket and only recreating on send failure for future reference when performance
becomes a concern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d413d65d-69f7-4f73-9a52-234038558cf3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
crates/mesh-llm-host-runtime/Cargo.tomlcrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/tests.rscrates/mesh-llm-host-runtime/src/network/discovery.rscrates/mesh-llm-host-runtime/src/network/lan_beacon.rscrates/mesh-llm-host-runtime/src/network/mdns_reverse_dial.rscrates/mesh-llm-host-runtime/src/network/mod.rscrates/mesh-llm-host-runtime/src/runtime/mod.rs
Contributor
|
This pull request has not been updated in at least 5 days. It will be closed after 7 days of inactivity to keep the active review queue current. Please update it within 2 days if the changes are still moving forward. |
Auto-pin QUIC to the primary private LAN interface so relay-less direct paths land on the fast path, add an mDNS reverse-dial loop and a UDP multicast LAN beacon so peers find each other regardless of which node hosts, and advertise the endpoint address over mDNS for dial-back. Tear down the LAN bootstrap loops on shutdown so they release sockets and stop dialing, and refresh stale dial-back targets when a peer rebinds to a new address under the same id.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
ndizazzo
force-pushed
the
micn/direct-lan-improvements
branch
from
June 21, 2026 21:53
4519e75 to
5f7bcf5
Compare
ndizazzo
approved these changes
Jun 21, 2026
michaelneale
added a commit
that referenced
this pull request
Jun 23, 2026
* origin/main: update guides for dev loop (#895) Add native MTP generation metadata to layer packages (#888) upgrade iroh to 1.0 (#894) fix(runtime): support relocating shared libs Improve LAN direct-path discovery and connection reliability (#853) Add GLM chat template fallback in llama (#890) # Conflicts: # crates/mesh-llm-config/src/model/built_in_schema.rs # crates/mesh-llm-config/src/validate.rs # crates/mesh-llm-host-runtime/src/inference/skippy/deployment.rs # crates/mesh-llm-host-runtime/src/inference/skippy/materialization.rs # crates/mesh-llm-host-runtime/src/inference/skippy/resolver/speculative.rs # crates/mesh-llm-host-runtime/src/inference/skippy/resolver/translation.rs # crates/mesh-llm-host-runtime/src/inference/skippy/stage/mod.rs # crates/mesh-llm-host-runtime/src/inference/skippy/stage/tests.rs # crates/mesh-llm-host-runtime/src/inference/skippy/stage/types.rs # crates/mesh-llm-host-runtime/src/mesh/mod.rs # crates/mesh-llm-host-runtime/src/mesh/tests.rs # crates/mesh-llm-host-runtime/src/runtime/local.rs # crates/skippy-protocol/proto/stage.proto # crates/skippy-server/src/binary_transport.rs # crates/skippy-server/src/frontend.rs # crates/skippy-server/src/frontend/embedded_execution.rs # crates/skippy-server/src/frontend/embedded_generation.rs # crates/skippy-server/src/frontend/generation_flow.rs # crates/skippy-server/src/frontend/prefix_cache.rs # docs/skippy/CONFIGURATION.md
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.
Makes mesh-llm find and use fast LAN direct paths more reliably, so peers on the same network connect directly instead of falling back to the relay.
What changed
Validation
cargo fmt/cargo clippy --all-targets -D warnings— cleancargo test -p mesh-llm-host-runtime --libmesh + network suites pass (incl. newremember_join_target_updates_address_on_peer_rebindtest)Summary by CodeRabbit
New Features
Tests
Chores