tokio-rs / io-uring

The `io_uring` library for Rust
Apache License 2.0
1.16k stars 129 forks source link

Fix broken test_udp_recvmsg_multishot_trunc test #284

Closed SUPERCILEX closed 4 months ago

SUPERCILEX commented 4 months ago

The bug is that I wasn't using buf_id to pick the buffers but rather assumed the kernel would return responses in order.

connortsui20 commented 4 months ago

Note that I'm running this on a laptop now, but still running the same version of Arch Linux

I now get this error:

test udp_recvmsg_multishot_trunc
thread 'main' panicked at io-uring-test/src/tests/net.rs:1523:5:
assertion `left == right` failed
  left: 4
 right: 5

Looking at the two lines right before this one, I'm pretty sure that this assertion should be equal to 4 and not 5 since I think that submit_and_wait will block until there are "wait_nr completion events" in the completion queue, as opposed to waiting until wait_nr operations complete that were originally in flight when io_uring_submit_and_wait was called.

After changing that to 4 I get a new error:

thread 'main' panicked at io-uring-test/src/tests/net.rs:1542:32:
index out of bounds: the len is 2 but the index is 2

I haven't looked into the code but this might be a logical error given the completion events == 5 thing?

SUPERCILEX commented 4 months ago

Nah, the test passes on my machine. I feel like io_uring is just broken on your kernel. The 4 events thing is fine, but that index out of bounds is because the kernel returned a buffer we never gave it. Like what? Maybe that was the old mechanism for signaling no more buffers? Lemme try that real quick.

SUPERCILEX commented 4 months ago

Alright, well I think these changes might be wrong (I added a mod the buffer size), but maybe that's how the kernel works? It just monotonically increases the buf ID? I wish this stuff was documented properly :sob:. https://lwn.net/Articles/815491/ helped a little bit.

connortsui20 commented 4 months ago

Test now passes on my machine!

SUPERCILEX commented 4 months ago

Sweet! I still feel iffy about the mod thing, but hey if it works I guess.

@quininer Do we want to delete the kernel version check code?

quininer commented 4 months ago

You can keep it, feel free.

quininer commented 4 months ago

Thank you!