Re: [PATCH] cifs: Fix problem with encrypted RDMA data read

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi David,

see below...

When the cifs client is talking to the ksmbd server by RDMA and the ksmbd
server has "smb3 encryption = yes" in its config file, the normal PDU
stream is encrypted, but the directly-delivered data isn't in the stream
(and isn't encrypted), but is rather delivered by DDP/RDMA packets (at
least with IWarp).

Currently, the direct delivery fails with:

    buf can not contain only a part of read data
    WARNING: CPU: 0 PID: 4619 at fs/cifs/smb2ops.c:4731 handle_read_data+0x393/0x405
    ...
    RIP: 0010:handle_read_data+0x393/0x405
    ...
     smb3_handle_read_data+0x30/0x37
     receive_encrypted_standard+0x141/0x224
     cifs_demultiplex_thread+0x21a/0x63b
     kthread+0xe7/0xef
     ret_from_fork+0x22/0x30

The problem apparently stemming from the fact that it's trying to manage
the decryption, but the data isn't in the smallbuf, the bigbuf or the page
array).

This can be fixed simply by inserting an extra case into handle_read_data()
that checks to see if use_rdma_mr is true, and if it is, just setting
rdata->got_bytes to the length of data delivered and allowing normal
continuation.

This can be seen in an IWarp packet trace.  With the upstream code, it does
a DDP/RDMA packet, which produces the warning above and then retries,
retrieving the data inline, spread across several SMBDirect messages that
get glued together into a single PDU.  With the patch applied, only the
DDP/RDMA packet is seen.

Note that this doesn't happen if the server isn't told to encrypt stuff and
it does also happen with softRoCE.

Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
cc: Steve French <smfrench@xxxxxxxxx>
cc: Tom Talpey <tom@xxxxxxxxxx>
cc: Long Li <longli@xxxxxxxxxxxxx>
cc: Namjae Jeon <linkinjeon@xxxxxxxxxx>
cc: Stefan Metzmacher <metze@xxxxxxxxx>
cc: linux-cifs@xxxxxxxxxxxxxxx
---

  fs/cifs/smb2ops.c |    3 +++
  1 file changed, 3 insertions(+)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 880cd494afea..8d459f60f27b 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4726,6 +4726,9 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid,
  		iov.iov_base = buf + data_offset;
  		iov.iov_len = data_len;
  		iov_iter_kvec(&iter, WRITE, &iov, 1, data_len);
+	} else if (use_rdma_mr) {
+		/* The data was delivered directly by RDMA. */
+		rdata->got_bytes = data_len;
  	} else {
  		/* read response payload cannot be in both buf and pages */
  		WARN_ONCE(1, "buf can not contain only a part of read data");

I'm not sure I understand why this would fix anything when encryption is enabled.

Is the payload still be offloaded as plaintext? Otherwise we wouldn't have use_rdma_mr...
So this rather looks like a fix for the non encrypted case.

Before smbd_register_mr() is called we typically have a check like this:

      if (server->rdma && !server->sign && wdata->bytes >=
              server->smbd_conn->rdma_readwrite_threshold) {

I'm wondering if server->sign is true for the encryption case, otherwise
we would have to add a !encrypt check in addition as we should never use
RDMA offload for encrypted connections.

Latest Windows servers allow encrypted/signed offload, but that needs to be
negotiated via MS-SMB2 2.2.3.1.6 SMB2_RDMA_TRANSFORM_CAPABILITIES, see
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/52b74a74-9838-4f51-b2b0-efeb23bd79d6
And SMB2_READFLAG_RESPONSE_RDMA_TRANSFORM in MS-SMB2 2.2.20 SMB2 READ Response
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/3e3d2f2c-0e2f-41ea-ad07-fbca6ffdfd90
As well as SMB2_CHANNEL_RDMA_TRANSFORM in 2.2.21 SMB2 WRITE Request
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/e7046961-3318-4350-be2a-a8d69bb59ce8
But none of this is implemented in Linux yet.

metze



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux