linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] use compounding to speed up readdir()
@ 2019-12-04 22:54 Ronnie Sahlberg
  2019-12-04 22:54 ` [PATCH 1/3] cifs: prepare SMB2_query_directory to be used with compounding Ronnie Sahlberg
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ronnie Sahlberg @ 2019-12-04 22:54 UTC (permalink / raw)
  To: linux-cifs

Steve, List

This three patches are the first step in using compounding to speed up
readdir() which currently takes a minimum of 4 roundtrips for any non-empty
directory.
With these patches we reduce one roundtrip and we can list a directory
in just 3, instead of 4, roundtrips which will benefit use-cases where
latency to the server is high.

I.e. this changes the sequence of operations for a small directory from
1, Open
2, Query and get data
3, Query loop until we get STATUS_NO_MORE_FILES
4, Close

To be the slightly better
1, Open + Query and get data
2, Query and get STATUS_NO_MORE_FILES
3, Close


In later patches we can do even better and drive this down to just 2 roundtrips
for a small nonempty directory by using
1, Open + Query + Query
2, Close
for the case where we get STATUS_NO_MORE_FILES for the second Query.
And bring it down to just two roundtrips.

That is probably the best we can do for Windows based servers since without
support for the SMB2_INDEX_SPECIFIED flag in the QueryDirectory request
we can not put the Close() as part of the compound.


IF we had SMB2_INDEX_SPECIFIED support on some server (Azure?) and IF we
had a way to reliably detect if the server supports this flag or not then
we could change the sequence to be
Open + Query + Query + Close
and if the second Query returned STATUS_NO_MORE_FILES we would have finished thereaddir() in a single roundtrip.
If the directory is large and the second query did not return this error then
we could just continue and use this compound instead to loop until we get to the end :
Open() + Query(SMB2_INDEX_SPECIFIED, Index) + Close()


regards
Ronnie Sahlberg



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] cifs: prepare SMB2_query_directory to be used with compounding
  2019-12-04 22:54 [PATCH 0/3] use compounding to speed up readdir() Ronnie Sahlberg
@ 2019-12-04 22:54 ` Ronnie Sahlberg
  2019-12-23 23:48   ` Pavel Shilovsky
  2019-12-04 22:54 ` [PATCH 2/3] cifs: create a helper function to parse the query-directory response buffer Ronnie Sahlberg
  2019-12-04 22:54 ` [PATCH 3/3] cifs: use compounding for open and first query-dir for readdir() Ronnie Sahlberg
  2 siblings, 1 reply; 7+ messages in thread
From: Ronnie Sahlberg @ 2019-12-04 22:54 UTC (permalink / raw)
  To: linux-cifs; +Cc: Ronnie Sahlberg

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2pdu.c   | 108 +++++++++++++++++++++++++++++++++++-----------------
 fs/cifs/smb2pdu.h   |   2 +
 fs/cifs/smb2proto.h |   5 +++
 3 files changed, 80 insertions(+), 35 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index ed77f94dbf1d..df903931590e 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -4214,56 +4214,36 @@ num_entries(char *bufstart, char *end_of_buf, char **lastentry, size_t size)
 /*
  * Readdir/FindFirst
  */
-int
-SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
-		     u64 persistent_fid, u64 volatile_fid, int index,
-		     struct cifs_search_info *srch_inf)
+int SMB2_query_directory_init(const unsigned int xid,
+			      struct cifs_tcon *tcon, struct smb_rqst *rqst,
+			      u64 persistent_fid, u64 volatile_fid,
+			      int index, int info_level)
 {
-	struct smb_rqst rqst;
+	struct TCP_Server_Info *server = tcon->ses->server;
 	struct smb2_query_directory_req *req;
-	struct smb2_query_directory_rsp *rsp = NULL;
-	struct kvec iov[2];
-	struct kvec rsp_iov;
-	int rc = 0;
-	int len;
-	int resp_buftype = CIFS_NO_BUFFER;
 	unsigned char *bufptr;
-	struct TCP_Server_Info *server;
-	struct cifs_ses *ses = tcon->ses;
 	__le16 asteriks = cpu_to_le16('*');
-	char *end_of_smb;
 	unsigned int output_size = CIFSMaxBufSize;
-	size_t info_buf_size;
-	int flags = 0;
 	unsigned int total_len;
-
-	if (ses && (ses->server))
-		server = ses->server;
-	else
-		return -EIO;
+	struct kvec *iov = rqst->rq_iov;
+	int len, rc;
 
 	rc = smb2_plain_req_init(SMB2_QUERY_DIRECTORY, tcon, (void **) &req,
 			     &total_len);
 	if (rc)
 		return rc;
 
-	if (smb3_encryption_required(tcon))
-		flags |= CIFS_TRANSFORM_REQ;
-
-	switch (srch_inf->info_level) {
+	switch (info_level) {
 	case SMB_FIND_FILE_DIRECTORY_INFO:
 		req->FileInformationClass = FILE_DIRECTORY_INFORMATION;
-		info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
 		break;
 	case SMB_FIND_FILE_ID_FULL_DIR_INFO:
 		req->FileInformationClass = FILEID_FULL_DIRECTORY_INFORMATION;
-		info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
 		break;
 	default:
 		cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
-			 srch_inf->info_level);
-		rc = -EINVAL;
-		goto qdir_exit;
+			info_level);
+		return -EINVAL;
 	}
 
 	req->FileIndex = cpu_to_le32(index);
@@ -4292,15 +4272,56 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
 	iov[1].iov_base = (char *)(req->Buffer);
 	iov[1].iov_len = len;
 
+	trace_smb3_query_dir_enter(xid, persistent_fid, tcon->tid,
+			tcon->ses->Suid, index, output_size);
+
+	return 0;
+}
+
+void SMB2_query_directory_free(struct smb_rqst *rqst)
+{
+	if (rqst && rqst->rq_iov) {
+		cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
+	}
+}
+
+int
+SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
+		     u64 persistent_fid, u64 volatile_fid, int index,
+		     struct cifs_search_info *srch_inf)
+{
+	struct smb_rqst rqst;
+	struct kvec iov[SMB2_QUERY_DIRECTORY_IOV_SIZE];
+	struct smb2_query_directory_rsp *rsp = NULL;
+	int resp_buftype = CIFS_NO_BUFFER;
+	struct kvec rsp_iov;
+	int rc = 0;
+	struct TCP_Server_Info *server;
+	struct cifs_ses *ses = tcon->ses;
+	char *end_of_smb;
+	size_t info_buf_size;
+	int flags = 0;
+
+	if (ses && (ses->server))
+		server = ses->server;
+	else
+		return -EIO;
+
+	if (smb3_encryption_required(tcon))
+		flags |= CIFS_TRANSFORM_REQ;
+
 	memset(&rqst, 0, sizeof(struct smb_rqst));
+	memset(&iov, 0, sizeof(iov));
 	rqst.rq_iov = iov;
-	rqst.rq_nvec = 2;
+	rqst.rq_nvec = SMB2_QUERY_DIRECTORY_IOV_SIZE;
 
-	trace_smb3_query_dir_enter(xid, persistent_fid, tcon->tid,
-			tcon->ses->Suid, index, output_size);
+	rc = SMB2_query_directory_init(xid, tcon, &rqst, persistent_fid,
+				       volatile_fid, index,
+				       srch_inf->info_level);
+	if (rc)
+		goto qdir_exit;
 
 	rc = cifs_send_recv(xid, ses, &rqst, &resp_buftype, flags, &rsp_iov);
-	cifs_small_buf_release(req);
 	rsp = (struct smb2_query_directory_rsp *)rsp_iov.iov_base;
 
 	if (rc) {
@@ -4318,6 +4339,20 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
 		goto qdir_exit;
 	}
 
+	switch (srch_inf->info_level) {
+	case SMB_FIND_FILE_DIRECTORY_INFO:
+		info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
+		break;
+	case SMB_FIND_FILE_ID_FULL_DIR_INFO:
+		info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
+		break;
+	default:
+		cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
+			 srch_inf->info_level);
+		rc = -EINVAL;
+		goto qdir_exit;
+	}
+
 	rc = smb2_validate_iov(le16_to_cpu(rsp->OutputBufferOffset),
 			       le32_to_cpu(rsp->OutputBufferLength), &rsp_iov,
 			       info_buf_size);
@@ -4353,11 +4388,14 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
 	else
 		cifs_tcon_dbg(VFS, "illegal search buffer type\n");
 
+	rsp = NULL;
+	resp_buftype = CIFS_NO_BUFFER;
+
 	trace_smb3_query_dir_done(xid, persistent_fid, tcon->tid,
 			tcon->ses->Suid, index, srch_inf->entries_in_buffer);
-	return rc;
 
 qdir_exit:
+	SMB2_query_directory_free(&rqst);
 	free_rsp_buf(resp_buftype, rsp);
 	return rc;
 }
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index f264e1d36fe1..caf323be0d7f 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -1272,6 +1272,8 @@ struct smb2_echo_rsp {
 #define SMB2_INDEX_SPECIFIED		0x04
 #define SMB2_REOPEN			0x10
 
+#define SMB2_QUERY_DIRECTORY_IOV_SIZE 2
+
 struct smb2_query_directory_req {
 	struct smb2_sync_hdr sync_hdr;
 	__le16 StructureSize; /* Must be 33 */
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index d21a5fcc8d06..ba48ce9af620 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -194,6 +194,11 @@ extern int SMB2_echo(struct TCP_Server_Info *server);
 extern int SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
 				u64 persistent_fid, u64 volatile_fid, int index,
 				struct cifs_search_info *srch_inf);
+extern int SMB2_query_directory_init(unsigned int xid, struct cifs_tcon *tcon,
+				     struct smb_rqst *rqst,
+				     u64 persistent_fid, u64 volatile_fid,
+				     int index, int info_level);
+extern void SMB2_query_directory_free(struct smb_rqst *rqst);
 extern int SMB2_set_eof(const unsigned int xid, struct cifs_tcon *tcon,
 			u64 persistent_fid, u64 volatile_fid, u32 pid,
 			__le64 *eof);
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] cifs: create a helper function to parse the query-directory response buffer
  2019-12-04 22:54 [PATCH 0/3] use compounding to speed up readdir() Ronnie Sahlberg
  2019-12-04 22:54 ` [PATCH 1/3] cifs: prepare SMB2_query_directory to be used with compounding Ronnie Sahlberg
