Mike Christie wrote:
> Dev, Vasu wrote:
>>> -----Original Message-----
>>> From: Dev, Vasu
>>> Sent: Thursday, August 28, 2008 8:52 PM
>>> To: 'Mike Christie'
>>> Cc: [email protected]
>>> Subject: RE: [Open-FCoE] [PATCH] libfc: fixed ESB_ST_ABNORMAL handling
>>> (ver-2)
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Mike Christie [mailto:[EMAIL PROTECTED]
>>>> Sent: Thursday, August 28, 2008 7:23 PM
>>>> To: Dev, Vasu
>>>> Cc: [email protected]
>>>> Subject: Re: [Open-FCoE] [PATCH] libfc: fixed ESB_ST_ABNORMAL handling
>>>> (ver-2)
>>>>
>>>> Mike Christie wrote:
>>>>> Vasu Dev wrote:
>>>>>> @@ -410,16 +449,19 @@ static void fc_exch_timeout(unsigned long
>> ep_arg)
>>>>>> if (e_stat & ESB_ST_REC_QUAL)
>>>>>> fc_exch_rrq(ep);
>>>>>> goto done;
>>>>>> - } else if (!(e_stat & ESB_ST_ABNORMAL)) {
>>>>>> - resp = ep->resp;
>>>>>> - arg = ep->resp_arg;
>>>>>> + } else if (ep->fh_type != FC_TYPE_FCP) {
>>>>> I do not think this works because srr is type FCP isn't it?. If that
>>>>> times out we probably want to want to handle it. Currently we
>> possibly
>>>>> retry the rec and then if that fails send an abort for the scsi
>> command.
>>>> I mean retry srr.
>>>>
>>>>> Not sure how handy the retries are, but at the very lest we need to
>> call
>>>>> exch_done on the srr ep, and we need to release the hold on the scsi
>>>>> command's fsp.
>>> This issue was already there before this patch, since prior to this
>> patch
>>> "} else if (!(e_stat & ESB_ST_ABNORMAL))" check would have prevented
>> any -
>>> FC_EX_TIMEOUT event to any upper layer of fc_exch.c since
>> ESB_ST_ABNORMAL
>>> is set on sending abort and it would have remained set on exch timeout
>> for
>>> no response to abort request.
>>>
>>> However you are correct that this is an issue for FCP and need to be
>> fixed
>>> when -FC_EX_TIMEOUT events are passed up to FCP and that will also
>> require
>>> adding -FC_EX_TIMEOUT handling in all exch resp handlers of FCP.
>>>
>>> So I'm updating comment work and description of this patch that this
>> patch
>>> is only fixing -FC_EX_TIMEOUT for only non FCP cases and FCP need to be
>>> fixed later in another patch. Is that okay?
>>
>> Never mind, ignore my comments since you are taking about normal
>> -FC_EX_TIMEOUT for REC and SRR.
>>
>> I'll stick to original comments of re-adding -FC_EX_TIMEOUT handling
>> with ver-1 of this patch.
>>
>
> I was thinking that for scsi commands do we need fc_exch.c to set a
> timeout for the abort? If not then we can just have fc_fcp call some
> other function that does not set the timer for the abort (or just modify
> the existing function to take an extra argument to indicate the timeout
> and if not set then fc_exch.c would not set one).
>
> The fc_fcp.c layer will always set its own abort - sort of I guess. If
> the abort is sent because the scsi command timed out at the scsi layer
> and the scsi eh is initiating it, then fc_fcp_pkt_abort will set its own
> timeout.
>
> And if the abort is send because fc_timeout_error is doing the abort,
> then the scsi command timer will eventually timeout and when the scsi eh
> runs it will see that we sent an abort and it did not complete and will
> then send a lu reset. So in one way or another fc_fcp.c will set a
> timeout for scsi_commands and does not need fc_exch.c to do it. So if
> fc_fcp_error is does not want the -FC_EX_TIMEOUT maybe we should set
> things up so it would not get one for the abort and not have fc_exch.c
> do it for the upper layer. I mean if the upper layer set a timeout it
> wants the -FC_EX_TIMEOUT. If it doers not set the timeout then it does
> not. So that way we would not need any special code in the fc_exch.c
> time out handler code to guess what the upper layer wanted.
>
> _______________________________________________
> devel mailing list
> [email protected]
> http://www.open-fcoe.org/mailman/listinfo/devel
This makes sense. If the abort was from an explicit call by the upper
layer (FCP) then don't time it out. Maybe even call the response
handler with no error but the actual response frame when the ABTS
response comes in. The recovery qualifier and RRQ should still be
handled by the exchange manager, though.
Joe
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
