diff mbox series

[v5,18/20] ksmdb: make smb2_get_ksmbd_tcon() callable with chained PDUs

Message ID 20211001120421.327245-19-slow@samba.org
State New
Headers show
Series Buffer validation patches | expand

Commit Message

Ralph Boehme Oct. 1, 2021, 12:04 p.m. UTC
Also track the tcon id of compound requests.

Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
---
 fs/ksmbd/ksmbd_work.h | 1 +
 fs/ksmbd/smb2pdu.c    | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

Namjae Jeon Oct. 2, 2021, 6 a.m. UTC | #1
2021-10-01 21:04 GMT+09:00, Ralph Boehme <slow@samba.org>:
> Also track the tcon id of compound requests.
>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Steve French <smfrench@gmail.com>
> Cc: Hyunchul Lee <hyc.lee@gmail.com>
> Signed-off-by: Ralph Boehme <slow@samba.org>
> ---
>  fs/ksmbd/ksmbd_work.h | 1 +
>  fs/ksmbd/smb2pdu.c    | 9 ++++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ksmbd/ksmbd_work.h b/fs/ksmbd/ksmbd_work.h
> index f7156bc50049..91363d508909 100644
> --- a/fs/ksmbd/ksmbd_work.h
> +++ b/fs/ksmbd/ksmbd_work.h
> @@ -46,6 +46,7 @@ struct ksmbd_work {
>  	u64				compound_fid;
>  	u64				compound_pfid;
>  	u64				compound_sid;
> +	u32				compound_tid;
>
>  	const struct cred		*saved_cred;
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 7d3344b5519c..5b1fead05c49 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -98,6 +98,8 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
>  	int tree_id;
>
>  	work->tcon = NULL;
> +	work->compound_tid = 0;
> +
>  	if (cmd == SMB2_TREE_CONNECT_HE ||
>  	    cmd ==  SMB2_CANCEL_HE ||
>  	    cmd ==  SMB2_LOGOFF_HE) {
> @@ -110,13 +112,18 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
>  		return -ENOENT;
>  	}
>
> -	tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
> +	if (req_hdr->Flags & SMB2_FLAGS_RELATED_OPERATIONS)
> +		tree_id = work->compound_tid;
Well, Isn't that a performance degradation due to unneeded lookups?
> +	else
> +		tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
> +
>  	work->tcon = ksmbd_tree_conn_lookup(work->sess, tree_id);
>  	if (!work->tcon) {
>  		pr_err("Invalid tid %d\n", tree_id);
>  		return -EINVAL;
>  	}
>
> +	work->compound_tid = tree_id;
>  	return 1;
>  }
>
> --
> 2.31.1
>
>
Ralph Boehme Oct. 2, 2021, 12:08 p.m. UTC | #2
Am 02.10.21 um 08:00 schrieb Namjae Jeon:
> 2021-10-01 21:04 GMT+09:00, Ralph Boehme <slow@samba.org>:
>> Also track the tcon id of compound requests.
>>
>> Cc: Namjae Jeon <linkinjeon@kernel.org>
>> Cc: Tom Talpey <tom@talpey.com>
>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>> Cc: Steve French <smfrench@gmail.com>
>> Cc: Hyunchul Lee <hyc.lee@gmail.com>
>> Signed-off-by: Ralph Boehme <slow@samba.org>
>> ---
>>   fs/ksmbd/ksmbd_work.h | 1 +
>>   fs/ksmbd/smb2pdu.c    | 9 ++++++++-
>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ksmbd/ksmbd_work.h b/fs/ksmbd/ksmbd_work.h
>> index f7156bc50049..91363d508909 100644
>> --- a/fs/ksmbd/ksmbd_work.h
>> +++ b/fs/ksmbd/ksmbd_work.h
>> @@ -46,6 +46,7 @@ struct ksmbd_work {
>>   	u64				compound_fid;
>>   	u64				compound_pfid;
>>   	u64				compound_sid;
>> +	u32				compound_tid;
>>
>>   	const struct cred		*saved_cred;
>>
>> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
>> index 7d3344b5519c..5b1fead05c49 100644
>> --- a/fs/ksmbd/smb2pdu.c
>> +++ b/fs/ksmbd/smb2pdu.c
>> @@ -98,6 +98,8 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
>>   	int tree_id;
>>
>>   	work->tcon = NULL;
>> +	work->compound_tid = 0;
>> +
>>   	if (cmd == SMB2_TREE_CONNECT_HE ||
>>   	    cmd ==  SMB2_CANCEL_HE ||
>>   	    cmd ==  SMB2_LOGOFF_HE) {
>> @@ -110,13 +112,18 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
>>   		return -ENOENT;
>>   	}
>>
>> -	tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
>> +	if (req_hdr->Flags & SMB2_FLAGS_RELATED_OPERATIONS)
>> +		tree_id = work->compound_tid;
> Well, Isn't that a performance degradation due to unneeded lookups?

I wouldn't think that this has any noticable performance impact, has it? 
This way the logic is easy to follow, but I'll rewrite to avoid the 
lookup for related operations.

-slow
diff mbox series

Patch

diff --git a/fs/ksmbd/ksmbd_work.h b/fs/ksmbd/ksmbd_work.h
index f7156bc50049..91363d508909 100644
--- a/fs/ksmbd/ksmbd_work.h
+++ b/fs/ksmbd/ksmbd_work.h
@@ -46,6 +46,7 @@  struct ksmbd_work {
 	u64				compound_fid;
 	u64				compound_pfid;
 	u64				compound_sid;
+	u32				compound_tid;
 
 	const struct cred		*saved_cred;
 
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 7d3344b5519c..5b1fead05c49 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -98,6 +98,8 @@  int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
 	int tree_id;
 
 	work->tcon = NULL;
+	work->compound_tid = 0;
+
 	if (cmd == SMB2_TREE_CONNECT_HE ||
 	    cmd ==  SMB2_CANCEL_HE ||
 	    cmd ==  SMB2_LOGOFF_HE) {
@@ -110,13 +112,18 @@  int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
 		return -ENOENT;
 	}
 
-	tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
+	if (req_hdr->Flags & SMB2_FLAGS_RELATED_OPERATIONS)
+		tree_id = work->compound_tid;
+	else
+		tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
+
 	work->tcon = ksmbd_tree_conn_lookup(work->sess, tree_id);
 	if (!work->tcon) {
 		pr_err("Invalid tid %d\n", tree_id);
 		return -EINVAL;
 	}
 
+	work->compound_tid = tree_id;
 	return 1;
 }