- 06 Mar, 2019 26 commits
-
-
Pavel Shilovsky authored
When we attempt to send a packet while the demultiplex thread is in the middle of cifs_reconnect() we may end up returning -ENOTSOCK to upper layers. The intent here is to retry the request once the TCP connection is up, so change it to return -EAGAIN instead. The latter error code is retryable and the upper layers will retry the request if needed. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Do not allow commands other than SMB2_NEGOTIATE to be sent over recently established TCP connections. Return -EAGAIN to let upper layers handle it properly. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
When we have a READ lease for a file and have just issued a write operation to the server we need to purge the cache and set oplock/lease level to NONE to avoid reading stale data. Currently we do that only if a write operation succedeed thus not covering cases when a request was sent to the server but a negative error code was returned later for some other reasons (e.g. -EIOCBQUEUED or -EINTR). Fix this by turning off caching regardless of the error code being returned. The patches fixes generic tests 075 and 112 from the xfs-tests. Cc: <stable@vger.kernel.org> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
-
Steve French authored
For kerberos mounts, the cruid is helpful to display in /proc/mounts in order to tell which uid's krb5 cache we got the ticket for and to tell in the multiuser krb5 case which local users (uids) we have Kerberos authentic sessions for. Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
-
Louis Taylor authored
When compiling with -Wformat, clang emits the following warnings: fs/cifs/smb1ops.c:312:20: warning: format specifies type 'unsigned short' but the argument has type 'unsigned int' [-Wformat] tgt_total_cnt, total_in_tgt); ^~~~~~~~~~~~ fs/cifs/cifs_dfs_ref.c:289:4: warning: format specifies type 'short' but the argument has type 'int' [-Wformat] ref->flags, ref->server_type); ^~~~~~~~~~ fs/cifs/cifs_dfs_ref.c:289:16: warning: format specifies type 'short' but the argument has type 'int' [-Wformat] ref->flags, ref->server_type); ^~~~~~~~~~~~~~~~ fs/cifs/cifs_dfs_ref.c:291:4: warning: format specifies type 'short' but the argument has type 'int' [-Wformat] ref->ref_flag, ref->path_consumed); ^~~~~~~~~~~~~ fs/cifs/cifs_dfs_ref.c:291:19: warning: format specifies type 'short' but the argument has type 'int' [-Wformat] ref->ref_flag, ref->path_consumed); ^~~~~~~~~~~~~~~~~~ The types of these arguments are unconditionally defined, so this patch updates the format character to the correct ones for ints and unsigned ints. Link: https://github.com/ClangBuiltLinux/linux/issues/378Signed-off-by: Louis Taylor <louis@kragniz.eu> Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
-
Steve French authored
Adds dynamic trace points for the query_info_enter and query_info_done (no error) case. We only had one existing trace point related to this which was on query_info errors. Note that these two new tracepoints are for the non-compounded query_info paths. Sample output (from: trace-cmd record -e smb3_query_info*) ls-24140 [001] .... 27811.866068: smb3_query_info_enter: xid=7 sid=0xd2d00587 tid=0xb5441939 fid=0xcf082bac class=18 type=0x1 ls-24140 [001] .... 27811.867656: smb3_query_info_done: xid=7 sid=0xd2d00587 tid=0xb5441939 fid=0xcf082bac class=18 type=0x1 getcifsacl-24149 [005] .... 27854.759873: smb3_query_info_enter: xid=15 sid=0xd2d00587 tid=0xb5441939 fid=0x99896e72 class=0 type=0x3 getcifsacl-24149 [005] .... 27854.761730: smb3_query_info_done: xid=15 sid=0xd2d00587 tid=0xb5441939 fid=0x99896e72 class=0 type=0x3 Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
-
Steve French authored
Add tracepoint before sending an SMB3 command on the wire (ie add an smb3_cmd_enter tracepoint). This allows us to look in much more detail at response times (between request and response). Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
-
Steve French authored
Add dynamic trace point for open_enter (and posix mkdir enter) Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
-
Steve French authored
When ENODATA returned we weren't logging the read completion (not an error, but can be indicated by logging length 0) which makes looking at read traces confusing for smb3. Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
-
Steve French authored
Allows tracing begin (not just completion) of read, write and query_dir which may be helpful in finding slow requests and other timing information Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
-
Steve French authored
Adds two tracepoints - one for query_dir done (no err) and one for query_dir_err Sanple output: To start the trace in one window: trace-cmd record -e smb3_query_dir* Then in another window after doing an ls /mnt View the trace output by: trace-cmd show Sample output: TASK-PID CPU# |||| TIMESTAMP FUNCTION | | | |||| | | ls-24869 [007] .... 90695.452009: smb3_query_dir_done: xid=7 sid=0x5027d24d tid=0xb95cf25a fid=0xc41a8c3e offset=0x0 len=0x16 ls-24869 [000] .... 90695.452764: smb3_query_dir_done: xid=8 sid=0x5027d24d tid=0xb95cf25a fid=0xc41a8c3e offset=0x0 len=0x0 ls-24874 [003] .... 90701.506342: smb3_query_dir_done: xid=11 sid=0x5027d24d tid=0xb95cf25a fid=0x33ad3601 offset=0x0 len=0x8 ls-24874 [003] .... 90701.506917: smb3_query_dir_done: xid=12 sid=0x5027d24d tid=0xb95cf25a fid=0x33ad3601 offset=0x0 len=0x0 Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Steve French authored
POSIX negotiate context now includes the GUID specifying which POSIX open context we support. Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Jeremy Allison <jra@samba.org>
-
Steve French authored
To 2.18 Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Currently we get one credit per compound part of the request individually. This may lead to being stuck on waiting for credits if multiple compounded operations happen in parallel. Try acquire credits for all compound parts at once. Return immediately if not enough credits and too few requests are in flight currently thus narrowing the possibility of infinite waiting for credits. The more advance fix is to return right away if not enough credits for the compound request and do not look at the number of requests in flight. The caller should handle such situations by falling back to sequential execution of SMB commands instead of compounding. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Now we just return NULL cifsFileInfo pointer in cases we didn't find or couldn't reopen a file. This hides errors from cifs_reopen_file() especially retryable errors which should be handled appropriately. Create new cifs_get_writable_file() routine that returns error codes from cifs_reopen_file() and use it in the writeback codepath. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Currently we check for an open file existence in wdata_send_pages() which doesn't provide an easy way to handle error codes that will be returned from find_writable_filehandle() once it is changed. Move the check to writepages. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Currently wdata_send_pages() unlocks pages after sending. This complicates further refactoring and doesn't align with the function name. Move unlocking to writepages. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Reorder finding and reopening a writable handle file and getting MTU credits in writepages because we may be stuck on low credits otherwise. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Currently we get MTU credits before we check an open file if it needs to be reopened. Reopening the file in such conditions leads to a possibility of being stuck waiting indefinitely for credits in the transport layer. Fix this by reopening the file first if needed and then getting MTU credits for async IO. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Currently we do proper accounting for credits in regards to reconnects and error handling, thus we do not need custom credit adjustments when reconnect is detected developed previously. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Currently we adjust MTU credits before sending an IO request and after reopening a file. This approach doesn't allow the reopen routine to use existing credits that are not needed for IO. Reorder credit adjustment and reopening a file to use credits available to the client more efficiently. Also unwrap complex if statement into few pieces to improve readability. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
The reconnect might have happended after we obtained credits and before we acquired srv_mutex. Check for that under the mutex and retry a sync operation if the reconnect is detected. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
The reconnect might have happended after we obtained credits and before we acquired srv_mutex. Check for that under the mutex and retry an async operation if the reconnect is detected. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Every time after a session reconnect we don't need to account for credits obtained in previous sessions. Make use of the recently added cifs_credits structure to properly calculate credits for non-MTU requests the same way we did for MTU ones. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Every time after a session reconnect we don't need to account for credits obtained in previous sessions. Introduce new struct cifs_credits which contains both credits value and reconnect instance of the time those credits were taken. Modify a routine that add credits back to handle the reconnect instance by assuming zero credits if the reconnect happened after the credits were obtained and before we decided to add them back due to some errors during sending. This patch fixes the MTU credits cases. The subsequent patch will handle non-MTU ones. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Currently we set reconnect instance to zero on the first connection but this is not convenient because we need to reserve some special value for credit handling on reconnects which is coming in subsequent patches. Fix this by starting with one when initiating a new TCP connection. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
- 05 Mar, 2019 14 commits
-
-
Pavel Shilovsky authored
There are a couple places where we still account for 4 bytes in the beginning of SMB2 packet which is not true in the current code. Fix this to use a header preamble size where possible. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Even if a response is malformed, we should count credits granted by the server to avoid miscalculations and unnecessary reconnects due to client or server bugs. If the response has been received partially, the session will be reconnected anyway on the next iteration of the demultiplex thread, so counting credits for such cases shouldn't break things. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Currently we only skip credits logging on reconnects. When unmounting a share the number of credits on the client doesn't matter, so skip logging in such cases too. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
Currently we skip setting a read error to -EIO if a stored result is -ENODATA and a response hasn't been received. With the recent changes in read error processing there shouldn't be cases when -ENODATA is set without a response from the server, so reset the error to -EIO unconditionally. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Namjae Jeon authored
Old windows version or Netapp SMB server will return NT_STATUS_NOT_SUPPORTED since they do not allow or implement FSCTL_VALIDATE_NEGOTIATE_INFO. The client should accept the response provided it's properly signed. See https://blogs.msdn.microsoft.com/openspecification/2012/06/28/smb3-secure-dialect-negotiation/ and MS-SMB2 validate negotiate response processing: https://msdn.microsoft.com/en-us/library/hh880630.aspx Samba client had already handled it. https://bugzilla.samba.org/attachment.cgi?id=13285&action=editSigned-off-by: Namjae Jeon <linkinjeon@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Pavel Shilovsky authored
When we hit failures during constructing MIDs or sending PDUs through the network, we end up not using message IDs assigned to the packet. The next SMB packet will skip those message IDs and continue with the next one. This behavior may lead to a server not granting us credits until we use the skipped IDs. Fix this by reverting the current ID to the original value if any errors occur before we push the packet through the network stack. This patch fixes the generic/310 test from the xfs-tests. Cc: <stable@vger.kernel.org> # 4.19.x Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Steve French authored
If we try large I/O (read or write) immediately after mount we won't typically have enough credits because we only request large amounts of credits on the first session setup. So if large I/O is attempted soon after mount we will typically only have about 43 credits rather than 105 credits (with this patch) available for the large i/o (which needs 64 credits minimum). This patch requests more credits during tree connect, which helps ensure that we have enough credits when mount completes (between these requests and the first session setup) in order to start large I/O immediately after mount if needed. Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
-
Steve French authored
We negotiate rsize mounts (and it can be overridden by user) to typically 4MB, so using larger default I/O sizes from userspace (changing to 1MB default i/o size returned by stat) the performance is much better (and not just for long latency network connections) in most use cases for SMB3 than the default I/O size (which ends up being 128K for cp and can be even smaller for cp). This can be 4x slower or worse depending on network latency. By changing inode->blocksize from 32K (which was perhaps ok for very old SMB1/CIFS) to a larger value, 1MB (but still less than max size negotiated with the server which is 4MB, in order to minimize risk) it significantly increases performance for the noncached case, and slightly increases it for the cached case. This can be changed by the user on mount (specifying bsize= values from 16K to 16MB) to tune better for performance for applications that depend on blocksize. Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> CC: Stable <stable@vger.kernel.org>
-
Pavel Shilovsky authored
Currently on lease break the client sets a caching level twice: when oplock is detected and when oplock is processed. While the 1st attempt sets the level to the value provided by the server, the 2nd one resets the level to None unconditionally. This happens because the oplock/lease processing code was changed to avoid races between page cache flushes and oplock breaks. The commit c11f1df5 ("cifs: Wait for writebacks to complete before attempting write.") fixed the races for oplocks but didn't apply the same changes for leases resulting in overwriting the server granted value to None. Fix this by properly processing lease breaks. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com> CC: Stable <stable@vger.kernel.org>
-
Steve French authored
/proc/fs/cifs/Stats bytes_read was double counting reads when uncached (ie mounted with cache=none) Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
-
Ronnie Sahlberg authored
BUGZILLA: https://bugzilla.kernel.org/show_bug.cgi?id=202007 When deleting an xattr/EA: SMB2/3 servers will return SUCCESS when clients delete non-existing EAs. This means that we need to first QUERY the server and check if the EA exists or not so that we can return -ENODATA correctly when this happens. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Ronnie Sahlberg authored
We should add any credits granted to us from unmatched server responses. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
-
Ronnie Sahlberg authored
a trivial patch that replaces all use of snprintf with scnprintf. scnprintf() is generally seen as a safer function to use than snprintf for many use cases. In our case, there is no actual difference between the two since we never look at the return value. Thus we did not have any of the bugs that scnprintf protects against and the patch does nothing. However, for people reading our code it will be a receipt that we have done our due dilligence and checked our code for this type of bugs. See the presentation "Making C Less Dangerous In The Linux Kernel" at this years LCA Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
-
Yao Liu authored
There is a NULL pointer dereference of devname in strspn() The oops looks something like: CIFS: Attempting to mount (null) BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 ... RIP: 0010:strspn+0x0/0x50 ... Call Trace: ? cifs_parse_mount_options+0x222/0x1710 [cifs] ? cifs_get_volume_info+0x2f/0x80 [cifs] cifs_setup_volume_info+0x20/0x190 [cifs] cifs_get_volume_info+0x50/0x80 [cifs] cifs_smb3_do_mount+0x59/0x630 [cifs] ? ida_alloc_range+0x34b/0x3d0 cifs_do_mount+0x11/0x20 [cifs] mount_fs+0x52/0x170 vfs_kern_mount+0x6b/0x170 do_mount+0x216/0xdc0 ksys_mount+0x83/0xd0 __x64_sys_mount+0x25/0x30 do_syscall_64+0x65/0x220 entry_SYSCALL_64_after_hwframe+0x49/0xbe Fix this by adding a NULL check on devname in cifs_parse_devname() Signed-off-by: Yao Liu <yotta.liu@ucloud.cn> Signed-off-by: Steve French <stfrench@microsoft.com>
-