v0.2 rewrite #2
v0.2 rewrite #2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR represents a major rewrite of the lantun codebase for version 0.2, transitioning from a workspace-based architecture to a unified single-package structure with a completely redesigned tunnel implementation.
Key Changes:
- Consolidated the multi-crate workspace (lantun-cli, lantun-core) into a single package structure
- Rewrote the tunnel architecture with improved state management using Rust's type state pattern (Stopped/Running states)
- Implemented a new forwarder module for bidirectional data flow between local sockets and QUIC streams
- Added comprehensive test coverage for both TCP and UDP tunnel operations including concurrent connections and large data transfers
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Converted from workspace root to single package configuration with updated dependencies |
| src/main.rs | New main entry point with CLI argument parsing and tunnel management |
| src/host.rs | Complete rewrite of host tunnel implementation with improved connection handling |
| src/client.rs | Complete rewrite of client tunnel implementation with better state management |
| src/forwarder.rs | New bidirectional data forwarding module for tunnel communication |
| src/config.rs | New configuration loading and persistence module |
| src/common.rs | Common types including Protocol enum and async I/O wrappers for UDP |
| src/tests/mod.rs | Test module declarations for TCP and UDP tunnel tests |
| src/tests/test_tcp_tunnel.rs | Comprehensive TCP tunnel test suite with 5 test scenarios |
| src/tests/test_udp_tunnel.rs | Comprehensive UDP tunnel test suite with 5 test scenarios |
| .gitignore | Minor cleanup with redundant commented entries |
| lantun-core/* | Removed entire lantun-core crate as part of consolidation |
| lantun-cli/* | Removed entire lantun-cli crate as part of consolidation |
| .typos.toml | Removed configuration file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // for some reason packets above ~10KB are dropped even locally (linux) | ||
| // TODO: figure out if this needs to be fixed |
There was a problem hiding this comment.
This TODO comment should be addressed or removed. If the issue with packets above ~10KB being dropped is a known limitation, consider documenting it in a more formal way (e.g., in module documentation or a GitHub issue) rather than leaving it as a TODO in the test code. If this is something that will be fixed, create a tracking issue and reference it here.
| // for some reason packets above ~10KB are dropped even locally (linux) | |
| // TODO: figure out if this needs to be fixed | |
| // Note: packets above ~10KB have been observed to be dropped even on local | |
| // Linux setups, so this test uses 9KB to avoid environment-specific issues. |
src/client.rs
Outdated
| println!( | ||
| "Bound socket at {} for client at {}", | ||
| addr, local_client_addr | ||
| ); | ||
|
|
||
| println!( |
There was a problem hiding this comment.
These println! statements should be replaced with tracing logs (e.g., tracing::debug! or tracing::info!) to maintain consistency with the logging strategy used throughout the codebase.
| println!( | |
| "Bound socket at {} for client at {}", | |
| addr, local_client_addr | |
| ); | |
| println!( | |
| tracing::info!( | |
| "Bound socket at {} for client at {}", | |
| addr, local_client_addr | |
| ); | |
| tracing::info!( |
| local_addr | ||
| ); | ||
|
|
||
| // let (stream_read, stream_write) = stream.into_split(); |
There was a problem hiding this comment.
This commented-out code should be removed as it appears to be leftover from refactoring.
| TcpSocket::new_v4().unwrap() | ||
| } else { | ||
| TcpSocket::new_v6().unwrap() | ||
| }; | ||
|
|
||
| socket.bind(client_addr).unwrap(); |
There was a problem hiding this comment.
These unwrap() calls could cause panics in production. TcpSocket::new_v4() and TcpSocket::new_v6() can fail if the system runs out of file descriptors or in other resource-constrained scenarios. socket.bind() can also fail for various reasons. Consider handling these errors properly by logging and returning early, similar to how the socket.connect() error is handled on line 108.
| TcpSocket::new_v4().unwrap() | |
| } else { | |
| TcpSocket::new_v6().unwrap() | |
| }; | |
| socket.bind(client_addr).unwrap(); | |
| let Ok(socket) = TcpSocket::new_v4() else { | |
| tracing::error!( | |
| "Failed to create IPv4 TCP socket for local service at {}", | |
| local | |
| ); | |
| return; | |
| }; | |
| socket | |
| } else { | |
| let Ok(socket) = TcpSocket::new_v6() else { | |
| tracing::error!( | |
| "Failed to create IPv6 TCP socket for local service at {}", | |
| local | |
| ); | |
| return; | |
| }; | |
| socket | |
| }; | |
| if let Err(e) = socket.bind(client_addr) { | |
| tracing::error!( | |
| "Failed to bind TCP socket for local service at {}: {}", | |
| local, | |
| e | |
| ); | |
| return; | |
| } |
No description provided.