Compare commits

..
Author SHA1 Message Date
enricobuehler 76832a5b86 fix(client/apple): two DualSenses stop fighting over one device, and a failed stop stops lying
apple / swift (pull_request) Successful in 1m25s
ci / web (pull_request) Successful in 1m23s
ci / docs-site (pull_request) Successful in 1m24s
apple / screenshots (pull_request) Skipped
ci / rust-arm64 (pull_request) Successful in 1m48s
ci / rust (pull_request) Successful in 7m11s
Five faults in the Apple client's feedback path.

With two DualSenses attached, each pad's renderer opened "the first connected
DualSense" — taken from an unordered Set, so the choice could differ between
two calls in one process. Both renderers could land on the same device, one
pad's rumble coming out of the other while their per-instance write dedupes
fought over it, or they could split by luck. Each renderer now asks for the
device its own controller is, correlating GameController's stable ordering with
IOKit's location ids; the selection rule is a pure function so it can be tested
without an IOHIDDevice, which cannot be constructed. Without a preference the
lowest location id wins — still arbitrary, but stable, which Set.first was not.

A failed HID write was logged and swallowed, so a write that never reached the
device still counted as a successful render. That matters most for a stop,
which has nothing behind it: the renderer stamped its write clock even on
failure, the keepalive only re-writes non-zero levels, the ticker is cancelled
once the target is zero, and on USB there is no firmware timeout. A swallowed
stop therefore left the motors running with nothing scheduled to try again.
The write result now reaches the caller, which drops the handle and falls back
to CoreHaptics rather than claiming success.

A half-failed split-handle setup reported HEALTHY. Only the all-nil case
counted as failure, so one surviving handle passed silently while rendering
something wrong in a direction that depended on which handle died: lose the
right one and render falls to the combined branch, playing max(low, high) on
the LEFT handle; lose the left and the split branch discards the heavy motor
outright. A half-open split now tears the survivor down and takes the combined
path, which at least renders both motors somewhere.

Session end never put the lightbar out. This class is what turned it on, and
every DS write is valid-flag-selective, so a game's last colour stayed lit in
firmware after the stream ended — a DS4 was cleared incidentally because its
player indicator IS the lightbar, a DualSense was not.

And the renderer's stop() ran on the main actor. It is a queue.sync whose body
is a per-motor CHHapticEngine.stop() — an XPC round trip the renderer's own
notes record as able to hang — plus a blocking HID write to a device that has
just departed, and it queues behind any in-flight setup(). It runs on every
unplug and every pin change, and the main thread drives the presenter's
CADisplayLink, so it hitched the picture mid-stream. It is detached now; the
renderer is already off routing by then, so nothing observes it.

Verified: swift build clean, 188 tests pass (185 before), and the three new
device-selection tests fail if the deterministic fallback is reverted.

Note for anyone rebuilding here: the checked-in xcframework was stale (it
predates punktfunk_connection_report_phase) and build-xcframework.sh still dies
on this Mac at its macOS-floor guard. A macos-arm64 slice assembled by hand
from `cargo build --target aarch64-apple-darwin` is enough to typecheck.

