mirror of https://github.com/ospab/ostp.git
fix(tun): TUN routing failed on every connect, freezing under load
A user's log showed the same two failures on all 9 connects, 198 route errors total: Added 0 bypass routes via 192.168.88.1 (if_index=11) Could not find ostp_tun index in routing table after 15s — traffic will NOT be captured Two independent bugs in the Windows route layer: 1. The TUN interface index was looked up by matching FriendlyName == "ostp_tun" through GetAdaptersAddresses. WinTun does not set the FriendlyName to the adapter name, so the match never succeeded — every connect burned the full 15s window and gave up. The tun crate hands the real index back directly via AbstractDevice::tun_index() (WinTun's own adapter index), which is instant and correct; the name lookup remains only as a fallback. 2. Every route add — the server-IP bypass and the TUN default route alike — went through the legacy CreateIpForwardEntry, which failed with error 160 (ERROR_BAD_ARGUMENTS) on this machine for all of them. With the server-IP bypass never installed, the server's own packets were routed INTO the tunnel: a loop that stalls the link for seconds under load (the reported "VPN drops ~8s into a game" — the tunnel never actually disconnected, it froze; the log showed gap recovery skipping up to 402 frames with no packet loss on the wire). add_ipv4_route now shells to route.exe, which resolves the interface and validates the gateway itself and is already what the teardown path uses; its command form was verified to be accepted (fails only on elevation, not syntax). CREATE_NO_WINDOW keeps it from flashing a console per route. Cannot be verified without the user's elevated TUN environment; the next log will read "Added N bypass routes" and "Default route via TUN ... added" instead of the failures.
This commit is contained in:
parent
44677c68e4
commit
321365efe3
|
|
@ -99,18 +99,34 @@ pub async fn create(opts: OstpTunOptions) -> Result<OstpTunInterface> {
|
||||||
|
|
||||||
let current_exe = std::env::current_exe()?.to_string_lossy().into_owned();
|
let current_exe = std::env::current_exe()?.to_string_lossy().into_owned();
|
||||||
|
|
||||||
// A freshly created WinTun adapter can take several seconds to appear in
|
// Take the interface index straight from the adapter WinTun just created,
|
||||||
// GetAdaptersAddresses (it only shows up once it has an operational IPv4
|
// via the tun crate. The old code looked it up by FriendlyName == "ostp_tun"
|
||||||
// binding). The default route via the TUN is what actually captures
|
// through GetAdaptersAddresses — but WinTun does NOT set the FriendlyName to
|
||||||
// traffic, so this lookup is critical — give it a generous window (~15s).
|
// the adapter name, so that match never succeeded: on every single connect
|
||||||
let mut tun_index = None;
|
// it spun the full 15s and then gave up with "traffic will NOT be captured",
|
||||||
for _ in 0..75 {
|
// leaving the default route (and, above, the server-IP bypass) uninstalled.
|
||||||
if let Some(idx) = windows_route::sys::get_interface_index("ostp_tun") {
|
// get_adapter_index() is instant and correct.
|
||||||
tun_index = Some(idx);
|
use tun::AbstractDevice;
|
||||||
break;
|
let tun_index = match dev.tun_index() {
|
||||||
|
Ok(idx) if idx > 0 => Some(idx as u32),
|
||||||
|
Ok(idx) => {
|
||||||
|
tracing::error!("WinTun reported a non-positive interface index ({idx})");
|
||||||
|
None
|
||||||
}
|
}
|
||||||
tokio::time::sleep(std::time::Duration::from_millis(200)).await;
|
Err(e) => {
|
||||||
}
|
// Fall back to the old name lookup rather than fail outright.
|
||||||
|
tracing::warn!("Could not read TUN index from the adapter ({e}); falling back to name lookup");
|
||||||
|
let mut idx = None;
|
||||||
|
for _ in 0..75 {
|
||||||
|
if let Some(i) = windows_route::sys::get_interface_index("ostp_tun") {
|
||||||
|
idx = Some(i);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
tokio::time::sleep(std::time::Duration::from_millis(200)).await;
|
||||||
|
}
|
||||||
|
idx
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
if let Some(idx) = tun_index {
|
if let Some(idx) = tun_index {
|
||||||
match windows_route::sys::add_ipv4_route(
|
match windows_route::sys::add_ipv4_route(
|
||||||
|
|
|
||||||
|
|
@ -17,7 +17,7 @@ pub mod sys {
|
||||||
use winapi::shared::minwindef::{DWORD, ULONG};
|
use winapi::shared::minwindef::{DWORD, ULONG};
|
||||||
use winapi::shared::winerror::{ERROR_INSUFFICIENT_BUFFER, NO_ERROR};
|
use winapi::shared::winerror::{ERROR_INSUFFICIENT_BUFFER, NO_ERROR};
|
||||||
use winapi::um::iphlpapi::{
|
use winapi::um::iphlpapi::{
|
||||||
CreateIpForwardEntry, DeleteIpForwardEntry, GetAdaptersAddresses, GetIpForwardTable,
|
DeleteIpForwardEntry, GetAdaptersAddresses, GetIpForwardTable,
|
||||||
};
|
};
|
||||||
use winapi::um::iptypes::{
|
use winapi::um::iptypes::{
|
||||||
GAA_FLAG_SKIP_ANYCAST, GAA_FLAG_SKIP_DNS_SERVER, GAA_FLAG_SKIP_MULTICAST, IP_ADAPTER_ADDRESSES,
|
GAA_FLAG_SKIP_ANYCAST, GAA_FLAG_SKIP_DNS_SERVER, GAA_FLAG_SKIP_MULTICAST, IP_ADAPTER_ADDRESSES,
|
||||||
|
|
@ -88,20 +88,47 @@ pub mod sys {
|
||||||
if_index: u32,
|
if_index: u32,
|
||||||
metric: u32,
|
metric: u32,
|
||||||
) -> Result<(), String> {
|
) -> Result<(), String> {
|
||||||
let mut row: MIB_IPFORWARDROW = unsafe { mem::zeroed() };
|
// Installed through route.exe rather than CreateIpForwardEntry.
|
||||||
row.dwForwardDest = ipv4_to_dword(dest);
|
//
|
||||||
row.dwForwardMask = ipv4_to_dword(mask);
|
// The legacy CreateIpForwardEntry API was failing here with error 160
|
||||||
row.dwForwardNextHop = ipv4_to_dword(nexthop);
|
// (ERROR_BAD_ARGUMENTS) on every single route — server-IP bypass and TUN
|
||||||
row.dwForwardIfIndex = if_index;
|
// default route alike — which left the server IP routed INTO the tunnel
|
||||||
row.ForwardType = if nexthop == Ipv4Addr::UNSPECIFIED || dest == nexthop { 3 } else { 4 };
|
// (a loop that froze the link for seconds under load) and the default
|
||||||
row.ForwardProto = 3; // MIB_IPPROTO_NETMGMT
|
// route uninstalled. route.exe resolves the interface and validates the
|
||||||
row.dwForwardMetric1 = metric;
|
// gateway itself, and is what the teardown path already uses, so it
|
||||||
|
// succeeds where the hand-built MIB_IPFORWARDROW did not.
|
||||||
|
use std::os::windows::process::CommandExt;
|
||||||
|
use std::process::Command;
|
||||||
|
const CREATE_NO_WINDOW: u32 = 0x0800_0000;
|
||||||
|
|
||||||
let ret = unsafe { CreateIpForwardEntry(&mut row) };
|
// route add <dest> mask <mask> <gateway> metric <m> if <ifindex>
|
||||||
if ret == NO_ERROR {
|
let out = Command::new("route")
|
||||||
|
.args([
|
||||||
|
"add",
|
||||||
|
&dest.to_string(),
|
||||||
|
"mask",
|
||||||
|
&mask.to_string(),
|
||||||
|
&nexthop.to_string(),
|
||||||
|
"metric",
|
||||||
|
&metric.to_string(),
|
||||||
|
"if",
|
||||||
|
&if_index.to_string(),
|
||||||
|
])
|
||||||
|
.creation_flags(CREATE_NO_WINDOW)
|
||||||
|
.output()
|
||||||
|
.map_err(|e| format!("could not run route.exe: {e}"))?;
|
||||||
|
|
||||||
|
if out.status.success() {
|
||||||
Ok(())
|
Ok(())
|
||||||
} else {
|
} else {
|
||||||
Err(format!("CreateIpForwardEntry failed: {}", ret))
|
// route.exe prints its diagnostics to stdout, not stderr.
|
||||||
|
let msg = String::from_utf8_lossy(&out.stdout);
|
||||||
|
let msg = msg.trim();
|
||||||
|
Err(format!(
|
||||||
|
"route add failed (exit {:?}): {}",
|
||||||
|
out.status.code(),
|
||||||
|
if msg.is_empty() { "no output" } else { msg }
|
||||||
|
))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue