- 02 Jan, 2019 33 commits
-
-
J. Bruce Fields authored
If we ignore the error we'll hit a null dereference a little later. Reported-by: syzbot+4b98281f2401ab849f4b@syzkaller.appspotmail.com Signed-off-by:
J. Bruce Fields <bfields@redhat.com> Signed-off-by:
Anna Schumaker <Anna.Schumaker@Netapp.com>
-
NeilBrown authored
As gte_current_cred() cannot return an error, this test is not necessary. It hasn't been necessary for years, but it wasn't so obvious before. Reported-by:
Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by:
NeilBrown <neilb@suse.com> Signed-off-by:
-
Chuck Lever authored
If a reply has been processed but the RPC is later retransmitted anyway, the req->rl_reply field still contains the only pointer to the old rpcrdma rep. When the next reply comes in, the reply handler will stomp on the rl_reply field, leaking the old rep. A trace event is added to capture such leaks. This problem seems to be worsened by the restructuring of the RPC Call path in v4.20. Fully addressing this issue will require at least a re-architecture of the disconnect logic, which is not appropriate during -rc. Signed-off-by:
Chuck Lever <chuck.lever@oracle.com> Signed-off-by:
-
Olga Kornievskaia authored
Original commit (e4648aa4 "NFS recover from destination server reboot for copies") used memcmp() and then it was changed to use nfs4_stateid_match_other() but that function returns opposite of memcmp. As the result, recovery can't find the copy leading to copy hanging. Fixes: 80f42368 ("NFSv4: Split out NFS v4.2 copy completion functions") Fixes: cb7a8384 ("NFS: Split out the body of nfs4_reclaim_open_state") Signed-of-by:
Olga Kornievskaia <kolga@netapp.com> Signed-off-by:
-
Chuck Lever authored
Defensive clean up. Don't set frwr->fr_mr until we know that the scatterlist allocation has succeeded. Signed-off-by:
-
Chuck Lever authored
Make a note of the function's dependency on an earlier ib_drain_qp. Signed-off-by:
-
Chuck Lever authored
Since commit 7c8d9e7c ("xprtrdma: Move Receive posting to Receive handler"), rpcrdma_ep_post is no longer responsible for posting Receive buffers. Update the documenting comment to reflect this change. Signed-off-by:
-
Chuck Lever authored
Commit f2877623 ("xprtrdma: Chain Send to FastReg WRs") was written before commit ce5b3717 ("xprtrdma: Replace all usage of "frmr" with "frwr""), but was merged afterwards. Thus it still refers to FRMR and MWs. Signed-off-by:
-
Chuck Lever authored
Clean up some warnings observed when building with "make W=1". Signed-off-by:
-
Chuck Lever authored
Clean up, no functional change is expected. Signed-off-by:
-
Chuck Lever authored
These symbolic values were not being displayed in string form. TRACE_DEFINE_ENUM was missing in many cases. It also turns out that __print_symbolic wants an unsigned long in the first field... Signed-off-by:
-
Chuck Lever authored
These are rare, but can be helpful at tracking down DMAR and other problems. Signed-off-by:
-
Chuck Lever authored
Name them "trace_xprtrdma_op_*" so they can be easily enabled as a group. No trace point is added where the generic layer already has observability. Signed-off-by:
-
Chuck Lever authored
The mr_map trace points were capturing information about the previous use of the MR rather than about the segment that was just mapped. Signed-off-by:
-
Chuck Lever authored
The chunk-related trace points capture nearly the same information as the MR-related trace points. Also, rename them so globbing can be used to enable or disable these trace points more easily. Signed-off-by:
-
Chuck Lever authored
Clean up. The last use of these fields was in commit 173b8f49 ("xprtrdma: Demote "connect" log messages") . Signed-off-by:
-
Chuck Lever authored
Clean up: Remove dprintk() call sites that report rare or impossible errors. Leave a few that display high-value low noise status information. Signed-off-by:
-
Chuck Lever authored
Clean up: There's little chance of contention between the use of rb_lock and rb_reqslock, so merge the two. This avoids having to take both in some (possibly future) cases. Transport tear-down is already serialized, thus there is no need for locking at all when destroying rpcrdma_reqs. Signed-off-by:
-
Chuck Lever authored
For better observability of parsing errors, return the error code generated in the decoders to the upper layer consumer. Signed-off-by:
-
Chuck Lever authored
Since commit ffe1f0df ("rpcrdma: Merge svcrdma and xprtrdma modules into one"), the forward and backchannel components are part of the same kernel module. A separate request_module() call in the backchannel code is no longer necessary. Signed-off-by:
-
Chuck Lever authored
Commit 431f6eb3 ("SUNRPC: Add a label for RPC calls that require allocation on receive") didn't update similar logic in rpc_rdma.c. I don't think this is a bug, per-se; the commit just adds more careful checking for broken upper layer behavior. Signed-off-by:
-
Chuck Lever authored
Having to specify "proto=rdma,port=20049" is cumbersome. RFC 8267 Section 6.3 requires NFSv4 clients to use "the alternative well-known port number", which is 20049. Make the use of the well- known port number automatic, just as it is for NFS/TCP and port 2049. For NFSv2/3, Section 4.2 allows clients to simply choose 20049 as the default or use rpcbind. I don't know of an NFS/RDMA server implementation that registers it's NFS/RDMA service with rpcbind, so automatically choosing 20049 seems like the better choice. The other widely-deployed NFS/RDMA client, Solaris, also uses 20049 as the default port. Signed-off-by:
-
Chuck Lever authored
Place the associated RPC transaction's XID in the upper 32 bits of each RDMA segment's rdma_offset field. There are two reasons to do this: - The R_key only has 8 bits that are different from registration to registration. The XID adds more uniqueness to each RDMA segment to reduce the likelihood of a software bug on the server reading from or writing into memory it's not supposed to. - On-the-wire RDMA Read and Write requests do not otherwise carry any identifier that matches them up to an RPC. The XID in the upper 32 bits will act as an eye-catcher in network captures. Suggested-by:
Tom Talpey <ttalpey@microsoft.com> Signed-off-by:
-
Chuck Lever authored
Clean up: Now that there is only FRWR, there is no need for a memory registration switch. The indirect calls to the memreg operations can be replaced with faster direct calls. Signed-off-by:
-
Chuck Lever authored
FMR is not supported on most recent RDMA devices. It is also less secure than FRWR because an FMR memory registration can expose adjacent bytes to remote reading or writing. As discussed during the RDMA BoF at LPC 2018, it is time to remove support for FMR in the NFS/RDMA client stack. Note that NFS/RDMA server-side uses either local memory registration or FRWR. FMR is not used. There are a few Infiniband/RoCE devices in the kernel tree that do not appear to support MEM_MGT_EXTENSIONS (FRWR), and therefore will not support client-side NFS/RDMA after this patch. These are: - mthca - qib - hns (RoCE) Users of these devices can use NFS/TCP on IPoIB instead. Signed-off-by:
-
Chuck Lever authored
Some devices advertise a large max_fast_reg_page_list_len capability, but perform optimally when MRs are significantly smaller than that depth -- probably when the MR itself is no larger than a page. By default, the RDMA R/W core API uses max_sge_rd as the maximum page depth for MRs. For some devices, the value of max_sge_rd is 1, which is also not optimal. Thus, when max_sge_rd is larger than 1, use that value. Otherwise use the value of the max_fast_reg_page_list_len attribute. I've tested this with CX-3 Pro, FastLinq, and CX-5 devices. It reproducibly improves the throughput of large I/Os by several percent. Signed-off-by:
-
Chuck Lever authored
With certain combinations of krb5i/p, MR size, and r/wsize, I/O can fail with EMSGSIZE. This is because the calculated value of ri_max_segs (the max number of MRs per RPC) exceeded RPCRDMA_MAX_HDR_SEGS, which caused Read or Write list encoding to walk off the end of the transport header. Once that was addressed, the ro_maxpages result has to be corrected to account for the number of MRs needed for Reply chunks, which is 2 MRs smaller than a normal Read or Write chunk. Signed-off-by:
-
Chuck Lever authored
Transport disconnect processing does a "wake pending tasks" at various points. Suppose an RPC Reply is being processed. The RPC task that Reply goes with is waiting on the pending queue. If a disconnect wake-up happens before reply processing is done, that reply, even if it is good, is thrown away, and the RPC has to be sent again. This window apparently does not exist for socket transports because there is a lock held while a reply is being received which prevents the wake-up call until after reply processing is done. To resolve this, all RPC replies being processed on an RPC-over-RDMA transport have to complete before pending tasks are awoken due to a transport disconnect. Callers that already hold the transport write lock may invoke ->ops->close directly. Others use a generic helper that schedules a close when the write lock can be taken safely. Signed-off-by:
-
Chuck Lever authored
After thinking about this more, and auditing other kernel ULP imple- mentations, I believe that a DISCONNECT cm_event will occur after a fatal QP event. If that's the case, there's no need for an explicit disconnect in the QP event handler. Signed-off-by:
-
Chuck Lever authored
To address a connection-close ordering problem, we need the ability to drain the RPC completions running on rpcrdma_receive_wq for just one transport. Give each transport its own RPC completion workqueue, and drain that workqueue when disconnecting the transport. Signed-off-by:
-
Chuck Lever authored
Clean up: Divide the work cleanly: - rpcrdma_wc_receive is responsible only for RDMA Receives - rpcrdma_reply_handler is responsible only for RPC Replies - the posted send and receive counts both belong in rpcrdma_ep Signed-off-by:
-
Chuck Lever authored
The recovery case in frwr_op_unmap_sync needs to DMA unmap each MR. frwr_release_mr does not DMA-unmap, but the recycle worker does. Fixes: 61da886b ("xprtrdma: Explicitly resetting MRs is ... ") Signed-off-by:
-
Chuck Lever authored
While chasing yet another set of DMAR fault reports, I noticed that the frwr recycler conflates whether or not an MR has been DMA unmapped with frwr->fr_state. Actually the two have only an indirect relationship. It's in fact impossible to guess reliably whether the MR has been DMA unmapped based on its fr_state field, especially as the surrounding code and its assumptions have changed over time. A better approach is to track the DMA mapping status explicitly so that the recycler is less brittle to unexpected situations, and attempts to DMA-unmap a second time are prevented. Signed-off-by:
-
- 21 Dec, 2018 1 commit
-
-
Chris Perl authored
This patch removes the check from nfs_compare_mount_options to see if a `sec' option was passed for the current mount before comparing auth flavors and instead just always compares auth flavors. Consider the following scenario: You have a server with the address 192.168.1.1 and two exports /export/a and /export/b. The first export supports `sys' and `krb5' security, the second just `sys'. Assume you start with no mounts from the server. The following results in EIOs being returned as the kernel nfs client incorrectly thinks it can share the underlying `struct nfs_server's: $ mkdir /tmp/{a,b} $ sudo mount -t nfs -o vers=3,sec=krb5 192.168.1.1:/export/a /tmp/a $ sudo mount -t nfs -o vers=3 192.168.1.1:/export/b /tmp/b $ df >/dev/null df: ‘/tmp/b’: Input/output error Signed-off-by:Chris Perl <cperl@janestreet.com> Signed-off-by:
-
- 19 Dec, 2018 6 commits
-
-
NeilBrown authored
Just use ->cr_cred->fsuid directly. Signed-off-by:
-
NeilBrown authored
1/ discard 'struct unx_cred'. We don't need any data that is not already in 'struct rpc_cred'. 2/ Don't keep these creds in a hash table. When a credential is needed, simply allocate it. When not needed, discard it. This can easily be faster than performing a lookup on a shared hash table. As the lookup can happen during write-out, use a mempool to ensure forward progress. This means that we cannot compare two credentials for equality by comparing the pointers, but we never do that anyway. Signed-off-by:
-
NeilBrown authored
This now always just does get_rpccred(), so we don't need an operation pointer to know to do that. Signed-off-by:
-
NeilBrown authored
This is no longer used. Signed-off-by:
-
NeilBrown authored
SUNRPC has two sorts of credentials, both of which appear as "struct rpc_cred". There are "generic credentials" which are supplied by clients such as NFS and passed in 'struct rpc_message' to indicate which user should be used to authorize the request, and there are low-level credentials such as AUTH_NULL, AUTH_UNIX, AUTH_GSS which describe the credential to be sent over the wires. This patch replaces all the generic credentials by 'struct cred' pointers - the credential structure used throughout Linux. For machine credentials, there is a special 'struct cred *' pointer which is statically allocated and recognized where needed as having a special meaning. A look-up of a low-level cred will map this to a machine credential. Signed-off-by:
-
NeilBrown authored
Use the common 'struct cred' to pass credentials for readdir. Signed-off-by:
-
