diff mbox series

[06/36] target: Return Function Complete

Message ID d25dbd79f4234f5b0d7574cc1ba02e42e3537d81.1657149962.git.Thinh.Nguyen@synopsys.com (mailing list archive)
State Superseded
Headers show
Series usb: gadget: f_tcm: Enhance UASP driver | expand

Commit Message

Thinh Nguyen July 6, 2022, 11:35 p.m. UTC
According to SAM-4 r14 section 7.2, for ABORT TASK function, a response
of FUNCTION COMPLETE shall indicate that the command was aborted or was
not in the task set. Currently we respond with TASK DOES NOT EXIST when
there's no command in the task set. Fix the response to FUNCTION
COMPLETE instead.

Fixes: 3d28934aaae5 ("target: Add TMR_ABORT_TASK task management support")
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
 drivers/target/target_core_tmr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dmitry Bogdanov July 7, 2022, 1:34 p.m. UTC | #1
Hi Thinh,

On Wed, Jul 06, 2022 at 04:35:01PM -0700, Thinh Nguyen wrote:
> According to SAM-4 r14 section 7.2, for ABORT TASK function, a response
> of FUNCTION COMPLETE shall indicate that the command was aborted or was
> not in the task set. Currently we respond with TASK DOES NOT EXIST when
> there's no command in the task set. Fix the response to FUNCTION
> COMPLETE instead.
SAM does not describe a response status encoding. But other specs
do describe. For example, iSCSI RFC7143 11.6.1.  Response
       0 - Function complete
       1 - Task does not exist
   The mapping of the response code into a SCSI service response code
   value, if needed, is outside the scope of this document.  However, in
   symbolic terms, Response values 0 and 1 map to the SCSI service
   response of FUNCTION COMPLETE. 

So, the current code is according to specs.
Moreover, TMR_TASK_DOES_NOT_EXIST is used in several fabric drivers to
handle some corner cases.
> 
> Fixes: 3d28934aaae5 ("target: Add TMR_ABORT_TASK task management support")
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
>  drivers/target/target_core_tmr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
> index 2af80d0998bf..724ddabda488 100644
> --- a/drivers/target/target_core_tmr.c
> +++ b/drivers/target/target_core_tmr.c
> @@ -170,9 +170,9 @@ void core_tmr_abort_task(
>  	if (dev->transport->tmr_notify)
>  		dev->transport->tmr_notify(dev, TMR_ABORT_TASK, &aborted_list);
>  
> -	printk("ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST for ref_tag: %lld\n",
> +	printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for ref_tag: %lld\n",
>  			tmr->ref_task_tag);
> -	tmr->response = TMR_TASK_DOES_NOT_EXIST;
> +	tmr->response = TMR_FUNCTION_COMPLETE;
>  	atomic_long_inc(&dev->aborts_no_task);
>  }
>
Thinh Nguyen July 8, 2022, 11:51 p.m. UTC | #2
On 7/7/2022, Dmitry Bogdanov wrote:
> Hi Thinh,
>
> On Wed, Jul 06, 2022 at 04:35:01PM -0700, Thinh Nguyen wrote:
>> According to SAM-4 r14 section 7.2, for ABORT TASK function, a response
>> of FUNCTION COMPLETE shall indicate that the command was aborted or was
>> not in the task set. Currently we respond with TASK DOES NOT EXIST when
>> there's no command in the task set. Fix the response to FUNCTION
>> COMPLETE instead.
> SAM does not describe a response status encoding. But other specs
> do describe. For example, iSCSI RFC7143 11.6.1.  Response
>         0 - Function complete
>         1 - Task does not exist
>     The mapping of the response code into a SCSI service response code
>     value, if needed, is outside the scope of this document.  However, in
>     symbolic terms, Response values 0 and 1 map to the SCSI service
>     response of FUNCTION COMPLETE.
>
> So, the current code is according to specs.
> Moreover, TMR_TASK_DOES_NOT_EXIST is used in several fabric drivers to
> handle some corner cases.

Ok, it's a bit confusing. So, the SCSI service response needs to reflect 
FUNCTION COMPLETE, but the FUNCTION COMPLETE can be different for other 
specs?

Thanks,
Thinh

