Commit `47c58cba59`

Vincent Rischmann <vincent@rischmann.fr>

2022-07-16 17:05:11

Fix io_uring tests (#12134)

* io_uring: fix the timeout_remove test The test does a IORING_OP_TIMEOUT followed with a IORING_OP_TIMEOUT_REMOVE and assumed we would get the CQEs in the same order. Linux v5.18 changed how this works and we now get them in the reverse order. The documentation doesn't explicitly say which CQE we should get first so just make the test work with both cases. * io_uring: fix the remove_buffers test The original test was buggy but accidentally worked with kernels < 5.18 The test assumed that IORING_OP_REMOVE_BUFFERS removed from the start of but in fact the documentation doesn't specify which buffer is removed, only that a certain number of buffers are removed. Starting with the kernel 5.18 the check for the `used_buffer_id` fails. Turns out that previous kernels removed buffers in such a way that the remaining buffer for this read would always be 0, however this isn't true anymore. Instead of checking a specific value just check that the `used_buffer_id` corresponds to a valid ID.

master

1 parent 9c66fda

Changed files (1)

lib

std

linux

io_uring.zig

@@ -2181,29 +2181,41 @@ test "timeout_remove" {
 
     try testing.expectEqual(@as(u32, 2), try ring.submit());
 
-    const cqe_timeout = try ring.copy_cqe();
-    // IORING_OP_TIMEOUT_REMOVE is not supported by this kernel version:
-    // Timeout remove operations set the fd to -1, which results in EBADF before EINVAL.
-    // We use IORING_FEAT_RW_CUR_POS as a safety check here to make sure we are at least pre-5.6.
-    // We don't want to skip this test for newer kernels.
-    if (cqe_timeout.user_data == 0x99999999 and
-        cqe_timeout.err() == .BADF and
-        (ring.features & linux.IORING_FEAT_RW_CUR_POS) == 0)
-    {
-        return error.SkipZigTest;
-    }
-    try testing.expectEqual(linux.io_uring_cqe{
-        .user_data = 0x88888888,
-        .res = -@as(i32, @enumToInt(linux.E.CANCELED)),
-        .flags = 0,
-    }, cqe_timeout);
+    // The order in which the CQE arrive is not clearly documented and it changed with kernel 5.18:
+    // * kernel 5.10 gives user data 0x88888888 first, 0x99999999 second
+    // * kernel 5.18 gives user data 0x99999999 first, 0x88888888 second
+
+    var cqes: [2]os.linux.io_uring_cqe = undefined;
+    try testing.expectEqual(@as(u32, 2), try ring.copy_cqes(cqes[0..], 2));
+
+    for (cqes) |cqe| {
+        // IORING_OP_TIMEOUT_REMOVE is not supported by this kernel version:
+        // Timeout remove operations set the fd to -1, which results in EBADF before EINVAL.
+        // We use IORING_FEAT_RW_CUR_POS as a safety check here to make sure we are at least pre-5.6.
+        // We don't want to skip this test for newer kernels.
+        if (cqe.user_data == 0x99999999 and
+            cqe.err() == .BADF and
+            (ring.features & linux.IORING_FEAT_RW_CUR_POS) == 0)
+        {
+            return error.SkipZigTest;
+        }
 
-    const cqe_timeout_remove = try ring.copy_cqe();
-    try testing.expectEqual(linux.io_uring_cqe{
-        .user_data = 0x99999999,
-        .res = 0,
-        .flags = 0,
-    }, cqe_timeout_remove);
+        try testing.expect(cqe.user_data == 0x88888888 or cqe.user_data == 0x99999999);
+
+        if (cqe.user_data == 0x88888888) {
+            try testing.expectEqual(linux.io_uring_cqe{
+                .user_data = 0x88888888,
+                .res = -@as(i32, @enumToInt(linux.E.CANCELED)),
+                .flags = 0,
+            }, cqe);
+        } else if (cqe.user_data == 0x99999999) {
+            try testing.expectEqual(linux.io_uring_cqe{
+                .user_data = 0x99999999,
+                .res = 0,
+                .flags = 0,
+            }, cqe);
+        }
+    }
 }
 
 test "accept/connect/recv/link_timeout" {
@@ -2989,7 +3001,7 @@ test "remove_buffers" {
         try testing.expectEqual(@as(u64, 0xcccccccc), cqe.user_data);
     }
 
-    // Remove the first 3 buffers
+    // Remove 3 buffers
 
     {
         var sqe = try ring.remove_buffers(0xbababababa, 3, group_id);
@@ -3021,7 +3033,7 @@ test "remove_buffers" {
 
         try testing.expect(cqe.flags & linux.IORING_CQE_F_BUFFER == linux.IORING_CQE_F_BUFFER);
         const used_buffer_id = cqe.flags >> 16;
-        try testing.expectEqual(used_buffer_id, 0);
+        try testing.expect(used_buffer_id >= 0 and used_buffer_id < 4);
         try testing.expectEqual(@as(i32, buffer_len), cqe.res);
         try testing.expectEqual(@as(u64, 0xdfdfdfdf), cqe.user_data);
         try testing.expectEqualSlices(u8, &([_]u8{0} ** buffer_len), buffers[used_buffer_id][0..@intCast(usize, cqe.res)]);

Commit 47c58cba59

Commit `47c58cba59`