@ 2019-12-04 22:54 ` Ronnie Sahlberg
  2019-12-04 22:54 ` [PATCH 3/3] cifs: use compounding for open and first query-dir for readdir() Ronnie Sahlberg
  2 siblings, 0 replies; 7+ messages in thread
From: Ronnie Sahlberg @ 2019-12-04 22:54 UTC (permalink / raw)
  To: linux-cifs; +Cc: Ronnie Sahlberg

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2pdu.c | 110 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 63 insertions(+), 47 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index df903931590e..4e1bd20d97ae 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -4286,6 +4286,67 @@ void SMB2_query_directory_free(struct smb_rqst *rqst)
 }
 
 int
+smb2_parse_query_directory(struct cifs_tcon *tcon,
+			   struct kvec *rsp_iov,
+			   int resp_buftype,
+			   struct cifs_search_info *srch_inf)
+{
+	struct smb2_query_directory_rsp *rsp;
+	size_t info_buf_size;
+	char *end_of_smb;
+	int rc;
+
+	rsp = (struct smb2_query_directory_rsp *)rsp_iov->iov_base;
+
+	switch (srch_inf->info_level) {
+	case SMB_FIND_FILE_DIRECTORY_INFO:
+		info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
+		break;
+	case SMB_FIND_FILE_ID_FULL_DIR_INFO:
+		info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
+		break;
+	default:
+		cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
+			 srch_inf->info_level);
+		return -EINVAL;
+	}
+
+	rc = smb2_validate_iov(le16_to_cpu(rsp->OutputBufferOffset),
+			       le32_to_cpu(rsp->OutputBufferLength), rsp_iov,
+			       info_buf_size);
+	if (rc)
+		return rc;
+
+	srch_inf->unicode = true;
+
+	if (srch_inf->ntwrk_buf_start) {
+		if (srch_inf->smallBuf)
+			cifs_small_buf_release(srch_inf->ntwrk_buf_start);
+		else
+			cifs_buf_release(srch_inf->ntwrk_buf_start);
+	}
+	srch_inf->ntwrk_buf_start = (char *)rsp;
+	srch_inf->srch_entries_start = srch_inf->last_entry =
+		(char *)rsp + le16_to_cpu(rsp->OutputBufferOffset);
+	end_of_smb = rsp_iov->iov_len + (char *)rsp;
+	srch_inf->entries_in_buffer =
+			num_entries(srch_inf->srch_entries_start, end_of_smb,
+				    &srch_inf->last_entry, info_buf_size);
+	srch_inf->index_of_last_entry += srch_inf->entries_in_buffer;
+	cifs_dbg(FYI, "num entries %d last_index %lld srch start %p srch end %p\n",
+		 srch_inf->entries_in_buffer, srch_inf->index_of_last_entry,
+		 srch_inf->srch_entries_start, srch_inf->last_entry);
+	if (resp_buftype == CIFS_LARGE_BUFFER)
+		srch_inf->smallBuf = false;
+	else if (resp_buftype == CIFS_SMALL_BUFFER)
+		srch_inf->smallBuf = true;
+	else
+		cifs_tcon_dbg(VFS, "illegal search buffer type\n");
+
+	return 0;
+}
+
+int
 SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
 		     u64 persistent_fid, u64 volatile_fid, int index,
 		     struct cifs_search_info *srch_inf)
