1
0
mirror of https://github.com/samba-team/samba.git synced 2024-12-25 23:21:54 +03:00
Commit Graph

25 Commits

Author SHA1 Message Date
Stefan Metzmacher
8dac16e82d smb2_query_directory: make 'return true' explicit in smb2_query_directory_next_entry()
'return req' should do the same as 'return true' for a bool function,
it's implicitly expanded as 'return (req!=NULL)?true:false.

There's no point in that as 'req' is always a valid pointer.

This was most likely just a copy and paste bug.

So we make this explicit now and avoid that Coverity reports this:

CID 1438158:  Null pointer dereferences  (REVERSE_INULL)
Null-checking "req" suggests that it may be null, but it has already
been dereferenced on all paths leading to the check.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>

Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Tue Jul 31 14:20:49 CEST 2018 on sn-devel-144
2018-07-31 14:20:49 +02:00
Ralph Boehme
0736fdcdb0 smbd: use async dos_mode_at_send in smbd_smb2_query_directory_send()
Finally: use the new dos_mode_at_send() in the directory enumeration
loop. This means that fetching the DOS attributes for directory entries
is done asynchronously with regard to the enumeration loop.

As the DOS attribute is typically read from an extended attribute in the
filesytem, this avoids sequentially blocking on IO. If the IO subsystem
is slow servicing these request, enabling async processing can result in
performance improvements.

A parametric option

  smbd:async dosmode = true | false (default: false)

can be used to enable the new async processing.

Simulating slow IO with usleep(5000) in the synchronous and asynchronous
versions of SMB_VFS_GET_DOS_ATTRIBUTES(), the results of enumerating a
directory with 10,000 files are:

    smbd:async dosmode = no:

        $ time bin/smbclient -U slow%x //localhost/test -c "ls dir\*" > /dev/null
        real    0m59.597s
        user    0m0.024s
        sys     0m0.012s

    smbd:async dosmode = yes:

        $ time bin/smbclient -U slow%x //localhost/test -c "ls dir\*" > /dev/null
        real    0m0.698s
        user    0m0.038s
        sys     0m0.025s

Performance gains in real world workloads depends on whether the actual
IO requests can be merged and parallelized by the kernel. Without such
wins at the IO layer, the async processing may even be slower then the
sync processing due to the additional overhead.

The following parameters can be used to adapt async processing behaviour
for specific workloads and systems:

        aio max threads = X (default: 100)
        smbd:max async dosmode = Y (default: "aio max threads" * 2)

By default we have at most twice the number of async requests in flight
as threads provided by the underlying threadpool. This ensures a worker
thread that finishes a job can directly pick up a new one without going
to sleep.

It may be advisable to reduce the number of threads to avoid scheduling
overhead while also increasing "smbd:max async dosmode".

