All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] target: loopback: fix READ with data and sensebytes
@ 2020-04-28 18:26 ` Bodo Stroesser
  0 siblings, 0 replies; 4+ messages in thread
From: Bodo Stroesser @ 2020-04-28 18:26 UTC (permalink / raw)
  To: martin.petersen, nab, linux-scsi, target-devel; +Cc: Bodo Stroesser

We use tcm_loop with tape emulations running on tcmu.
In case application reads a short tape block with a longer READ,
or a long tape block with a short READ, according to SCC spec
data has to be tranferred _and_ sensebytes with ILI set and
information field containing the residual count.
Similar problem also exists when using fixed block size in READ.

Up to now tcm_loop is not prepared to handle sensebytes if input
data is provided, as in tcm_loop_queue_data_in() it only sets
SAM_STAT_GOOD and if necessary the residual count.

To fix the bug, the same handling for sensebytes as present in
tcm_loop_queue_status() must be done in tcm_loop_queue_data_in()
also.

After adding this handling, the two function now are nearly
identical, so I created a single function with two wrappers.

Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
---
 drivers/target/loopback/tcm_loop.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 3305b47fdf53..16d5a4e117a2 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -545,32 +545,15 @@ static int tcm_loop_write_pending(struct se_cmd *se_cmd)
 	return 0;
 }
 
-static int tcm_loop_queue_data_in(struct se_cmd *se_cmd)
+static int tcm_loop_queue_data_or_status(const char *func,
+		struct se_cmd *se_cmd, u8 scsi_status)
 {
 	struct tcm_loop_cmd *tl_cmd = container_of(se_cmd,
 				struct tcm_loop_cmd, tl_se_cmd);
 	struct scsi_cmnd *sc = tl_cmd->sc;
 
 	pr_debug("%s() called for scsi_cmnd: %p cdb: 0x%02x\n",
-		 __func__, sc, sc->cmnd[0]);
-
-	sc->result = SAM_STAT_GOOD;
-	set_host_byte(sc, DID_OK);
-	if ((se_cmd->se_cmd_flags & SCF_OVERFLOW_BIT) ||
-	    (se_cmd->se_cmd_flags & SCF_UNDERFLOW_BIT))
-		scsi_set_resid(sc, se_cmd->residual_count);
-	sc->scsi_done(sc);
-	return 0;
-}
-
-static int tcm_loop_queue_status(struct se_cmd *se_cmd)
-{
-	struct tcm_loop_cmd *tl_cmd = container_of(se_cmd,
-				struct tcm_loop_cmd, tl_se_cmd);
-	struct scsi_cmnd *sc = tl_cmd->sc;
-
-	pr_debug("%s() called for scsi_cmnd: %p cdb: 0x%02x\n",
-		 __func__, sc, sc->cmnd[0]);
+		 func, sc, sc->cmnd[0]);
 
 	if (se_cmd->sense_buffer &&
 	   ((se_cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) ||
@@ -581,7 +564,7 @@ static int tcm_loop_queue_status(struct se_cmd *se_cmd)
 		sc->result = SAM_STAT_CHECK_CONDITION;
 		set_driver_byte(sc, DRIVER_SENSE);
 	} else
-		sc->result = se_cmd->scsi_status;
+		sc->result = scsi_status;
 
 	set_host_byte(sc, DID_OK);
 	if ((se_cmd->se_cmd_flags & SCF_OVERFLOW_BIT) ||
@@ -591,6 +574,17 @@ static int tcm_loop_queue_status(struct se_cmd *se_cmd)
 	return 0;
 }
 
+static int tcm_loop_queue_data_in(struct se_cmd *se_cmd)
+{
+	return tcm_loop_queue_data_or_status(__func__, se_cmd, SAM_STAT_GOOD);
+}
+
+static int tcm_loop_queue_status(struct se_cmd *se_cmd)
+{
+	return tcm_loop_queue_data_or_status(__func__,
+					     se_cmd, se_cmd->scsi_status);
+}
+
 static void tcm_loop_queue_tm_rsp(struct se_cmd *se_cmd)
 {
 	struct tcm_loop_cmd *tl_cmd = container_of(se_cmd,
-- 
2.12.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] target: loopback: fix READ with data and sensebytes
@ 2020-04-28 18:26 ` Bodo Stroesser
  0 siblings, 0 replies; 4+ messages in thread
From: Bodo Stroesser @ 2020-04-28 18:26 UTC (permalink / raw)
  To: martin.petersen, nab, linux-scsi, target-devel; +Cc: Bodo Stroesser

We use tcm_loop with tape emulations running on tcmu.
In case application reads a short tape block with a longer READ,
or a long tape block with a short READ, according to SCC spec
data has to be tranferred _and_ sensebytes with ILI set and
information field containing the residual count.
Similar problem also exists when using fixed block size in READ.

Up to now tcm_loop is not prepared to handle sensebytes if input
data is provided, as in tcm_loop_queue_data_in() it only sets
SAM_STAT_GOOD and if necessary the residual count.

To fix the bug, the same handling for sensebytes as present in
tcm_loop_queue_status() must be done in tcm_loop_queue_data_in()
also.