@@ -4298,8 +4359,6 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
 	int rc = 0;
 	struct TCP_Server_Info *server;
 	struct cifs_ses *ses = tcon->ses;
-	char *end_of_smb;
-	size_t info_buf_size;
 	int flags = 0;
 
 	if (ses && (ses->server))
@@ -4339,56 +4398,13 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
 		goto qdir_exit;
 	}
 
-	switch (srch_inf->info_level) {
-	case SMB_FIND_FILE_DIRECTORY_INFO:
-		info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
-		break;
-	case SMB_FIND_FILE_ID_FULL_DIR_INFO:
-		info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
-		break;
-	default:
-		cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
-			 srch_inf->info_level);
-		rc = -EINVAL;
-		goto qdir_exit;
-	}
-
-	rc = smb2_validate_iov(le16_to_cpu(rsp->OutputBufferOffset),
-			       le32_to_cpu(rsp->OutputBufferLength), &rsp_iov,
-			       info_buf_size);
+	rc = smb2_parse_query_directory(tcon, &rsp_iov,	resp_buftype,
+					srch_inf);
 	if (rc) {
 		trace_smb3_query_dir_err(xid, persistent_fid, tcon->tid,
 			tcon->ses->Suid, index, 0, rc);
 		goto qdir_exit;
 	}
-
-	srch_inf->unicode = true;
-
-	if (srch_inf->ntwrk_buf_start) {
-		if (srch_inf->smallBuf)
-			cifs_small_buf_release(srch_inf->ntwrk_buf_start);
-		else
-			cifs_buf_release(srch_inf->ntwrk_buf_start);
-	}
-	srch_inf->ntwrk_buf_start = (char *)rsp;
-	srch_inf->srch_entries_start = srch_inf->last_entry =
-		(char *)rsp + le16_to_cpu(rsp->OutputBufferOffset);
-	end_of_smb = rsp_iov.iov_len + (char *)rsp;
-	srch_inf->entries_in_buffer =
-			num_entries(srch_inf->srch_entries_start, end_of_smb,
-				    &srch_inf->last_entry, info_buf_size);
-	srch_inf->index_of_last_entry += srch_inf->entries_in_buffer;
-	cifs_dbg(FYI, "num entries %d last_index %lld srch start %p srch end %p\n",
-		 srch_inf->entries_in_buffer, srch_inf->index_of_last_entry,
-		 srch_inf->srch_entries_start, srch_inf->last_entry);
-	if (resp_buftype == CIFS_LARGE_BUFFER)
-		srch_inf->smallBuf = false;
-	else if (resp_buftype == CIFS_SMALL_BUFFER)
-		srch_inf->smallBuf = true;
-	else
-		cifs_tcon_dbg(VFS, "illegal search buffer type\n");
-
-	rsp = NULL;
 	resp_buftype = CIFS_NO_BUFFER;
 
 	trace_smb3_query_dir_done(xid, persistent_fid, tcon->tid,
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] cifs: use compounding for open and first query-dir for readdir()
  2019-12-04 22:54 [PATCH 0/3] use compounding to speed up readdir() Ronnie Sahlberg
  2019-12-04 22:54 ` [PATCH 1/3] cifs: prepare SMB2_query_directory to be used with compounding Ronnie Sahlberg
  2019-12-04 22:54 ` [PATCH 2/3] cifs: create a helper function to parse the query-directory response buffer Ronnie Sahlberg
