lkml.org 
[lkml]   [2022]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] cifs: Fix problem with encrypted RDMA data read
    2022-11-16 9:57 GMT+09:00, Stefan Metzmacher <metze@samba.org>:
    > 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@redhat.com>
    >> cc: Steve French <smfrench@gmail.com>
    >> cc: Tom Talpey <tom@talpey.com>
    >> cc: Long Li <longli@microsoft.com>
    >> cc: Namjae Jeon <linkinjeon@kernel.org>
    >> cc: Stefan Metzmacher <metze@samba.org>
    >> cc: linux-cifs@vger.kernel.org
    >> ---
    >>
    >> 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.
    ksmbd doesn't encrypt RDMA payload on read/write operation, Currently
    only smb2 response is encrypted for this. And as you pointed out, We
    need to implement SMB2 RDMA Transform to encrypt it.

    >
    > 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
    >

    \
     
     \ /
      Last update: 2022-11-16 06:20    [W:4.670 / U:0.208 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site