After adding this handling, the two function now are nearly
identical, so I created a single function with two wrappers.

Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
---
 drivers/target/loopback/tcm_loop.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 3305b47fdf53..16d5a4e117a2 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -545,32 +545,15 @@ static int tcm_loop_write_pending(struct se_cmd *se_cmd)
 	return 0;
 }
 
-static int tcm_loop_queue_data_in(struct se_cmd *se_cmd)
+static int tcm_loop_queue_data_or_status(const char *func,
+		struct se_cmd *se_cmd, u8 scsi_status)
 {
 	struct tcm_loop_cmd *tl_cmd = container_of(se_cmd,
 				struct tcm_loop_cmd, tl_se_cmd);
 	struct scsi_cmnd *sc = tl_cmd->sc;
 
 	pr_debug("%s() called for scsi_cmnd: %p cdb: 0x%02x\n",
-		 __func__, sc, sc->cmnd[0]);
-
-	sc->result = SAM_STAT_GOOD;
-	set_host_byte(sc, DID_OK);
-	if ((se_cmd->se_cmd_flags & SCF_OVERFLOW_BIT) ||
-	    (se_cmd->se_cmd_flags & SCF_UNDERFLOW_BIT))
-		scsi_set_resid(sc, se_cmd->residual_count);
-	sc->scsi_done(sc);
-	return 0;
-}
-
-static int tcm_loop_queue_status(struct se_cmd *se_cmd)
-{
-	struct tcm_loop_cmd *tl_cmd = container_of(se_cmd,
-				struct tcm_loop_cmd, tl_se_cmd);
-	struct scsi_cmnd *sc = tl_cmd->sc;
-
-	pr_debug("%s() called for scsi_cmnd: %p cdb: 0x%02x\n",
-		 __func__, sc, sc->cmnd[0]);
+		 func, sc, sc->cmnd[0]);
 
 	if (se_cmd->sense_buffer &&
 	   ((se_cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) ||
@@ -581,7 +564,7 @@ static int tcm_loop_queue_status(struct se_cmd *se_cmd)
 		sc->result = SAM_STAT_CHECK_CONDITION;
 		set_driver_byte(sc, DRIVER_SENSE);
 	} else
-		sc->result = se_cmd->scsi_status;
+		sc->result = scsi_status;
 
 	set_host_byte(sc, DID_OK);
 	if ((se_cmd->se_cmd_flags & SCF_OVERFLOW_BIT) ||
@@ -591,6 +574,17 @@ static int tcm_loop_queue_status(struct se_cmd *se_cmd)
 	return 0;
 }
 
+static int tcm_loop_queue_data_in(struct se_cmd *se_cmd)
+{
+	return tcm_loop_queue_data_or_status(__func__, se_cmd, SAM_STAT_GOOD);
+}
+
+static int tcm_loop_queue_status(struct se_cmd *se_cmd)
+{
+	return tcm_loop_queue_data_or_status(__func__,
+					     se_cmd, se_cmd->scsi_status);
+}
+
 static void tcm_loop_queue_tm_rsp(struct se_cmd *se_cmd)
 {
 	struct tcm_loop_cmd *tl_cmd = container_of(se_cmd,
-- 
2.12.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] target: loopback: fix READ with data and sensebytes
  2020-04-28 18:26 ` Bodo Stroesser
@ 2020-05-12  3:28   ` Martin K. Petersen
  -1 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2020-05-12  3:28 UTC (permalink / raw)
  To: nab, target-devel, linux-scsi, Bodo Stroesser; +Cc: Martin K . Petersen

On Tue, 28 Apr 2020 20:26:17 +0200, Bodo Stroesser wrote:

> We use tcm_loop with tape emulations running on tcmu.
> In case application reads a short tape block with a longer READ,
> or a long tape block with a short READ, according to SCC spec
> data has to be tranferred _and_ sensebytes with ILI set and
> information field containing the residual count.
> Similar problem also exists when using fixed block size in READ.
> 
> [...]

Applied to 5.8/scsi-queue, thanks!

[1/1] scsi: target: loopback: Fix READ with data and sensebytes
      https://git.kernel.org/mkp/scsi/c/c68a56736c12

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] target: loopback: fix READ with data and sensebytes
@ 2020-05-12  3:28   ` Martin K. Petersen
  0 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2020-05-12  3:28 UTC (permalink / raw)
  To: nab, target-devel, linux-scsi, Bodo Stroesser; +Cc: Martin K . Petersen

On Tue, 28 Apr 2020 20:26:17 +0200, Bodo Stroesser wrote:

> We use tcm_loop with tape emulations running on tcmu.
> In case application reads a short tape block with a longer READ,
> or a long tape block with a short READ, according to SCC spec
> data has to be tranferred _and_ sensebytes with ILI set and
> information field containing the residual count.
> Similar problem also exists when using fixed block size in READ.
> 
> [...]

Applied to 5.8/scsi-queue, thanks!

[1/1] scsi: target: loopback: Fix READ with data and sensebytes
      https://git.kernel.org/mkp/scsi/c/c68a56736c12

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-05-12  3:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-28 18:26 [PATCH] target: loopback: fix READ with data and sensebytes Bodo Stroesser
2020-04-28 18:26 ` Bodo Stroesser
2020-05-12  3:28 ` Martin K. Petersen
2020-05-12  3:28   ` Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.