@ 2019-12-04 22:54 ` Ronnie Sahlberg
  2 siblings, 0 replies; 7+ messages in thread
From: Ronnie Sahlberg @ 2019-12-04 22:54 UTC (permalink / raw)
  To: linux-cifs; +Cc: Ronnie Sahlberg

Combine the initial SMB2_Open and the first SMB2_Query_Directory in a compound.
This shaves one round-trip of each directory listing, changing it from 4 to 3
for small directories.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cifsproto.h |  3 ++
 fs/cifs/smb2ops.c   | 96 ++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 87 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 1ed695336f62..b276281ec0d8 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -595,6 +595,9 @@ bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface);
 
 void extract_unc_hostname(const char *unc, const char **h, size_t *len);
 int copy_path_name(char *dst, const char *src);
+int smb2_parse_query_directory(struct cifs_tcon *tcon, struct kvec *rsp_iov,
+			       int resp_buftype,
+			       struct cifs_search_info *srch_inf);
 
 #ifdef CONFIG_CIFS_DFS_UPCALL
 static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index a7f328f79c6f..09949beea769 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1997,14 +1997,33 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
 		     struct cifs_search_info *srch_inf)
 {
 	__le16 *utf16_path;
-	int rc;
-	__u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
+	struct smb_rqst rqst[2];
+	struct kvec rsp_iov[2];
+	int resp_buftype[2];
+	struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
+	struct kvec qd_iov[SMB2_QUERY_DIRECTORY_IOV_SIZE];
+	int rc, flags = 0;
+	u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
 	struct cifs_open_parms oparms;
+	struct smb2_query_directory_rsp *qd_rsp = NULL;
+	struct smb2_create_rsp *op_rsp = NULL;
 
 	utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
 	if (!utf16_path)
 		return -ENOMEM;
 
+	if (smb3_encryption_required(tcon))
+		flags |= CIFS_TRANSFORM_REQ;
+
+	memset(rqst, 0, sizeof(rqst));
+	resp_buftype[0] = resp_buftype[1] = CIFS_NO_BUFFER;
+	memset(rsp_iov, 0, sizeof(rsp_iov));
+
+	/* Open */
+	memset(&open_iov, 0, sizeof(open_iov));
+	rqst[0].rq_iov = open_iov;
+	rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
+
 	oparms.tcon = tcon;
 	oparms.desired_access = FILE_READ_ATTRIBUTES | FILE_READ_DATA;
 	oparms.disposition = FILE_OPEN;
@@ -2015,22 +2034,75 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
 	oparms.fid = fid;
 	oparms.reconnect = false;
 
-	rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL);
-	kfree(utf16_path);
-	if (rc) {
-		cifs_dbg(FYI, "open dir failed rc=%d\n", rc);
-		return rc;
-	}
+	rc = SMB2_open_init(tcon, &rqst[0], &oplock, &oparms, utf16_path);
+	if (rc)
+		goto qdf_free;
+	smb2_set_next_command(tcon, &rqst[0]);
 
+	/* Query directory */
 	srch_inf->entries_in_buffer = 0;
 	srch_inf->index_of_last_entry = 2;
 
-	rc = SMB2_query_directory(xid, tcon, fid->persistent_fid,
-				  fid->volatile_fid, 0, srch_inf);
-	if (rc) {
-		cifs_dbg(FYI, "query directory failed rc=%d\n", rc);
+	memset(&qd_iov, 0, sizeof(qd_iov));
+	rqst[1].rq_iov = qd_iov;
+	rqst[1].rq_nvec = SMB2_QUERY_DIRECTORY_IOV_SIZE;
+
+	rc = SMB2_query_directory_init(xid, tcon, &rqst[1],
+				       COMPOUND_FID, COMPOUND_FID,
+				       0, srch_inf->info_level);
+	if (rc)
+		goto qdf_free;
+
+	smb2_set_related(&rqst[1]);
+
+	rc = compound_send_recv(xid, tcon->ses, flags, 2, rqst,
+				resp_buftype, rsp_iov);
+
+	/* If the open failed there is nothing to do */
+	op_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
+	if (op_rsp == NULL || op_rsp->sync_hdr.Status != STATUS_SUCCESS) {
+		cifs_dbg(FYI, "query_dir_first: open failed rc=%d\n", rc);
+		goto qdf_free;
+	}
+	fid->persistent_fid = op_rsp->PersistentFileId;
+	fid->volatile_fid = op_rsp->VolatileFileId;
+
+	/* Anything else than ENODATA means a genuine error */
+	if (rc && rc != -ENODATA) {
 		SMB2_close(xid, tcon, fid->persistent_fid, fid->volatile_fid);
+		cifs_dbg(FYI, "query_dir_first: query directory failed rc=%d\n", rc);
+		trace_smb3_query_dir_err(xid, fid->persistent_fid,
+					 tcon->tid, tcon->ses->Suid, 0, 0, rc);
+		goto qdf_free;
 	}
+
+	qd_rsp = (struct smb2_query_directory_rsp *)rsp_iov[1].iov_base;
+	if (qd_rsp->sync_hdr.Status == STATUS_NO_MORE_FILES) {
+		trace_smb3_query_dir_done(xid, fid->persistent_fid,
+					  tcon->tid, tcon->ses->Suid, 0, 0);
+		srch_inf->endOfSearch = true;
+		rc = 0;
+		goto qdf_free;
+	}
+
+	rc = smb2_parse_query_directory(tcon, &rsp_iov[1], resp_buftype[1],
+					srch_inf);
+	if (rc) {
+		trace_smb3_query_dir_err(xid, fid->persistent_fid, tcon->tid,
+			tcon->ses->Suid, 0, 0, rc);
+		goto qdf_free;
+	}
+	resp_buftype[1] = CIFS_NO_BUFFER;
+
+	trace_smb3_query_dir_done(xid, fid->persistent_fid, tcon->tid,
+			tcon->ses->Suid, 0, srch_inf->entries_in_buffer);
+
+ qdf_free:
+	kfree(utf16_path);
+	SMB2_open_free(&rqst[0]);
+	SMB2_query_directory_free(&rqst[1]);
+	free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
+	free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
 	return rc;
 }
 
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] cifs: prepare SMB2_query_directory to be used with compounding
  2019-12-04 22:54 ` [PATCH 1/3] cifs: prepare SMB2_query_directory to be used with compounding Ronnie Sahlberg
@ 2019-12-23 23:48   ` Pavel Shilovsky
  2020-01-08  3:09     ` ronnie sahlberg
  0 siblings, 1 reply; 7+ messages in thread
From: Pavel Shilovsky @ 2019-12-23 23:48 UTC (permalink / raw)
  To: Ronnie Sahlberg; +Cc: linux-cifs

