Skip to content

Commit 1d7c4ea

Browse files
ELaresELaresclaude
authored
feat: io_uring fixed-buffer send (write_fixed) (#284) (#497)
* feat: io_uring fixed-buffer send (write_fixed) (#284) Completes the fixed recv/send primitive pair: `send_fixed` writes a reply from a REGISTERED buffer via write_fixed_all, the write-side analog of recv_fixed. New in ironcache-runtime::fixed_datapath. It stages `data` into a checked-out registered buffer (copy_nonoverlapping into the buffer's stable_mut_ptr + set_init, in one small documented unsafe block -- data.len() is guarded <= the buffer capacity, distinct allocations, exactly data.len() bytes written then marked initialized, so write_fixed_all reads only those bytes) and writes it without a per-write pin. Its fall-back contract MIRRORS recv_fixed's for one clean caller branch: it returns None -- "use the owned-buffer send" -- when the reply does NOT fit one buffer (> ring.buf_size(), a rare large bulk value) OR the slab is drained. The single-buffer size decision stays with this primitive; the serve-loop wiring picks fixed-vs-owned send off the None. Validated on a real ring (local Linux container kernel 6.8 + the CI io_uring datapath job): a full fixed recv+send round-trip (request read via read_fixed, reply written via write_fixed_all, client observes it byte-exact) plus the oversized-reply -> None fall-back. 43 io_uring lib tests pass; clippy -D warnings over --features io_uring clean (incl. the unsafe block); macOS build + fmt + invariant lints clean; 0 dashes. No throughput claim (deferred to a pinned host). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: ELares <zeke@butlr.io> * fix: slice fixed-buffer send to data.len() to not leak stale bytes (#284) Adversarial review (verified against the tokio-uring 0.5 source) caught a real correctness + information-disclosure defect in send_fixed. `write_fixed_all(buf)` submits `buf.bytes_init()` bytes, but tokio-uring's `set_init` is GROW-ONLY and `FixedBufPool` RETAINS init_len across buffer reuse. So a reused buffer whose retained init_len exceeds the reply length would send the reply FOLLOWED BY the stale trailing bytes of a prior (longer) recv/reply through that buffer -- and because the slab is shared across all connections on the shard, those leaked bytes can be another client's request/reply data. Not UB / not OOB (the stale bytes are within init_len = previously-initialized memory); a semantic bug: `set_init(data.len())` cannot SHRINK init_len, so it does not bound the write. Fix: write `buf.slice(0..data.len())` so the SQE length is the slice length (`end - begin`), independent of the buffer's retained init_len. The reply is pinned to exactly the fresh bytes. Regression test added (fixed_send_of_a_short_reply_after_a_longer_recv_leaks_no_ stale_bytes): a pool of ONE buffer forces send_fixed to reuse the exact buffer a prior 20-byte recv filled, then sends a 5-byte reply. VERIFIED it FAILS on the old `write_fixed_all(buf)` (client received "+OK\r\n" + 15 stale 'q' bytes from the prior request) and PASSES with the slice fix (exactly 5 bytes). My earlier round-trip test masked the bug only because its 7-byte reply happened to be LONGER than the 6-byte prior recv (set_init grew, no stale tail). 44 io_uring lib tests pass; clippy -D warnings over --features io_uring clean; macOS build + fmt + invariant lints clean; 0 dashes. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: ELares <zeke@butlr.io> --------- Signed-off-by: ELares <zeke@butlr.io> Co-authored-by: ELares <zeke@butlr.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d933916 commit 1d7c4ea

2 files changed

Lines changed: 140 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,12 @@ release.
164164

165165
### Added
166166

167+
- io_uring fixed-buffer SEND (issue #284): `ironcache-runtime::fixed_datapath` gains `send_fixed`,
168+
the reply side over registered buffers, completing the fixed recv/send pair. It stages the reply
169+
into a checked-out registered buffer and writes it via `write_fixed_all` (no per-write pin). Its
170+
fall-back contract mirrors `recv_fixed`: it returns `None` (use the owned-buffer send) when the
171+
reply does not fit one buffer (a rare large bulk value) OR the slab is drained, so the caller has
172+
one clean branch. Functionally validated on a real ring by a full fixed recv+send round-trip.
167173
- io_uring OneShotFixed datapath substrate (issue #284, IOURING_DATAPATH.md "Registered fixed-buffer
168174
slab"): `ironcache-runtime`'s new `fixed_datapath` module adds `FixedRing` (a per-shard slab of
169175
buffers registered once with the ring via tokio-uring's `FixedBufPool`) + `recv_fixed`, which reads

crates/ironcache-runtime/src/fixed_datapath.rs

Lines changed: 134 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use std::io;
2020

2121
use tokio_uring::buf::fixed::{FixedBuf, FixedBufPool};
22+
use tokio_uring::buf::{BoundedBuf, IoBufMut};
2223
use tokio_uring::net::TcpStream;
2324

2425
/// A per-shard REGISTERED fixed-buffer group for the OneShotFixed datapath: `count` buffers of
@@ -86,9 +87,44 @@ pub async fn recv_fixed(
8687
Some(stream.read_fixed(buf).await)
8788
}
8889

90+
/// Write `data` to `stream` from a checked-out REGISTERED buffer via `write_fixed_all`: the reply is
91+
/// staged into the pre-registered slab and written without a per-write pin. The contract mirrors
92+
/// `recv_fixed`'s fall-back shape -- `None` means "the fixed send does not apply, use the owned-buffer
93+
/// send" -- so the caller has one clean branch:
94+
/// - `None` if `data` does not fit ONE buffer (`> ring.buf_size()`, a rare large bulk reply) OR the
95+
/// group is drained (no buffer to stage into); either way the caller falls back to the owned send.
96+
/// - `Some(Ok(()))` on a fully-written fixed send; `Some(Err(e))` on a write error.
97+
pub async fn send_fixed(
98+
stream: &TcpStream,
99+
ring: &FixedRing,
100+
data: &[u8],
101+
) -> Option<io::Result<()>> {
102+
// A reply larger than one fixed buffer uses the owned send (signalled by None), not a multi-
103+
// buffer fixed write -- keeps this primitive single-buffer and the size decision in the caller.
104+
if data.len() > ring.buf_size() {
105+
return None;
106+
}
107+
let mut buf = ring.checkout()?; // None = drained -> owned-send fall-back
108+
// SAFETY: `data.len() <= ring.buf_size() <= capacity` (the `>` guard above + `Vec::with_capacity`
109+
// never under-allocates), so `stable_mut_ptr()` is valid for `data.len()` writes. `data` and
110+
// `buf` are distinct allocations (non-overlapping). We write exactly `data.len()` bytes then grow
111+
// the init length to cover them, so the `0..data.len()` slice below has an INITIALIZED range.
112+
unsafe {
113+
std::ptr::copy_nonoverlapping(data.as_ptr(), buf.stable_mut_ptr(), data.len());
114+
buf.set_init(data.len());
115+
}
116+
// Write EXACTLY `data.len()` bytes by SLICING to `0..data.len()`: the SQE length is the slice
117+
// length, NOT the buffer's `bytes_init()`. tokio-uring's `set_init` is GROW-ONLY and the pool
118+
// RETAINS `init_len` across buffer reuse, so a reused buffer can carry a LARGER init_len from a
119+
// prior (longer) recv/reply; writing `bytes_init()` would then append those stale, possibly
120+
// cross-connection, trailing bytes after the reply. The slice pins the write to the fresh reply.
121+
let (res, _buf) = stream.write_fixed_all(buf.slice(0..data.len())).await;
122+
Some(res)
123+
}
124+
89125
#[cfg(test)]
90126
mod tests {
91-
use super::{FixedRing, recv_fixed};
127+
use super::{FixedRing, recv_fixed, send_fixed};
92128
use crate::io_uring_rt::IoUringRuntime;
93129
use crate::{Runtime, tokio_rt};
94130
use tokio_uring::net::TcpListener;
@@ -141,6 +177,103 @@ mod tests {
141177
});
142178
}
143179

180+
/// The full FIXED round-trip: the server reads the request via `recv_fixed` AND writes the reply
181+
/// via `send_fixed` (both over the registered slab). Also asserts the fall-back contract: a reply
182+
/// larger than one buffer yields `None` (caller uses the owned send).
183+
#[test]
184+
fn fixed_ring_recv_and_send_round_trip_over_the_registered_slab() {
185+
tokio_uring::start(async {
186+
let std_listener =
187+
tokio_rt::bind_reuseport_std("127.0.0.1:0".parse().unwrap()).unwrap();
188+
let addr = std_listener.local_addr().unwrap();
189+
let listener = TcpListener::from_std(std_listener);
190+
191+
let server = tokio_uring::spawn(async move {
192+
let runtime = IoUringRuntime::new();
193+
let (stream, _peer) = runtime.accept(&listener).await.unwrap();
194+
let ring = FixedRing::register(8, 4096).unwrap();
195+
196+
let (res, buf) = recv_fixed(&stream, &ring).await.expect("buffer available");
197+
let n = res.unwrap();
198+
assert_eq!(&buf[..n], b"PING\r\n");
199+
drop(buf);
200+
201+
// A reply that does NOT fit one buffer -> None (caller falls back to owned send).
202+
let too_big = vec![b'x'; ring.buf_size() + 1];
203+
assert!(
204+
send_fixed(&stream, &ring, &too_big).await.is_none(),
205+
"oversized reply must signal owned-send fall-back"
206+
);
207+
208+
// The fitting reply is written from the registered slab.
209+
send_fixed(&stream, &ring, b"+PONG\r\n")
210+
.await
211+
.expect("fits + not drained")
212+
.unwrap();
213+
});
214+
215+
let client = IoUringRuntime::new();
216+
let mut peer = client.connect(addr).await.unwrap();
217+
let _ = client.send(&mut peer, b"PING\r\n".to_vec()).await.unwrap();
218+
let res = client
219+
.recv(&mut peer, Vec::with_capacity(16))
220+
.await
221+
.unwrap();
222+
assert_eq!(&res.buf[..res.n], b"+PONG\r\n");
223+
server.await.unwrap();
224+
});
225+
}
226+
227+
/// Regression: a SHORT reply after a LONGER recv on the SAME reused buffer must write only the
228+
/// reply, never the stale trailing bytes of the prior read. tokio-uring's `set_init` is grow-only
229+
/// and the pool retains `init_len` across reuse, so `write_fixed_all(buf)` would send
230+
/// `bytes_init()` (= the prior 20) bytes; `send_fixed` slices to `data.len()` to prevent that
231+
/// leak. A pool of ONE buffer forces `send_fixed` to reuse the exact buffer `recv_fixed` filled.
232+
#[test]
233+
fn fixed_send_of_a_short_reply_after_a_longer_recv_leaks_no_stale_bytes() {
234+
tokio_uring::start(async {
235+
let std_listener =
236+
tokio_rt::bind_reuseport_std("127.0.0.1:0".parse().unwrap()).unwrap();
237+
let addr = std_listener.local_addr().unwrap();
238+
let listener = TcpListener::from_std(std_listener);
239+
240+
let server = tokio_uring::spawn(async move {
241+
let runtime = IoUringRuntime::new();
242+
let (stream, _peer) = runtime.accept(&listener).await.unwrap();
243+
// ONE buffer: send_fixed necessarily reuses the buffer recv_fixed just filled, whose
244+
// retained init_len is 20 -- the exact grow-only trap.
245+
let ring = FixedRing::register(1, 4096).unwrap();
246+
let (res, buf) = recv_fixed(&stream, &ring).await.expect("buffer available");
247+
let n = res.unwrap();
248+
assert_eq!(
249+
n, 20,
250+
"the long request set the reused buffer's init_len to 20"
251+
);
252+
drop(buf);
253+
// A 5-byte reply through that same buffer: must be exactly 5 bytes on the wire.
254+
send_fixed(&stream, &ring, b"+OK\r\n")
255+
.await
256+
.expect("fits + not drained")
257+
.unwrap();
258+
});
259+
260+
let client = IoUringRuntime::new();
261+
let mut peer = client.connect(addr).await.unwrap();
262+
let _ = client.send(&mut peer, vec![b'q'; 20]).await.unwrap();
263+
let res = client
264+
.recv(&mut peer, Vec::with_capacity(64))
265+
.await
266+
.unwrap();
267+
// With the bug this would be 20 bytes ("+OK\r\n" + 15 stale 'q's); the slice makes it 5.
268+
assert_eq!(
269+
&res.buf[..res.n],
270+
b"+OK\r\n",
271+
"reply must be exactly the 5 fresh bytes, no stale trailing bytes leaked"
272+
);
273+
server.await.unwrap();
274+
});
275+
}
276+
144277
/// Draining the group yields `None` from `checkout` (the back-pressure signal): after checking
145278
/// out all `count` buffers, the next check-out is `None`, and returning one makes it available
146279
/// again.

0 commit comments

Comments
 (0)