Directory cache and SMB2

[Ralph Böhme]

On Thu, Mar 29, 2018 at 07:47:01PM +0200, Ralph Böhme wrote:
> On Thu, Mar 29, 2018 at 10:12:42AM -0700, Jeremy Allison wrote:
> > On Thu, Mar 29, 2018 at 03:43:07PM +0200, Ralph Böhme wrote:
> > > Hi!
> > > 
> > > It seems for SMB2 we still populate the directory cache, but we never use it. Am
> > > I missing something?
> > > 
> > > It gets populated via smbd_smb2_query_directory_send ->
> > > smbd_dirptr_lanman2_entry -> smbd_dirptr_get_entry -> DirCacheAdd for SMB2.
> > > 
> > > But the only point where it's used is via call_trans2findnext -> dptr_SearchDir
> > > -> SearchDir which is SMB1 only.
> > 
> > Yes, that's completely correct. That's because SMB1 uses a name
> > to resume from, wheras SMB2 always continues from the next position
> > (unless it's told to restart or close-and-reopen the listing).
> > 
> > We could make DirCacheAdd a noop for SMB2. Wanna do a patch ?
> 
> yup, I just needed the confirmation before jumping at it. :) Thanks!

attached, preventing the call to DirCacheAdd() as well as changing the
initialisation to not allocate the cache array.

I've logged a bug as I thought this would be worthwile backporting.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From d2bf8f181409768cd9df4ff47fef83005c6c163e Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 29 Mar 2018 20:06:47 +0200
Subject: [PATCH] s3:smbd: don't use the directory cache for SMB2/3

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13363

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 .../smbdotconf/misc/directorynamecachesize.xml     |  5 ++--
 source3/smbd/dir.c                                 | 32 ++++++++++++++++++++--
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/docs-xml/smbdotconf/misc/directorynamecachesize.xml b/docs-xml/smbdotconf/misc/directorynamecachesize.xml
index 7a89bf2ed64..22999a6964e 100644
--- a/docs-xml/smbdotconf/misc/directorynamecachesize.xml
+++ b/docs-xml/smbdotconf/misc/directorynamecachesize.xml
@@ -4,8 +4,9 @@
                  xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
 <description>
 	<para>
-	This parameter specifies the size of the directory name cache.
-	It will be needed to turn this off for *BSD systems.
+	This parameter specifies the size of the directory name cache for SMB1
+	connections. It is not used for SMB2. It will be needed to turn this off
+	for *BSD systems.
 	</para>
 
 </description>
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 2ed71752977..8bb66b81936 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -1224,7 +1224,15 @@ bool smbd_dirptr_get_entry(TALLOC_CTX *ctx,
 			mask, smb_fname_str_dbg(&smb_fname),
 			dname, fname));
 
-		DirCacheAdd(dirptr->dir_hnd, dname, cur_offset);
+		if (!conn->sconn->using_smb2) {
+			/*
+			 * The dircache is only needed for SMB1 because SMB1
+			 * uses a name for the resume wheras SMB2 always
+			 * continues from the next position (unless it's told to
+			 * restart or close-and-reopen the listing).
+			 */
+			DirCacheAdd(dirptr->dir_hnd, dname, cur_offset);
+		}
 
 		TALLOC_FREE(dname);
 
@@ -1651,7 +1659,16 @@ static struct smb_Dir *OpenDir_internal(TALLOC_CTX *mem_ctx,
 	}
 
 	dirp->conn = conn;
-	dirp->name_cache_size = lp_directory_name_cache_size(SNUM(conn));
+
+	if (!conn->sconn->using_smb2) {
+		/*
+		 * The dircache is only needed for SMB1 because SMB1 uses a name
+		 * for the resume wheras SMB2 always continues from the next
+		 * position (unless it's told to restart or close-and-reopen the
+		 * listing).
+		 */
+		dirp->name_cache_size = lp_directory_name_cache_size(SNUM(conn));
+	}
 
 	if (sconn && !sconn->using_smb2) {
 		sconn->searches.dirhandles_open++;
@@ -1773,7 +1790,16 @@ static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn,
 	}
 
 	dirp->conn = conn;
-	dirp->name_cache_size = lp_directory_name_cache_size(SNUM(conn));
+
+	if (!conn->sconn->using_smb2) {
+		/*
+		 * The dircache is only needed for SMB1 because SMB1 uses a name
+		 * for the resume wheras SMB2 always continues from the next
+		 * position (unless it's told to restart or close-and-reopen the
+		 * listing).
+		 */
+		dirp->name_cache_size = lp_directory_name_cache_size(SNUM(conn));
+	}
 
 	dirp->dir_smb_fname = cp_smb_filename(dirp, fsp->fsp_name);
 	if (!dirp->dir_smb_fname) {
-- 
2.13.6