s3: libsmbclient: Re-resolving targetcli on every read/write/lseek/ftruncate/close is both incorrect and slow.

Cache targetcli on file open in the SMBCFILE struct.

Bug 11295 - Excessive cli_resolve_path() usage can slow down transmission.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11295

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>

2 files changed, 22 insertions, 185 deletions

diff --git a/source3/include/libsmb_internal.h b/source3/include/libsmb_internal.h
index fcdb5130154..cf51f34fcca 100644
--- a/source3/include/libsmb_internal.h
+++ b/source3/include/libsmb_internal.h
@@ -100,6 +100,11 @@ struct smbc_dir_list {
  */
 struct _SMBCFILE {
 	int cli_fd;
+	/*
+	 * cache of cli_state we opened cli_fd on.
+	 * Due to DFS can be a subsidiary connection to srv->cli
+	 */
+	struct cli_state *targetcli;
 	char *fname;
 	off_t offset;
 	struct _SMBCSRV *srv;
diff --git a/source3/libsmb/libsmb_file.c b/source3/libsmb/libsmb_file.c
index fe5703521eb..29a6a0fbbec 100644
--- a/source3/libsmb/libsmb_file.c
+++ b/source3/libsmb/libsmb_file.c
@@ -144,6 +144,14 @@ SMBC_open_ctx(SMBCCTX *context,
 		file->srv     = srv;
 		file->offset  = 0;
 		file->file    = True;
+		/*
+		 * targetcli is either equal to srv->cli or
+		 * is a subsidiary DFS connection. Either way
+		 * file->cli_fd belongs to it so we must cache
+		 * it for read/write/close, not re-resolve each time.
+		 * Re-resolving is both slow and incorrect.
+		 */
+		file->targetcli = targetcli;
 
 		DLIST_ADD(context->internal->files, file);
 
@@ -228,11 +236,6 @@ SMBC_read_ctx(SMBCCTX *context,
               size_t count)
 {
 	size_t ret;
-	char *server = NULL, *share = NULL, *user = NULL, *password = NULL;
-	char *path = NULL;
-	char *targetpath = NULL;
-	struct cli_state *targetcli = NULL;
-	uint16_t port = 0;
 	TALLOC_CTX *frame = talloc_stackframe();
 	NTSTATUS status;
 
@@ -271,39 +274,10 @@ SMBC_read_ctx(SMBCCTX *context,
 		return -1;
 	}
 
-	/*d_printf(">>>read: parsing %s\n", file->fname);*/
-	if (SMBC_parse_path(frame,
-                            context,
-                            file->fname,
-                            NULL,
-                            &server,
-                            &port,
-                            &share,
-                            &path,
-                            &user,
-                            &password,
-                            NULL)) {
-                errno = EINVAL;
-		TALLOC_FREE(frame);
-                return -1;
-        }
-
-	/*d_printf(">>>read: resolving %s\n", path);*/
-	status = cli_resolve_path(frame, "", context->internal->auth_info,
-				  file->srv->cli, path,
-				  &targetcli, &targetpath);
-	if (!NT_STATUS_IS_OK(status)) {
-		d_printf("Could not resolve %s\n", path);
-                errno = ENOENT;
-		TALLOC_FREE(frame);
-		return -1;
-	}
-	/*d_printf(">>>fstat: resolved path as %s\n", targetpath);*/
-
-	status = cli_read(targetcli, file->cli_fd, (char *)buf, offset,
+	status = cli_read(file->targetcli, file->cli_fd, (char *)buf, offset,
 			  count, &ret);
 	if (!NT_STATUS_IS_OK(status)) {
-		errno = SMBC_errno(context, targetcli);
+		errno = SMBC_errno(context, file->targetcli);
 		TALLOC_FREE(frame);
 		return -1;
 	}
@@ -436,11 +410,6 @@ SMBC_write_ctx(SMBCCTX *context,
                size_t count)
 {
         off_t offset;
-	char *server = NULL, *share = NULL, *user = NULL, *password = NULL;
-	char *path = NULL;
-	char *targetpath = NULL;
-	struct cli_state *targetcli = NULL;
-	uint16_t port = 0;
 	TALLOC_CTX *frame = talloc_stackframe();
 	NTSTATUS status;
 
@@ -468,36 +437,7 @@ SMBC_write_ctx(SMBCCTX *context,
 
         offset = file->offset; /* See "offset" comment in SMBC_read_ctx() */
 
-	/*d_printf(">>>write: parsing %s\n", file->fname);*/
-	if (SMBC_parse_path(frame,
-                            context,
-                            file->fname,
-                            NULL,
-                            &server,
-                            &port,
-                            &share,
-                            &path,
-                            &user,
-                            &password,
-                            NULL)) {
-                errno = EINVAL;
-		TALLOC_FREE(frame);
-                return -1;
-        }
-
-	/*d_printf(">>>write: resolving %s\n", path);*/
-	status = cli_resolve_path(frame, "", context->internal->auth_info,
-				  file->srv->cli, path,
-				  &targetcli, &targetpath);
-	if (!NT_STATUS_IS_OK(status)) {
-		d_printf("Could not resolve %s\n", path);
-                errno = ENOENT;
-		TALLOC_FREE(frame);
-		return -1;
-	}
-	/*d_printf(">>>write: resolved path as %s\n", targetpath);*/
-
-	status = cli_writeall(targetcli, file->cli_fd,
+	status = cli_writeall(file->targetcli, file->cli_fd,
 			      0, (const uint8_t *)buf, offset, count, NULL);
 	if (!NT_STATUS_IS_OK(status)) {
 		errno = map_errno_from_nt_status(status);
@@ -519,14 +459,7 @@ int
 SMBC_close_ctx(SMBCCTX *context,
                SMBCFILE *file)
 {
-        SMBCSRV *srv;
-	char *server = NULL, *share = NULL, *user = NULL, *password = NULL;
-	char *path = NULL;
-	char *targetpath = NULL;
-	uint16_t port = 0;
-	struct cli_state *targetcli = NULL;
 	TALLOC_CTX *frame = talloc_stackframe();
-	NTSTATUS status;
 
 	if (!context || !context->internal->initialized) {
 		errno = EINVAL;
@@ -546,41 +479,13 @@ SMBC_close_ctx(SMBCCTX *context,
 		return smbc_getFunctionClosedir(context)(context, file);
 	}
 
-	/*d_printf(">>>close: parsing %s\n", file->fname);*/
-	if (SMBC_parse_path(frame,
-                            context,
-                            file->fname,
-                            NULL,
-                            &server,
-                            &port,
-                            &share,
-                            &path,
-                            &user,
-                            &password,
-                            NULL)) {
-                errno = EINVAL;
-		TALLOC_FREE(frame);
-                return -1;
-        }
-
-	/*d_printf(">>>close: resolving %s\n", path);*/
-	status = cli_resolve_path(frame, "", context->internal->auth_info,
-				  file->srv->cli, path,
-				  &targetcli, &targetpath);
-	if (!NT_STATUS_IS_OK(status)) {
-		d_printf("Could not resolve %s\n", path);
-                errno = ENOENT;
-		TALLOC_FREE(frame);
-		return -1;
-	}
-	/*d_printf(">>>close: resolved path as %s\n", targetpath);*/
-
-	if (!NT_STATUS_IS_OK(cli_close(targetcli, file->cli_fd))) {
+	if (!NT_STATUS_IS_OK(cli_close(file->targetcli, file->cli_fd))) {
+		SMBCSRV *srv;
 		DEBUG(3, ("cli_close failed on %s. purging server.\n",
 			  file->fname));
 		/* Deallocate slot and remove the server
 		 * from the server cache if unused */
-		errno = SMBC_errno(context, targetcli);
+		errno = SMBC_errno(context, file->targetcli);
 		srv = file->srv;
 		DLIST_REMOVE(context->internal->files, file);
 		SAFE_FREE(file->fname);
@@ -816,13 +721,7 @@ SMBC_lseek_ctx(SMBCCTX *context,
                int whence)
 {
 	off_t size;
-	char *server = NULL, *share = NULL, *user = NULL, *password = NULL;
-	char *path = NULL;
-	char *targetpath = NULL;
-	struct cli_state *targetcli = NULL;
-	uint16_t port = 0;
 	TALLOC_CTX *frame = talloc_stackframe();
-	NTSTATUS status;
 
 	if (!context || !context->internal->initialized) {
 		errno = EINVAL;
@@ -850,41 +749,12 @@ SMBC_lseek_ctx(SMBCCTX *context,
 		file->offset += offset;
 		break;
 	case SEEK_END:
-		/*d_printf(">>>lseek: parsing %s\n", file->fname);*/
-		if (SMBC_parse_path(frame,
-                                    context,
-                                    file->fname,
-                                    NULL,
-                                    &server,
-                                    &port,
-                                    &share,
-                                    &path,
-                                    &user,
-                                    &password,
-                                    NULL)) {
-			errno = EINVAL;
-			TALLOC_FREE(frame);
-			return -1;
-		}
-
-		/*d_printf(">>>lseek: resolving %s\n", path);*/
-		status = cli_resolve_path(
-			frame, "", context->internal->auth_info,
-			file->srv->cli, path, &targetcli, &targetpath);
-		if (!NT_STATUS_IS_OK(status)) {
-			d_printf("Could not resolve %s\n", path);
-                        errno = ENOENT;
-			TALLOC_FREE(frame);
-			return -1;
-		}
-
-		/*d_printf(">>>lseek: resolved path as %s\n", targetpath);*/
 		if (!NT_STATUS_IS_OK(cli_qfileinfo_basic(
-					     targetcli, file->cli_fd, NULL,
+					     file->targetcli, file->cli_fd, NULL,
 					     &size, NULL, NULL, NULL, NULL,
 					     NULL))) {
                         off_t b_size = size;
-			if (!NT_STATUS_IS_OK(cli_getattrE(targetcli, file->cli_fd,
+			if (!NT_STATUS_IS_OK(cli_getattrE(file->targetcli, file->cli_fd,
                                           NULL, &b_size, NULL, NULL, NULL))) {
                                 errno = EINVAL;
                                 TALLOC_FREE(frame);
@@ -914,16 +784,7 @@ SMBC_ftruncate_ctx(SMBCCTX *context,
                    off_t length)
 {
 	off_t size = length;
-	char *server = NULL;
-	char *share = NULL;
-	char *user = NULL;
-	char *password = NULL;
-	char *path = NULL;
-        char *targetpath = NULL;
-	uint16_t port = 0;
-	struct cli_state *targetcli = NULL;
 	TALLOC_CTX *frame = talloc_stackframe();
-	NTSTATUS status;
 
 	if (!context || !context->internal->initialized) {
 		errno = EINVAL;
@@ -943,36 +804,7 @@ SMBC_ftruncate_ctx(SMBCCTX *context,
 		return -1;
 	}
 
-	/*d_printf(">>>fstat: parsing %s\n", file->fname);*/
-	if (SMBC_parse_path(frame,
-                            context,
-                            file->fname,
-                            NULL,
-                            &server,
-                            &port,
-                            &share,
-                            &path,
-                            &user,
-                            &password,
-                            NULL)) {
-                errno = EINVAL;
-		TALLOC_FREE(frame);
-                return -1;
-        }
-
-	/*d_printf(">>>fstat: resolving %s\n", path);*/
-	status = cli_resolve_path(frame, "", context->internal->auth_info,
-				  file->srv->cli, path,
-				  &targetcli, &targetpath);
-	if (!NT_STATUS_IS_OK(status)) {
-		d_printf("Could not resolve %s\n", path);
-                errno = ENOENT;
-		TALLOC_FREE(frame);
-		return -1;
-	}
-	/*d_printf(">>>fstat: resolved path as %s\n", targetpath);*/
-
-        if (!NT_STATUS_IS_OK(cli_ftruncate(targetcli, file->cli_fd, (uint64_t)size))) {
+        if (!NT_STATUS_IS_OK(cli_ftruncate(file->targetcli, file->cli_fd, (uint64_t)size))) {
                 errno = EINVAL;
                 TALLOC_FREE(frame);
                 return -1;