mmap: publish the completion queue to userspace - #76
Open
afrog33k wants to merge 2 commits into
Open
Conversation
Every post_send and every poll_cq on this driver is a uverbs ioctl. Measured with
perf and strace on a two-machine pingpong: ~96 ioctls per iteration, and the poll
loop is where almost all of them come from (the receiver issues ~95 polls per
round trip; one ioctl per 256 KiB message is all the data path actually needs).
This adds the standard kernel-bypass shape for the completion path: the kernel
publishes completions into a page shared with the provider, and the provider polls
it by reading memory.
KERNEL
- per-CQ buffer is one vmalloc_user() allocation: page 0 is a shared header,
the kernel's ib_wc ring follows, then the ib_uverbs_wc ring the provider
reads. The whole thing is mapped by a new .mmap op via
rdma_user_mmap_entry_insert(), and released with .mmap_free.
- the ring carries struct ib_uverbs_wc -- the uapi wire format -- NOT the
kernel's struct ib_wc (which holds a struct ib_qp * and a port number and is
not a wire format) and not userspace's struct ibv_wc. tbv_cqe_from_wc()
translates on push; tbv_poll_cq() translates back.
- create_cq returns a vendor response tail (struct tbv_uresp_create_cq) with
the mmap offset, the cqe count, the shm ABI and map_len. The kernel fills it
only when the provider's outbuf is large enough, so an old provider sees
nothing and never mmaps, and a new provider on an old kernel reads zeros and
falls back to ioctls.
- occupancy is a pair of monotonic totals with ONE WRITER EACH: `produced`
(kernel) and `consumed` (consumer). The obvious design -- publish head/tail
and derive occupancy -- cannot distinguish full from empty at cqe == 1, which
ib_send_lat's send CQ actually asks for (TX depth 1), so the "ring full" test
degenerates to a tautology and the CQ declares overflow before storing
anything.
- overflow is a monotonic `ovf_seq`, not a sticky flag, and the consumer keeps
its own copy: a latched flag makes the CQ permanently dead and perftest's
`do { } while (ne == 0)` cannot escape it.
USERSPACE
- create_cq maps the buffer when the kernel offers it and polls lock-free:
acquire load of the tail, read the entries, release store of the head.
- when the kernel offers nothing, or the map fails, the cq keeps its mmap
state null and every call falls through to the existing ibv_cmd_* path --
byte-identical with the previous provider.
MEASURED (two machines, Thunderbolt 4 cable, ib_send_lat, interleaved A/B,
6 rounds, both arms md5-verified on install, every run asserting its own
iteration count in the same output as the number taken from it):
pristine provider 9.14 ioctls/iteration
mmap provider 2.02 ioctls/iteration
The residual 2.02 is exactly post_send + post_recv per iteration.
HONESTLY: throughput and latency do NOT improve. Across a queue-pair and message
size sweep the difference is inside the run-to-run spread. The syscall path was
never the throughput limit; this removes CPU from the completion path and is the
prerequisite for a lockless one, and it should be judged on that.
Co-Authored-By: Claude Code <noreply@anthropic.com>
…inter leak) SECURITY FIX. The mapped buffer held three things -- the shared header, the kernel's ib_wc ring, and the uapi ib_uverbs_wc ring -- and map_len covered the whole allocation. The process that creates the CQ could therefore read the kernel's own ib_wc ring, and struct ib_wc carries `struct ib_qp *qp`: a live kernel pointer. That is an information disclosure with a KASLR/heap-layout bypass attached, on a buffer deliberately handed to userspace. The kernel ring is now kcalloc'd separately and is NOT in the mapped buffer. Only the header and the uapi ring are mapped; ring_offset becomes PAGE_SIZE and map_len shrinks to match. The uapi ring is safe to share because ib_uverbs_wc carries qp_num (a u32), not a pointer -- which is the whole reason the ring was moved to the uapi format in the first place. The provider needs no change: it takes ring_offset and map_len from the shared header, so it follows the kernel's layout rather than assuming it. Found by an automated security review of the pushed commit. Co-Authored-By: Claude Code <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every
post_sendand everypoll_cqon this driver is a uverbs ioctl. Measured on a two-machine pingpong with perf and strace: ~96 ioctls per iteration, and the poll loop is where nearly all of them come from — the receiver issues ~95 polls per round trip, while the data path itself needs one ioctl per message. This publishes the completion queue to userspace so the provider can poll it by reading memory.Design
The per-CQ buffer becomes one
vmalloc_user()allocation: page 0 is a shared header, the kernel'sib_wcring follows, then theib_uverbs_wcring the provider reads. A new.mmapop exposes it throughrdma_user_mmap_entry_insert(), and.mmap_freereleases it.The ring carries
struct ib_uverbs_wc— the uapi wire format — not the kernel'sstruct ib_wc(which holds astruct ib_qp *and is not a wire format) and not userspace'sstruct ibv_wc.tbv_cqe_from_wc()translates on push,tbv_poll_cq()translates back.Occupancy is a pair of monotonic totals with one writer each:
produced(kernel) andconsumed(consumer). The obvious design — publishhead/tailand derive occupancy — cannot distinguish full from empty atcqe == 1. That case is not hypothetical:ib_send_latcreates its send CQ with capacity 1 (TX depth 1), so(tail + 1) % cqe == headis a tautology (x % 1 == 0), and the CQ declared overflow before storing a single completion.Overflow is a monotonic
ovf_seq, not a sticky flag, and the consumer latches its own copy. A latched flag makes the CQ permanently dead, and perftest'sdo { } while (ne == 0)poll loop cannot escape it.create_cqreturns a vendor response tail (struct tbv_uresp_create_cq) with the mmap offset, cqe count, shm ABI andmap_len. The kernel fills it only when the provider's outbuf is large enough, so an older provider sees nothing and never mmaps, and a newer provider on an older kernel reads zeros from its own calloc and falls back to ioctls.On the userspace side,
create_cqmaps the buffer when offered and polls it lock-free — acquire load of the tail, read the entries, release store of the head. When the kernel offers nothing, or the map fails, the cq's mmap state stays null and every call falls through to the existingibv_cmd_*path, byte-identical with the current provider.Measured
Two machines, one Thunderbolt 4 cable,
ib_send_lat, interleaved A/B, 6 rounds, both arms md5-verified on install, and every run asserting its own iteration count in the same output as the number taken from it (a failed run otherwise reports a flatteringly small syscall count):The residual 2.02 is exactly
post_send+post_recv, one each.What this does not do
Throughput and latency do not improve. Across a queue-pair sweep (1–8) and a message-size sweep (64 KiB–1 MiB) the difference sits inside the run-to-run spread, and at q=4 it is nil. The syscall path was never the throughput limit on this hardware. This change removes CPU from the completion path and is the prerequisite for a lockless one; it should be judged on that, not on a bandwidth claim it cannot support.
Build
make -j8inkernel/, no errors).proto/tbv_cq_shm.his included by both sides. Both are Linux and both take the fixed-width types from<linux/types.h>— note that a userspace branch declaring its own__u32/__aligned_u64does not compile against rdma-core, which already defines them; that was the first build failure of this header.🤖 Generated with Claude Code