* [PATCH v2 1/3] cifs: introduce cifs_io_parms in smb2_async_writev()
2023-02-01 15:21 [PATCH v2 0/3] avoid plaintext rdma offload if encryption is required Stefan Metzmacher
@ 2023-02-01 15:21 ` Stefan Metzmacher
2023-02-01 15:21 ` [PATCH v2 2/3] cifs: split out smb3_use_rdma_offload() helper Stefan Metzmacher
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Stefan Metzmacher @ 2023-02-01 15:21 UTC (permalink / raw)
To: linux-cifs
Cc: Stefan Metzmacher, Steve French, Tom Talpey, Long Li, Namjae Jeon,
David Howells, stable
This will simplify the following changes and makes it easy to get
in passed in from the caller in future.
Signed-off-by: Stefan Metzmacher <metze@samba.org>
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: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable@vger.kernel.org
---
fs/cifs/smb2pdu.c | 53 ++++++++++++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 14 deletions(-)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 2d5c3df2277d..64e2c8b438f4 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -4504,10 +4504,27 @@ smb2_async_writev(struct cifs_writedata *wdata,
struct kvec iov[1];
struct smb_rqst rqst = { };
unsigned int total_len;
+ struct cifs_io_parms _io_parms;
+ struct cifs_io_parms *io_parms = NULL;
if (!wdata->server)
server = wdata->server = cifs_pick_channel(tcon->ses);
+ /*
+ * in future we may get cifs_io_parms passed in from the caller,
+ * but for now we construct it here...
+ */
+ _io_parms = (struct cifs_io_parms) {
+ .tcon = tcon,
+ .server = server,
+ .offset = wdata->offset,
+ .length = wdata->bytes,
+ .persistent_fid = wdata->cfile->fid.persistent_fid,
+ .volatile_fid = wdata->cfile->fid.volatile_fid,
+ .pid = wdata->pid,
+ };
+ io_parms = &_io_parms;
+
rc = smb2_plain_req_init(SMB2_WRITE, tcon, server,
(void **) &req, &total_len);
if (rc)
@@ -4517,26 +4534,31 @@ smb2_async_writev(struct cifs_writedata *wdata,
flags |= CIFS_TRANSFORM_REQ;
shdr = (struct smb2_hdr *)req;
- shdr->Id.SyncId.ProcessId = cpu_to_le32(wdata->cfile->pid);
+ shdr->Id.SyncId.ProcessId = cpu_to_le32(io_parms->pid);
- req->PersistentFileId = wdata->cfile->fid.persistent_fid;
- req->VolatileFileId = wdata->cfile->fid.volatile_fid;
+ req->PersistentFileId = io_parms->persistent_fid;
+ req->VolatileFileId = io_parms->volatile_fid;
req->WriteChannelInfoOffset = 0;
req->WriteChannelInfoLength = 0;
req->Channel = 0;
- req->Offset = cpu_to_le64(wdata->offset);
+ req->Offset = cpu_to_le64(io_parms->offset);
req->DataOffset = cpu_to_le16(
offsetof(struct smb2_write_req, Buffer));
req->RemainingBytes = 0;
- trace_smb3_write_enter(0 /* xid */, wdata->cfile->fid.persistent_fid,
- tcon->tid, tcon->ses->Suid, wdata->offset, wdata->bytes);
+ trace_smb3_write_enter(0 /* xid */,
+ io_parms->persistent_fid,
+ io_parms->tcon->tid,
+ io_parms->tcon->ses->Suid,
+ io_parms->offset,
+ io_parms->length);
+
#ifdef CONFIG_CIFS_SMB_DIRECT
/*
* If we want to do a server RDMA read, fill in and append
* smbd_buffer_descriptor_v1 to the end of write request
*/
- if (server->rdma && !server->sign && wdata->bytes >=
+ if (server->rdma && !server->sign && io_parms->length >=
server->smbd_conn->rdma_readwrite_threshold) {
struct smbd_buffer_descriptor_v1 *v1;
@@ -4590,14 +4612,14 @@ smb2_async_writev(struct cifs_writedata *wdata,
}
#endif
cifs_dbg(FYI, "async write at %llu %u bytes\n",
- wdata->offset, wdata->bytes);
+ io_parms->offset, io_parms->length);
#ifdef CONFIG_CIFS_SMB_DIRECT
/* For RDMA read, I/O size is in RemainingBytes not in Length */
if (!wdata->mr)
- req->Length = cpu_to_le32(wdata->bytes);
+ req->Length = cpu_to_le32(io_parms->length);
#else
- req->Length = cpu_to_le32(wdata->bytes);
+ req->Length = cpu_to_le32(io_parms->length);
#endif
if (wdata->credits.value > 0) {
@@ -4605,7 +4627,7 @@ smb2_async_writev(struct cifs_writedata *wdata,
SMB2_MAX_BUFFER_SIZE));
shdr->CreditRequest = cpu_to_le16(le16_to_cpu(shdr->CreditCharge) + 8);
- rc = adjust_credits(server, &wdata->credits, wdata->bytes);
+ rc = adjust_credits(server, &wdata->credits, io_parms->length);
if (rc)
goto async_writev_out;
@@ -4618,9 +4640,12 @@ smb2_async_writev(struct cifs_writedata *wdata,
if (rc) {
trace_smb3_write_err(0 /* no xid */,
- req->PersistentFileId,
- tcon->tid, tcon->ses->Suid, wdata->offset,
- wdata->bytes, rc);
+ io_parms->persistent_fid,
+ io_parms->tcon->tid,
+ io_parms->tcon->ses->Suid,
+ io_parms->offset,
+ io_parms->length,
+ rc);
kref_put(&wdata->refcount, release);
cifs_stats_fail_inc(tcon, SMB2_WRITE_HE);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/3] cifs: split out smb3_use_rdma_offload() helper
2023-02-01 15:21 [PATCH v2 0/3] avoid plaintext rdma offload if encryption is required Stefan Metzmacher
2023-02-01 15:21 ` [PATCH v2 1/3] cifs: introduce cifs_io_parms in smb2_async_writev() Stefan Metzmacher
@ 2023-02-01 15:21 ` Stefan Metzmacher
2023-02-01 15:21 ` [PATCH v2 3/3] cifs: don't try to use rdma offload on encrypted connections Stefan Metzmacher
2023-02-02 0:05 ` [PATCH v2 0/3] avoid plaintext rdma offload if encryption is required Steve French
3 siblings, 0 replies; 5+ messages in thread
From: Stefan Metzmacher @ 2023-02-01 15:21 UTC (permalink / raw)
To: linux-cifs
Cc: Stefan Metzmacher, Steve French, Tom Talpey, Long Li, Namjae Jeon,
David Howells, stable
We should have the logic to decide if we want rdma offload
in a single spot in order to advance it in future.
Signed-off-by: Stefan Metzmacher <metze@samba.org>
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: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable@vger.kernel.org
---
fs/cifs/smb2pdu.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 64e2c8b438f4..6a4d621241dd 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -4063,6 +4063,32 @@ SMB2_flush(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
return rc;
}
+#ifdef CONFIG_CIFS_SMB_DIRECT
+static inline bool smb3_use_rdma_offload(struct cifs_io_parms *io_parms)
+{
+ struct TCP_Server_Info *server = io_parms->server;
+ struct cifs_tcon *tcon = io_parms->tcon;
+
+ /* we can only offload if we're connected */
+ if (!server || !tcon)
+ return false;
+
+ /* we can only offload on an rdma connection */
+ if (!server->rdma || !server->smbd_conn)
+ return false;
+
+ /* we don't support signed offload yet */
+ if (server->sign)
+ return false;
+
+ /* offload also has its overhead, so only do it if desired */
+ if (io_parms->length < server->smbd_conn->rdma_readwrite_threshold)
+ return false;
+
+ return true;
+}
+#endif /* CONFIG_CIFS_SMB_DIRECT */
+
/*
* To form a chain of read requests, any read requests after the first should
* have the end_of_chain boolean set to true.
@@ -4106,9 +4132,7 @@ smb2_new_read_req(void **buf, unsigned int *total_len,
* If we want to do a RDMA write, fill in and append
* smbd_buffer_descriptor_v1 to the end of read request
*/
- if (server->rdma && rdata && !server->sign &&
- rdata->bytes >= server->smbd_conn->rdma_readwrite_threshold) {
-
+ if (smb3_use_rdma_offload(io_parms)) {
struct smbd_buffer_descriptor_v1 *v1;
bool need_invalidate = server->dialect == SMB30_PROT_ID;
@@ -4558,9 +4582,7 @@ smb2_async_writev(struct cifs_writedata *wdata,
* If we want to do a server RDMA read, fill in and append
* smbd_buffer_descriptor_v1 to the end of write request
*/
- if (server->rdma && !server->sign && io_parms->length >=
- server->smbd_conn->rdma_readwrite_threshold) {
-
+ if (smb3_use_rdma_offload(io_parms)) {
struct smbd_buffer_descriptor_v1 *v1;
bool need_invalidate = server->dialect == SMB30_PROT_ID;
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 3/3] cifs: don't try to use rdma offload on encrypted connections
2023-02-01 15:21 [PATCH v2 0/3] avoid plaintext rdma offload if encryption is required Stefan Metzmacher
2023-02-01 15:21 ` [PATCH v2 1/3] cifs: introduce cifs_io_parms in smb2_async_writev() Stefan Metzmacher
2023-02-01 15:21 ` [PATCH v2 2/3] cifs: split out smb3_use_rdma_offload() helper Stefan Metzmacher
@ 2023-02-01 15:21 ` Stefan Metzmacher
2023-02-02 0:05 ` [PATCH v2 0/3] avoid plaintext rdma offload if encryption is required Steve French
3 siblings, 0 replies; 5+ messages in thread
From: Stefan Metzmacher @ 2023-02-01 15:21 UTC (permalink / raw)
To: linux-cifs
Cc: Stefan Metzmacher, Steve French, Tom Talpey, Long Li, Namjae Jeon,
David Howells, stable
The aim of using encryption on a connection is to keep
the data confidential, so we must not use plaintext rdma offload
for that data!
It seems that current windows servers and ksmbd would allow
this, but that's no reason to expose the users data in plaintext!
And servers hopefully reject this in future.
Note modern windows servers support signed or encrypted offload,
see MS-SMB2 2.2.3.1.6 SMB2_RDMA_TRANSFORM_CAPABILITIES, but we don't
support that yet.
Signed-off-by: Stefan Metzmacher <metze@samba.org>
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: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable@vger.kernel.org
---
fs/cifs/smb2pdu.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 6a4d621241dd..c5cb2639b3f1 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -4081,6 +4081,10 @@ static inline bool smb3_use_rdma_offload(struct cifs_io_parms *io_parms)
if (server->sign)
return false;
+ /* we don't support encrypted offload yet */
+ if (smb3_encryption_required(tcon))
+ return false;
+
/* offload also has its overhead, so only do it if desired */
if (io_parms->length < server->smbd_conn->rdma_readwrite_threshold)
return false;
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread