The branch, master has been updated via 39df9f4a593 s3: smbd: Fix schedule_smb2_aio_read() to allow the last read in a compound to go async. via 0bb4810719c s3: smbd: Fix schedule_aio_smb2_write() to allow the last write in a compound to go async. via 088b8a1e3e5 s4: torture: Add compound_async.read_read test to show we don't go async on the last read in a compound. via ffd9b94fe0f s4: torture: Add compound_async.write_write test to show we don't go async on the last write in a compound. via fc6c76e6dab s4: torture: Tweak the compound padding streamfile test to send 3 reads instead of 2, and check the middle read padding. via 48b12f11a5c s4: torture: Tweak the compound padding basefile test to send 3 reads instead of 2, and check the middle read padding. via f5b2ae58093 s3: tests: Change smb2.compound_async to run against share aio_delay_inject instead of tmp. from 49b40a13343 s4:torture: Fix segfault in multichannel test
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 39df9f4a593f4dd1f19c8b720fd7fd55081c29d1 Author: Jeremy Allison <j...@samba.org> Date: Fri Nov 18 10:50:35 2022 -0800 s3: smbd: Fix schedule_smb2_aio_read() to allow the last read in a compound to go async. Remove knownfail. Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> Autobuild-User(master): Ralph Böhme <s...@samba.org> Autobuild-Date(master): Thu Dec 1 16:04:07 UTC 2022 on sn-devel-184 commit 0bb4810719ce0864114d84b72f8d3b206f1a7d0e Author: Jeremy Allison <j...@samba.org> Date: Fri Nov 18 10:45:19 2022 -0800 s3: smbd: Fix schedule_aio_smb2_write() to allow the last write in a compound to go async. Remove knownfail. Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit 088b8a1e3e56cc24a7c2a469042d1ece9e84df38 Author: Jeremy Allison <j...@samba.org> Date: Thu Nov 17 15:50:30 2022 -0800 s4: torture: Add compound_async.read_read test to show we don't go async on the last read in a compound. Add knownfail. Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit ffd9b94fe0f59c2b552402543db406cb69003745 Author: Jeremy Allison <j...@samba.org> Date: Thu Nov 17 15:39:16 2022 -0800 s4: torture: Add compound_async.write_write test to show we don't go async on the last write in a compound. Add knownfail. Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit fc6c76e6dabdc20bc7401cc2268baa6edb635ee1 Author: Jeremy Allison <j...@samba.org> Date: Fri Nov 18 13:30:05 2022 -0800 s4: torture: Tweak the compound padding streamfile test to send 3 reads instead of 2, and check the middle read padding. The protocol allows the last read in a related compound to be split off and possibly go async (and smbd soon will do this). If the last read is split off, then the padding is different. By sending 3 reads and checking the padding on the 2nd read, we cope with the smbd change and are still correctly checking the padding on a compound related read. Do this for the stream filename compound padding test. Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit 48b12f11a5c4ebd9affb2a2589f47881b46b659b Author: Jeremy Allison <j...@samba.org> Date: Fri Nov 18 13:23:48 2022 -0800 s4: torture: Tweak the compound padding basefile test to send 3 reads instead of 2, and check the middle read padding. The protocol allows the last read in a related compound to be split off and possibly go async (and smbd soon will do this). If the last read is split off, then the padding is different. By sending 3 reads and checking the padding on the 2nd read, we cope with the smbd change and are still correctly checking the padding on a compound related read. Do this for the base filename compound padding test. Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit f5b2ae58093a0920c7be0394f638b73736fbebc2 Author: Jeremy Allison <j...@samba.org> Date: Fri Nov 18 09:53:23 2022 -0800 s3: tests: Change smb2.compound_async to run against share aio_delay_inject instead of tmp. It doesn't hurt the fsync compound async tests, and we need this for the next commits to ensure smb2_read/smb2_write compound tests take longer than 500ms so can be sure the last read/write in the compound will go async. Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/selftest/tests.py | 2 +- source3/smbd/smb2_aio.c | 22 +++- source4/torture/smb2/compound.c | 258 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 277 insertions(+), 5 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 8e9a4aaba47..1630fdd2035 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -1170,7 +1170,7 @@ for t in tests: plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/aio -U$USERNAME%$PASSWORD', 'aio') plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD') elif t == "smb2.compound_async": - plansmbtorture4testsuite(t, "fileserver", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') + plansmbtorture4testsuite(t, "fileserver", '//$SERVER_IP/aio_delay_inject -U$USERNAME%$PASSWORD') elif t == "smb2.ea": plansmbtorture4testsuite(t, "fileserver", '//$SERVER/ea_acl_xattr --option=torture:acl_xattr_name=hackme -U$USERNAME%$PASSWORD') elif t == "rpc.samba3.netlogon" or t == "rpc.samba3.sessionkey": diff --git a/source3/smbd/smb2_aio.c b/source3/smbd/smb2_aio.c index 76a5b644ef8..8c01c76a3e9 100644 --- a/source3/smbd/smb2_aio.c +++ b/source3/smbd/smb2_aio.c @@ -289,6 +289,8 @@ NTSTATUS schedule_smb2_aio_read(connection_struct *conn, struct aio_extra *aio_ex; size_t min_aio_read_size = lp_aio_read_size(SNUM(conn)); struct tevent_req *req; + bool is_compound = false; + bool is_last_in_compound = false; bool ok; ok = vfs_valid_pread_range(startpos, smb_maxcnt); @@ -316,7 +318,14 @@ NTSTATUS schedule_smb2_aio_read(connection_struct *conn, return NT_STATUS_RETRY; } - if (smbd_smb2_is_compound(smbreq->smb2req)) { + is_compound = smbd_smb2_is_compound(smbreq->smb2req); + is_last_in_compound = smbd_smb2_is_last_in_compound(smbreq->smb2req); + + if (is_compound && !is_last_in_compound) { + /* + * Only allow going async if this is the last + * request in a compound. + */ return NT_STATUS_RETRY; } @@ -433,6 +442,8 @@ NTSTATUS schedule_aio_smb2_write(connection_struct *conn, struct aio_extra *aio_ex = NULL; size_t min_aio_write_size = lp_aio_write_size(SNUM(conn)); struct tevent_req *req; + bool is_compound = false; + bool is_last_in_compound = false; if (fsp_is_alternate_stream(fsp)) { /* No AIO on streams yet */ @@ -455,7 +466,14 @@ NTSTATUS schedule_aio_smb2_write(connection_struct *conn, return NT_STATUS_RETRY; } - if (smbd_smb2_is_compound(smbreq->smb2req)) { + is_compound = smbd_smb2_is_compound(smbreq->smb2req); + is_last_in_compound = smbd_smb2_is_last_in_compound(smbreq->smb2req); + + if (is_compound && !is_last_in_compound) { + /* + * Only allow going async if this is the last + * request in a compound. + */ return NT_STATUS_RETRY; } diff --git a/source4/torture/smb2/compound.c b/source4/torture/smb2/compound.c index 47a550f0873..a9dfd727a7f 100644 --- a/source4/torture/smb2/compound.c +++ b/source4/torture/smb2/compound.c @@ -20,6 +20,7 @@ */ #include "includes.h" +#include "tevent.h" #include "libcli/smb2/smb2.h" #include "libcli/smb2/smb2_calls.h" #include "torture/torture.h" @@ -44,6 +45,13 @@ ret = false; \ }} while (0) +#define WAIT_FOR_ASYNC_RESPONSE(req) \ + while (!req->cancel.can_cancel && req->state <= SMB2_REQUEST_RECV) { \ + if (tevent_loop_once(tctx->ev) != 0) { \ + break; \ + } \ + } + static struct { struct smb2_handle handle; uint8_t level; @@ -1076,6 +1084,7 @@ static bool test_compound_padding(struct torture_context *tctx, struct smb2_handle h; struct smb2_create cr; struct smb2_read r; + struct smb2_read r2; const char *fname = "compound_read.dat"; const char *sname = "compound_read.dat:foo"; struct smb2_request *req[3]; @@ -1123,7 +1132,7 @@ static bool test_compound_padding(struct torture_context *tctx, smb2_util_close(tree, h); /* Check compound read from basefile */ - smb2_transport_compound_start(tree->session->transport, 2); + smb2_transport_compound_start(tree->session->transport, 3); ZERO_STRUCT(cr); cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; @@ -1138,6 +1147,14 @@ static bool test_compound_padding(struct torture_context *tctx, smb2_transport_compound_set_related(tree->session->transport, true); + /* + * We send 2 reads in the compound here as the protocol + * allows the last read to be split off and possibly + * go async. Check the padding on the first read returned, + * not the second as the second may not be part of the + * returned compound. + */ + ZERO_STRUCT(r); h.data[0] = UINT64_MAX; h.data[1] = UINT64_MAX; @@ -1147,6 +1164,15 @@ static bool test_compound_padding(struct torture_context *tctx, r.in.min_count = 1; req[1] = smb2_read_send(tree, &r); + ZERO_STRUCT(r2); + h.data[0] = UINT64_MAX; + h.data[1] = UINT64_MAX; + r2.in.file.handle = h; + r2.in.length = 3; + r2.in.offset = 0; + r2.in.min_count = 1; + req[2] = smb2_read_send(tree, &r2); + status = smb2_create_recv(req[0], tree, &cr); CHECK_STATUS(status, NT_STATUS_OK); @@ -1170,10 +1196,14 @@ static bool test_compound_padding(struct torture_context *tctx, status = smb2_read_recv(req[1], tree, &r); CHECK_STATUS(status, NT_STATUS_OK); + /* Pick up the second, possibly async, read. */ + status = smb2_read_recv(req[2], tree, &r2); + CHECK_STATUS(status, NT_STATUS_OK); + smb2_util_close(tree, cr.out.file.handle); /* Check compound read from stream */ - smb2_transport_compound_start(tree->session->transport, 2); + smb2_transport_compound_start(tree->session->transport, 3); ZERO_STRUCT(cr); cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; @@ -1188,6 +1218,14 @@ static bool test_compound_padding(struct torture_context *tctx, smb2_transport_compound_set_related(tree->session->transport, true); + /* + * We send 2 reads in the compound here as the protocol + * allows the last read to be split off and possibly + * go async. Check the padding on the first read returned, + * not the second as the second may not be part of the + * returned compound. + */ + ZERO_STRUCT(r); h.data[0] = UINT64_MAX; h.data[1] = UINT64_MAX; @@ -1197,6 +1235,15 @@ static bool test_compound_padding(struct torture_context *tctx, r.in.min_count = 1; req[1] = smb2_read_send(tree, &r); + ZERO_STRUCT(r2); + h.data[0] = UINT64_MAX; + h.data[1] = UINT64_MAX; + r2.in.file.handle = h; + r2.in.length = 3; + r2.in.offset = 0; + r2.in.min_count = 1; + req[2] = smb2_read_send(tree, &r2); + status = smb2_create_recv(req[0], tree, &cr); CHECK_STATUS(status, NT_STATUS_OK); @@ -1220,6 +1267,10 @@ static bool test_compound_padding(struct torture_context *tctx, status = smb2_read_recv(req[1], tree, &r); CHECK_STATUS(status, NT_STATUS_OK); + /* Pick up the second, possibly async, read. */ + status = smb2_read_recv(req[2], tree, &r2); + CHECK_STATUS(status, NT_STATUS_OK); + h = cr.out.file.handle; /* Check 2 compound (unrelateated) reads from existing stream handle */ @@ -2274,6 +2325,205 @@ static bool test_compound_async_flush_flush(struct torture_context *tctx, return ret; } +/* + * For Samba/smbd this test must be run against the aio_delay_inject share + * as we need to ensure the last write in the compound takes longer than + * 500 us, which is the threshold for going async in smbd SMB2 writes. + */ + +static bool test_compound_async_write_write(struct torture_context *tctx, + struct smb2_tree *tree) +{ + struct smb2_handle fhandle = { .data = { 0, 0 } }; + struct smb2_handle relhandle = { .data = { UINT64_MAX, UINT64_MAX } }; + struct smb2_write w1; + struct smb2_write w2; + const char *fname = "compound_async_write_write"; + struct smb2_request *req[2]; + NTSTATUS status; + bool is_smbd = torture_setting_bool(tctx, "smbd", true); + bool ret = false; + + /* Start clean. */ + smb2_util_unlink(tree, fname); + + /* Create a file. */ + status = torture_smb2_testfile_access(tree, + fname, + &fhandle, + SEC_RIGHTS_FILE_ALL); + CHECK_STATUS(status, NT_STATUS_OK); + + /* Now do a compound write + write handle. */ + smb2_transport_compound_start(tree->session->transport, 2); + + ZERO_STRUCT(w1); + w1.in.file.handle = fhandle; + w1.in.offset = 0; + w1.in.data = data_blob_talloc_zero(tctx, 64); + req[0] = smb2_write_send(tree, &w1); + + torture_assert_not_null_goto(tctx, req[0], ret, done, + "smb2_write_send (1) failed\n"); + + smb2_transport_compound_set_related(tree->session->transport, true); + + ZERO_STRUCT(w2); + w2.in.file.handle = relhandle; + w2.in.offset = 64; + w2.in.data = data_blob_talloc_zero(tctx, 64); + req[1] = smb2_write_send(tree, &w2); + + torture_assert_not_null_goto(tctx, req[0], ret, done, + "smb2_write_send (2) failed\n"); + + status = smb2_write_recv(req[0], &w1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_write_recv (1) failed."); + + if (!is_smbd) { + /* + * Windows and other servers don't go async. + */ + status = smb2_write_recv(req[1], &w2); + } else { + /* + * For smbd, the second write should go async + * as it's the last element of a compound. + */ + WAIT_FOR_ASYNC_RESPONSE(req[1]); + CHECK_VALUE(req[1]->cancel.can_cancel, true); + /* + * Now pick up the real return. + */ + status = smb2_write_recv(req[1], &w2); + } + + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_write_recv (2) failed."); + + status = smb2_util_close(tree, fhandle); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed."); + ZERO_STRUCT(fhandle); + + ret = true; + + done: + + if (fhandle.data[0] != 0) { + smb2_util_close(tree, fhandle); + } + + smb2_util_unlink(tree, fname); + return ret; +} + +/* + * For Samba/smbd this test must be run against the aio_delay_inject share + * as we need to ensure the last read in the compound takes longer than + * 500 us, which is the threshold for going async in smbd SMB2 reads. + */ + +static bool test_compound_async_read_read(struct torture_context *tctx, + struct smb2_tree *tree) +{ + struct smb2_handle fhandle = { .data = { 0, 0 } }; + struct smb2_handle relhandle = { .data = { UINT64_MAX, UINT64_MAX } }; + struct smb2_write w; + struct smb2_read r1; + struct smb2_read r2; + const char *fname = "compound_async_read_read"; + struct smb2_request *req[2]; + NTSTATUS status; + bool is_smbd = torture_setting_bool(tctx, "smbd", true); + bool ret = false; + + /* Start clean. */ + smb2_util_unlink(tree, fname); + + /* Create a file. */ + status = torture_smb2_testfile_access(tree, + fname, + &fhandle, + SEC_RIGHTS_FILE_ALL); + CHECK_STATUS(status, NT_STATUS_OK); + + /* Write 128 bytes. */ + ZERO_STRUCT(w); + w.in.file.handle = fhandle; + w.in.offset = 0; + w.in.data = data_blob_talloc_zero(tctx, 128); + status = smb2_write(tree, &w); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_write_recv (1) failed."); + + /* Now do a compound read + read handle. */ + smb2_transport_compound_start(tree->session->transport, 2); + + ZERO_STRUCT(r1); + r1.in.file.handle = fhandle; + r1.in.length = 64; + r1.in.offset = 0; + req[0] = smb2_read_send(tree, &r1); + + torture_assert_not_null_goto(tctx, req[0], ret, done, + "smb2_read_send (1) failed\n"); + + smb2_transport_compound_set_related(tree->session->transport, true); + + ZERO_STRUCT(r2); + r2.in.file.handle = relhandle; + r2.in.length = 64; + r2.in.offset = 64; + req[1] = smb2_read_send(tree, &r2); + + torture_assert_not_null_goto(tctx, req[0], ret, done, + "smb2_read_send (2) failed\n"); + + status = smb2_read_recv(req[0], tree, &r1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_read_recv (1) failed."); + + if (!is_smbd) { + /* + * Windows and other servers don't go async. + */ + status = smb2_read_recv(req[1], tree, &r2); + } else { + /* + * For smbd, the second write should go async + * as it's the last element of a compound. + */ + WAIT_FOR_ASYNC_RESPONSE(req[1]); + CHECK_VALUE(req[1]->cancel.can_cancel, true); + /* + * Now pick up the real return. + */ + status = smb2_read_recv(req[1], tree, &r2); + } + + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_read_recv (2) failed."); + + status = smb2_util_close(tree, fhandle); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed."); + ZERO_STRUCT(fhandle); + + ret = true; + + done: + + if (fhandle.data[0] != 0) { + smb2_util_close(tree, fhandle); + } + + smb2_util_unlink(tree, fname); + return ret; +} + + struct torture_suite *torture_smb2_compound_init(TALLOC_CTX *ctx) { struct torture_suite *suite = torture_suite_create(ctx, "compound"); @@ -2334,6 +2584,10 @@ struct torture_suite *torture_smb2_compound_async_init(TALLOC_CTX *ctx) test_compound_async_flush_close); torture_suite_add_1smb2_test(suite, "flush_flush", test_compound_async_flush_flush); + torture_suite_add_1smb2_test(suite, "write_write", + test_compound_async_write_write); + torture_suite_add_1smb2_test(suite, "read_read", + test_compound_async_read_read); suite->description = talloc_strdup(suite, "SMB2-COMPOUND-ASYNC tests"); -- Samba Shared Repository