ср, 4 дек. 2019 г. в 14:54, Ronnie Sahlberg <lsahlber@redhat.com>:
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/smb2pdu.c   | 108 +++++++++++++++++++++++++++++++++++-----------------
>  fs/cifs/smb2pdu.h   |   2 +
>  fs/cifs/smb2proto.h |   5 +++
>  3 files changed, 80 insertions(+), 35 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index ed77f94dbf1d..df903931590e 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -4214,56 +4214,36 @@ num_entries(char *bufstart, char *end_of_buf, char **lastentry, size_t size)
>  /*
>   * Readdir/FindFirst
>   */
> -int
> -SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> -                    u64 persistent_fid, u64 volatile_fid, int index,
> -                    struct cifs_search_info *srch_inf)
> +int SMB2_query_directory_init(const unsigned int xid,
> +                             struct cifs_tcon *tcon, struct smb_rqst *rqst,
> +                             u64 persistent_fid, u64 volatile_fid,
> +                             int index, int info_level)
>  {
> -       struct smb_rqst rqst;
> +       struct TCP_Server_Info *server = tcon->ses->server;
>         struct smb2_query_directory_req *req;
> -       struct smb2_query_directory_rsp *rsp = NULL;
> -       struct kvec iov[2];
> -       struct kvec rsp_iov;
> -       int rc = 0;
> -       int len;
> -       int resp_buftype = CIFS_NO_BUFFER;
>         unsigned char *bufptr;
> -       struct TCP_Server_Info *server;
> -       struct cifs_ses *ses = tcon->ses;
>         __le16 asteriks = cpu_to_le16('*');
> -       char *end_of_smb;
>         unsigned int output_size = CIFSMaxBufSize;

I think we need to account for compound responses here. The response
buffer size is MAX_SMB2_HDR_SIZE + CIFSMaxBufSize, where
MAX_SMB2_HDR_SIZE is 52 transform hdr + 64 hdr + 88 create rsp. While
output_size being CIFSMaxBufSize worked for non-compounded query
directory responses it may not fit into buffer for compounded ones
when encryption is used.

It seem like we need to set it to CIFSMaxBufSize -
MAX_SMB2_CREATE_RESPONSE_SIZE - MAX_SMB2_CLOSE_RESPONSE_SIZE like to
do in smb2_query_eas().

I found other occurrences of the similar issue in smb2_query_symlink()
and smb2_ioctl_query_info(): SMB2_ioctl_init() should use
CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
MAX_SMB2_CLOSE_RESPONSE_SIZE too instead of CIFSMaxBufSize. Could you
fix those together with this one?


> -       size_t info_buf_size;
> -       int flags = 0;
>         unsigned int total_len;
> -
> -       if (ses && (ses->server))
> -               server = ses->server;
> -       else
> -               return -EIO;
> +       struct kvec *iov = rqst->rq_iov;
> +       int len, rc;
>
>         rc = smb2_plain_req_init(SMB2_QUERY_DIRECTORY, tcon, (void **) &req,
>                              &total_len);
>         if (rc)
>                 return rc;
>
> -       if (smb3_encryption_required(tcon))
> -               flags |= CIFS_TRANSFORM_REQ;
> -
> -       switch (srch_inf->info_level) {
> +       switch (info_level) {
>         case SMB_FIND_FILE_DIRECTORY_INFO:
>                 req->FileInformationClass = FILE_DIRECTORY_INFORMATION;
> -               info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
>                 break;
>         case SMB_FIND_FILE_ID_FULL_DIR_INFO:
>                 req->FileInformationClass = FILEID_FULL_DIRECTORY_INFORMATION;
> -               info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
>                 break;
>         default:
>                 cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
> -                        srch_inf->info_level);
> -               rc = -EINVAL;
> -               goto qdir_exit;
> +                       info_level);
> +               return -EINVAL;
>         }
>
>         req->FileIndex = cpu_to_le32(index);
> @@ -4292,15 +4272,56 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
>         iov[1].iov_base = (char *)(req->Buffer);
>         iov[1].iov_len = len;
>
> +       trace_smb3_query_dir_enter(xid, persistent_fid, tcon->tid,
> +                       tcon->ses->Suid, index, output_size);
> +
> +       return 0;
> +}
> +
> +void SMB2_query_directory_free(struct smb_rqst *rqst)
> +{
> +       if (rqst && rqst->rq_iov) {
> +               cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
> +       }
> +}
> +
> +int
> +SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> +                    u64 persistent_fid, u64 volatile_fid, int index,
> +                    struct cifs_search_info *srch_inf)
> +{
> +       struct smb_rqst rqst;
> +       struct kvec iov[SMB2_QUERY_DIRECTORY_IOV_SIZE];
> +       struct smb2_query_directory_rsp *rsp = NULL;
> +       int resp_buftype = CIFS_NO_BUFFER;
> +       struct kvec rsp_iov;
> +       int rc = 0;
> +       struct TCP_Server_Info *server;
> +       struct cifs_ses *ses = tcon->ses;
> +       char *end_of_smb;
> +       size_t info_buf_size;
> +       int flags = 0;
> +
> +       if (ses && (ses->server))
> +               server = ses->server;
> +       else
> +               return -EIO;
> +
> +       if (smb3_encryption_required(tcon))
> +               flags |= CIFS_TRANSFORM_REQ;
> +
>         memset(&rqst, 0, sizeof(struct smb_rqst));
> +       memset(&iov, 0, sizeof(iov));
>         rqst.rq_iov = iov;
> -       rqst.rq_nvec = 2;
> +       rqst.rq_nvec = SMB2_QUERY_DIRECTORY_IOV_SIZE;
>
> -       trace_smb3_query_dir_enter(xid, persistent_fid, tcon->tid,
> -                       tcon->ses->Suid, index, output_size);
> +       rc = SMB2_query_directory_init(xid, tcon, &rqst, persistent_fid,
> +                                      volatile_fid, index,
> +                                      srch_inf->info_level);
> +       if (rc)
> +               goto qdir_exit;
>
>         rc = cifs_send_recv(xid, ses, &rqst, &resp_buftype, flags, &rsp_iov);
> -       cifs_small_buf_release(req);
>         rsp = (struct smb2_query_directory_rsp *)rsp_iov.iov_base;
>
>         if (rc) {
> @@ -4318,6 +4339,20 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
>                 goto qdir_exit;
>         }
>
> +       switch (srch_inf->info_level) {
> +       case SMB_FIND_FILE_DIRECTORY_INFO:
> +               info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
> +               break;
> +       case SMB_FIND_FILE_ID_FULL_DIR_INFO:
> +               info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
> +               break;
> +       default:
> +               cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
> +                        srch_inf->info_level);
> +               rc = -EINVAL;
> +               goto qdir_exit;
> +       }
> +
>         rc = smb2_validate_iov(le16_to_cpu(rsp->OutputBufferOffset),
>                                le32_to_cpu(rsp->OutputBufferLength), &rsp_iov,
>                                info_buf_size);
> @@ -4353,11 +4388,14 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
>         else
>                 cifs_tcon_dbg(VFS, "illegal search buffer type\n");
>
> +       rsp = NULL;

minor: the subsequent patch removes this assignment, probably better
to not do it here.

