Message ID | 20230131182855.4027499-12-dhowells@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp63829wrn; Tue, 31 Jan 2023 10:33:43 -0800 (PST) X-Google-Smtp-Source: AK7set/LABc0UrP9rGiAs7U8WTp5FagPajsD6V9oTHqNsNGDz56pVrJHdqVkWLbvtwNhhik77HIT X-Received: by 2002:a17:902:c412:b0:196:82a0:4187 with SMTP id k18-20020a170902c41200b0019682a04187mr10963544plk.36.1675190022938; Tue, 31 Jan 2023 10:33:42 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675190022; cv=none; d=google.com; s=arc-20160816; b=VU8J8lEd7DCuA5/4UPcwEQN2tSdej6pCqkZ2PL1ryKfIvviP1NvQUGcAZsfVW1KjEb GNUUoNr/rQ/6bfAEHvbjXFddt76QHnu+rN6V1V2OOuZUUDdv9tZo4My8JPrhLMY6WOKd YxHA9pk0ygsNXXs21GRY5lDLPJfnFht7ZPoObHGyRNLl1Z5HZfEKryA/QNkvQZFvislS OUFvA49P/fcKFK/fSmbFZ3pW7aKvGvdSL7xX9pnQjwk8LYWgMb1WS/KTbDyQnzjM9lQn vhp8pb9vgCN+75uyh7HdrBW93NEkPmOd04/nI3CTUfnKGLdePC4OYNY4wmEmUP6cWb9E q1RA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=eyDj/YozGMP7rLtsc0cWS+zeHfZsaS4iMdGY5XNJpoE=; b=eQKRJapUv+SwxlCNbvNm1Bs1L3RZ2WcYCu0emiiHIDUiT8osbqEVnBYXbQeW2+fPN7 wZ/73RVy4YViSP9smm/7fvrbZVKgJWOGndmsplXlnhaBpGnm1fv54uOB7odwfsUnd/4m IlBmV6clII7TBEpfS88tSLMoZAiqVTZC+Y53ziDZD/pFJFk4RvT0cu1Opsz0WNLVYy1F +ybIF9aWNBR5hRjBK/ooJt3nQXeVRJgB1ssSc8X1O4n+OjNPCuz32DEijMJQZdWZUwaR k3Rpc5sPXw03mb9iCMqZnHPLV1NFNkEATG2C7E9jSFiE1thiTh9Edi6JXip4bPi8dXM9 OFrQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=JFim05CV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id x3-20020a170902820300b00194d51f6607si12674748pln.456.2023.01.31.10.33.28; Tue, 31 Jan 2023 10:33:42 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=JFim05CV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231838AbjAaSbo (ORCPT <rfc822;maxin.john@gmail.com> + 99 others); Tue, 31 Jan 2023 13:31:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54482 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231726AbjAaSa6 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 31 Jan 2023 13:30:58 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7B60059245 for <linux-kernel@vger.kernel.org>; Tue, 31 Jan 2023 10:30:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675189803; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=eyDj/YozGMP7rLtsc0cWS+zeHfZsaS4iMdGY5XNJpoE=; b=JFim05CVIchBE3rpyyeZJvZyFg1RJsteorMJFS9SktS3OEGzGRMrNGIRtuYZvpDYyUYtxm 0ArwVHDA2gCGCZVcpq1750N0YVOxTQcaB4DIuK3m2irkKGgojSCcjvaO/pxjW+9858ZcmA bGtztNqLaOMJYKdKwPVsrjuo/Tq51cw= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-663-HP0ebrMgPvOTCqiYZVeThg-1; Tue, 31 Jan 2023 13:29:59 -0500 X-MC-Unique: HP0ebrMgPvOTCqiYZVeThg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C1DBD89522E; Tue, 31 Jan 2023 18:29:26 +0000 (UTC) Received: from warthog.procyon.org.uk.com (unknown [10.33.36.97]) by smtp.corp.redhat.com (Postfix) with ESMTP id 14B9C422F2; Tue, 31 Jan 2023 18:29:24 +0000 (UTC) From: David Howells <dhowells@redhat.com> To: Steve French <smfrench@gmail.com> Cc: David Howells <dhowells@redhat.com>, Al Viro <viro@zeniv.linux.org.uk>, Shyam Prasad N <nspmangalore@gmail.com>, Rohith Surabattula <rohiths.msft@gmail.com>, Tom Talpey <tom@talpey.com>, Stefan Metzmacher <metze@samba.org>, Christoph Hellwig <hch@infradead.org>, Matthew Wilcox <willy@infradead.org>, Jeff Layton <jlayton@kernel.org>, linux-cifs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Long Li <longli@microsoft.com>, Namjae Jeon <linkinjeon@kernel.org> Subject: [PATCH 11/12] cifs: Fix problem with encrypted RDMA data read Date: Tue, 31 Jan 2023 18:28:54 +0000 Message-Id: <20230131182855.4027499-12-dhowells@redhat.com> In-Reply-To: <20230131182855.4027499-1-dhowells@redhat.com> References: <20230131182855.4027499-1-dhowells@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1756564053721114697?= X-GMAIL-MSGID: =?utf-8?q?1756564053721114697?= |
Series |
smb3: Use iov_iters down to the network transport and fix DIO page pinning
|
|
Commit Message
David Howells
Jan. 31, 2023, 6:28 p.m. UTC
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
Link: https://lore.kernel.org/r/166855224228.1998592.2212551359609792175.stgit@warthog.procyon.org.uk/ # v1
---
fs/cifs/smb2ops.c | 3 +++
1 file changed, 3 insertions(+)
Comments
On Tue, Jan 31, 2023 at 06:28:54PM +0000, David Howells wrote: > 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). This really needs to be split into a separate backportable fix series. And it seems like Stefan has just send such a series.
Am 01.02.23 um 14:36 schrieb Christoph Hellwig: > On Tue, Jan 31, 2023 at 06:28:54PM +0000, David Howells wrote: >> 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). > > This really needs to be split into a separate backportable fix series. > And it seems like Stefan has just send such a series. My changes are triggered by this commit... Now I'm looking more closely as I didn't understand it...
Am 31.01.23 um 19:28 schrieb David Howells: > 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 > > Link: https://lore.kernel.org/r/166855224228.1998592.2212551359609792175.stgit@warthog.procyon.org.uk/ # v1 > --- > fs/cifs/smb2ops.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index cea578a45ed8..73b66ac86abf 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -4733,6 +4733,9 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid, > if (length < 0) > return length; > rdata->got_bytes = data_len; > + } else if (use_rdma_mr) { > + /* The data was delivered directly by RDMA. */ > + rdata->got_bytes = data_len; I actually don't understand why this would only be a problem with encryption. I guess there's much more needed and data_offset should most likely be ignored completely in the rdma offload case. So I'd guess its just luck that we don't trigger the below warning/error more often. > } else { > /* read response payload cannot be in both buf and pages */ > WARN_ONCE(1, "buf can not contain only a part of read data"); >
Am 01.02.23 um 15:05 schrieb Stefan Metzmacher: > Am 31.01.23 um 19:28 schrieb David Howells: >> 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 >> >> Link: https://lore.kernel.org/r/166855224228.1998592.2212551359609792175.stgit@warthog.procyon.org.uk/ # v1 >> --- >> fs/cifs/smb2ops.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c >> index cea578a45ed8..73b66ac86abf 100644 >> --- a/fs/cifs/smb2ops.c >> +++ b/fs/cifs/smb2ops.c >> @@ -4733,6 +4733,9 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid, >> if (length < 0) >> return length; >> rdata->got_bytes = data_len; >> + } else if (use_rdma_mr) { >> + /* The data was delivered directly by RDMA. */ >> + rdata->got_bytes = data_len; > > I actually don't understand why this would only be a problem with encryption. > > I guess there's much more needed and data_offset should most likely be > ignored completely in the rdma offload case. So I'd guess its just luck > that we don't trigger the below warning/error more often. I guess it might be related to smb3_handle_read_data passing server->pdu_size to handle_read_data(), while server->pdu_size is the outer size of the transform and not the size of the decrypted pdu. Maybe receive_encrypted_standard() needs to reset server->pdu_size during this: if (mid_entry && mid_entry->handle) ret = mid_entry->handle(server, mid_entry); else ret = cifs_handle_standard(server, mid_entry); But this code is so overly complex, that's way to hard to understand...
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index cea578a45ed8..73b66ac86abf 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -4733,6 +4733,9 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid, if (length < 0) return length; rdata->got_bytes = 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");