[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Thu Dec 1 16:05:01 UTC 2022


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 <jra at 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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Thu Dec  1 16:04:07 UTC 2022 on sn-devel-184

commit 0bb4810719ce0864114d84b72f8d3b206f1a7d0e
Author: Jeremy Allison <jra at 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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 088b8a1e3e56cc24a7c2a469042d1ece9e84df38
Author: Jeremy Allison <jra at 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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit ffd9b94fe0f59c2b552402543db406cb69003745
Author: Jeremy Allison <jra at 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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit fc6c76e6dabdc20bc7401cc2268baa6edb635ee1
Author: Jeremy Allison <jra at 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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 48b12f11a5c4ebd9affb2a2589f47881b46b659b
Author: Jeremy Allison <jra at 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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit f5b2ae58093a0920c7be0394f638b73736fbebc2
Author: Jeremy Allison <jra at 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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at 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



More information about the samba-cvs mailing list