From the 2026-08-03 force-feedback sweep (B14, B15, B18, B19, B20).
2026-08-04 08:20:12 +02:00
10 changed files with 172 additions and 350 deletions
@@ -21,8 +21,12 @@ import os
private let log = Logger(subsystem: "io.unom.punktfunk", category: "gamepad")
/// Opens the first connected Sony DualSense and forwards motor rumble to it over raw HID.
/// Single-pad model (we forward exactly one controller), so the first match is the right one.
/// Opens one connected Sony DualSense and forwards motor rumble to it over raw HID.
///
/// A caller that owns a particular pad passes the location id it wants (see
/// `open(preferringLocationID:)`); the renderer takes that from the `GCController` it is bound to,
/// so with two DualSenses attached each renderer drives its own device. Without a preference the
/// lowest location id wins an arbitrary but *stable* choice, where `Set.first` was neither.
final class DualSenseHID {
private let manager: IOHIDManager
private var device: IOHIDDevice?
@@ -43,9 +47,57 @@ final class DualSenseHID {
deinit { close() }
/// Find and open the first connected DualSense. Returns false if none is present or it can't
/// be opened (caller then falls back to CoreHaptics).
func open() -> Bool {
/// The IOKit location id of the device this instance opened the handle a caller correlates
/// with its `GCController`. `nil` until a successful `open`.
private(set) var locationID: UInt32?
/// A device's location id, or `nil` if IOKit does not report one.
static func locationID(of dev: IOHIDDevice) -> UInt32? {
IOHIDDeviceGetProperty(dev, kIOHIDLocationIDKey as CFString) as? UInt32
}
/// Every connected DualSense/Edge, by location id what a caller pairs against its controllers.
static func attachedLocationIDs() -> [UInt32] {
let mgr = IOHIDManagerCreate(kCFAllocatorDefault, IOOptionBits(kIOHIDOptionsTypeNone))
let matches = productIDs.map { pid in
[kIOHIDVendorIDKey: vendorSony, kIOHIDProductIDKey: pid] as CFDictionary
}
IOHIDManagerSetDeviceMatchingMultiple(mgr, matches as CFArray)
guard IOHIDManagerOpen(mgr, IOOptionBits(kIOHIDOptionsTypeNone)) == kIOReturnSuccess else {
return []
}
defer { IOHIDManagerClose(mgr, IOOptionBits(kIOHIDOptionsTypeNone)) }
let devices = IOHIDManagerCopyDevices(mgr) as? Set<IOHIDDevice> ?? []
return devices.compactMap(locationID(of:)).sorted()
}
/// Which attached device to drive, as an index into `ids` the whole selection rule, pure so
/// it can be tested without an `IOHIDDevice` (which cannot be constructed).
///
/// `IOHIDManagerCopyDevices` returns an unordered `Set`, so the previous `Set.first` was not
/// merely arbitrary it can differ between two calls in one process. With two DualSenses that
/// made each renderer's paddevice binding a coin flip: both could land on the same device
/// (one pad's rumble coming out of the other, and the two per-instance write dedupes fighting
/// over it) or split by luck. An explicit location id makes the binding deterministic; the
/// lowest-id fallback at least makes it stable. `nil` ids sort last so a device IOKit cannot
/// place never displaces one it can.
static func preferredIndex(among ids: [UInt32?], preferring wanted: UInt32?) -> Int? {
if let wanted, let hit = ids.firstIndex(where: { $0 == wanted }) { return hit }
return ids.indices.min { (ids[$0] ?? .max) < (ids[$1] ?? .max) }
}
/// Pick the device to drive from everything attached (see [`preferredIndex`]).
static func pick(_ devices: Set<IOHIDDevice>, preferring wanted: UInt32?) -> IOHIDDevice? {
let ordered = Array(devices)
guard let i = preferredIndex(among: ordered.map(locationID(of:)), preferring: wanted) else {
return nil
}
return ordered[i]
}
/// Find and open a connected DualSense, preferring the one at `preferredLocationID`. Returns
/// false if none is present or it can't be opened (caller then falls back to CoreHaptics).
func open(preferringLocationID preferred: UInt32? = nil) -> Bool {
let matches = Self.productIDs.map { pid in
[kIOHIDVendorIDKey: Self.vendorSony, kIOHIDProductIDKey: pid] as CFDictionary
}
@@ -55,13 +107,21 @@ final class DualSenseHID {
return false
}
guard let devices = IOHIDManagerCopyDevices(manager) as? Set<IOHIDDevice>,
let dev = devices.first
let dev = Self.pick(devices, preferring: preferred)
else {
log.info("rumble: no DualSense HID device found — falling back to CoreHaptics")
IOHIDManagerClose(manager, IOOptionBits(kIOHIDOptionsTypeNone))
return false
}
device = dev
locationID = Self.locationID(of: dev)
if let preferred, locationID != preferred {
// Not fatal one pad still gets rumble but with two pads attached it means this
// renderer is driving the wrong one, and it is invisible without the log line.
log.error(
"rumble: wanted DualSense at location \(preferred, privacy: .public) but opened \(self.locationID.map(String.init) ?? "unknown", privacy: .public)"
)
}
let transport = IOHIDDeviceGetProperty(dev, kIOHIDTransportKey as CFString) as? String
bluetooth = transport?.lowercased().contains("bluetooth") ?? false
log.info("rumble: DualSense raw-HID rumble active (transport=\(self.transport, privacy: .public))")
@@ -70,8 +130,16 @@ final class DualSenseHID {
/// Drive the motors. `low` = left/heavy (low-frequency), `high` = right/light (high-frequency),
/// each 0...255. (0, 0) stops.
func rumble(low: UInt8, high: UInt8) {
guard let dev = device else { return }
///
/// Returns whether the write reached the device. The caller needs this: it used to be logged
/// and swallowed, so a failed write still counted as a successful render. That matters most
/// for a **stop**, which has nothing behind it the renderer stamps its write clock even on
/// failure, the keepalive re-write only fires for non-zero levels, and the ticker is cancelled
/// once the target is `(0, 0)`. On USB there is no firmware timeout either, so a swallowed
/// stop left the motors running with nothing scheduled to try again.
@discardableResult
func rumble(low: UInt8, high: UInt8) -> Bool {
guard let dev = device else { return false }
let report = bluetooth
? Self.bluetoothReport(low: low, high: high)
: Self.usbReport(low: low, high: high)
@@ -81,7 +149,9 @@ final class DualSenseHID {
}
if rc != kIOReturnSuccess {
log.error("rumble: IOHIDDeviceSetReport failed (0x\(String(format: "%08x", rc), privacy: .public))")
return false
}
return true
}
func close() {
@@ -117,7 +117,15 @@ public final class GamepadFeedback {
reset(slot.controller)
slots[pad] = nil
let renderer = withRouting { rumbleByPad.removeValue(forKey: pad) }
renderer?.stop()
// OFF the main actor. `RumbleRenderer.stop()` is a `queue.sync`, and its body is a
// per-motor `CHHapticEngine.stop()` an XPC round trip to gamecontrollerd, which the
// renderer's own notes record as able to hang plus `DualSenseHID.close()`, whose
// blocking `IOHIDDeviceSetReport` goes to a device that has just departed. It also
// queues behind any in-flight `setup()`. This runs on every unplug and every pin
// change, and the main thread is what drives the presenter's CADisplayLink, so
// blocking here hitches the picture mid-stream. The renderer is already detached from
// routing above, so nothing observes it after this point.
if let renderer { Task.detached { renderer.stop() } }
}
for (pad, controller) in want {
if let slot = slots[pad] {
@@ -282,6 +290,12 @@ public final class GamepadFeedback {
private func reset(_ controller: GCController?) {
guard let c = controller else { return }
c.playerIndex = .indexUnset
// Put the lightbar out too. This class is what turned it on (see the `Led` and
// `PlayerLeds` arms), and every DS write is valid-flag-selective, so a colour the game
// set stays lit in firmware after the stream ends back at the launcher, or for a pad
// that merely left the forwarded set. A DS4 is cleared incidentally because its player
// indicator IS the lightbar; a DualSense is not.
c.light?.color = GCColor(red: 0, green: 0, blue: 0)
if let ds = c.extendedGamepad as? GCDualSenseGamepad {
ds.leftTrigger.setModeOff()
ds.rightTrigger.setModeOff()
@@ -459,6 +459,18 @@ final class RumbleRenderer: @unchecked Sendable {
if split {
low = makeMotor(haptics, .leftHandle, sharpness: RumbleTuning.sharpnessLow)
high = makeMotor(haptics, .rightHandle, sharpness: RumbleTuning.sharpnessHigh)
// HALF a split is worse than none, and it used to pass silently: only the all-nil case
// below counts as failure, so one surviving handle left `ok` true and `reportHealth(nil)`
// announced HEALTHY. What actually rendered was wrong in a direction that depends on
// which handle died lose `high` and `render` falls to the combined branch (selected
// purely by `high != nil`), playing max(low, high) on the LEFT handle at the combined
// sharpness; lose `low` and the split branch's reconcile no-ops on the nil slot, so the
// heavy motor is discarded outright. Tear the survivor down and take the combined path,
// which at least renders both motors somewhere.
if low == nil || high == nil {
log.warning("rumble: only one split-handle engine came up — falling back to combined")
teardown() // disarms handlers, stops the survivor's players + engine, nils both
}
} else {
low = makeMotor(haptics, .default, sharpness: RumbleTuning.sharpnessCombined)
}
@@ -587,7 +599,9 @@ final class RumbleRenderer: @unchecked Sendable {
#if os(macOS)
guard let c, c.extendedGamepad is GCDualSenseGamepad else { return false }
let hid = DualSenseHID()
guard hid.open() else { return false }
// Ask for the device this renderer's controller actually is, so two attached DualSenses
// do not both get driven through whichever one an unordered Set happened to yield first.
guard hid.open(preferringLocationID: Self.hidLocationID(for: c)) else { return false }
dualSenseHID = hid
return true
#else
@@ -595,6 +609,24 @@ final class RumbleRenderer: @unchecked Sendable {
#endif
}
#if os(macOS)
/// Correlate a `GCController` with an IOKit location id.
///
/// GameController exposes no location id, so there is no direct mapping. What it does expose is
/// a stable per-controller ordering, and IOKit's location ids are stable per port: pairing the
/// two by rank makes each renderer pick a *distinct* device, which is the property that was
/// missing. With one pad attached this is the same device it always was.
static func hidLocationID(for c: GCController) -> UInt32? {
let ids = DualSenseHID.attachedLocationIDs()
guard ids.count > 1 else { return ids.first }
let peers = GCController.controllers().filter { $0.extendedGamepad is GCDualSenseGamepad }
guard let rank = peers.firstIndex(where: { $0 === c }), rank < ids.count else {
return ids.first
}
return ids[rank]
}
#endif
/// Write the target to the DualSense over HID if that's the active backend; false not a
/// HID pad, so the caller renders via CoreHaptics. Deduped on the pad's 0...255 resolution,
/// with a periodic keepalive re-write while nonzero (the ticker calls back in here).
@@ -605,8 +637,20 @@ final class RumbleRenderer: @unchecked Sendable {
let keepalive = levels != (0, 0)
&& seconds(since: lastHidWrite.at) > RumbleTuning.hidKeepaliveSeconds
if levels != lastHidWrite.levels || keepalive {
hid.rumble(low: levels.0, high: levels.1)
lastHidWrite = (levels, .now())
if hid.rumble(low: levels.0, high: levels.1) {
lastHidWrite = (levels, .now())
} else {
// The write did not reach the device. Do NOT stamp the clock that would claim a
// render that never happened, and for a stop there is nothing behind it: the
// keepalive only re-writes non-zero levels and the ticker is cancelled once the
// target is (0, 0), so the motors would keep running with nothing scheduled.
// Drop the handle instead: the pad reverts to CoreHaptics, and a reconnect
// rebuilds it. Health is reported so the state is visible rather than silent.
log.error("rumble: HID write failed — dropping the handle, falling back")
closeHID()
reportHealth("Lost the direct connection to this DualSense; using the system path.")
return false
}
}
return true
#else
@@ -43,5 +43,33 @@ final class DualSenseHIDTests: XCTestCase {
let crc = DualSenseHID.crc32(seed: UInt8(ascii: "1"), Array("23456789".utf8))
XCTAssertEqual(crc, 0xCBF4_3926)
}
// MARK: - Device selection (B14)
/// With two DualSenses attached, each renderer must drive its OWN device. The old code took
/// `Set.first` from an unordered set, so the paddevice binding was a coin flip that could
/// point both renderers at the same pad.
func testPreferredIndexHonoursAnExplicitLocation() {
let ids: [UInt32?] = [0x1D18_0000, 0x1420_0000, 0x1411_0000]
XCTAssertEqual(DualSenseHID.preferredIndex(among: ids, preferring: 0x1420_0000), 1)
XCTAssertEqual(DualSenseHID.preferredIndex(among: ids, preferring: 0x1D18_0000), 0)
}
/// No preference (or one the pad no longer has): fall back to the LOWEST id arbitrary, but
/// stable across calls, which `Set.first` was not.
func testPreferredIndexFallsBackToTheLowestIdDeterministically() {
let ids: [UInt32?] = [0x1D18_0000, 0x1420_0000, 0x1411_0000]
XCTAssertEqual(DualSenseHID.preferredIndex(among: ids, preferring: nil), 2)
// A wanted id that is gone (pad unplugged between enumeration and open) must not fail the
// open it degrades to the same stable fallback.
XCTAssertEqual(DualSenseHID.preferredIndex(among: ids, preferring: 0xDEAD_BEEF), 2)
}
/// A device IOKit reports no location for must never displace one it can place.
func testPreferredIndexSortsUnplaceableDevicesLast() {
XCTAssertEqual(DualSenseHID.preferredIndex(among: [nil, 0x1420_0000], preferring: nil), 1)
XCTAssertEqual(DualSenseHID.preferredIndex(among: [nil, nil], preferring: nil), 0)
XCTAssertNil(DualSenseHID.preferredIndex(among: [], preferring: nil))
}
}
#endif
-112
View File
@@ -341,50 +341,6 @@ pub fn mtu1500_shard_payload_for(peer: core::net::IpAddr) -> usize {
}
}
/// Floor for a negotiated `shard_payload` (even, well under every real path). A path whose UDP
/// budget lands below this can't carry the QUIC control plane either (QUIC's own minimum is a
/// 1200-byte UDP payload), so shrinking video shards further buys nothing — the clamp helpers
/// bottom out here instead of producing degenerate confetti-sized shards.
pub const MIN_SHARD_PAYLOAD: usize = 512;
/// The sealed wire size of a video datagram carrying `shard_payload` bytes of shard — what
/// actually leaves the socket as UDP payload (punktfunk header + shard + crypto overhead).
pub const fn sealed_datagram_bytes(shard_payload: usize) -> usize {
HEADER_LEN + shard_payload + CRYPTO_OVERHEAD
}
/// The UDP-payload size a path must carry for full-size IPv4 video datagrams: the sealed size
/// of the [`mtu1500_shard_payload`] default (= 1472, the exact 1500-MTU IPv4 ceiling). Doubles
/// as the QUIC MTU-discovery probe ceiling (`quic/endpoint.rs`): with the ceiling set to
/// exactly this value, a control connection whose discovery settles AT the ceiling has proven
/// the path carries full-size video datagrams, and one that settles BELOW it has proven the
/// path cannot — a discrimination quinn's stock 1452 ceiling can't make in either direction.
pub const fn video_datagram_udp_ceiling() -> usize {
sealed_datagram_bytes(mtu1500_shard_payload())
}
/// Largest even shard payload whose sealed datagram fits in `udp_budget` bytes of UDP payload
/// (the quantity QUIC MTU discovery measures — [`video_datagram_udp_ceiling`] is its probe
/// ceiling). Clamped to the peer's family default ([`mtu1500_shard_payload_for`]) so a generous
/// budget never grows packets past today's wire, and floored at [`MIN_SHARD_PAYLOAD`].
pub fn shard_payload_for_udp_budget(udp_budget: usize, peer: core::net::IpAddr) -> usize {
let p = udp_budget.saturating_sub(HEADER_LEN + CRYPTO_OVERHEAD);
let p = p - p % 2; // FEC requires even shards
p.clamp(MIN_SHARD_PAYLOAD, mtu1500_shard_payload_for(peer))
}
/// [`shard_payload_for_udp_budget`] for an operator-supplied ON-WIRE IP MTU (the number
/// `netsh interface ipv4 show subinterfaces` / `ip link` shows): subtracts the family's IP+UDP
/// headers first — 28 for IPv4 (and IPv4-mapped), 48 for IPv6.
pub fn shard_payload_for_wire_mtu(wire_mtu: usize, peer: core::net::IpAddr) -> usize {
let ip_udp = match peer {
core::net::IpAddr::V4(_) => 28,
core::net::IpAddr::V6(v6) if v6.to_ipv4_mapped().is_some() => 28,
core::net::IpAddr::V6(_) => 48,
};
shard_payload_for_udp_budget(wire_mtu.saturating_sub(ip_udp), peer)
}
/// Everything needed to construct a [`Session`](crate::session::Session).
///
/// `Debug` is implemented by hand to redact `key`/`salt`, and `key`/`salt` are zeroized
@@ -558,74 +514,6 @@ mod tests {
assert!(HEADER_LEN + (p + 2) + CRYPTO_OVERHEAD > 1452, "not maximal");
}
/// The video-datagram ceiling IS the exact v4 sealed size — the QUIC MTU-discovery probe
/// ceiling (endpoint.rs) relies on this equality for its settled-at-vs-below verdict.
#[test]
fn video_datagram_ceiling_is_the_sealed_default() {
assert_eq!(
video_datagram_udp_ceiling(),
HEADER_LEN + mtu1500_shard_payload() + CRYPTO_OVERHEAD
);
assert_eq!(video_datagram_udp_ceiling(), 1472);
}
/// Budget-derived sizing: even, sealed-fits-the-budget, clamped to the family default
/// above and [`MIN_SHARD_PAYLOAD`] below.
#[test]
fn shard_payload_for_udp_budget_math() {
use core::net::IpAddr;
let v4: IpAddr = "192.168.1.50".parse().unwrap();
let v6: IpAddr = "fd00::50".parse().unwrap();
// The full ceiling reproduces the default exactly.
assert_eq!(
shard_payload_for_udp_budget(video_datagram_udp_ceiling(), v4),
mtu1500_shard_payload()
);
// A WARP/Tailscale-shaped 1280 budget: sealed result must fit the budget, stay even.
let p = shard_payload_for_udp_budget(1280, v4);
assert_eq!(p % 2, 0);
assert!(sealed_datagram_bytes(p) <= 1280);
assert!(sealed_datagram_bytes(p + 2) > 1280, "not maximal");
// Odd budgets round down to even shards.
assert_eq!(shard_payload_for_udp_budget(1281, v4) % 2, 0);
// A generous budget never grows past the family default (either family).
assert_eq!(
shard_payload_for_udp_budget(9000, v4),
mtu1500_shard_payload()
);
assert_eq!(
shard_payload_for_udp_budget(9000, v6),
mtu1500_shard_payload_v6()
);
// Degenerate budgets bottom out at the floor instead of confetti.
assert_eq!(shard_payload_for_udp_budget(100, v4), MIN_SHARD_PAYLOAD);
}
/// Operator-facing wire-MTU sizing subtracts the right IP+UDP header per family, and 1500
/// reproduces today's defaults exactly.
#[test]
fn shard_payload_for_wire_mtu_math() {
use core::net::IpAddr;
let v4: IpAddr = "192.168.1.50".parse().unwrap();
let v6: IpAddr = "fd00::50".parse().unwrap();
let mapped: IpAddr = "::ffff:192.168.1.50".parse().unwrap();
assert_eq!(
shard_payload_for_wire_mtu(1500, v4),
mtu1500_shard_payload()
);
assert_eq!(
shard_payload_for_wire_mtu(1500, mapped),
mtu1500_shard_payload()
);
assert_eq!(
shard_payload_for_wire_mtu(1500, v6),
mtu1500_shard_payload_v6()
);
// 1280 wire 28 64 = 1188 (v4); 48 64 = 1168 (v6).
assert_eq!(shard_payload_for_wire_mtu(1280, v4), 1188);
assert_eq!(shard_payload_for_wire_mtu(1280, v6), 1168);
}
/// Family selection: genuine v6 remotes get the v6 size; v4 — including the IPv4-mapped v6
/// form a dual-stack `[::]` socket reports for a v4 client — keeps the v4 size.
#[test]
@@ -47,20 +47,6 @@ fn stream_transport_idle(idle: std::time::Duration) -> Arc<quinn::TransportConfi
// plane latest-wins at the source — ~200 ms of stereo Opus (proportionally less at
// surround bitrates), so sustained congestion costs concealable drops, never lag.
t.datagram_send_buffer_size(4 * 1024);
// MTU discovery probes up to EXACTLY the sealed size of a full IPv4 video datagram (1472)
// instead of quinn's stock 1452. Two reasons: (a) on a clean 1500-MTU path QUIC gets the
// last 20 bytes per packet; (b) the ceiling turns discovery into a video-path verdict the
// host's wire-MTU watcher reads (`punktfunk-host` `native/wire_mtu.rs`) — settled == ceiling
// proves the path carries full-size video datagrams, settled BELOW it proves it cannot (a
// VPN/overlay adapter at MTU ~1280 blackholes every video packet while all the small flows
// pass: the "connects fine, black screen forever" field shape). With the stock 1452 ceiling
// a healthy path and a constrained one are indistinguishable at the top. This is the ONLY
// behavioral change on healthy paths, and it's confined to discovery: probes are padded
// PINGs quinn already expects to lose above a constrained hop — a lost probe settles the
// search lower, exactly as it did before.
let mut mtud = quinn::MtuDiscoveryConfig::default();
mtud.upper_bound(crate::config::video_datagram_udp_ceiling() as u16);
t.mtu_discovery_config(Some(mtud));
Arc::new(t)
}
+3 -4
View File
@@ -26,7 +26,9 @@
#![deny(clippy::undocumented_unsafe_blocks)]
use anyhow::{anyhow, Context, Result};
use punktfunk_core::config::{CompositorPref, FecConfig, FecScheme, GamepadPref, Role};
use punktfunk_core::config::{
mtu1500_shard_payload_for, CompositorPref, FecConfig, FecScheme, GamepadPref, Role,
};
use punktfunk_core::input::{InputEvent, InputKind};
use punktfunk_core::packet::{FLAG_PIC, FLAG_PROBE, FLAG_SOF};
use punktfunk_core::quic::{
@@ -70,9 +72,6 @@ use input::{input_thread, ClientInput};
/// The Hello→Welcome→Start negotiation (plan §W1); `serve_session` calls `handshake::negotiate`
/// after the pairing gate.
mod handshake;
/// MTU resilience for the video data plane: `PUNKTFUNK_WIRE_MTU` override, the per-session
/// path-MTU watch on the control connection, and the per-peer learned shard-payload clamp.
mod wire_mtu;
/// The mid-stream control task (plan §W1); `serve_session` spawns `control::run` after the
/// handshake to multiplex renegotiation / speed-test control messages onto the data-plane channels.
+1 -10
View File
@@ -491,12 +491,7 @@ pub(super) async fn negotiate(
// per-datagram loss on Wi-Fi — the "100 Mbps badly fails on the phone" root cause.
// Negotiated, so the client follows. Jumbo (≈8900) is a future negotiated bump (needs
// MAX_DATAGRAM_BYTES raised + end-to-end 9000 MTU).
// Resolution order (wire_mtu.rs): `PUNKTFUNK_WIRE_MTU` operator override, then a path
// budget learned from a prior session whose QUIC MTU discovery settled below the
// video-datagram ceiling (the "VPN on the host blackholes every video packet" field
// shape — small flows pass, the stream is an endless black screen), then this family
// default. Healthy paths take the default branch and are byte-identical to before.
shard_payload: wire_mtu::negotiated_shard_payload(peer.ip()) as u16,
shard_payload: mtu1500_shard_payload_for(peer.ip()) as u16,
encrypt: true,
key,
salt,
@@ -663,10 +658,6 @@ pub(super) async fn negotiate(
let start =
Start::decode(&io::read_msg(recv).await?).map_err(|e| anyhow!("Start decode: {e:?}"))?;
bringup.mark("start");
// The session is real: watch this connection's MTU discovery settle and turn it into a
// path verdict (WARN + learned clamp for the next session on a constrained path; clears a
// stale clamp on a healthy one). Bounded ~10 s task, ends by itself.
wire_mtu::spawn_watch(conn.clone(), welcome.shard_payload as usize);
Ok::<_, anyhow::Error>((
hello,
welcome,
@@ -1,192 +0,0 @@
//! MTU resilience for the video data plane (the "connects fine, black screen forever" field
//! shape).
//!
//! Video datagrams are sealed at a per-session `shard_payload` sized for a clean 1500-byte MTU
//! (1472-byte UDP payloads). A host whose route to the client runs through a smaller-MTU hop —
//! a VPN/overlay adapter (Tailscale/WARP/ZeroTier default to 1280) claiming the LAN route, or a
//! lowered NIC MTU — delivers every SMALL flow (QUIC control, hole punch, input, audio) while
//! 100 % of video datagrams die by fragmentation or local `WSAEMSGSIZE`: the client sits on a
//! black screen reporting `loss_ppm=0` (it can't see gaps in packets it never saw any of) and
//! the host streams into the void with every gauge green. Neither side observes the failure
//! directly — but the control connection CAN: its MTU discovery probes up to exactly the sealed
//! video-datagram size ([`video_datagram_udp_ceiling`], set in `quic/endpoint.rs`), so its
//! settled MTU is a verdict on the path.
//!
//! Three legs, none of which changes a session on a healthy path:
//! - **`PUNKTFUNK_WIRE_MTU=<bytes>`** — operator override; the shard payload is derived from
//! the given on-wire IP MTU. Wire-compatible with every deployed client:
//! `Welcome::shard_payload` is already negotiated per session (the v4/v6 split ships two
//! values today) and clients follow the negotiated value.
//! - **Watch** — a per-session task samples the control connection's discovered MTU once the
//! search has had time to finish. A connection still alive that settled BELOW the ceiling is
//! proof the path can't carry full-size video: log an actionable WARN and record the measured
//! budget for the peer.
//! - **Heal** — the next handshake from that peer clamps `shard_payload` to the recorded
//! budget, so a reconnect fixes the stream. A later session that reaches the ceiling erases
//! the record (the learn/heal loop is self-correcting in both directions).
use std::collections::HashMap;
use std::net::IpAddr;
use std::sync::{Mutex, OnceLock};
use punktfunk_core::config::{
mtu1500_shard_payload_for, sealed_datagram_bytes, shard_payload_for_udp_budget,
shard_payload_for_wire_mtu, video_datagram_udp_ceiling,
};
/// Measured UDP-payload budget per peer IP, learned from live control connections whose MTU
/// discovery settled below the video-datagram ceiling. In-memory only: a host restart
/// re-learns in one session, and entries self-correct (a later ceiling-hit erases, a lower
/// re-measure overwrites).
fn learned() -> &'static Mutex<HashMap<IpAddr, u16>> {
static LEARNED: OnceLock<Mutex<HashMap<IpAddr, u16>>> = OnceLock::new();
LEARNED.get_or_init(|| Mutex::new(HashMap::new()))
}
/// The shard payload for a new session to `peer`: `PUNKTFUNK_WIRE_MTU` override, else the
/// peer's learned path budget, else the family default (today's exact behavior). Logs whenever
/// the result differs from the default.
pub(super) fn negotiated_shard_payload(peer: IpAddr) -> usize {
let env = match std::env::var("PUNKTFUNK_WIRE_MTU") {
Ok(v) => match v.trim().parse::<usize>() {
Ok(mtu) => Some(mtu),
Err(_) => {
tracing::warn!(value = %v, "PUNKTFUNK_WIRE_MTU is not a number — ignoring it");
None
}
},
Err(_) => None,
};
let learned_budget = learned().lock().unwrap().get(&peer).copied();
resolve(env, learned_budget, peer)
}
/// Pure resolution (env override > learned budget > family default) — the tested core of
/// [`negotiated_shard_payload`].
fn resolve(env_wire_mtu: Option<usize>, learned_udp_budget: Option<u16>, peer: IpAddr) -> usize {
let default = mtu1500_shard_payload_for(peer);
if let Some(mtu) = env_wire_mtu {
let p = shard_payload_for_wire_mtu(mtu, peer);
if p != default {
tracing::info!(
wire_mtu = mtu,
shard_payload = p,
default,
"wire MTU: shard payload set from PUNKTFUNK_WIRE_MTU"
);
}
return p;
}
if let Some(budget) = learned_udp_budget {
let p = shard_payload_for_udp_budget(budget as usize, peer);
if p != default {
tracing::info!(
peer = %peer,
udp_budget = budget,
shard_payload = p,
default,
"wire MTU: shard payload clamped to this peer's measured path MTU (learned \
from a prior session's QUIC MTU discovery) video datagrams now fit the \
constrained hop"
);
return p;
}
}
default
}
/// Sample the control connection's discovered MTU after the search has settled and turn it
/// into a verdict. Spawned once per negotiated session; the task ends by itself after the
/// final sample (bounded ~10 s lifetime, holding only a cheap `Connection` handle).
pub(super) fn spawn_watch(conn: quinn::Connection, session_shard_payload: usize) {
tokio::spawn(async move {
let peer = conn.remote_address().ip();
let ceiling = video_datagram_udp_ceiling() as u16;
// Discovery finishes in a handful of RTTs on a LAN (well under the first sample) but
// needs a loss timeout per failed probe on a constrained path — the second sample
// covers that with margin. Max, because discovery only ever raises `current_mtu`.
let mut settled = 0u16;
for wait_s in [3u64, 7] {
tokio::time::sleep(std::time::Duration::from_secs(wait_s)).await;
settled = settled.max(conn.stats().path.current_mtu);
if settled >= ceiling {
break;
}
}
if settled >= ceiling {
// The path carries full-size video datagrams — erase any stale learned clamp so
// the next session returns to the default wire.
if learned().lock().unwrap().remove(&peer).is_some() {
tracing::info!(peer = %peer,
"wire MTU: path re-measured at full size — learned clamp cleared");
}
return;
}
// A closed connection stops discovering, so a session that ended before the final
// sample proves nothing (a healthy high-RTT path could still be mid-search): learn
// only from a connection that stayed alive through the whole window.
if conn.close_reason().is_some() {
return;
}
learned().lock().unwrap().insert(peer, settled);
if sealed_datagram_bytes(session_shard_payload) <= settled as usize {
// This session was already clamped small enough — the path is still constrained
// (keep the record fresh) but video fits, so no alarm.
tracing::info!(peer = %peer, discovered_udp_mtu = settled,
"wire MTU: constrained path re-measured; this session's video is sized to fit");
} else {
tracing::warn!(
peer = %peer,
discovered_udp_mtu = settled,
needed_udp_mtu = ceiling,
"wire MTU: this path CANNOT carry full-size video datagrams — the control \
plane works but every video packet is oversized for a hop, which streams as \
an endless black screen with zero reported loss. Typical cause: a VPN/overlay \
adapter (Tailscale / Cloudflare WARP / ZeroTier) claiming the LAN route, or a \
lowered NIC MTU compare `ping <client> -f -l 1450` vs `-l 1200` and check \
`netsh interface ipv4 show subinterfaces` (Windows) / `ip link` (Linux). The \
measured budget is recorded: the NEXT session from this client sizes video to \
fit automatically. To pin it for all sessions set PUNKTFUNK_WIRE_MTU."
);
}
});
}
#[cfg(test)]
mod tests {
use super::*;
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
const V4: IpAddr = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 2));
const V6: IpAddr = IpAddr::V6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1));
#[test]
fn default_when_nothing_known() {
assert_eq!(resolve(None, None, V4), mtu1500_shard_payload_for(V4));
assert_eq!(resolve(None, None, V6), mtu1500_shard_payload_for(V6));
}
#[test]
fn env_override_beats_learned() {
// 1280 wire 28 IP/UDP 64 header/crypto = 1188.
assert_eq!(resolve(Some(1280), Some(1472), V4), 1188);
}
#[test]
fn learned_budget_clamps() {
// A WARP-shaped path: 1280-byte UDP budget → 1280 64 = 1216.
assert_eq!(resolve(None, Some(1280), V4), 1216);
}
#[test]
fn learned_at_or_above_ceiling_is_the_default_wire() {
assert_eq!(resolve(None, Some(1472), V4), mtu1500_shard_payload_for(V4));
assert_eq!(resolve(None, Some(2000), V4), mtu1500_shard_payload_for(V4));
}
#[test]
fn env_full_mtu_is_the_default_wire_both_families() {
assert_eq!(resolve(Some(1500), None, V4), mtu1500_shard_payload_for(V4));
assert_eq!(resolve(Some(1500), None, V6), mtu1500_shard_payload_for(V6));
}
}
-6
View File
@@ -333,12 +333,6 @@
#define INBOUND_REQ_FLAG 2147483648
#endif
// Floor for a negotiated `shard_payload` (even, well under every real path). A path whose UDP
// budget lands below this can't carry the QUIC control plane either (QUIC's own minimum is a
// 1200-byte UDP payload), so shrinking video shards further buys nothing — the clamp helpers
// bottom out here instead of producing degenerate confetti-sized shards.
#define MIN_SHARD_PAYLOAD 512
// 16-byte AEAD authentication tag appended by either session cipher.
#define TAG_LEN 16