> +       resp_buftype = CIFS_NO_BUFFER;
> +
>         trace_smb3_query_dir_done(xid, persistent_fid, tcon->tid,
>                         tcon->ses->Suid, index, srch_inf->entries_in_buffer);
> -       return rc;
>
>  qdir_exit:
> +       SMB2_query_directory_free(&rqst);
>         free_rsp_buf(resp_buftype, rsp);
>         return rc;
>  }
> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> index f264e1d36fe1..caf323be0d7f 100644
> --- a/fs/cifs/smb2pdu.h
> +++ b/fs/cifs/smb2pdu.h
> @@ -1272,6 +1272,8 @@ struct smb2_echo_rsp {
>  #define SMB2_INDEX_SPECIFIED           0x04
>  #define SMB2_REOPEN                    0x10
>
> +#define SMB2_QUERY_DIRECTORY_IOV_SIZE 2
> +
>  struct smb2_query_directory_req {
>         struct smb2_sync_hdr sync_hdr;
>         __le16 StructureSize; /* Must be 33 */
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index d21a5fcc8d06..ba48ce9af620 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -194,6 +194,11 @@ extern int SMB2_echo(struct TCP_Server_Info *server);
>  extern int SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
>                                 u64 persistent_fid, u64 volatile_fid, int index,
>                                 struct cifs_search_info *srch_inf);
> +extern int SMB2_query_directory_init(unsigned int xid, struct cifs_tcon *tcon,
> +                                    struct smb_rqst *rqst,
> +                                    u64 persistent_fid, u64 volatile_fid,
> +                                    int index, int info_level);
> +extern void SMB2_query_directory_free(struct smb_rqst *rqst);
>  extern int SMB2_set_eof(const unsigned int xid, struct cifs_tcon *tcon,
>                         u64 persistent_fid, u64 volatile_fid, u32 pid,
>                         __le64 *eof);
> --
> 2.13.6
>

Other than the note about buffer sizes above the series looks good at
the first glance.

--
Best regards,
Pavel Shilovsky

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] cifs: prepare SMB2_query_directory to be used with compounding
  2019-12-23 23:48   ` Pavel Shilovsky
@ 2020-01-08  3:09     ` ronnie sahlberg
  0 siblings, 0 replies; 7+ messages in thread
From: ronnie sahlberg @ 2020-01-08  3:09 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: Ronnie Sahlberg, linux-cifs

Thanks,

I have resent it after addressing your concerns.
I added an extra patch to fix the two places calling smb2_ioctl_init()
that were unrelated to opendir/readdir

regards
ronnie sahlberg

