From e5d0c01947eccafaab6930e69c8c8fabf95b6fd0 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 23 May 2018 11:07:54 -0700 Subject: [PATCH 1/5] selftest: Add dfq_cache share with 'dfree cache time' set BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446 Signed-off-by: Christof Schmitt Reviewed-by: Jeremy Allison (cherry picked from commit 7ffcbd5ce1222971cb9879f78765d87cdc4102a8) --- selftest/target/Samba3.pm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 52c7d3e..7b0303a 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2140,6 +2140,12 @@ sub provision($$$$$$$$$) vfs objects = acl_xattr fake_acls xattr_tdb fake_dfq admin users = $unix_name include = $dfqconffile +[dfq_cache] + path = $shrdir/dfree + vfs objects = acl_xattr fake_acls xattr_tdb fake_dfq + admin users = $unix_name + include = $dfqconffile + dfree cache time = 60 [dfq_owner] path = $shrdir/dfree vfs objects = acl_xattr fake_acls xattr_tdb fake_dfq -- 1.8.3.1 From 1bc25ae8d35acfd7c444bf3184123204b7a193f7 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 23 May 2018 11:25:42 -0700 Subject: [PATCH 2/5] selftest: Add test for 'dfree cache' BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446 Signed-off-by: Christof Schmitt Reviewed-by: Jeremy Allison (cherry picked from commit a55b3d2fcc2f7737a2702bf908dcf1f80969bf21) --- selftest/knownfail | 1 + source3/script/tests/test_dfree_quota.sh | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/selftest/knownfail b/selftest/knownfail index 710fd33..0709b2d 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -343,3 +343,4 @@ # Disabling NTLM means you can't use samr to change the password ^samba.tests.ntlmdisabled.python\(ktest\).ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\) ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\) +^samba3.blackbox.dfree_quota \(fileserver\).Test dfree cache\(fileserver\) diff --git a/source3/script/tests/test_dfree_quota.sh b/source3/script/tests/test_dfree_quota.sh index 6e227c4..abd82b4 100755 --- a/source3/script/tests/test_dfree_quota.sh +++ b/source3/script/tests/test_dfree_quota.sh @@ -130,6 +130,35 @@ test_smbclient_dfree() { return $status } +# Issue two queries to different directories in one session to test +# caching effects +test_smbclient_dfree_2() { + name="$1" + share="$2" + dir1="$3" + dir2="$4" + confs="$5" + expected="$6" + subunit_start_test "$name" + setup_conf $confs + output=$($VALGRIND $smbclient //$SERVER/$share \ + -c "cd $dir1; du; cd ..; cd $dir2 ; du" $@ 2>&1) + status=$? + if [ "$status" = "0" ]; then + received=$(echo "$output" | \ + awk '/blocks of size/ {print $1, $5, $6}' | \ + tr '\n' ' ') + if [ "$expected" = "$received" ]; then + subunit_pass_test "$name" + else + echo "$output" | subunit_fail_test "$name" + fi + else + echo "$output" | subunit_fail_test "$name" + fi + return $status +} + test_smbcquotas() { name="$1" conf="$2" @@ -164,6 +193,12 @@ test_smbclient_dfree "Test large disk" dfq "." "conf3 ." "1125899906842624 1024. #basic quota test (SMB1 only) test_smbcquotas "Test user quota" confq1 $USERNAME "40960/4096000/3072000" -U$USERNAME%$PASSWORD --option=clientmaxprotocol=NT1 || failed=`expr $failed + 1` +# Test dfree cache through queries in two different directories +test_smbclient_dfree_2 "Test dfree cache" dfq_cache "." "subdir1" \ + "conf1 . conf2 subdir1" "10 1024. 5 20 1024. 10 " \ + -U$USERNAME%$PASSWORD --option=clientmaxprotocol=SMB3 \ + || failed=`expr $failed + 1` + #quota limit > disk size, remaining quota > disk free test_smbclient_dfree "Test dfree share root df vs quota case 1" dfq "." "confdfq1 ." "80 1024. 40" -U$USERNAME%$PASSWORD --option=clientmaxprotocol=SMB3 || failed=`expr $failed + 1` #quota limit > disk size, remaining quota < disk free -- 1.8.3.1 From 6777e3de8c33d1e4a270ac2f7e11be4ed4d2853a Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 16 May 2018 13:05:36 -0700 Subject: [PATCH 3/5] memcache: Add new cache type for dfree information BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446 Signed-off-by: Christof Schmitt Reviewed-by: Jeremy Allison (cherry picked from commit f5d05562679f6aa691b98b4a75952f7dda7ed343) --- lib/util/memcache.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/util/memcache.h b/lib/util/memcache.h index 670cbd6..4331c2f 100644 --- a/lib/util/memcache.h +++ b/lib/util/memcache.h @@ -45,7 +45,8 @@ enum memcache_number { SINGLETON_CACHE, SMB1_SEARCH_OFFSET_MAP, SHARE_MODE_LOCK_CACHE, /* talloc */ - VIRUSFILTER_SCAN_RESULTS_CACHE_TALLOC /* talloc */ + VIRUSFILTER_SCAN_RESULTS_CACHE_TALLOC, /* talloc */ + DFREE_CACHE, }; /* -- 1.8.3.1 From 544beec2d7c839396459423b3d1aeebc8d5290ef Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 16 May 2018 13:17:52 -0700 Subject: [PATCH 4/5] smbd: Cache dfree information based on query path Sub directories in a SMB share can have different free space information (e.g. when a different file system is mounted there). Caching the dfree information per SMB share will return invalid data. Address this by switching to memcache and store the cached data based on the query path. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446 Signed-off-by: Christof Schmitt Reviewed-by: Jeremy Allison (cherry picked from commit 8f121747b06ca78cf51801a3931b2ddd1a424c77) --- selftest/knownfail | 1 - source3/smbd/dfree.c | 99 ++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 80 insertions(+), 20 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index 0709b2d..710fd33 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -343,4 +343,3 @@ # Disabling NTLM means you can't use samr to change the password ^samba.tests.ntlmdisabled.python\(ktest\).ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\) ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\) -^samba3.blackbox.dfree_quota \(fileserver\).Test dfree cache\(fileserver\) diff --git a/source3/smbd/dfree.c b/source3/smbd/dfree.c index a702d08..5b20707 100644 --- a/source3/smbd/dfree.c +++ b/source3/smbd/dfree.c @@ -21,6 +21,7 @@ #include "smbd/smbd.h" #include "smbd/globals.h" #include "lib/util_file.h" +#include "lib/util/memcache.h" /**************************************************************************** Normalise for DOS usage. @@ -167,48 +168,108 @@ dfree_done: /**************************************************************************** Potentially returned cached dfree info. + + Depending on the file system layout and file system features, the free space + information can be different for different sub directories underneath a SMB + share. Store the cache information in memcache using the query path as the + key to accomodate this. ****************************************************************************/ uint64_t get_dfree_info(connection_struct *conn, struct smb_filename *fname, uint64_t *bsize, uint64_t *dfree, uint64_t *dsize) { int dfree_cache_time = lp_dfree_cache_time(SNUM(conn)); - struct dfree_cached_info *dfc = conn->dfree_info; + struct dfree_cached_info *dfc = NULL; + struct dfree_cached_info dfc_new = { 0 }; uint64_t dfree_ret; + char tmpbuf[PATH_MAX]; + char *full_path = NULL; + char *to_free = NULL; + char *key_path = NULL; + size_t len; + DATA_BLOB key, value; + bool found; if (!dfree_cache_time) { return sys_disk_free(conn, fname, bsize, dfree, dsize); } + len = full_path_tos(conn->connectpath, + fname->base_name, + tmpbuf, + sizeof(tmpbuf), + &full_path, + &to_free); + if (len == -1) { + errno = ENOMEM; + return -1; + } + + if (VALID_STAT(fname->st) && S_ISREG(fname->st.st_ex_mode)) { + /* + * In case of a file use the parent directory to reduce number + * of cache entries. + */ + bool ok; + + ok = parent_dirname(talloc_tos(), + full_path, + &key_path, + NULL); + TALLOC_FREE(to_free); /* We're done with full_path */ + + if (!ok) { + errno = ENOMEM; + return -1; + } + + /* + * key_path is always a talloced object. + */ + to_free = key_path; + } else { + /* + * key_path might not be a talloced object; rely on + * to_free set from full_path_tos. + */ + key_path = full_path; + } + + key = data_blob_const(key_path, strlen(key_path)); + found = memcache_lookup(smbd_memcache(), + DFREE_CACHE, + key, + &value); + dfc = found ? (struct dfree_cached_info *)value.data : NULL; + if (dfc && (conn->lastused - dfc->last_dfree_time < dfree_cache_time)) { - /* Return cached info. */ + DBG_DEBUG("Returning dfree cache entry for %s\n", key_path); *bsize = dfc->bsize; *dfree = dfc->dfree; *dsize = dfc->dsize; - return dfc->dfree_ret; + dfree_ret = dfc->dfree_ret; + goto out; } dfree_ret = sys_disk_free(conn, fname, bsize, dfree, dsize); if (dfree_ret == (uint64_t)-1) { /* Don't cache bad data. */ - return dfree_ret; - } - - /* No cached info or time to refresh. */ - if (!dfc) { - dfc = talloc(conn, struct dfree_cached_info); - if (!dfc) { - return dfree_ret; - } - conn->dfree_info = dfc; + goto out; } - dfc->bsize = *bsize; - dfc->dfree = *dfree; - dfc->dsize = *dsize; - dfc->dfree_ret = dfree_ret; - dfc->last_dfree_time = conn->lastused; - + DBG_DEBUG("Creating dfree cache entry for %s\n", key_path); + dfc_new.bsize = *bsize; + dfc_new.dfree = *dfree; + dfc_new.dsize = *dsize; + dfc_new.dfree_ret = dfree_ret; + dfc_new.last_dfree_time = conn->lastused; + memcache_add(smbd_memcache(), + DFREE_CACHE, + key, + data_blob_const(&dfc_new, sizeof(dfc_new))); + +out: + TALLOC_FREE(to_free); return dfree_ret; } -- 1.8.3.1 From bc3dd6f476567d213005c8692be6452c89155635 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Fri, 18 May 2018 20:51:58 -0700 Subject: [PATCH 5/5] smbd: Flush dfree memcache on service reload BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446 Signed-off-by: Christof Schmitt Reviewed-by: Jeremy Allison (cherry picked from commit e30d0c0e0d11f65b2d1886be3c0fe9e32eaf3926) --- source3/smbd/dfree.c | 5 +++++ source3/smbd/proto.h | 1 + source3/smbd/server_reload.c | 1 + 3 files changed, 7 insertions(+) diff --git a/source3/smbd/dfree.c b/source3/smbd/dfree.c index 5b20707..d280e1e 100644 --- a/source3/smbd/dfree.c +++ b/source3/smbd/dfree.c @@ -273,3 +273,8 @@ out: TALLOC_FREE(to_free); return dfree_ret; } + +void flush_dfree_cache(void) +{ + memcache_flush(smbd_memcache(), DFREE_CACHE); +} diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index fe37640..9bb919d 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -176,6 +176,7 @@ uint64_t sys_disk_free(connection_struct *conn, struct smb_filename *fname, uint64_t *bsize, uint64_t *dfree, uint64_t *dsize); uint64_t get_dfree_info(connection_struct *conn, struct smb_filename *fname, uint64_t *bsize, uint64_t *dfree, uint64_t *dsize); +void flush_dfree_cache(void); /* The following definitions come from smbd/dir.c */ diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c index c93c077..9b62096 100644 --- a/source3/smbd/server_reload.c +++ b/source3/smbd/server_reload.c @@ -164,6 +164,7 @@ bool reload_services(struct smbd_server_connection *sconn, mangle_reset_cache(); reset_stat_cache(); + flush_dfree_cache(); /* this forces service parameters to be flushed */ set_current_service(NULL,0,True); -- 1.8.3.1