Note that we disable async processing for certain VFS modules in the VFS
connect function to avoid the overhead of triggering the sync fallback
in dos_mode_at_send(). This is done for VFS modules that implement the
sync SMB_VFS_GET_DOS_ATTRIBUTES(), but not the async version (gpfs), and
for VFS modules that don't share a real filesystem where fchdir() can be
used (ceph, gluster). It is disabled for catia, because we realized that
the catia name translation macros used on
fsps (CATIA_FETCH_FSP_[PRE|POST]_NEXT) have a bug (#13547).

We use threadpool = smb_vfs_ev_glue_tp_chdir_safe() and then
pthreadpool_tevent_max_threads(threadpool) to get the number of maximum
worker threads which matches the pool used by the low level
SMB_VFS_GETXATTRAT_[SEND|RECV] implementation in vfs_default.

This is a terrible abstraction leak that should be removed in the future
by maybe making it possible to ask a VFS function which threadpool it
uses, internally suporting chaining so VFS function FOO that internally
uses BAR can forward the question to BAR.

On a hyphotetical system that had a getxattrat(dirfd, path, ...)
syscall and at the same time doesn't support per-thread current working
directories (eg FreeBSD doesn't have the latter) but has support for
per-thread-credentials, pthreadpool_tevent_max_threads() on the
tp_chdir_safe threadpool returns 1.

So when hooking the hyphotetical getxattrat() into the async
SMB_VFS_GETXATTRAT_[SEND|RECV] implementation in an VFS module, the
implementation could use the tp_path_safe threadpool, but the SMB2
layer would use the wrong threadpool in the call to
pthreadpool_tevent_max_threads(), resulting in no parallelism.

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2018-07-27 13:07:15 +02:00
Ralph Boehme
8884036ba2 smbd: let smbd_dirptr_lanman2_entry return smb_fname
Note that smb_fname is relative to fsp, not conn!

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2018-07-27 13:07:15 +02:00
Ralph Boehme
00a26ac985 smbd: deal with fsp->aio_requests in close_directory()
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2018-07-27 13:07:15 +02:00
Ralph Boehme
0e1d11eebd smbd: fix a long line in smb2_query_directory_next_entry()
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2018-07-27 13:07:15 +02:00
Ralph Boehme
abef6ca706 smbd: factor out smb2_query_directory_next_entry() from smbd_smb2_query_directory_send()
This paves the way for adding async functions into the enumeration loop.

Best viewed with: git show -w

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2018-07-27 13:07:15 +02:00
Ralph Boehme
8de5018c9f smbd: add "get_dosmode" argument to smbd_dirptr_lanman2_entry()
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2018-07-27 13:07:15 +02:00
Stefan Metzmacher
894e5001c7 smbd: add an effective {smb,smbd_smb2}_request->ev_ctx that holds the event context used for the request processing
In future this will an impersonation wrapper tevent_context based on the
user session.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2018-06-18 08:59:21 +02:00
Jeremy Allison
8dabcf8948 s3: debug: smb2: Create a new DBGC_SMB2 debug class and mark all smbd/smb2_*.c files with it.
Will allow easier smb2-specific debugging.

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Böhme <slow@samba.org>
2018-03-22 02:15:13 +01:00
Jeremy Allison
5ad5e7966f s3: smbd: Fix possible directory fd leak if the underlying OS doesn't support fdopendir()
HPUX has this problem.

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Fri Feb 23 22:56:35 CET 2018 on sn-devel-144
2018-02-23 22:56:35 +01:00
Ralph Boehme
7e0b2af4c0 s3/smbd: sticky write time offset miscalculation causes broken timestamps
The offset calculation for the offset that got passed to
fetch_write_time_send() in the enumeration loop was wrong as it passed
the offset before smbd_dirptr_lanman2_entry() added required padding.

This resulted in broken timestamps in the find response.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Tue Sep 12 02:45:46 CEST 2017 on sn-devel-144
2017-09-12 02:45:46 +02:00
Jeremy Allison
f2f936a961 s3: smbd: We can now remove the 'bool dfs_path' parameter from filename_convert().
Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Uri Simchoni <uri@samba.org>
2017-05-22 18:41:16 +02:00
Volker Lendecke
b19f3730fc smbd: Fix a 32-bit problem
On 32-bit freebsd11, size_t is 32 bit. %zu does not cover
64 bits.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2017-05-02 17:19:26 +02:00
Ralph Boehme
9c95eca0f4 s3/smbd: add "smbd:find async delay usec" to SMB2 FIND
This is just a hack for selftest that will be used in subsequent commits
for torturing compound find requests.

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2017-04-18 22:54:16 +02:00
Ralph Boehme
d99bd1c48b s3/smbd: make write time fetching async
Finally use the new async dbwrap_parse_record_send/recv() functions
respectively the fetch_share_mode_send/recv wrappers for fetching the
write time from locking.tdb.

Previously for a directory with n files we would sit idle in the
directory enumeration loop fo n * m seconds waiting for responses from
ctdb, where m is the response time in seconds for a dbwrap request via
ctbd.

This is known to kill performance and we even have a parameter
"smbd:search ask sharemode" that can be used to disable fetching the
write time from locking.tdb.

Using fetch_write_time_send() works this way: in the directory
enumeration loop that calls smbd_dirptr_lanman2_entry() to marshall the
directory entries we

1. call fetch_write_time_send() after calling smbd_dirptr_lanman2_entry
   passing a pointer to the current position in the marshall buffer.

2. If fetch_write_time_send() has set the out parameter "stop", we exit
   the enumeration loop. This is necessary because we only send dbwrap
   requests but don't consume the results. This has the potential to
   deadlock so we must stop sending requests as soon as our ctdb send
   queue is full.

3. In the fetch_write_time_done() callback, if the recv function got a
   locking.tdb record, we push the write time into the marshall buffer
   at the offet saved in the request state.

This new feature is still off by default as it doesn't
give any improvement in the non-clustered usecase.
"smbd:async search ask sharemode" can be used to activate it,
which makes only sense with "clustering = yes" (execept for testing).

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2017-04-18 22:54:16 +02:00
Ralph Boehme
d1f8d3e18b s3/smbd: ask_sharemode is not needed for info_level SMB_FIND_FILE_NAMES_INFO
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2017-04-18 22:54:16 +02:00
Ralph Boehme
f589919d67 s3/smbd: add file_id return arg to smbd_dirptr_lanman2_entry
Not used for now, needed for async write_time updates in
smbd_smb2_query_directory_send().

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2017-04-18 22:54:16 +02:00
Ralph Boehme
47b6b6f8f5 CVE-2017-2619: s3/smbd: re-open directory after dptr_CloseDir()
dptr_CloseDir() will close and invalidate the fsp's file descriptor, we
have to reopen it.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Uri Simchoni <uri@samba.org>
2017-03-23 19:10:19 +01:00
Jeremy Allison
247481c12c s3: smbd: Change dptr_create() to take a const struct smb_filename * instead of const char *.
Also internally change path storage inside struct dptr_struct
to a struct smb_filename *.

This allows me to remove several of the synthetic_smb_fname()
calls I had to add in the previous patches, as we're now
dealing with struct smb_filename * throughout the dptr and
OpenDir code.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Tue Mar  1 18:34:24 CET 2016 on sn-devel-144
2016-03-01 18:34:24 +01:00
Jeremy Allison
398ee270de s3: smbd: Replace lp_posix_pathnames() with smbreq->posix_pathnames in smb2_query_directory.c.
Currently SMB2/3 doesn't do posix pathname processing, leave this
as a placeholder for when SMB2 unix extensions are added.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Volker Lendecke <Volker.Lendecke@SerNet.DE>
2015-12-23 18:23:17 +01:00
Jeremy Allison
274e8b5409 s3: smbd: In smb2_query_directory.c.c, add in UCF_POSIX_PATHNAMES to the ucf_flags if lp_posix_pathnames() requested.
Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <rb@sernet.de>
2015-12-23 03:31:10 +01:00
Jeremy Allison
acf6600132 s3: smbd: In smb2_query_directory.c Use ucf_flags variable instead of passing as parameter.
This will allow us to move lp_posix_pathnames() out of unix_convert()
later.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <rb@sernet.de>
2015-12-23 03:31:10 +01:00
Volker Lendecke
0062177d81 smbd: Fix CID 1343333 Uninitialized variables
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2015-12-15 20:29:04 +01:00
Jeremy Allison
dea2addab1 s3: smbd: Moving lp_posix_pathnames() out of the lower-level code.
Prepare to remove lp_posix_pathnames() out of ms_has_wild().
Check before calls to ms_has_wild().

Fix smbd_smb2_query_directory_send().

No SMB2 client uses unix extensions yet, but this is a placeholder
for when we move the POSIX pathnames bit into the SMB2 request
when moving to handle based code.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2015-12-13 22:59:27 +01:00
Richard Sharpe
f0e9ba91c0 Rename SMB2_OP_FIND to SMB2_OP_QUERY_DIRECTORY so that it conforms with the MS document MS-SMB2.
Signed-off-by: Richard Sharpe <rsharpe@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Fri Mar 27 01:24:47 CET 2015 on sn-devel-104
2015-03-27 01:24:47 +01:00