Attachment 10620 Details for Bug 10123 – new patch addressing points in comment#3

[patch] new patch addressing points in comment#3

0001-libsmb-reuse-connections-derived-from-DFS-referrals.patch (text/plain), 6.91 KB, created by David Disseldorp on 2015-01-14 17:36:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Disseldorp

Created: 2015-01-14 17:36:59 UTC

Size: 6.91 KB

patch

obsolete

>From 83d798f2e7a3c933be52a379b12e3e7a48115c98 Mon Sep 17 00:00:00 2001
>From: David Disseldorp <ddiss@samba.org>
>Date: Wed, 14 Jan 2015 13:39:05 +0100
>Subject: [PATCH] libsmb: reuse connections derived from DFS referrals
>
>[MS-DFSC] 3.2.1.1 and 3.2.1.2 states that DFS targets with the same site
>location or relative cost are placed in random order in a DFS referral
>response.
>
>libsmbclient currently resolves DFS referrals on every API call, always
>using the first entry in the referral response. With random ordering,
>libsmbclient may open a new server connection, rather than reuse an
>existing (cached) connection established in a previous DFS referred API
>call.
>
>This change sees libsmbclient check the connection cache for any of the
>DFS referral response entries before creating a new connection.
>
>This change is based on a patch by Har Gagan Sahai
><SHarGagan@novell.com>.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10123
>
>Signed-off-by: David Disseldorp <ddiss@samba.org>
>---
> source3/libsmb/clidfs.c | 103 +++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 79 insertions(+), 24 deletions(-)
>
>diff --git a/source3/libsmb/clidfs.c b/source3/libsmb/clidfs.c
>index e5c03a8..c554def 100644
>--- a/source3/libsmb/clidfs.c
>+++ b/source3/libsmb/clidfs.c
>@@ -834,6 +834,11 @@ NTSTATUS cli_dfs_get_referral(TALLOC_CTX *ctx,
> 
> /********************************************************************
> ********************************************************************/
>+struct cli_dfs_path_split {
>+	char *server;
>+	char *share;
>+	char *extrapath;
>+};
> 
> NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
> 			  const char *mountpt,
>@@ -851,9 +856,9 @@ NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
> 	char *cleanpath = NULL;
> 	char *extrapath = NULL;
> 	int pathlen;
>-	char *server = NULL;
>-	char *share = NULL;
> 	struct cli_state *newcli = NULL;
>+	struct cli_state *ccli = NULL;
>+	int count = 0;
> 	char *newpath = NULL;
> 	char *newmount = NULL;
> 	char *ppath = NULL;
>@@ -862,6 +867,7 @@ NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
> 	NTSTATUS status;
> 	struct smbXcli_tcon *root_tcon = NULL;
> 	struct smbXcli_tcon *target_tcon = NULL;
>+	struct cli_dfs_path_split *dfs_refs = NULL;
> 
> 	if ( !rootcli || !path || !targetcli ) {
> 		return NT_STATUS_INVALID_PARAMETER;
>@@ -951,26 +957,83 @@ NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
> 		return status;
> 	}
> 
>-	/* Just store the first referral for now. */
>-
> 	if (!refs[0].dfspath) {
> 		return NT_STATUS_NOT_FOUND;
> 	}
>-	if (!split_dfs_path(ctx, refs[0].dfspath, &server, &share,
>-			    &extrapath)) {
>-		return NT_STATUS_NOT_FOUND;
>+
>+	/*
>+	 * Bug#10123 - DFS referal entries can be provided in a random order,
>+	 * so check the connection cache for each item to avoid unnecessary
>+	 * reconnections.
>+	 */
>+	dfs_refs = talloc_array(ctx, struct cli_dfs_path_split, num_refs);
>+	if (dfs_refs == NULL) {
>+		return NT_STATUS_NO_MEMORY;
>+	}
>+
>+	for (count = 0; count < num_refs; count++) {
>+		if (!split_dfs_path(dfs_refs, refs[count].dfspath,
>+				    &dfs_refs[count].server,
>+				    &dfs_refs[count].share,
>+				    &dfs_refs[count].extrapath)) {
>+			TALLOC_FREE(dfs_refs);
>+			return NT_STATUS_NOT_FOUND;
>+		}
>+
>+		ccli = cli_cm_find(rootcli, dfs_refs[count].server,
>+				   dfs_refs[count].share);
>+		if (ccli != NULL) {
>+			extrapath = dfs_refs[count].extrapath;
>+			*targetcli = ccli;
>+			break;
>+		}
>+	}
>+
>+	/*
>+	 * If no cached connection was found, then connect to the first live
>+	 * referral server in the list.
>+	 */
>+	for (count = 0; (ccli == NULL) && (count < num_refs); count++) {
>+		/* Connect to the target server & share */
>+		status = cli_cm_connect(ctx, rootcli,
>+				dfs_refs[count].server,
>+				dfs_refs[count].share,
>+				dfs_auth_info,
>+				false,
>+				smb1cli_conn_encryption_on(rootcli->conn),
>+				smbXcli_conn_protocol(rootcli->conn),
>+				0,
>+				0x20,
>+				targetcli);
>+		if (!NT_STATUS_IS_OK(status)) {
>+			d_printf("Unable to follow dfs referral [\\%s\\%s]\n",
>+				 dfs_refs[count].server,
>+				 dfs_refs[count].share);
>+			continue;
>+		} else {
>+			extrapath = dfs_refs[count].extrapath;
>+			break;
>+		}
>+	}
>+
>+	/* No available referral server for the connection */
>+	if (*targetcli == NULL) {
>+		TALLOC_FREE(dfs_refs);
>+		return status;
> 	}
> 
> 	/* Make sure to recreate the original string including any wildcards. */
> 
> 	dfs_path = cli_dfs_make_full_path(ctx, rootcli, path);
> 	if (!dfs_path) {
>+		TALLOC_FREE(dfs_refs);
> 		return NT_STATUS_NO_MEMORY;
> 	}
> 	pathlen = strlen(dfs_path);
> 	consumed = MIN(pathlen, consumed);
> 	*pp_targetpath = talloc_strdup(ctx, &dfs_path[consumed]);
> 	if (!*pp_targetpath) {
>+		TALLOC_FREE(dfs_refs);
> 		return NT_STATUS_NO_MEMORY;
> 	}
> 	dfs_path[consumed] = '\0';
>@@ -981,23 +1044,6 @@ NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
> 	 * (in \server\share\path format).
>  	 */
> 
>-	/* Open the connection to the target server & share */
>-	status = cli_cm_open(ctx, rootcli,
>-			     server,
>-			     share,
>-			     dfs_auth_info,
>-			     false,
>-			     smb1cli_conn_encryption_on(rootcli->conn),
>-			     smbXcli_conn_protocol(rootcli->conn),
>-			     0,
>-			     0x20,
>-			     targetcli);
>-	if (!NT_STATUS_IS_OK(status)) {
>-		d_printf("Unable to follow dfs referral [\\%s\\%s]\n",
>-			server, share );
>-		return status;
>-	}
>-
> 	if (extrapath && strlen(extrapath) > 0) {
> 		/* EMC Celerra NAS version 5.6.50 (at least) doesn't appear to */
> 		/* put the trailing \ on the path, so to be save we put one in if needed */
>@@ -1013,6 +1059,7 @@ NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
> 						  *pp_targetpath);
> 		}
> 		if (!*pp_targetpath) {
>+			TALLOC_FREE(dfs_refs);
> 			return NT_STATUS_NO_MEMORY;
> 		}
> 	}
>@@ -1026,18 +1073,21 @@ NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
> 		d_printf("cli_resolve_path: "
> 			"dfs_path (%s) not in correct format.\n",
> 			dfs_path );
>+		TALLOC_FREE(dfs_refs);
> 		return NT_STATUS_NOT_FOUND;
> 	}
> 
> 	ppath++; /* Now pointing at start of server name. */
> 
> 	if ((ppath = strchr_m( dfs_path, '\\' )) == NULL) {
>+		TALLOC_FREE(dfs_refs);
> 		return NT_STATUS_NOT_FOUND;
> 	}
> 
> 	ppath++; /* Now pointing at start of share name. */
> 
> 	if ((ppath = strchr_m( ppath+1, '\\' )) == NULL) {
>+		TALLOC_FREE(dfs_refs);
> 		return NT_STATUS_NOT_FOUND;
> 	}
> 
>@@ -1045,6 +1095,7 @@ NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
> 
> 	newmount = talloc_asprintf(ctx, "%s\\%s", mountpt, ppath );
> 	if (!newmount) {
>+		TALLOC_FREE(dfs_refs);
> 		return NT_STATUS_NOT_FOUND;
> 	}
> 
>@@ -1069,6 +1120,7 @@ NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
>  			 */
> 			*targetcli = newcli;
> 			*pp_targetpath = newpath;
>+			TALLOC_FREE(dfs_refs);
> 			return status;
> 		}
> 	}
>@@ -1085,14 +1137,17 @@ NTSTATUS cli_resolve_path(TALLOC_CTX *ctx,
> 	if (smbXcli_tcon_is_dfs_share(target_tcon)) {
> 		dfs_path = talloc_strdup(ctx, *pp_targetpath);
> 		if (!dfs_path) {
>+			TALLOC_FREE(dfs_refs);
> 			return NT_STATUS_NO_MEMORY;
> 		}
> 		*pp_targetpath = cli_dfs_make_full_path(ctx, *targetcli, dfs_path);
> 		if (*pp_targetpath == NULL) {
>+			TALLOC_FREE(dfs_refs);
> 			return NT_STATUS_NO_MEMORY;
> 		}
> 	}
> 
>+	TALLOC_FREE(dfs_refs);
> 	return NT_STATUS_OK;
> }
> 
>-- 
>2.1.2
>

Actions: View

Attachments on bug 10123: 9785 | 10620 | 10624 | 10632 | 10633 | 10634 | 10635