decrypt large read offload patch

decrypt large read offload patch

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

I am getting EBADMSG from the call (in crypt_message in smb2ops.c) to
crypto_wait_req when trying to decrypt a 512K array of pages from an
SMB3 read in a worker thread (rather than in the usual cifsd thread
which works) - see attached patch (doesn't fail with non-offload
case).

Any obvious bug anyone spots here?  Looking at the crypto library for
CCM wasn't exactly clear to me what could be going on

-- 
Thanks,

Steve

[-- Attachment #2: read-decrypt-offload-rfc.patch --]
[-- Type: text/x-patch, Size: 6284 bytes --]

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index de90e665ef11..0bd68b0c9e36 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -109,6 +109,7 @@ extern mempool_t *cifs_req_poolp;
 extern mempool_t *cifs_mid_poolp;
 
 struct workqueue_struct	*cifsiod_wq;
+struct workqueue_struct	*decrypt_wq;
 struct workqueue_struct	*cifsoplockd_wq;
 __u32 cifs_lock_secret;
 
@@ -1499,11 +1500,19 @@ init_cifs(void)
 		goto out_clean_proc;
 	}
 
+	/* BB Consider set limit!=0 so don't launch too many worker threads */
+	decrypt_wq = alloc_workqueue("smb3decryptd",
+				     WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
+	if (!decrypt_wq) {
+		rc = -ENOMEM;
+		goto out_destroy_cifsiod_wq;
+	}
+
 	cifsoplockd_wq = alloc_workqueue("cifsoplockd",
 					 WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 	if (!cifsoplockd_wq) {
 		rc = -ENOMEM;
-		goto out_destroy_cifsiod_wq;
+		goto out_destroy_decrypt_wq;
 	}
 
 	rc = cifs_fscache_register();
@@ -1569,6 +1578,8 @@ init_cifs(void)
 	cifs_fscache_unregister();
 out_destroy_cifsoplockd_wq:
 	destroy_workqueue(cifsoplockd_wq);
+out_destroy_decrypt_wq:
+	destroy_workqueue(decrypt_wq);
 out_destroy_cifsiod_wq:
 	destroy_workqueue(cifsiod_wq);
 out_clean_proc:
@@ -1595,6 +1606,7 @@ exit_cifs(void)
 	cifs_destroy_inodecache();
 	cifs_fscache_unregister();
 	destroy_workqueue(cifsoplockd_wq);
+	destroy_workqueue(decrypt_wq);
 	destroy_workqueue(cifsiod_wq);
 	cifs_proc_clean();
 }
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 1f53dee211d8..d66106ac031a 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1892,6 +1892,7 @@ void cifs_queue_oplock_break(struct cifsFileInfo *cfile);
 
 extern const struct slow_work_ops cifs_oplock_break_ops;
 extern struct workqueue_struct *cifsiod_wq;
+extern struct workqueue_struct *decrypt_wq;
 extern struct workqueue_struct *cifsoplockd_wq;
 extern __u32 cifs_lock_secret;
 
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 070d0b7b21dc..4f985f517bbb 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3683,6 +3683,8 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 
 	if (!rc && enc)
 		memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
+	else
+		cifs_dbg(VFS, "crypto_wait_req returned %d with enc %d\n", rc, enc); /* BB REMOVEME */
 
 	kfree(iv);
 free_sg:
@@ -3814,7 +3816,7 @@ decrypt_raw_data(struct TCP_Server_Info *server, char *buf,
 	rqst.rq_tailsz = (page_data_size % PAGE_SIZE) ? : PAGE_SIZE;
 
 	rc = crypt_message(server, 1, &rqst, 0);
-	cifs_dbg(FYI, "Decrypt message returned %d\n", rc);
+	cifs_dbg(VFS, "Decrypt message returned %d with pages %p npages %d tailsz %d\n", rc, pages, npages, rqst.rq_tailsz); /* BB REMOVEME */
 
 	if (rc)
 		return rc;
@@ -4017,8 +4019,65 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid,
 	return length;
 }
 
+struct smb2_decrypt_work {
+	struct work_struct decrypt;
+	struct TCP_Server_Info *server;
+	struct page **ppages;
+	char *buf;
+	unsigned int npages;
+	unsigned int len;
+};
+
+
+static void smb2_decrypt_offload(struct work_struct *work)
+{
+	struct smb2_decrypt_work *dw = container_of(work,
+				struct smb2_decrypt_work, decrypt);
+	int i, rc;
+	struct mid_q_entry *mid;
+
+	rc = decrypt_raw_data(dw->server, dw->buf, dw->server->vals->read_rsp_size,
+			      dw->ppages, dw->npages, dw->len);
+	if (rc) {
+		cifs_dbg(VFS, "error decrypting rc=%d\n", rc);
+		goto free_pages;
+	}
+
+	mid = smb2_find_mid(dw->server, dw->buf);
+	if (mid == NULL)
+		cifs_dbg(VFS, "mid not found\n");
+	else {
+		cifs_dbg(VFS, "mid found %lld\n", mid->mid); /* BB removeme */
+		mid->decrypted = true;
+		rc = handle_read_data(dw->server, mid, dw->buf,
+				      dw->server->vals->read_rsp_size,
+				      dw->ppages, dw->npages, dw->len);
+	}
+
+	dw->server->lstrp = jiffies;
+
+	mid->callback(mid);
+
+	cifs_mid_q_entry_release(mid);
+
+free_pages:
+	/* BB TODO double check that we are freeing the right number of pages */
+	for (i = dw->npages; i >= 0; i--) {
+		cifs_dbg(VFS, "free page %d %p\n", i, dw->ppages[i-1]);
+		put_page(dw->ppages[i-1]);
+		msleep(10);
+	}
+	kfree(dw->buf);
+/* FIXME */
+/* discard_data:
+	cifs_discard_remaining_data(server);
+	goto free_pages; */
+}
+
+
 static int
-receive_encrypted_read(struct TCP_Server_Info *server, struct mid_q_entry **mid)
+receive_encrypted_read(struct TCP_Server_Info *server, struct mid_q_entry **mid,
+		       int *num_mids)
 {
 	char *buf = server->smallbuf;
 	struct smb2_transform_hdr *tr_hdr = (struct smb2_transform_hdr *)buf;
@@ -4028,7 +4087,9 @@ receive_encrypted_read(struct TCP_Server_Info *server, struct mid_q_entry **mid)
 	unsigned int buflen = server->pdu_size;
 	int rc;
 	int i = 0;
+	struct smb2_decrypt_work *dw;
 
+	*num_mids = 1;
 	len = min_t(unsigned int, buflen, server->vals->read_rsp_size +
 		sizeof(struct smb2_transform_hdr)) - HEADER_SIZE(server) + 1;
 
@@ -4064,6 +4125,33 @@ receive_encrypted_read(struct TCP_Server_Info *server, struct mid_q_entry **mid)
 	if (rc)
 		goto free_pages;
 
+	/*
+	 * For large reads, offload to different thread for better performance,
+	 * use more cores decrypting which can be expensive
+	 */
+
+	if (server->pdu_size > (512 * 1024)) {
+		dw = kmalloc(sizeof(struct smb2_decrypt_work), GFP_KERNEL);
+		if (dw == NULL)
+			goto non_offloaded_decrypt;
+		dw->buf = kmalloc(sizeof(struct smb2_transform_hdr), GFP_KERNEL);
+		if (dw->buf == NULL) {
+			kfree(dw);
+			goto non_offloaded_decrypt;
+		}
+		memcpy(dw->buf, buf, sizeof(struct smb2_transform_hdr));
+		INIT_WORK(&dw->decrypt, smb2_decrypt_offload);
+
+		dw->npages = npages;
+		dw->server = server;
+		dw->ppages = pages;
+		dw->len = len;
+		queue_work(cifsiod_wq, &dw->decrypt);
+		*num_mids = 0; /* worker thread takes care of finding mid */
+		return -1;
+	}
+
+non_offloaded_decrypt:
 	rc = decrypt_raw_data(server, buf, server->vals->read_rsp_size,
 			      pages, npages, len);
 	if (rc)
@@ -4210,8 +4298,7 @@ smb3_receive_transform(struct TCP_Server_Info *server,
 
 	/* TODO: add support for compounds containing READ. */
 	if (pdu_length > CIFSMaxBufSize + MAX_HEADER_SIZE(server)) {
-		*num_mids = 1;
-		return receive_encrypted_read(server, &mids[0]);
+		return receive_encrypted_read(server, &mids[0], num_mids);
 	}
 
 	return receive_encrypted_standard(server, mids, bufs, num_mids);

Re: decrypt large read offload patch

[Pavel Shilovsky]

чт, 5 сент. 2019 г. в 16:24, Steve French <smfrench@gmail.com>:
>
> I am getting EBADMSG from the call (in crypt_message in smb2ops.c) to
> crypto_wait_req when trying to decrypt a 512K array of pages from an
> SMB3 read in a worker thread (rather than in the usual cifsd thread
> which works) - see attached patch (doesn't fail with non-offload
> case).
>
> Any obvious bug anyone spots here?  Looking at the crypto library for
> CCM wasn't exactly clear to me what could be going on
>

+ if (server->pdu_size > (512 * 1024)) {
+ dw = kmalloc(sizeof(struct smb2_decrypt_work), GFP_KERNEL);
+ if (dw == NULL)
+ goto non_offloaded_decrypt;
+ dw->buf = kmalloc(sizeof(struct smb2_transform_hdr), GFP_KERNEL);
+ if (dw->buf == NULL) {
+ kfree(dw);
+ goto non_offloaded_decrypt;
+ }
+ memcpy(dw->buf, buf, sizeof(struct smb2_transform_hdr));

^^^
here buf contains transform header + read rsp -- see
decrypt_raw_data() function for details. Should be sizeof(struct
smb2_transform_hdr) + server->vals->read_rsp_size.

--
Best regards,
Pavel Shilovsky