On Tue, Dec 24, 2019 at 9:50 AM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> ср, 4 дек. 2019 г. в 14:54, Ronnie Sahlberg <lsahlber@redhat.com>:
> >
> > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > ---
> >  fs/cifs/smb2pdu.c   | 108 +++++++++++++++++++++++++++++++++++-----------------
> >  fs/cifs/smb2pdu.h   |   2 +
> >  fs/cifs/smb2proto.h |   5 +++
> >  3 files changed, 80 insertions(+), 35 deletions(-)
> >
> > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> > index ed77f94dbf1d..df903931590e 100644
> > --- a/fs/cifs/smb2pdu.c
> > +++ b/fs/cifs/smb2pdu.c
> > @@ -4214,56 +4214,36 @@ num_entries(char *bufstart, char *end_of_buf, char **lastentry, size_t size)
> >  /*
> >   * Readdir/FindFirst
> >   */
> > -int
> > -SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> > -                    u64 persistent_fid, u64 volatile_fid, int index,
> > -                    struct cifs_search_info *srch_inf)
> > +int SMB2_query_directory_init(const unsigned int xid,
> > +                             struct cifs_tcon *tcon, struct smb_rqst *rqst,
> > +                             u64 persistent_fid, u64 volatile_fid,
> > +                             int index, int info_level)
> >  {
> > -       struct smb_rqst rqst;
> > +       struct TCP_Server_Info *server = tcon->ses->server;
> >         struct smb2_query_directory_req *req;
> > -       struct smb2_query_directory_rsp *rsp = NULL;
> > -       struct kvec iov[2];
> > -       struct kvec rsp_iov;
> > -       int rc = 0;
> > -       int len;
> > -       int resp_buftype = CIFS_NO_BUFFER;
> >         unsigned char *bufptr;
> > -       struct TCP_Server_Info *server;
> > -       struct cifs_ses *ses = tcon->ses;
> >         __le16 asteriks = cpu_to_le16('*');
> > -       char *end_of_smb;
> >         unsigned int output_size = CIFSMaxBufSize;
>
> I think we need to account for compound responses here. The response
> buffer size is MAX_SMB2_HDR_SIZE + CIFSMaxBufSize, where
> MAX_SMB2_HDR_SIZE is 52 transform hdr + 64 hdr + 88 create rsp. While
> output_size being CIFSMaxBufSize worked for non-compounded query
> directory responses it may not fit into buffer for compounded ones
> when encryption is used.
>
> It seem like we need to set it to CIFSMaxBufSize -
> MAX_SMB2_CREATE_RESPONSE_SIZE - MAX_SMB2_CLOSE_RESPONSE_SIZE like to
> do in smb2_query_eas().
>
> I found other occurrences of the similar issue in smb2_query_symlink()
> and smb2_ioctl_query_info(): SMB2_ioctl_init() should use
> CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
> MAX_SMB2_CLOSE_RESPONSE_SIZE too instead of CIFSMaxBufSize. Could you
> fix those together with this one?
>
>
> > -       size_t info_buf_size;
> > -       int flags = 0;
> >         unsigned int total_len;
> > -
> > -       if (ses && (ses->server))
> > -               server = ses->server;
> > -       else
> > -               return -EIO;
> > +       struct kvec *iov = rqst->rq_iov;
> > +       int len, rc;
> >
> >         rc = smb2_plain_req_init(SMB2_QUERY_DIRECTORY, tcon, (void **) &req,
> >                              &total_len);
> >         if (rc)
> >                 return rc;
> >
> > -       if (smb3_encryption_required(tcon))
> > -               flags |= CIFS_TRANSFORM_REQ;
> > -
> > -       switch (srch_inf->info_level) {
> > +       switch (info_level) {
> >         case SMB_FIND_FILE_DIRECTORY_INFO:
> >                 req->FileInformationClass = FILE_DIRECTORY_INFORMATION;
> > -               info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
> >                 break;
> >         case SMB_FIND_FILE_ID_FULL_DIR_INFO:
> >                 req->FileInformationClass = FILEID_FULL_DIRECTORY_INFORMATION;
> > -               info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
> >                 break;
> >         default:
> >                 cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
> > -                        srch_inf->info_level);
> > -               rc = -EINVAL;
> > -               goto qdir_exit;
> > +                       info_level);
> > +               return -EINVAL;
> >         }
> >
> >         req->FileIndex = cpu_to_le32(index);
> > @@ -4292,15 +4272,56 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> >         iov[1].iov_base = (char *)(req->Buffer);
> >         iov[1].iov_len = len;
> >
> > +       trace_smb3_query_dir_enter(xid, persistent_fid, tcon->tid,
> > +                       tcon->ses->Suid, index, output_size);
> > +
> > +       return 0;
> > +}
> > +
> > +void SMB2_query_directory_free(struct smb_rqst *rqst)
> > +{
> > +       if (rqst && rqst->rq_iov) {
> > +               cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
> > +       }
> > +}
> > +
> > +int
> > +SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> > +                    u64 persistent_fid, u64 volatile_fid, int index,
> > +                    struct cifs_search_info *srch_inf)
> > +{
> > +       struct smb_rqst rqst;
> > +       struct kvec iov[SMB2_QUERY_DIRECTORY_IOV_SIZE];
> > +       struct smb2_query_directory_rsp *rsp = NULL;
> > +       int resp_buftype = CIFS_NO_BUFFER;
> > +       struct kvec rsp_iov;
> > +       int rc = 0;
> > +       struct TCP_Server_Info *server;
> > +       struct cifs_ses *ses = tcon->ses;
> > +       char *end_of_smb;
> > +       size_t info_buf_size;
> > +       int flags = 0;
> > +
> > +       if (ses && (ses->server))
> > +               server = ses->server;
> > +       else
> > +               return -EIO;
> > +
> > +       if (smb3_encryption_required(tcon))
> > +               flags |= CIFS_TRANSFORM_REQ;
> > +
> >         memset(&rqst, 0, sizeof(struct smb_rqst));
> > +       memset(&iov, 0, sizeof(iov));
> >         rqst.rq_iov = iov;
> > -       rqst.rq_nvec = 2;
> > +       rqst.rq_nvec = SMB2_QUERY_DIRECTORY_IOV_SIZE;
> >
> > -       trace_smb3_query_dir_enter(xid, persistent_fid, tcon->tid,
> > -                       tcon->ses->Suid, index, output_size);
> > +       rc = SMB2_query_directory_init(xid, tcon, &rqst, persistent_fid,
> > +                                      volatile_fid, index,
> > +                                      srch_inf->info_level);
> > +       if (rc)
> > +               goto qdir_exit;
> >
> >         rc = cifs_send_recv(xid, ses, &rqst, &resp_buftype, flags, &rsp_iov);
> > -       cifs_small_buf_release(req);
> >         rsp = (struct smb2_query_directory_rsp *)rsp_iov.iov_base;
> >
> >         if (rc) {
> > @@ -4318,6 +4339,20 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> >                 goto qdir_exit;
> >         }
> >
> > +       switch (srch_inf->info_level) {
> > +       case SMB_FIND_FILE_DIRECTORY_INFO:
> > +               info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
> > +               break;
> > +       case SMB_FIND_FILE_ID_FULL_DIR_INFO:
> > +               info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
> > +               break;
> > +       default:
> > +               cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
> > +                        srch_inf->info_level);
> > +               rc = -EINVAL;
> > +               goto qdir_exit;
> > +       }
> > +
> >         rc = smb2_validate_iov(le16_to_cpu(rsp->OutputBufferOffset),
> >                                le32_to_cpu(rsp->OutputBufferLength), &rsp_iov,
> >                                info_buf_size);
> > @@ -4353,11 +4388,14 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> >         else
> >                 cifs_tcon_dbg(VFS, "illegal search buffer type\n");
> >
> > +       rsp = NULL;
>
> minor: the subsequent patch removes this assignment, probably better
> to not do it here.
>
> > +       resp_buftype = CIFS_NO_BUFFER;
> > +
> >         trace_smb3_query_dir_done(xid, persistent_fid, tcon->tid,
> >                         tcon->ses->Suid, index, srch_inf->entries_in_buffer);
> > -       return rc;
> >
> >  qdir_exit:
> > +       SMB2_query_directory_free(&rqst);
> >         free_rsp_buf(resp_buftype, rsp);
> >         return rc;
> >  }
> > diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> > index f264e1d36fe1..caf323be0d7f 100644
> > --- a/fs/cifs/smb2pdu.h
> > +++ b/fs/cifs/smb2pdu.h
> > @@ -1272,6 +1272,8 @@ struct smb2_echo_rsp {
> >  #define SMB2_INDEX_SPECIFIED           0x04
> >  #define SMB2_REOPEN                    0x10
> >
> > +#define SMB2_QUERY_DIRECTORY_IOV_SIZE 2
> > +
> >  struct smb2_query_directory_req {
> >         struct smb2_sync_hdr sync_hdr;
> >         __le16 StructureSize; /* Must be 33 */
> > diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> > index d21a5fcc8d06..ba48ce9af620 100644
> > --- a/fs/cifs/smb2proto.h
> > +++ b/fs/cifs/smb2proto.h
> > @@ -194,6 +194,11 @@ extern int SMB2_echo(struct TCP_Server_Info *server);
> >  extern int SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> >                                 u64 persistent_fid, u64 volatile_fid, int index,
> >                                 struct cifs_search_info *srch_inf);
> > +extern int SMB2_query_directory_init(unsigned int xid, struct cifs_tcon *tcon,
> > +                                    struct smb_rqst *rqst,
> > +                                    u64 persistent_fid, u64 volatile_fid,
> > +                                    int index, int info_level);
> > +extern void SMB2_query_directory_free(struct smb_rqst *rqst);
> >  extern int SMB2_set_eof(const unsigned int xid, struct cifs_tcon *tcon,
> >                         u64 persistent_fid, u64 volatile_fid, u32 pid,
> >                         __le64 *eof);
> > --
> > 2.13.6
> >
>
> Other than the note about buffer sizes above the series looks good at
> the first glance.
>
> --
> Best regards,
> Pavel Shilovsky

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] cifs: use compounding for open and first query-dir for readdir()
  2019-12-02  7:28 [PATCH 0/3] WIP start using compounding " Ronnie Sahlberg
@ 2019-12-02  7:29 ` Ronnie Sahlberg
  0 siblings, 0 replies; 7+ messages in thread
From: Ronnie Sahlberg @ 2019-12-02  7:29 UTC (permalink / raw)
  To: linux-cifs; +Cc: Ronnie Sahlberg

Combine the initial SMB2_Open and the first SMB2_Query_Directory in a compound.
This shaves one round-trip of each directory listing, changing it from 4 to 3
for small directories.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cifsproto.h |  3 ++
 fs/cifs/smb2ops.c   | 95 ++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 86 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 1ed695336f62..b276281ec0d8 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -595,6 +595,9 @@ bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface);
 
 void extract_unc_hostname(const char *unc, const char **h, size_t *len);
 int copy_path_name(char *dst, const char *src);
+int smb2_parse_query_directory(struct cifs_tcon *tcon, struct kvec *rsp_iov,
+			       int resp_buftype,
+			       struct cifs_search_info *srch_inf);
 
 #ifdef CONFIG_CIFS_DFS_UPCALL
 static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index a7f328f79c6f..58b9b694b62f 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1997,14 +1997,33 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
 		     struct cifs_search_info *srch_inf)
 {
 	__le16 *utf16_path;
-	int rc;
-	__u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
+	struct smb_rqst rqst[2];
+	struct kvec rsp_iov[2];
+	int resp_buftype[2];
+	struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
+	struct kvec qd_iov[SMB2_QUERY_DIRECTORY_IOV_SIZE];
+	int rc, flags = 0;
+	u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
 	struct cifs_open_parms oparms;
+	struct smb2_query_directory_rsp *qd_rsp = NULL;
+	struct smb2_create_rsp *op_rsp = NULL;
 
 	utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
 	if (!utf16_path)
 		return -ENOMEM;
 
+	if (smb3_encryption_required(tcon))
+		flags |= CIFS_TRANSFORM_REQ;
+
+	memset(rqst, 0, sizeof(rqst));
+	resp_buftype[0] = resp_buftype[1] = CIFS_NO_BUFFER;
+	memset(rsp_iov, 0, sizeof(rsp_iov));
+
+	/* Open */
+	memset(&open_iov, 0, sizeof(open_iov));
+	rqst[0].rq_iov = open_iov;
+	rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
+
 	oparms.tcon = tcon;
 	oparms.desired_access = FILE_READ_ATTRIBUTES | FILE_READ_DATA;
 	oparms.disposition = FILE_OPEN;
@@ -2015,22 +2034,74 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
 	oparms.fid = fid;
 	oparms.reconnect = false;
 
-	rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL);
-	kfree(utf16_path);
-	if (rc) {
-		cifs_dbg(FYI, "open dir failed rc=%d\n", rc);
-		return rc;
-	}
+	rc = SMB2_open_init(tcon, &rqst[0], &oplock, &oparms, utf16_path);
+	if (rc)
+		goto qdf_free;
+	smb2_set_next_command(tcon, &rqst[0]);
 
+	/* Query directory */
 	srch_inf->entries_in_buffer = 0;
 	srch_inf->index_of_last_entry = 2;
 
-	rc = SMB2_query_directory(xid, tcon, fid->persistent_fid,
-				  fid->volatile_fid, 0, srch_inf);
+	memset(&qd_iov, 0, sizeof(qd_iov));
+	rqst[1].rq_iov = qd_iov;
+	rqst[1].rq_nvec = SMB2_QUERY_DIRECTORY_IOV_SIZE;
+
+	rc = SMB2_query_directory_init(xid, tcon, &rqst[1],
+				       COMPOUND_FID, COMPOUND_FID,
+				       0, srch_inf->info_level);
+	if (rc)
+		goto qdf_free;
+
+	smb2_set_related(&rqst[1]);
+
+	rc = compound_send_recv(xid, tcon->ses, flags, 2, rqst,
+				resp_buftype, rsp_iov);
+	if (!rc) {
+		op_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
+		if (op_rsp->sync_hdr.Status == STATUS_SUCCESS) {
+			fid->persistent_fid = op_rsp->PersistentFileId;
+			fid->volatile_fid = op_rsp->VolatileFileId;
+		} else {
+			cifs_dbg(FYI, "query_dir_first: open failed rc=%d\n", rc);
+			goto qdf_free;
+		}
+	} else {
+		qd_rsp = (struct smb2_query_directory_rsp *)rsp_iov[1].iov_base;
+		if (rc == -ENODATA &&
+		    qd_rsp->sync_hdr.Status == STATUS_NO_MORE_FILES) {
+			trace_smb3_query_dir_done(xid, fid->persistent_fid,
+				tcon->tid, tcon->ses->Suid, 0, 0);
+			srch_inf->endOfSearch = true;
+			rc = 0;
+		} else {
+			SMB2_close(xid, tcon, fid->persistent_fid,
+				   fid->volatile_fid);
+			cifs_dbg(FYI, "query_dir_first: query directory failed rc=%d\n", rc);
+			trace_smb3_query_dir_err(xid, fid->persistent_fid,
+				tcon->tid, tcon->ses->Suid, 0, 0, rc);
+		}
+		goto qdf_free;
+	}
+
+	rc = smb2_parse_query_directory(tcon, &rsp_iov[1], resp_buftype[1],
+					srch_inf);
 	if (rc) {
-		cifs_dbg(FYI, "query directory failed rc=%d\n", rc);
-		SMB2_close(xid, tcon, fid->persistent_fid, fid->volatile_fid);
+		trace_smb3_query_dir_err(xid, fid->persistent_fid, tcon->tid,
+			tcon->ses->Suid, 0, 0, rc);
+		goto qdf_free;
 	}
+	resp_buftype[1] = CIFS_NO_BUFFER;
+
+	trace_smb3_query_dir_done(xid, fid->persistent_fid, tcon->tid,
+			tcon->ses->Suid, 0, srch_inf->entries_in_buffer);
+
+ qdf_free:
+	kfree(utf16_path);
+	SMB2_open_free(&rqst[0]);
+	SMB2_query_directory_free(&rqst[1]);
+	free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
+	free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
 	return rc;
 }
 
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-01-08  3:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-04 22:54 [PATCH 0/3] use compounding to speed up readdir() Ronnie Sahlberg
2019-12-04 22:54 ` [PATCH 1/3] cifs: prepare SMB2_query_directory to be used with compounding Ronnie Sahlberg
2019-12-23 23:48   ` Pavel Shilovsky
2020-01-08  3:09     ` ronnie sahlberg
2019-12-04 22:54 ` [PATCH 2/3] cifs: create a helper function to parse the query-directory response buffer Ronnie Sahlberg
2019-12-04 22:54 ` [PATCH 3/3] cifs: use compounding for open and first query-dir for readdir() Ronnie Sahlberg
  -- strict thread matches above, loose matches on Subject: below --
2019-12-02  7:28 [PATCH 0/3] WIP start using compounding " Ronnie Sahlberg
2019-12-02  7:29 ` [PATCH 3/3] cifs: use compounding for open and first query-dir " Ronnie Sahlberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).