decrypt large read offload patch

Commit Message

Steve French Sept. 5, 2019, 8:22 p.m. UTC

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

Comments

Pavel Shilovsky Sept. 6, 2019, 6:23 p.m. UTC | #1

чт, 5 сент. 2019 г. в 16:24, Steve French <[e-mail]>:
>
> 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

Patch

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