Re: [PATCH v1 3/3] TARGET/sbc,loopback: Adjust command data length in case pi exists on the wire

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sagi & Co,

On Sun, 2014-06-08 at 13:27 +0300, Sagi Grimberg wrote:
> In various areas of the code, it is assumed that
> se_cmd->data_length describes pure data. In case
> that protection information exists over the wire
> (protect bits is are on) the target core decrease
> the protection length from the data length (instead
> of each transport peeking in the cdb).
> 
> Modify loopback device to include protection information
> in the transferred data length (like other scsi transports).
> 
> Signed-off-by: Sagi Grimberg <sagig@xxxxxxxxxxxx>
> ---
>  drivers/target/loopback/tcm_loop.c |   15 ++++++++++++---
>  drivers/target/target_core_sbc.c   |   15 +++++++++++++--
>  2 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
> index c886ad1..1f4c015 100644
> --- a/drivers/target/loopback/tcm_loop.c
> +++ b/drivers/target/loopback/tcm_loop.c
> @@ -179,7 +179,7 @@ static void tcm_loop_submission_work(struct work_struct *work)
>  	struct tcm_loop_hba *tl_hba;
>  	struct tcm_loop_tpg *tl_tpg;
>  	struct scatterlist *sgl_bidi = NULL;
> -	u32 sgl_bidi_count = 0;
> +	u32 sgl_bidi_count = 0, transfer_length;
>  	int rc;
>  
>  	tl_hba = *(struct tcm_loop_hba **)shost_priv(sc->device->host);
> @@ -213,12 +213,21 @@ static void tcm_loop_submission_work(struct work_struct *work)
>  
>  	}
>  
> -	if (!scsi_prot_sg_count(sc) && scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
> +	transfer_length = scsi_transfer_length(sc);
> +	if (!scsi_prot_sg_count(sc) &&
> +	    scsi_get_prot_op(sc) != SCSI_PROT_NORMAL) {
>  		se_cmd->prot_pto = true;
> +		/*
> +		 * loopback transport doesn't support
> +		 * WRITE_GENERATE, READ_STRIP protection
> +		 * information operations, go ahead unprotected.
> +		 */
> +		transfer_length = scsi_bufflen(sc);
> +	}
>  
>  	rc = target_submit_cmd_map_sgls(se_cmd, tl_nexus->se_sess, sc->cmnd,
>  			&tl_cmd->tl_sense_buf[0], tl_cmd->sc->device->lun,
> -			scsi_bufflen(sc), tcm_loop_sam_attr(sc),
> +			transfer_length, tcm_loop_sam_attr(sc),
>  			sc->sc_data_direction, 0,
>  			scsi_sglist(sc), scsi_sg_count(sc),
>  			sgl_bidi, sgl_bidi_count,
> diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
> index e022959..06f8ecd 100644
> --- a/drivers/target/target_core_sbc.c
> +++ b/drivers/target/target_core_sbc.c
> @@ -665,8 +665,19 @@ sbc_check_prot(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb,
>  
>  	cmd->prot_type = dev->dev_attrib.pi_prot_type;
>  	cmd->prot_length = dev->prot_length * sectors;
> -	pr_debug("%s: prot_type=%d, prot_length=%d prot_op=%d prot_checks=%d\n",
> -		 __func__, cmd->prot_type, cmd->prot_length,
> +
> +	/**
> +	 * In case protection information exists over the wire
> +	 * we modify command data length to describe pure data.
> +	 * The actual transfer length is data length + protection
> +	 * length
> +	 **/
> +	if (protect)
> +		cmd->data_length -= cmd->prot_length;
> +

This fabric wide assumption breaks the vhost-scsi + tcm_qla2xxx PI code
already queued for v3.16-rc1..

A vhost-scsi side conversion to combine exp_data_len + prot_bytes is
easy enough in target-pending/for-next (see below), given that
vhost-scsi's exp_data_len is coming from virtio buffer iovecs, so
changing virtio-scsi LLD to use scsi_transfer_length() doesn't really
make any sense. (Adding CC's for MST + Paolo)

The qla2xxx target T10 PI patch in scsi/for-next (adding Quinn CC) will
also need a similar change to update qlt_do_work() to include PI bytes
for data_length being passed into tgt_ops->handle_cmd(), following what
qlt_build_ctio_crc2_pkt() is already doing for calculating
transfer_length for HW offload.

That said, there is one other small qla2xxx change to enable per-session
PI that is currently missing in Quinn's patch in scsi/for-next code. 

Sooo, I'll go ahead and include Sagi's patches with the vhost-scsi
change below if there are no objections from MKP and/or driver
maintainers, and post an -rc2 series after the merge window closes to
address the two remaining qla2xxx specific items.

--nab

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 667e72d..03e484f 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1144,7 +1144,8 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
                }
 
                cmd = vhost_scsi_get_tag(vq, tpg, cdb, tag, lun, task_attr,
-                                        exp_data_len, data_direction);
+                                        exp_data_len + prot_bytes,
+                                        data_direction);
                if (IS_ERR(cmd)) {
                        vq_err(vq, "vhost_scsi_get_tag failed %ld\n",
                                        PTR_ERR(cmd));

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux