Re: [PATCH] ibmvscsis: Do not send aborted task response

Michael Cyr
2017-04-07T21:14:30Z

On 4/7/17 12:01 PM, Bryant G. Ly wrote:
>
>
> On 4/7/17 11:36 AM, Bart Van Assche wrote:
>> On Fri, 2017-04-07 at 11:12 -0500, Bryant G. Ly wrote:
>>> So from this stack trace it looks like the ibmvscsis driver is
>>> sending an
>>> extra response through send_messages even though an abort was issued.
>>> IBMi, reported this issue internally when they were testing the driver,
>>> because their client didn't like getting a response back for the
>>> aborted op.
>>> They are only expecting a TMR from the abort request, NOT the
>>> aborted op.
>> Hello Bryant,
>>
>> What is the root cause of this behavior? Why is it that the behavior of
>> the ibmvscsi_tgt contradicts what has been implemented in the LIO core?
>> Sorry but the patch at the start of this thread looks to me like an
>> attempt to paper over the problem instead of addressing the root cause.
>>
>> Thanks,
>>
> IBMi clients received a response for an aborted operation, so they
> sent an abort tm request.
> Afterwards they get a CRQ response to the op that they aborted. That
> should not happen, because they are only supposed to get a response
> for the tm request NOT the aborted operation.
> Looking at the code it looks like it is due to send messages,
> processing a response without checking to see if it was an aborted op.
> This patch addresses a bug within the ibmvscsis driver and the fact
> that it SENT a response to the aborted operation(which is wrong since)
> without looking at what LIO core had done.
> The driver isn't supposed to send any response to the aborted
> operation, BUT only a response to the abort tm request, which LIO core
> currently does.
>
> -Bryant
>
I think I can clarify the issue here: ibmvscsis_tgt does not send the
response to the client until release_cmd time. The reason for this was
because if we did it at queue_status time, then the client would be free
to reuse the tag for that command, but we're still using the tag until
the command is released at release_cmd time, so we chose to delay
sending the response until then.

That then caused this issue, because release_cmd is always called, even
if queue_status is not. Perhaps it would be cleaner to set some sort of
status valid flag during queue_status instead of setting a flag in
aborted_task?