>> Fixes: 3d28934aaae5 ("target: Add TMR_ABORT_TASK task management support")
>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>> ---
>>   drivers/target/target_core_tmr.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
>> index 2af80d0998bf..724ddabda488 100644
>> --- a/drivers/target/target_core_tmr.c
>> +++ b/drivers/target/target_core_tmr.c
>> @@ -170,9 +170,9 @@ void core_tmr_abort_task(
>>   	if (dev->transport->tmr_notify)
>>   		dev->transport->tmr_notify(dev, TMR_ABORT_TASK, &aborted_list);
>>   
>> -	printk("ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST for ref_tag: %lld\n",
>> +	printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for ref_tag: %lld\n",
>>   			tmr->ref_task_tag);
>> -	tmr->response = TMR_TASK_DOES_NOT_EXIST;
>> +	tmr->response = TMR_FUNCTION_COMPLETE;
>>   	atomic_long_inc(&dev->aborts_no_task);
>>   }
>>
Dmitry Bogdanov July 11, 2022, 9:53 a.m. UTC | #3
On Fri, Jul 08, 2022 at 11:51:38PM +0000, Thinh Nguyen wrote:
> 
> On 7/7/2022, Dmitry Bogdanov wrote:
> > Hi Thinh,
> >
> > On Wed, Jul 06, 2022 at 04:35:01PM -0700, Thinh Nguyen wrote:
> >> According to SAM-4 r14 section 7.2, for ABORT TASK function, a response
> >> of FUNCTION COMPLETE shall indicate that the command was aborted or was
> >> not in the task set. Currently we respond with TASK DOES NOT EXIST when
> >> there's no command in the task set. Fix the response to FUNCTION
> >> COMPLETE instead.
> > SAM does not describe a response status encoding. But other specs
> > do describe. For example, iSCSI RFC7143 11.6.1.  Response
> >         0 - Function complete
> >         1 - Task does not exist
> >     The mapping of the response code into a SCSI service response code
> >     value, if needed, is outside the scope of this document.  However, in
> >     symbolic terms, Response values 0 and 1 map to the SCSI service
> >     response of FUNCTION COMPLETE.
> >
> > So, the current code is according to specs.
> > Moreover, TMR_TASK_DOES_NOT_EXIST is used in several fabric drivers to
> > handle some corner cases.
> 
> Ok, it's a bit confusing. So, the SCSI service response needs to reflect
> FUNCTION COMPLETE, but the FUNCTION COMPLETE can be different for other
> specs?
Yes, SCSI service response code may have a different values to differentiate
a level of "success" :) or a reason of failure.
> 
> Thanks,
> Thinh
> 
> >> Fixes: 3d28934aaae5 ("target: Add TMR_ABORT_TASK task management support")
> >> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> >> ---
> >>   drivers/target/target_core_tmr.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
> >> index 2af80d0998bf..724ddabda488 100644
> >> --- a/drivers/target/target_core_tmr.c
> >> +++ b/drivers/target/target_core_tmr.c
> >> @@ -170,9 +170,9 @@ void core_tmr_abort_task(
> >>      if (dev->transport->tmr_notify)
> >>              dev->transport->tmr_notify(dev, TMR_ABORT_TASK, &aborted_list);
> >>
> >> -    printk("ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST for ref_tag: %lld\n",
> >> +    printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for ref_tag: %lld\n",
> >>                      tmr->ref_task_tag);
> >> -    tmr->response = TMR_TASK_DOES_NOT_EXIST;
> >> +    tmr->response = TMR_FUNCTION_COMPLETE;
> >>      atomic_long_inc(&dev->aborts_no_task);
> >>   }
> >>
>
diff mbox series

Patch

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 2af80d0998bf..724ddabda488 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -170,9 +170,9 @@  void core_tmr_abort_task(
 	if (dev->transport->tmr_notify)
 		dev->transport->tmr_notify(dev, TMR_ABORT_TASK, &aborted_list);
 
-	printk("ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST for ref_tag: %lld\n",
+	printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for ref_tag: %lld\n",
 			tmr->ref_task_tag);
-	tmr->response = TMR_TASK_DOES_NOT_EXIST;
+	tmr->response = TMR_FUNCTION_COMPLETE;
 	atomic_long_inc(&dev->aborts_no_task);
 }