From patchwork Tue Jun 10 08:04:10 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 4327041 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A210D9F390 for ; Tue, 10 Jun 2014 08:04:24 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 91F4020221 for ; Tue, 10 Jun 2014 08:04:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 788DD2022D for ; Tue, 10 Jun 2014 08:04:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751164AbaFJIEU (ORCPT ); Tue, 10 Jun 2014 04:04:20 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:50618 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750742AbaFJIER (ORCPT ); Tue, 10 Jun 2014 04:04:17 -0400 Received: from [192.168.1.64] (75-37-193-228.lightspeed.lsatca.sbcglobal.net [75.37.193.228]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: nab) by linux-iscsi.org (Postfix) with ESMTPSA id 511B622D9A6; Tue, 10 Jun 2014 07:39:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=linux-iscsi.org; s=default.private; t=1402385944; bh=MAXnsh4ur/gP1+ZjsqwkWgnpAa9o8wK 5tIFu82Vdkkc=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To: References:Content-Type:Mime-Version:Content-Transfer-Encoding; b=R7jUXhe3sHgWW7HpVu5M3c/UhOkniVmnb+bZX7o8e89SD/c5W3dWm5BpVh5OKGiSz EWIyDtc4kTuwL0dz1WWX9J5y2oZwUunQxfvxp3cOFnxrqCkIWalE3r/U3z+J6KGgvRl H8Pz2UAjS5/nwkYBLXt+cvSXTzlN6j0ZiosIksA= Message-ID: <1402387450.5774.56.camel@haakon3.risingtidesystems.com> Subject: Re: [PATCH v1 3/3] TARGET/sbc, loopback: Adjust command data length in case pi exists on the wire From: "Nicholas A. Bellinger" To: Sagi Grimberg Cc: michaelc@cs.wisc.edu, martin.petersen@oracle.com, roland@kernel.org, linux-scsi@vger.kernel.org, target-devel@vger.kernel.org, linux-rdma@vger.kernel.org, Quinn Tran , "Michael S. Tsirkin" , Paolo Bonzini Date: Tue, 10 Jun 2014 01:04:10 -0700 In-Reply-To: <1402223228-23768-4-git-send-email-sagig@mellanox.com> References: <1402223228-23768-1-git-send-email-sagig@mellanox.com> <1402223228-23768-4-git-send-email-sagig@mellanox.com> X-Mailer: Evolution 3.4.4-1 Mime-Version: 1.0 Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 > --- > 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 --- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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));