[PATCH v2] scsi: libiscsi: Set expecting_cc_ua flag when stop_conn

[PATCH v2] scsi: libiscsi: Set expecting_cc_ua flag when stop_conn

[Xiang Zhang]

Initiator need to recover session and reconnect to target, after calling stop_conn. And target will rebuild new session info, and mark ASC_POWERON_RESET ua sense for scsi devices belong to the target(device reset). After recovery, first scsi command(scmd) request to target will get ASC_POWERON_RESET(ua sense) + SAM_STAT_CHECK_CONDITION(status) in response.
For command's response coming, according to scsi function calling: "scsi_done --> scsi_complete --> scsi_decide_disposition --> scsi_check_sense", if expecting_cc_ua = 0, scmd response with ASC_POWERON_RESET(ua sense) will make scsi_complete ignore "cmd->retries <= cmd->allowed", fail directly. It will cause SCSI return io_error to upper layer without retry.
If we set expecting_cc_ua=1 in fail_scsi_tasks, scsi_complete will retry scmd which is response with ASC_POWERON_RESET. The scmd second request to target can successful, because target will clear ASC_POWERON_RESET in device pending ua_sense_list after first scmd request.

Signed-off-by: Xiang Zhang <hawkxiang.cpp@gmail.com>
---
V1 -> V2: Fix build variable 'sc' is uninitialized warning(Reported-by: kernel test robot <lkp@intel.com>).
---
 drivers/scsi/libiscsi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 0fda8905eabd..f6bfe0c4f8a4 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -621,6 +621,7 @@ static void __fail_scsi_task(struct iscsi_task *task, int err)
 	if (cleanup_queued_task(task))
 		return;
 
+	sc = task->sc;
 	if (task->state == ISCSI_TASK_PENDING) {
 		/*
 		 * cmd never made it to the xmit thread, so we should not count
@@ -629,12 +630,12 @@ static void __fail_scsi_task(struct iscsi_task *task, int err)
 		conn->session->queued_cmdsn--;
 		/* it was never sent so just complete like normal */
 		state = ISCSI_TASK_COMPLETED;
-	} else if (err == DID_TRANSPORT_DISRUPTED)
+	} else if (err == DID_TRANSPORT_DISRUPTED) {
 		state = ISCSI_TASK_ABRT_SESS_RECOV;
-	else
+		sc->device->expecting_cc_ua = 1;
+	} else
 		state = ISCSI_TASK_ABRT_TMF;
 
-	sc = task->sc;
 	sc->result = err << 16;
 	scsi_set_resid(sc, scsi_bufflen(sc));
 	iscsi_complete_task(task, state);
-- 
2.44.0

Re: [PATCH v2] scsi: libiscsi: Set expecting_cc_ua flag when stop_conn

[Mike Christie]

On 10/14/24 12:36 AM, Xiang Zhang wrote:
> Initiator need to recover session and reconnect to target, after calling stop_conn. And target will rebuild new session info, and mark ASC_POWERON_RESET ua sense for scsi devices belong to the target(device reset). After recovery, first scsi command(scmd) request to target will get ASC_POWERON_RESET(ua sense) + SAM_STAT_CHECK_CONDITION(status) in response.
> For command's response coming, according to scsi function calling: "scsi_done --> scsi_complete --> scsi_decide_disposition --> scsi_check_sense", if expecting_cc_ua = 0, scmd response with ASC_POWERON_RESET(ua sense) will make scsi_complete ignore "cmd->retries <= cmd->allowed", fail directly. It will cause SCSI return io_error to upper layer without retry.
> If we set expecting_cc_ua=1 in fail_scsi_tasks, scsi_complete will retry scmd which is response with ASC_POWERON_RESET. The scmd second request to target can successful, because target will clear ASC_POWERON_RESET in device pending ua_sense_list after first scmd request.
> 
> Signed-off-by: Xiang Zhang <hawkxiang.cpp@gmail.com>
> ---
> V1 -> V2: Fix build variable 'sc' is uninitialized warning(Reported-by: kernel test robot <lkp@intel.com>).
> ---
>  drivers/scsi/libiscsi.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index 0fda8905eabd..f6bfe0c4f8a4 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -621,6 +621,7 @@ static void __fail_scsi_task(struct iscsi_task *task, int err)
>  	if (cleanup_queued_task(task))
>  		return;
>  
> +	sc = task->sc;
>  	if (task->state == ISCSI_TASK_PENDING) {
>  		/*
>  		 * cmd never made it to the xmit thread, so we should not count
> @@ -629,12 +630,12 @@ static void __fail_scsi_task(struct iscsi_task *task, int err)
>  		conn->session->queued_cmdsn--;
>  		/* it was never sent so just complete like normal */
>  		state = ISCSI_TASK_COMPLETED;
> -	} else if (err == DID_TRANSPORT_DISRUPTED)
> +	} else if (err == DID_TRANSPORT_DISRUPTED) {
>  		state = ISCSI_TASK_ABRT_SESS_RECOV;
> -	else
> +		sc->device->expecting_cc_ua = 1;
> +	} else
>  		state = ISCSI_TASK_ABRT_TMF;
>  
> -	sc = task->sc;
>  	sc->result = err << 16;
>  	scsi_set_resid(sc, scsi_bufflen(sc));
>  	iscsi_complete_task(task, state);


This should be fixed in a common way like I mentioned in the other thread.