Add safer abstractions for buffer rings #256
Conversation
| pub struct BufRing { | ||
| ring: Mmap, | ||
| ring_entries: u16, | ||
| entry_size: u32, |
There was a problem hiding this comment.
you might want to decouple this in the future.
Although its terribly complicated, you might want to reuse parts of existing buffers.
Lets say you have 1k buffers, but a message was received with only 8 bytes. In theory you could immediately requeue the rest of that buffer onto the buf_ring for the next network data. The hard part is obviously accounting for this and soon you might have built an allocator internally - but possibly worth allowing for this use case in the future.
There was a problem hiding this comment.
Yeah, originally I wanted to enable full flexibility, but that made the API a pain to use for the simple case. I forgot to mention that for creation I decided against allowing you to specify arbitrary buffer sizes because that effectively limits you to the smallest buffer size since the kernel can't pick the buffer size it wants and has to go in order. Similar logic applies for what you described: if the data is tiny and processing will take a while, you should probably just copy it off the buffer, but if the data is large, then you'll be adding back a small fraction of the buffer to the ring making it have limited utility when the kernel tries to use it.
Anyway, that was the rationale. But I don't think this current architecture prevents flexibility in the future (or I could add it now). I think you could have an unsafe get method get returns a buffer struct and an unsafe add method that adds a buffer struct. Then we could special case entry_size of 0 to not initialize any of the buffers so you can allocate your own.
There was a problem hiding this comment.
i think that sounds generally sensible - was just in case you hadn't thought of it.
There was a problem hiding this comment.
Yeah, totally fair. :)
SUPERCILEX
force-pushed
the
pbuf
branch
3 times, most recently
from
191d728
to
2715f05
Compare
Signed-off-by: Alex Saveau <[email protected]>
|
Ok, I've been running this for a while and I'm pretty confident it works at this point. I also expanded the API to support back pressure by carrying the buffer token through several io_uring steps before releasing it back to the kernel (that way if a client spams requests, it'll run out of buffers and need to wait until the server is ready to accept more). I'm not really happy with the API, but I do think it gets the job done. Thoughts on it? I can add docs if we're going forward with it. Also in case anyone's interested, here's how I'm using this: https://github.com/SUPERCILEX/clipboard-history/blob/418b2612f8e62693e42057029df78f6fbf49de3e/server/src/reactor.rs#L248 |
See tokio-rs/tokio-uring#112 (comment)
I'm leaving this in the draft state until I've fully built out my server and fuzzed it while using this API to battle test it.