Re: nvme tcp receive errors

[Sagi Grimberg <sagi@grimberg.me>]

>> What should I put on the Reported-by: and Tested-by: tags?
> 
> This report and testing was done courtesy of
> 
>    Narayan Ayalasomayajula <Narayan.Ayalasomayajula@wdc.com>
> 
> Before you submit a patch, though, we did additional testing with data
> digest enabled and observe a regression with the following error:
> 
>    nvme nvme0: queue 0: data digest flag is cleared
> 
>  From looking at the patch, the following part looks a bit suspicious:
> 
>> @@ -776,19 +776,20 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb,
>> 		req->data_recvd += recv_len;
>> 	}
>>
>> -	if (!queue->data_remaining) {
>> +	if (!queue->data_remaining)
>> +		nvme_tcp_init_recv_ctx(queue);
> 
> The code had previously called nvme_tcp_init_recv_ctx() only if
> queue->data_digest wasn't set, but now it's called all the time. I see
> that calling this function clears ddgst_remaining, so does that explain
> the new errors?
> 
>> +	if (req->data_recvd == req->data_len) {
>>   		if (queue->data_digest) {
>>   			nvme_tcp_ddgst_final(queue->rcv_hash, &queue->exp_ddgst);
>>   			queue->ddgst_remaining = NVME_TCP_DIGEST_LENGTH;
>>   		} else {
>> -			BUG_ON(req->data_recvd != req->data_len);
>>   			req->cmd_state = NVME_TCP_CMD_DATA_DONE;
>>   			if (pdu->hdr.flags & NVME_TCP_F_DATA_SUCCESS) {
>>   				req->cmd_state = NVME_TCP_CMD_DONE;
>>   				nvme_tcp_end_request(rq, NVME_SC_SUCCESS);
>>   				queue->nr_cqe++;
>>   			}
>> -			nvme_tcp_init_recv_ctx(queue);
>>   		}
>>   	}

You are exactly right, I think this should do the trick:
--
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 8e55d8bc0c50..eb1feaacd11a 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -55,6 +55,7 @@ struct nvme_tcp_request {
         /* send state */
         size_t                  offset;
         size_t                  data_sent;
+       size_t                  data_received;
         enum nvme_tcp_send_state state;
  };

@@ -751,9 +752,12 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue 
*queue, struct sk_buff *skb,
                 *len -= recv_len;
                 *offset += recv_len;
                 queue->data_remaining -= recv_len;
+               req->data_received += recv_len;
         }

         if (!queue->data_remaining) {
+               if (req->data_received < req->data_len)
+                       return 0;
                 if (queue->data_digest) {
                         nvme_tcp_ddgst_final(queue->rcv_hash, 
&queue->exp_ddgst);
                         queue->ddgst_remaining = NVME_TCP_DIGEST_LENGTH;
@@ -2274,6 +2278,7 @@ static blk_status_t nvme_tcp_setup_cmd_pdu(struct 
nvme_ns *ns,
         req->state = NVME_TCP_SEND_CMD_PDU;
         req->offset = 0;
         req->data_sent = 0;
+       req->data_received = 0;
         req->pdu_len = 0;
         req->pdu_sent = 0;
         req->data_len = blk_rq_nr_phys_segments(rq) ?
--

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme