diff mbox series

[07/17] nvme-tcp: allocate socket file

Message ID 20230810150630.134991-8-hare@suse.de (mailing list archive)
State Superseded
Headers show
Series nvme: In-kernel TLS support for TCP | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch, async

Commit Message

Hannes Reinecke Aug. 10, 2023, 3:06 p.m. UTC 
When using the TLS upcall we need to allocate a socket file such
that the userspace daemon is able to use the socket.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/host/tcp.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Simon Horman Aug. 11, 2023, 10:19 a.m. UTC | #1 
On Thu, Aug 10, 2023 at 05:06:20PM +0200, Hannes Reinecke wrote:
> When using the TLS upcall we need to allocate a socket file such
> that the userspace daemon is able to use the socket.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

...

> @@ -1512,6 +1514,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid)
>  	struct nvme_tcp_ctrl *ctrl = to_tcp_ctrl(nctrl);
>  	struct nvme_tcp_queue *queue = &ctrl->queues[qid];
>  	int ret, rcv_pdu_size;
> +	struct file *sock_file;
>  
>  	mutex_init(&queue->queue_lock);
>  	queue->ctrl = ctrl;
> @@ -1534,6 +1537,13 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid)
>  		goto err_destroy_mutex;
>  	}
>  
> +	sock_file = sock_alloc_file(queue->sock, O_CLOEXEC, NULL);
> +	if (IS_ERR(sock_file)) {
> +		sock_release(queue->sock);

Hi Hannes,

Is it correct to call sock_release() here?
sock_alloc_file() already does so on error.

Flagged by Smatch.

> +		queue->sock = NULL;
> +		ret = PTR_ERR(sock_file);
> +		goto err_destroy_mutex;
> +	}
>  	nvme_tcp_reclassify_socket(queue->sock);
>  
>  	/* Single syn retry */

...
Hannes Reinecke Aug. 11, 2023, 10:32 a.m. UTC | #2 
On 8/11/23 12:19, Simon Horman wrote:
> On Thu, Aug 10, 2023 at 05:06:20PM +0200, Hannes Reinecke wrote:
>> When using the TLS upcall we need to allocate a socket file such
>> that the userspace daemon is able to use the socket.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> 
> ...
> 
>> @@ -1512,6 +1514,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid)
>>   	struct nvme_tcp_ctrl *ctrl = to_tcp_ctrl(nctrl);
>>   	struct nvme_tcp_queue *queue = &ctrl->queues[qid];
>>   	int ret, rcv_pdu_size;
>> +	struct file *sock_file;
>>   
>>   	mutex_init(&queue->queue_lock);
>>   	queue->ctrl = ctrl;
>> @@ -1534,6 +1537,13 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid)
>>   		goto err_destroy_mutex;
>>   	}
>>   
>> +	sock_file = sock_alloc_file(queue->sock, O_CLOEXEC, NULL);
>> +	if (IS_ERR(sock_file)) {
>> +		sock_release(queue->sock);
> 
> Hi Hannes,
> 
> Is it correct to call sock_release() here?
> sock_alloc_file() already does so on error.
> 
> Flagged by Smatch.
> 
Ah, no, you are correct. Will be fixing it up.

Cheers,

Hannes
diff mbox series

Patch

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 9ce417cd32a7..c05037c29da0 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1338,7 +1338,9 @@  static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
 	}
 
 	noreclaim_flag = memalloc_noreclaim_save();
-	sock_release(queue->sock);
+	/* ->sock will be released by fput() */
+	fput(queue->sock->file);
+	queue->sock = NULL;
 	memalloc_noreclaim_restore(noreclaim_flag);
 
 	kfree(queue->pdu);
@@ -1512,6 +1514,7 @@  static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid)
 	struct nvme_tcp_ctrl *ctrl = to_tcp_ctrl(nctrl);
 	struct nvme_tcp_queue *queue = &ctrl->queues[qid];
 	int ret, rcv_pdu_size;
+	struct file *sock_file;
 
 	mutex_init(&queue->queue_lock);
 	queue->ctrl = ctrl;
@@ -1534,6 +1537,13 @@  static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid)
 		goto err_destroy_mutex;
 	}
 
+	sock_file = sock_alloc_file(queue->sock, O_CLOEXEC, NULL);
+	if (IS_ERR(sock_file)) {
+		sock_release(queue->sock);
+		queue->sock = NULL;
+		ret = PTR_ERR(sock_file);
+		goto err_destroy_mutex;
+	}
 	nvme_tcp_reclassify_socket(queue->sock);
 
 	/* Single syn retry */
@@ -1640,7 +1650,8 @@  static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid)
 	if (queue->hdr_digest || queue->data_digest)
 		nvme_tcp_free_crypto(queue);
 err_sock:
-	sock_release(queue->sock);
+	/* ->sock will be released by fput() */
+	fput(queue->sock->file);
 	queue->sock = NULL;
 err_destroy_mutex:
 	mutex_destroy(&queue->send_mutex);