Re: [PATCH] nvme-tcp: fix incorrect h2cdata pdu offset accounting

[Sagi Grimberg <sagi@grimberg.me>]

>>> Unfortunately this was unsuccessful. The same issue is still occuring. Please
>>> let me know if you have another patch to try. I'll also keep looking for a
>>> solution as well.
>>
>> Really? That's unexpected, this patch should ensure that the request
>> is not advanced after the last payload send. The req->data_sent and
>> req->data_len are recorded before we actually perform the send so
>> the request should not be advanced if (e.g. last send):
>> 	(req_data_sent + ret == req_data_len)
>>
>> So I'm surprised that the same issue is still occurring.
> 
> Only thing I thought of so far is if req_data_len is not aligned with
> nvme_tcp_req_cur_length(). When this was working, the request would not
> advance based on the nvme_tcp_req_cur_length() value; now the criteria
> is based on 'req->data_len'.

That is exactly the bug fix that broke this. Before a multi-pdu request
would incorrectly avoid advancing the iterator.
nvme_tcp_req_cur_length() is the length we need to send based
on the pdu, not the full request. In that case, we shouldn't have
an issue advancing the request.

My recollection was that the original issue you guys reported was
on the request last payload send, where the controller sent a completion
and the request was re-executed as a new request before the send
context advanced the request, which led to a use-after-free. I suspected
that this specific issue would have been addressed by this patch.

Think of a 8K write where the target sends 0-4k r2t and then 4k-8k
r2t.

The host needs to send 0-4k, on the second r2t reception send 4k-8k but
due to the fact that a completion may arrive, avoid advancing the
request (like the original reported issue).

I think we can also solve this a different way with refcounting the
request, but if the suggested patch doesn't resolve the issue then
we might still be missing something I don't yet understand.

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