[PATCH 0/3] iscsi bidi & varlen support

[PATCH 0/3] iscsi bidi & varlen support

[Boaz Harrosh]

Cheers after 1.3 years these can go in.

[PATCH 1/3] iscsi: extended cdb support
   The varlen support is not yet in mainline for
  block and scsi-ml. But the API for drivers will
  not change. All LLD need to do is max_command to
  the it's maximum and be ready for bigger commands.
  This is what's done here. Once these commands start
  coming iscsi will be ready for them.

[PATCH 2/3] iscsi: bidi support - libiscsi
[PATCH 3/3] iscsi: bidi support - iscsi_tcp
  bidirectional commands support in iscsi.
  iSER is not yet ready, but it will not break.
  There is already a mechanism in libiscsi that will
  return error if bidi commands are sent iSER way.

Pete please send me the iSER bits so we can port them
to this latest version.

Mike these patches are ontop of iscs branch of the iscsi
git tree, they will apply but for compilation you will need
to sync with Linus mainline. The patches are for the in-tree
iscsi code. I own you the compat patch for the out-off-tree
code, but this I will only be Sunday.

If we do it fast it might get accepted to 2.6.25 merge window

Everybody is invited to a party at Shila ben-yhuda 52 Tel-Aviv
9:45 pm. Drinks and wonderful see-food on us :)

Boaz
 

[PATCH 1/3] iscsi: extended cdb support

[Boaz Harrosh]

  Support for extended CDBs in iscsi.
  All we need is to check if command spills over 16 bytes then allocate
  an iscsi-extended-header for the leftovers.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/iscsi_tcp.c   |    2 +-
 drivers/scsi/libiscsi.c    |   56 ++++++++++++++++++++++++++++++++++++++++----
 include/scsi/iscsi_proto.h |    6 +++-
 3 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 8a17867..ee7acfa 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -1975,7 +1975,7 @@ static struct iscsi_transport iscsi_tcp_transport = {
 	.host_template		= &iscsi_sht,
 	.conndata_size		= sizeof(struct iscsi_conn),
 	.max_conn		= 1,
-	.max_cmd_len		= 16,
+	.max_cmd_len		= 260,
 	/* session management */
 	.create_session		= iscsi_tcp_session_create,
 	.destroy_session	= iscsi_tcp_session_destroy,
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index cfffabd..498cea3 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -137,6 +137,45 @@ static int iscsi_add_hdr(struct iscsi_cmd_task *ctask, unsigned len)
 	return 0;
 }
 
+/*
+ * make an extended cdb AHS
+ */
+static int iscsi_prep_ecdb_ahs(struct iscsi_cmd_task *ctask)
+{
+	struct scsi_cmnd *cmd = ctask->sc;
+	unsigned rlen, pad_len;
+	unsigned short ahslength;
+	struct iscsi_ecdb_ahdr *ecdb_ahdr;
+	int rc;
+
+	ecdb_ahdr = iscsi_next_hdr(ctask);
+	rlen = cmd->cmd_len - ISCSI_CDB_SIZE;
+
+	BUG_ON(rlen > sizeof(ecdb_ahdr->ecdb));
+	ahslength = rlen + sizeof(ecdb_ahdr->reserved);
+
+	pad_len = iscsi_padding(rlen);
+
+	rc = iscsi_add_hdr(ctask, sizeof(ecdb_ahdr->ahslength) +
+	              sizeof(ecdb_ahdr->ahstype) + ahslength + pad_len);
+	if (rc)
+		return rc;
+
+	if (pad_len)
+		memset(&ecdb_ahdr->ecdb[rlen], 0, pad_len);
+
+	ecdb_ahdr->ahslength = cpu_to_be16(ahslength);
+	ecdb_ahdr->ahstype = ISCSI_AHSTYPE_CDB;
+	ecdb_ahdr->reserved = 0;
+	memcpy(ecdb_ahdr->ecdb, cmd->cmnd + ISCSI_CDB_SIZE, rlen);
+
+	debug_scsi("iscsi_prep_ecdb_ahs: varlen_cdb_len %d "
+		"rlen %d pad_len %d ahs_length %d iscsi_headers_size %u\n",
+		cmd->cmd_len, rlen, pad_len, ahslength, ctask->hdr_len);
+
+	return 0;
+}
+
 /**
  * iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
  * @ctask: iscsi cmd task
@@ -150,7 +189,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	struct iscsi_session *session = conn->session;
 	struct iscsi_cmd *hdr = ctask->hdr;
 	struct scsi_cmnd *sc = ctask->sc;
-	unsigned hdrlength;
+	unsigned hdrlength, cmd_len;
 	int rc;
 
 	ctask->hdr_len = 0;
@@ -165,10 +204,17 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	hdr->cmdsn = cpu_to_be32(session->cmdsn);
 	session->cmdsn++;
 	hdr->exp_statsn = cpu_to_be32(conn->exp_statsn);
-	memcpy(hdr->cdb, sc->cmnd, sc->cmd_len);
-	if (sc->cmd_len < MAX_COMMAND_SIZE)
-		memset(&hdr->cdb[sc->cmd_len], 0,
-			MAX_COMMAND_SIZE - sc->cmd_len);
+	cmd_len = sc->cmd_len;
+	if (cmd_len < ISCSI_CDB_SIZE)
+		memset(&hdr->cdb[cmd_len], 0,
+			ISCSI_CDB_SIZE - cmd_len);
+	else if (cmd_len > ISCSI_CDB_SIZE) {
+		rc = iscsi_prep_ecdb_ahs(ctask);
+		if (rc)
+			return rc;
+		cmd_len = ISCSI_CDB_SIZE;
+	}
+	memcpy(hdr->cdb, sc->cmnd, cmd_len);
 
 	ctask->imm_count = 0;
 	if (sc->sc_data_direction == DMA_TO_DEVICE) {
diff --git a/include/scsi/iscsi_proto.h b/include/scsi/iscsi_proto.h
index 5ffec8a..e0593bf 100644
--- a/include/scsi/iscsi_proto.h
+++ b/include/scsi/iscsi_proto.h
@@ -112,6 +112,7 @@ struct iscsi_ahs_hdr {
 
 #define ISCSI_AHSTYPE_CDB		1
 #define ISCSI_AHSTYPE_RLENGTH		2
+#define ISCSI_CDB_SIZE			16
 
 /* iSCSI PDU Header */
 struct iscsi_cmd {
@@ -125,7 +126,7 @@ struct iscsi_cmd {
 	__be32 data_length;
 	__be32 cmdsn;
 	__be32 exp_statsn;
-	uint8_t cdb[16];	/* SCSI Command Block */
+	uint8_t cdb[ISCSI_CDB_SIZE];	/* SCSI Command Block */
 	/* Additional Data (Command Dependent) */
 };
 
@@ -154,7 +155,8 @@ struct iscsi_ecdb_ahdr {
 	__be16 ahslength;	/* CDB length - 15, including reserved byte */
 	uint8_t ahstype;
 	uint8_t reserved;
-	uint8_t ecdb[260 - 16];	/* 4-byte aligned extended CDB spillover */
+	/* 4-byte aligned extended CDB spillover */
+	uint8_t ecdb[260 - ISCSI_CDB_SIZE];
 };
 
 /* SCSI Response Header */
-- 
1.5.3.3

[PATCH 2/3] iscsi: bidi support - libiscsi

[Boaz Harrosh]

  iscsi bidi support at the generic libiscsi level
  - prepare the additional bidi_read rlength header.
  - access the right scsi_in() and/or scsi_out() side of things.
    also for resid.
  - Handle BIDI underflow overflow from target

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/libiscsi.c |   82 ++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 498cea3..df81f6f 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -176,6 +176,31 @@ static int iscsi_prep_ecdb_ahs(struct iscsi_cmd_task *ctask)
 	return 0;
 }
 
+static int iscsi_prep_bidi_ahs(struct iscsi_cmd_task *ctask)
+{
+	struct scsi_cmnd *sc = ctask->sc;
+	struct iscsi_rlength_ahdr *rlen_ahdr;
+	int rc;
+
+	rlen_ahdr = iscsi_next_hdr(ctask);
+	rc = iscsi_add_hdr(ctask, sizeof(*rlen_ahdr));
+	if (rc)
+		return rc;
+
+	rlen_ahdr->ahslength =
+		cpu_to_be16(sizeof(rlen_ahdr->read_length) +
+			            sizeof(rlen_ahdr->reserved));
+	rlen_ahdr->ahstype = ISCSI_AHSTYPE_RLENGTH;
+	rlen_ahdr->reserved = 0;
+	rlen_ahdr->read_length = cpu_to_be32(scsi_in(sc)->length);
+
+	debug_scsi("bidi-in rlen_ahdr->read_length(%d) "
+		   "rlen_ahdr->ahslength(%d)\n",
+		   be32_to_cpu(rlen_ahdr->read_length),
+		   be16_to_cpu(rlen_ahdr->ahslength));
+	return 0;
+}
+
 /**
  * iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
  * @ctask: iscsi cmd task
@@ -200,7 +225,6 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	hdr->flags = ISCSI_ATTR_SIMPLE;
 	int_to_scsilun(sc->device->lun, (struct scsi_lun *)hdr->lun);
 	hdr->itt = build_itt(ctask->itt, session->age);
-	hdr->data_length = cpu_to_be32(scsi_bufflen(sc));
 	hdr->cmdsn = cpu_to_be32(session->cmdsn);
 	session->cmdsn++;
 	hdr->exp_statsn = cpu_to_be32(conn->exp_statsn);
@@ -217,7 +241,15 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	memcpy(hdr->cdb, sc->cmnd, cmd_len);
 
 	ctask->imm_count = 0;
+	if (scsi_bidi_cmnd(sc)) {
+		hdr->flags |= ISCSI_FLAG_CMD_READ;
+		rc = iscsi_prep_bidi_ahs(ctask);
+		if (rc)
+			return rc;
+	}
 	if (sc->sc_data_direction == DMA_TO_DEVICE) {
+		unsigned out_len = scsi_out(sc)->length;
+		hdr->data_length = cpu_to_be32(out_len);
 		hdr->flags |= ISCSI_FLAG_CMD_WRITE;
 		/*
 		 * Write counters:
@@ -238,19 +270,19 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 		ctask->unsol_datasn = 0;
 
 		if (session->imm_data_en) {
-			if (scsi_bufflen(sc) >= session->first_burst)
+			if (out_len >= session->first_burst)
 				ctask->imm_count = min(session->first_burst,
 							conn->max_xmit_dlength);
 			else
-				ctask->imm_count = min(scsi_bufflen(sc),
+				ctask->imm_count = min(out_len,
 							conn->max_xmit_dlength);
 			hton24(hdr->dlength, ctask->imm_count);
 		} else
 			zero_data(hdr->dlength);
 
 		if (!session->initial_r2t_en) {
-			ctask->unsol_count = min((session->first_burst),
-				(scsi_bufflen(sc))) - ctask->imm_count;
+			ctask->unsol_count = min(session->first_burst, out_len)
+							     - ctask->imm_count;
 			ctask->unsol_offset = ctask->imm_count;
 		}
 
@@ -260,6 +292,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	} else {
 		hdr->flags |= ISCSI_FLAG_CMD_FINAL;
 		zero_data(hdr->dlength);
+		hdr->data_length = cpu_to_be32(scsi_in(sc)->length);
 
 		if (sc->sc_data_direction == DMA_FROM_DEVICE)
 			hdr->flags |= ISCSI_FLAG_CMD_READ;
@@ -278,10 +311,12 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 		return EIO;
 
 	conn->scsicmd_pdus_cnt++;
-	debug_scsi("iscsi prep [%s cid %d sc %p cdb 0x%x itt 0x%x len %d "
-		"cmdsn %d win %d]\n",
-		sc->sc_data_direction == DMA_TO_DEVICE ? "write" : "read",
-		conn->id, sc, sc->cmnd[0], ctask->itt, scsi_bufflen(sc),
+	debug_scsi("iscsi prep [%s cid %d sc %p cdb 0x%x itt 0x%x "
+		"len %d bidi_len %d cmdsn %d win %d]\n",
+		scsi_bidi_cmnd(sc) ? "bidirectional" :
+		    sc->sc_data_direction == DMA_TO_DEVICE ? "write" : "read",
+		conn->id, sc, sc->cmnd[0], ctask->itt,
+		scsi_bufflen(sc), scsi_bidi_cmnd(sc) ? scsi_in(sc)->length : 0,
 		session->cmdsn, session->max_cmdsn - session->exp_cmdsn + 1);
 	return 0;
 }
@@ -344,7 +379,12 @@ static void fail_command(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask,
 		conn->session->tt->cleanup_cmd_task(conn, ctask);
 
 	sc->result = err;
-	scsi_set_resid(sc, scsi_bufflen(sc));
+	if (!scsi_bidi_cmnd(sc))
+		scsi_set_resid(sc, scsi_bufflen(sc));
+	else {
+		scsi_out(sc)->resid = scsi_out(sc)->length;
+		scsi_in(sc)->resid = scsi_in(sc)->length;
+	}
 	if (conn->ctask == ctask)
 		conn->ctask = NULL;
 	/* release ref from queuecommand */
@@ -479,6 +519,20 @@ invalid_datalen:
 			   min_t(uint16_t, senselen, SCSI_SENSE_BUFFERSIZE));
 	}
 
+	if (scsi_bidi_cmnd(sc) &&
+		(rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW |
+					ISCSI_FLAG_CMD_BIDI_OVERFLOW))) {
+		int res_count = be32_to_cpu(rhdr->bi_residual_count);
+
+		if (res_count > 0 &&
+			(rhdr->flags & ISCSI_FLAG_CMD_BIDI_OVERFLOW ||
+			res_count <= scsi_in(sc)->length))
+			scsi_in(sc)->resid = res_count;
+		else
+			sc->result =
+				(DID_BAD_TARGET << 16) | rhdr->cmd_status;
+	}
+
 	if (rhdr->flags & (ISCSI_FLAG_CMD_UNDERFLOW |
 	                   ISCSI_FLAG_CMD_OVERFLOW)) {
 		int res_count = be32_to_cpu(rhdr->residual_count);
@@ -486,6 +540,7 @@ invalid_datalen:
 		if (res_count > 0 &&
 		    (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
 		     res_count <= scsi_bufflen(sc)))
+			/* write side for bidi or uni-io set_resid */
 			scsi_set_resid(sc, res_count);
 		else
 			sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
@@ -1149,7 +1204,12 @@ reject:
 fault:
 	spin_unlock(&session->lock);
 	debug_scsi("iscsi: cmd 0x%x is not queued (%d)\n", sc->cmnd[0], reason);
-	scsi_set_resid(sc, scsi_bufflen(sc));
+	if (!scsi_bidi_cmnd(sc))
+		scsi_set_resid(sc, scsi_bufflen(sc));
+	else {
+		scsi_out(sc)->resid = scsi_out(sc)->length;
+		scsi_in(sc)->resid = scsi_in(sc)->length;
+	}
 	sc->scsi_done(sc);
 	spin_lock(host->host_lock);
 	return 0;
-- 
1.5.3.3

[PATCH 3/3] iscsi: bidi support - iscsi_tcp

[Boaz Harrosh]

  bidi support for iscsi_tcp
  - access the right scsi_in() and/or scsi_out() side of things.
    also for resid

 Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/iscsi_tcp.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index ee7acfa..073dea7 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -528,6 +528,7 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 	struct iscsi_session *session = conn->session;
 	struct scsi_cmnd *sc = ctask->sc;
 	int datasn = be32_to_cpu(rhdr->datasn);
+	unsigned total_in_length = scsi_in(sc)->length;
 
 	iscsi_update_cmdsn(session, (struct iscsi_nopin*)rhdr);
 	if (tcp_conn->in.datalen == 0)
@@ -542,10 +543,10 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 	tcp_ctask->exp_datasn++;
 
 	tcp_ctask->data_offset = be32_to_cpu(rhdr->offset);
-	if (tcp_ctask->data_offset + tcp_conn->in.datalen > scsi_bufflen(sc)) {
+	if (tcp_ctask->data_offset + tcp_conn->in.datalen > total_in_length) {
 		debug_tcp("%s: data_offset(%d) + data_len(%d) > total_length_in(%d)\n",
 		          __FUNCTION__, tcp_ctask->data_offset,
-		          tcp_conn->in.datalen, scsi_bufflen(sc));
+		          tcp_conn->in.datalen, total_in_length);
 		return ISCSI_ERR_DATA_OFFSET;
 	}
 
@@ -558,8 +559,8 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 
 			if (res_count > 0 &&
 			    (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
-			     res_count <= scsi_bufflen(sc)))
-				scsi_set_resid(sc, res_count);
+			     res_count <= total_in_length))
+				scsi_in(sc)->resid = res_count;
 			else
 				sc->result = (DID_BAD_TARGET << 16) |
 					rhdr->cmd_status;
@@ -670,11 +671,11 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 			    r2t->data_length, session->max_burst);
 
 	r2t->data_offset = be32_to_cpu(rhdr->data_offset);
-	if (r2t->data_offset + r2t->data_length > scsi_bufflen(ctask->sc)) {
+	if (r2t->data_offset + r2t->data_length > scsi_out(ctask->sc)->length) {
 		iscsi_conn_printk(KERN_ERR, conn,
 				  "invalid R2T with data len %u at offset %u "
 				  "and total length %d\n", r2t->data_length,
-				  r2t->data_offset, scsi_bufflen(ctask->sc));
+				  r2t->data_offset, scsi_out(ctask->sc)->length);
 		__kfifo_put(tcp_ctask->r2tpool.queue, (void*)&r2t,
 			    sizeof(void*));
 		return ISCSI_ERR_DATALEN;
@@ -771,6 +772,7 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 		if (tcp_conn->in.datalen) {
 			struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
 			struct hash_desc *rx_hash = NULL;
+			struct scsi_data_buffer *sdb = scsi_in(ctask->sc);
 
 			/*
 			 * Setup copy of Data-In into the Scsi_Cmnd
@@ -788,8 +790,8 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 				  tcp_ctask->data_offset,
 				  tcp_conn->in.datalen);
 			return iscsi_segment_seek_sg(&tcp_conn->in.segment,
-						     scsi_sglist(ctask->sc),
-						     scsi_sg_count(ctask->sc),
+						     sdb->table.sgl,
+						     sdb->table.nents,
 						     tcp_ctask->data_offset,
 						     tcp_conn->in.datalen,
 						     iscsi_tcp_process_data_in,
@@ -1332,7 +1334,8 @@ iscsi_tcp_ctask_init(struct iscsi_cmd_task *ctask)
 		return 0;
 
 	/* If we have immediate data, attach a payload */
-	err = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc), scsi_sg_count(sc),
+	err = iscsi_tcp_send_data_prep(conn, scsi_out(sc)->table.sgl,
+				       scsi_out(sc)->table.nents,
 				       0, ctask->imm_count);
 	if (err)
 		return err;
@@ -1386,6 +1389,7 @@ iscsi_tcp_ctask_xmit(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 {
 	struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
 	struct scsi_cmnd *sc = ctask->sc;
+	struct scsi_data_buffer *sdb = scsi_out(sc);
 	int rc = 0;
 
 flush:
@@ -1412,9 +1416,8 @@ flush:
 				ctask->itt, tcp_ctask->sent, ctask->data_count);
 
 		iscsi_tcp_send_hdr_prep(conn, hdr, sizeof(*hdr));
-		rc = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc),
-					      scsi_sg_count(sc),
-					      tcp_ctask->sent,
+		rc = iscsi_tcp_send_data_prep(conn, sdb->table.sgl,
+					      sdb->table.nents, tcp_ctask->sent,
 					      ctask->data_count);
 		if (rc)
 			goto fail;
@@ -1460,8 +1463,8 @@ flush:
 		iscsi_tcp_send_hdr_prep(conn, &r2t->dtask.hdr,
 					sizeof(struct iscsi_hdr));
 
-		rc = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc),
-					      scsi_sg_count(sc),
+		rc = iscsi_tcp_send_data_prep(conn, sdb->table.sgl,
+					      sdb->table.nents,
 					      r2t->data_offset + r2t->sent,
 					      r2t->data_count);
 		if (rc)
-- 
1.5.3.3

Re: [PATCH 2/3] iscsi: bidi support - libiscsi

[Pete Wyckoff]

bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org wrote on Thu, 31 Jan 2008 22:29 +0200:
>   iscsi bidi support at the generic libiscsi level
>   - prepare the additional bidi_read rlength header.
>   - access the right scsi_in() and/or scsi_out() side of things.
>     also for resid.
>   - Handle BIDI underflow overflow from target
> 
> Signed-off-by: Boaz Harrosh <bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>

I see you do this a bit differently than in your previous patch set.
In particular, the residual handling in libiscsi.c.  (I'm editing in
a bit more context to the patch below.)

> +	if (scsi_bidi_cmnd(sc) &&
> +		(rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW |
> +					ISCSI_FLAG_CMD_BIDI_OVERFLOW))) {
> +		int res_count = be32_to_cpu(rhdr->bi_residual_count);
> +
> +		if (res_count > 0 &&
> +			(rhdr->flags & ISCSI_FLAG_CMD_BIDI_OVERFLOW ||
> +			res_count <= scsi_in(sc)->length))
> +			scsi_in(sc)->resid = res_count;
> +		else
> +			sc->result =
> +				(DID_BAD_TARGET << 16) | rhdr->cmd_status;
> +	}
> +
>  	if (rhdr->flags & (ISCSI_FLAG_CMD_UNDERFLOW |
>  	                   ISCSI_FLAG_CMD_OVERFLOW)) {
>  		int res_count = be32_to_cpu(rhdr->residual_count);
>
>  		if (res_count > 0 &&
>  		    (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
>  		     res_count <= scsi_bufflen(sc)))
> +			/* write side for bidi or uni-io set_resid */
>  			scsi_set_resid(sc, res_count);
>  		else
>  			sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
>       } else if (rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW |
>                                 ISCSI_FLAG_CMD_BIDI_OVERFLOW))
>               sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;

I haven't tested this, but, consider a bidi command that results in
an overflow on the read part, and no overflow on the write part.
E.g., the user supplied a data-in buffer that wasn't big enough to
hold the returned data from the target, but the data-out buffer was
just right.

Then this code will set scsi_in(sc)->resid properly, informing the
caller that there were extra bytes that were not transferred.  But
the "else" clause at the bottom will also set sc->result to be bad.
I don't think we want this.

Your earlier patch got rid of the second bidi_overflow handler and
just did all the logic for both bidi and non-bidi cases in a single
if clause.  Seemed better.

		-- Pete

Re: [PATCH 2/3] iscsi: bidi support - libiscsi

[Boaz Harrosh]

On Mon, Feb 11 2008 at 17:43 +0200, Pete Wyckoff <pw@osc.edu> wrote:
> bharrosh@panasas.com wrote on Thu, 31 Jan 2008 22:29 +0200:
>>   iscsi bidi support at the generic libiscsi level
>>   - prepare the additional bidi_read rlength header.
>>   - access the right scsi_in() and/or scsi_out() side of things.
>>     also for resid.
>>   - Handle BIDI underflow overflow from target
>>
>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> 
> I see you do this a bit differently than in your previous patch set.
> In particular, the residual handling in libiscsi.c.  (I'm editing in
> a bit more context to the patch below.)
> 
>> +	if (scsi_bidi_cmnd(sc) &&
>> +		(rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW |
>> +					ISCSI_FLAG_CMD_BIDI_OVERFLOW))) {
>> +		int res_count = be32_to_cpu(rhdr->bi_residual_count);
>> +
>> +		if (res_count > 0 &&
>> +			(rhdr->flags & ISCSI_FLAG_CMD_BIDI_OVERFLOW ||
>> +			res_count <= scsi_in(sc)->length))
>> +			scsi_in(sc)->resid = res_count;
>> +		else
>> +			sc->result =
>> +				(DID_BAD_TARGET << 16) | rhdr->cmd_status;
>> +	}
>> +
>>  	if (rhdr->flags & (ISCSI_FLAG_CMD_UNDERFLOW |
>>  	                   ISCSI_FLAG_CMD_OVERFLOW)) {
>>  		int res_count = be32_to_cpu(rhdr->residual_count);
>>
>>  		if (res_count > 0 &&
>>  		    (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
>>  		     res_count <= scsi_bufflen(sc)))
>> +			/* write side for bidi or uni-io set_resid */
>>  			scsi_set_resid(sc, res_count);
>>  		else
>>  			sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
>>       } else if (rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW |
>>                                 ISCSI_FLAG_CMD_BIDI_OVERFLOW))
>>               sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
> 
> I haven't tested this, but, consider a bidi command that results in
> an overflow on the read part, and no overflow on the write part.
> E.g., the user supplied a data-in buffer that wasn't big enough to
> hold the returned data from the target, but the data-out buffer was
> just right.
> 
> Then this code will set scsi_in(sc)->resid properly, informing the
> caller that there were extra bytes that were not transferred.  But
> the "else" clause at the bottom will also set sc->result to be bad.
> I don't think we want this.
> 
> Your earlier patch got rid of the second bidi_overflow handler and
> just did all the logic for both bidi and non-bidi cases in a single
> if clause.  Seemed better.
> 
> 		-- Pete
You are most probably right I will investigate what happened. It looks
like I went back to some old version right? or a merge fallout
Thanks for reviewing.

Please also test latest head-of-line code if possible + iscsi patches
+ last varlen I sent.

Is there any new patches I need for 2.6.24 or head-of-line for my 
osd-dev tree?

Boaz

Re: [PATCH 2/3] iscsi: bidi support - libiscsi

[Pete Wyckoff]

bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org wrote on Mon, 11 Feb 2008 18:05 +0200:
> You are most probably right I will investigate what happened. It looks
> like I went back to some old version right? or a merge fallout
> Thanks for reviewing.
> 
> Please also test latest head-of-line code if possible + iscsi patches
> + last varlen I sent.
> 
> Is there any new patches I need for 2.6.24 or head-of-line for my 
> osd-dev tree?

Testing now.  My patch set is actually shrinking (!) thanks to some
merges.  Some rebasing was required to apply your three varlen
patches and three bidi patches, but I'm sure you'll update your tree
and push those upstream soon enough.  I'll take a look at iser bidi
then update my patch list and let you know soon.

		-- Pete

Re: [PATCH 0/3] iscsi bidi & varlen support

[Pete Wyckoff]

bharrosh@panasas.com wrote on Thu, 31 Jan 2008 20:08 +0200:
> Cheers after 1.3 years these can go in.
> 
> [PATCH 1/3] iscsi: extended cdb support
>    The varlen support is not yet in mainline for
>   block and scsi-ml. But the API for drivers will
>   not change. All LLD need to do is max_command to
>   the it's maximum and be ready for bigger commands.
>   This is what's done here. Once these commands start
>   coming iscsi will be ready for them.
> 
> [PATCH 2/3] iscsi: bidi support - libiscsi
> [PATCH 3/3] iscsi: bidi support - iscsi_tcp
>   bidirectional commands support in iscsi.
>   iSER is not yet ready, but it will not break.
>   There is already a mechanism in libiscsi that will
>   return error if bidi commands are sent iSER way.
> 
> Pete please send me the iSER bits so we can port them
> to this latest version.
> 
> Mike these patches are ontop of iscs branch of the iscsi
> git tree, they will apply but for compilation you will need
> to sync with Linus mainline. The patches are for the in-tree
> iscsi code. I own you the compat patch for the out-off-tree
> code, but this I will only be Sunday.
> 
> If we do it fast it might get accepted to 2.6.25 merge window
> 
> Everybody is invited to a party at Shila ben-yhuda 52 Tel-Aviv
> 9:45 pm. Drinks and wonderful see-food on us :)

Here's the patch to add bidi support to iSER too.  It works
with my setup, but could use more testing.  Note that this does
rely on the 3 patches quoted above.

		-- Pete


From: Pete Wyckoff <pw@osc.edu>

Support bidirectional SCSI commands in iSCSI iSER.  The handling of
data buffers for each direction was already mostly separated.  It was
necessary to separate the unmapping of data bufs when one was found
to be unaligned.  This was done by adding a "direction" parameter to
iser_dma_unmap_task_data() and calling it appropriately.  Also
iser_ctask_rdma_finalize() used local variables in such a way that
it assumed a unidirectional command; that was split up to make it
cleaner and order the operations correctly for bidi.

Signed-off-by: Pete Wyckoff <pw@osc.edu>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h     |    3 +-
 drivers/infiniband/ulp/iser/iser_initiator.c |   79 +++++++++++---------------
 drivers/infiniband/ulp/iser/iser_memory.c    |    9 ++-
 3 files changed, 41 insertions(+), 50 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 66905df..f5497b0 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -356,5 +356,6 @@ int iser_dma_map_task_data(struct iscsi_iser_cmd_task *iser_ctask,
 			    enum   iser_data_dir       iser_dir,
 			    enum   dma_data_direction  dma_dir);
 
-void iser_dma_unmap_task_data(struct iscsi_iser_cmd_task *iser_ctask);
+void iser_dma_unmap_task_data(struct iscsi_iser_cmd_task *iser_ctask,
+			      enum iser_data_dir cmd_dir);
 #endif
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index ea3f5dc..491ffab 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -325,9 +325,8 @@ int iser_send_command(struct iscsi_conn     *conn,
 {
 	struct iscsi_iser_conn *iser_conn = conn->dd_data;
 	struct iscsi_iser_cmd_task *iser_ctask = ctask->dd_data;
-	struct iser_dto *send_dto = NULL;
-	unsigned long edtl;
-	int err = 0;
+	struct iser_dto *send_dto;
+	int err;
 	struct iser_data_buf *data_buf;
 
 	struct iscsi_cmd *hdr =  ctask->hdr;
@@ -340,37 +339,32 @@ int iser_send_command(struct iscsi_conn     *conn,
 	if (iser_check_xmit(conn, ctask))
 		return -ENOBUFS;
 
-	edtl = ntohl(hdr->data_length);
-
 	/* build the tx desc regd header and add it to the tx desc dto */
 	iser_ctask->desc.type = ISCSI_TX_SCSI_COMMAND;
 	send_dto = &iser_ctask->desc.dto;
 	send_dto->ctask = iser_ctask;
 	iser_create_send_desc(iser_conn, &iser_ctask->desc, ctask->hdr_len);
 
-	if (hdr->flags & ISCSI_FLAG_CMD_READ)
-		data_buf = &iser_ctask->data[ISER_DIR_IN];
-	else
-		data_buf = &iser_ctask->data[ISER_DIR_OUT];
-
-	if (scsi_sg_count(sc)) { /* using a scatter list */
-		data_buf->buf  = scsi_sglist(sc);
-		data_buf->size = scsi_sg_count(sc);
-	}
-
-	data_buf->data_len = scsi_bufflen(sc);
-
 	if (hdr->flags & ISCSI_FLAG_CMD_READ) {
-		err = iser_prepare_read_cmd(ctask, edtl);
+		data_buf = &iser_ctask->data[ISER_DIR_IN];
+		data_buf->buf  = scsi_in(sc)->table.sgl;
+		data_buf->size = scsi_in(sc)->table.nents;
+		data_buf->data_len = scsi_in(sc)->length;
+		err = iser_prepare_read_cmd(ctask, data_buf->data_len);
 		if (err)
 			goto send_command_error;
 	}
+
 	if (hdr->flags & ISCSI_FLAG_CMD_WRITE) {
+		data_buf = &iser_ctask->data[ISER_DIR_OUT];
+		data_buf->buf  = scsi_out(sc)->table.sgl;
+		data_buf->size = scsi_out(sc)->table.nents;
+		data_buf->data_len = scsi_out(sc)->length;
 		err = iser_prepare_write_cmd(ctask,
 					     ctask->imm_count,
 				             ctask->imm_count +
 					     ctask->unsol_count,
-					     edtl);
+					     data_buf->data_len);
 		if (err)
 			goto send_command_error;
 	}
@@ -648,45 +642,40 @@ void iser_ctask_rdma_init(struct iscsi_iser_cmd_task *iser_ctask)
 	       sizeof(struct iser_regd_buf));
 }
 
-void iser_ctask_rdma_finalize(struct iscsi_iser_cmd_task *iser_ctask)
+static void iser_ctask_rdma_finalize_dir(struct iscsi_iser_cmd_task *iser_ctask,
+				         enum iser_data_dir cmd_dir)
 {
 	int deferred;
 	int is_rdma_aligned = 1;
 	struct iser_regd_buf *regd;
 
-	/* if we were reading, copy back to unaligned sglist,
-	 * anyway dma_unmap and free the copy
+	/*
+	 * If we were reading, copy back to unaligned sglist,
+	 * anyway dma_unmap and free the copy.
 	 */
-	if (iser_ctask->data_copy[ISER_DIR_IN].copy_buf != NULL) {
-		is_rdma_aligned = 0;
-		iser_finalize_rdma_unaligned_sg(iser_ctask, ISER_DIR_IN);
-	}
-	if (iser_ctask->data_copy[ISER_DIR_OUT].copy_buf != NULL) {
+	if (iser_ctask->data_copy[cmd_dir].copy_buf != NULL) {
 		is_rdma_aligned = 0;
-		iser_finalize_rdma_unaligned_sg(iser_ctask, ISER_DIR_OUT);
+		iser_finalize_rdma_unaligned_sg(iser_ctask, cmd_dir);
 	}
 
-	if (iser_ctask->dir[ISER_DIR_IN]) {
-		regd = &iser_ctask->rdma_regd[ISER_DIR_IN];
+	if (iser_ctask->dir[cmd_dir]) {
+		regd = &iser_ctask->rdma_regd[cmd_dir];
 		deferred = iser_regd_buff_release(regd);
-		if (deferred) {
-			iser_err("%d references remain for BUF-IN rdma reg\n",
-				 atomic_read(&regd->ref_count));
-		}
+		if (deferred)
+			iser_err("%d references remain for %s rdma reg\n",
+				 atomic_read(&regd->ref_count),
+				 cmd_dir == ISER_DIR_IN ? "IN" : "OUT");
 	}
 
-	if (iser_ctask->dir[ISER_DIR_OUT]) {
-		regd = &iser_ctask->rdma_regd[ISER_DIR_OUT];
-		deferred = iser_regd_buff_release(regd);
-		if (deferred) {
-			iser_err("%d references remain for BUF-OUT rdma reg\n",
-				 atomic_read(&regd->ref_count));
-		}
-	}
+	/* if the data was unaligned, it was already unmapped and then copied */
+	if (is_rdma_aligned)
+		iser_dma_unmap_task_data(iser_ctask, cmd_dir);
+}
 
-       /* if the data was unaligned, it was already unmapped and then copied */
-       if (is_rdma_aligned)
-		iser_dma_unmap_task_data(iser_ctask);
+void iser_ctask_rdma_finalize(struct iscsi_iser_cmd_task *iser_ctask)
+{
+	iser_ctask_rdma_finalize_dir(iser_ctask, ISER_DIR_IN);
+	iser_ctask_rdma_finalize_dir(iser_ctask, ISER_DIR_OUT);
 }
 
 void iser_dto_buffs_release(struct iser_dto *dto)
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 4a17743..aa3f299 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -393,19 +393,20 @@ int iser_dma_map_task_data(struct iscsi_iser_cmd_task *iser_ctask,
 	return 0;
 }
 
-void iser_dma_unmap_task_data(struct iscsi_iser_cmd_task *iser_ctask)
+void iser_dma_unmap_task_data(struct iscsi_iser_cmd_task *iser_ctask,
+			      enum iser_data_dir cmd_dir)
 {
 	struct ib_device *dev;
 	struct iser_data_buf *data;
 
 	dev = iser_ctask->iser_conn->ib_conn->device->ib_device;
 
-	if (iser_ctask->dir[ISER_DIR_IN]) {
+	if (cmd_dir == ISER_DIR_IN && iser_ctask->dir[ISER_DIR_IN]) {
 		data = &iser_ctask->data[ISER_DIR_IN];
 		ib_dma_unmap_sg(dev, data->buf, data->size, DMA_FROM_DEVICE);
 	}
 
-	if (iser_ctask->dir[ISER_DIR_OUT]) {
+	if (cmd_dir == ISER_DIR_OUT && iser_ctask->dir[ISER_DIR_OUT]) {
 		data = &iser_ctask->data[ISER_DIR_OUT];
 		ib_dma_unmap_sg(dev, data->buf, data->size, DMA_TO_DEVICE);
 	}
@@ -439,7 +440,7 @@ int iser_reg_rdma_mem(struct iscsi_iser_cmd_task *iser_ctask,
 		iser_data_buf_dump(mem, ibdev);
 
 		/* unmap the command data before accessing it */
-		iser_dma_unmap_task_data(iser_ctask);
+		iser_dma_unmap_task_data(iser_ctask, cmd_dir);
 
 		/* allocate copy buf, if we are writing, copy the */
 		/* unaligned scatterlist, dma map the copy        */
-- 
1.5.3.8

Re: [PATCH 0/3] iscsi bidi & varlen support

[Pete Wyckoff]

pw@osc.edu wrote on Tue, 12 Feb 2008 15:12 -0500:
> bharrosh@panasas.com wrote on Thu, 31 Jan 2008 20:08 +0200:
> > Cheers after 1.3 years these can go in.
> > 
> > [PATCH 1/3] iscsi: extended cdb support
> >    The varlen support is not yet in mainline for
> >   block and scsi-ml. But the API for drivers will
> >   not change. All LLD need to do is max_command to
> >   the it's maximum and be ready for bigger commands.
> >   This is what's done here. Once these commands start
> >   coming iscsi will be ready for them.
> > 
> > [PATCH 2/3] iscsi: bidi support - libiscsi
> > [PATCH 3/3] iscsi: bidi support - iscsi_tcp
> >   bidirectional commands support in iscsi.
> >   iSER is not yet ready, but it will not break.
> >   There is already a mechanism in libiscsi that will
> >   return error if bidi commands are sent iSER way.
> > 
> > Pete please send me the iSER bits so we can port them
> > to this latest version.
> > 
> > Mike these patches are ontop of iscs branch of the iscsi
> > git tree, they will apply but for compilation you will need
> > to sync with Linus mainline. The patches are for the in-tree
> > iscsi code. I own you the compat patch for the out-off-tree
> > code, but this I will only be Sunday.
> 
> Here's the patch to add bidi support to iSER too.  It works
> with my setup, but could use more testing.  Note that this does
> rely on the 3 patches quoted above.

Similar, for varlen support to iSER.  Probably apply this one before
the bidi one I just sent.

		-- Pete


From: Pete Wyckoff <pw@osc.edu>
Subject: [PATCH] iscsi iser: varlen

Handle variable-length CDBs in iSER.

Signed-off-by: Pete Wyckoff <pw@osc.edu>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c     |    5 +++--
 drivers/infiniband/ulp/iser/iscsi_iser.h     |    2 +-
 drivers/infiniband/ulp/iser/iser_initiator.c |   16 ++++++++++------
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 5f2284d..9dfc310 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -401,7 +401,8 @@ iscsi_iser_session_create(struct iscsi_transport *iscsit,
 		ctask      = session->cmds[i];
 		iser_ctask = ctask->dd_data;
 		ctask->hdr = (struct iscsi_cmd *)&iser_ctask->desc.iscsi_header;
-		ctask->hdr_max = sizeof(iser_ctask->desc.iscsi_header);
+		ctask->hdr_max = sizeof(iser_ctask->desc.iscsi_header) +
+				 sizeof(iser_ctask->desc.hdrextbuf);
 	}
 
 	for (i = 0; i < session->mgmtpool_max; i++) {
@@ -604,7 +605,7 @@ static struct iscsi_transport iscsi_iser_transport = {
 	.host_template          = &iscsi_iser_sht,
 	.conndata_size		= sizeof(struct iscsi_conn),
 	.max_lun                = ISCSI_ISER_MAX_LUN,
-	.max_cmd_len            = ISCSI_ISER_MAX_CMD_LEN,
+	.max_cmd_len            = 260,
 	/* session management */
 	.create_session         = iscsi_iser_session_create,
 	.destroy_session        = iscsi_session_teardown,
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index db8f81a..66905df 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -90,7 +90,6 @@
 /* FMR space for 1 MB of 4k-page transfers, plus 1 if not page aligned */
 #define ISCSI_ISER_SG_TABLESIZE         (((1<<20) >> SHIFT_4K) + 1)
 #define ISCSI_ISER_MAX_LUN		256
-#define ISCSI_ISER_MAX_CMD_LEN		16
 
 /* QP settings */
 /* Maximal bounds on received asynchronous PDUs */
@@ -217,6 +216,7 @@ enum iser_desc_type {
 struct iser_desc {
 	struct iser_hdr              iser_header;
 	struct iscsi_hdr             iscsi_header;
+	char			     hdrextbuf[ISCSI_MAX_AHS_SIZE];
 	struct iser_regd_buf         hdr_regd_buf;
 	void                         *data;         /* used by RX & TX_CONTROL */
 	struct iser_regd_buf         data_regd_buf; /* used by RX & TX_CONTROL */
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 83247f1..ea3f5dc 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -246,9 +246,13 @@ post_rx_kmalloc_failure:
 	return err;
 }
 
-/* creates a new tx descriptor and adds header regd buffer */
+/**
+ * iser_create_send_desc - creates a new tx descriptor and adds header regd buffer
+ * @iscsi_hdr_len: length of &struct iscsi_hdr and any AHSes in hdrextbuf
+ */
 static void iser_create_send_desc(struct iscsi_iser_conn *iser_conn,
-				  struct iser_desc       *tx_desc)
+				  struct iser_desc       *tx_desc,
+				  int iscsi_hdr_len)
 {
 	struct iser_regd_buf *regd_hdr = &tx_desc->hdr_regd_buf;
 	struct iser_dto      *send_dto = &tx_desc->dto;
@@ -256,7 +260,7 @@ static void iser_create_send_desc(struct iscsi_iser_conn *iser_conn,
 	memset(regd_hdr, 0, sizeof(struct iser_regd_buf));
 	regd_hdr->device  = iser_conn->ib_conn->device;
 	regd_hdr->virt_addr  = tx_desc; /* == &tx_desc->iser_header */
-	regd_hdr->data_size  = ISER_TOTAL_HEADERS_LEN;
+	regd_hdr->data_size  = sizeof(struct iser_hdr) + iscsi_hdr_len;
 
 	send_dto->ib_conn         = iser_conn->ib_conn;
 	send_dto->notify_enable   = 1;
@@ -342,7 +346,7 @@ int iser_send_command(struct iscsi_conn     *conn,
 	iser_ctask->desc.type = ISCSI_TX_SCSI_COMMAND;
 	send_dto = &iser_ctask->desc.dto;
 	send_dto->ctask = iser_ctask;
-	iser_create_send_desc(iser_conn, &iser_ctask->desc);
+	iser_create_send_desc(iser_conn, &iser_ctask->desc, ctask->hdr_len);
 
 	if (hdr->flags & ISCSI_FLAG_CMD_READ)
 		data_buf = &iser_ctask->data[ISER_DIR_IN];
@@ -435,7 +439,7 @@ int iser_send_data_out(struct iscsi_conn     *conn,
 	/* build the tx desc regd header and add it to the tx desc dto */
 	send_dto = &tx_desc->dto;
 	send_dto->ctask = iser_ctask;
-	iser_create_send_desc(iser_conn, tx_desc);
+	iser_create_send_desc(iser_conn, tx_desc, ctask->hdr_len);
 
 	iser_reg_single(iser_conn->ib_conn->device,
 			send_dto->regd[0], DMA_TO_DEVICE);
@@ -492,7 +496,7 @@ int iser_send_control(struct iscsi_conn *conn,
 	mdesc->type = ISCSI_TX_CONTROL;
 	send_dto = &mdesc->dto;
 	send_dto->ctask = NULL;
-	iser_create_send_desc(iser_conn, mdesc);
+	iser_create_send_desc(iser_conn, mdesc, sizeof(struct iscsi_hdr));
 
 	device = iser_conn->ib_conn->device;
 
-- 
1.5.3.8

[PATCH 0/3 ver2] iscsi bidi & varlen support

[Boaz Harrosh]

On Thu, Jan 31 2008 at 20:08 +0200, Boaz Harrosh <bharrosh@panasas.com> wrote:
> Cheers after 1.3 years these can go in.
> 
> [PATCH 1/3] iscsi: extended cdb support
>    The varlen support is not yet in mainline for
>   block and scsi-ml. But the API for drivers will
>   not change. All LLD need to do is max_command to
>   the it's maximum and be ready for bigger commands.
>   This is what's done here. Once these commands start
>   coming iscsi will be ready for them.
> 
> [PATCH 2/3] iscsi: bidi support - libiscsi
> [PATCH 3/3] iscsi: bidi support - iscsi_tcp
>   bidirectional commands support in iscsi.
>   iSER is not yet ready, but it will not break.
>   There is already a mechanism in libiscsi that will
>   return error if bidi commands are sent iSER way.
> 
> Pete please send me the iSER bits so we can port them
> to this latest version.
> 
> Mike these patches are ontop of iscs branch of the iscsi
> git tree, they will apply but for compilation you will need
> to sync with Linus mainline. The patches are for the in-tree
> iscsi code. I own you the compat patch for the out-off-tree
> code, but this I will only be Sunday.
> 
> If we do it fast it might get accepted to 2.6.25 merge window
> 
> Everybody is invited to a party at Shila ben-yhuda 52 Tel-Aviv
> 9:45 pm. Drinks and wonderful see-food on us :)
> 
> Boaz
>  
> -
Everything the same as before. But working this time. Also
Pete's comment about second patch, was correct and code is now
fixed.

I have got Mike's Signed-off-by, on these they were tested and
approved by him. So they are for scsi-misc. But ... James? is
there any chance these can go into scsi-rc-fixes for the 2.6.25
kernel? The reason they are so late was mainly because of a fallout
in the merge process and a bug that was introduced because of that,
but they were intended to go together with bidi into 2.6.25. Also
as an important client code to the bidi-api that is introduced in
2.6.25 kernel.

Thanks
Boaz

[PATCH 1/3 ver2] iscsi: extended cdb support

[Boaz Harrosh]

Support for extended CDBs in iscsi.
All we need is to check if command spills over 16 bytes then allocate
an iscsi-extended-header for the leftovers.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
Reviewed-by: Pete Wyckoff <pw@osc.edu>
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/libiscsi.c    |   55 ++++++++++++++++++++++++++++++++++++++++----
 include/scsi/iscsi_proto.h |    6 +++-
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 59f8445..a43b8ee 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -137,6 +137,45 @@ static int iscsi_add_hdr(struct iscsi_cmd_task *ctask, unsigned len)
 	return 0;
 }
 
+/*
+ * make an extended cdb AHS
+ */
+static int iscsi_prep_ecdb_ahs(struct iscsi_cmd_task *ctask)
+{
+	struct scsi_cmnd *cmd = ctask->sc;
+	unsigned rlen, pad_len;
+	unsigned short ahslength;
+	struct iscsi_ecdb_ahdr *ecdb_ahdr;
+	int rc;
+
+	ecdb_ahdr = iscsi_next_hdr(ctask);
+	rlen = cmd->cmd_len - ISCSI_CDB_SIZE;
+
+	BUG_ON(rlen > sizeof(ecdb_ahdr->ecdb));
+	ahslength = rlen + sizeof(ecdb_ahdr->reserved);
+
+	pad_len = iscsi_padding(rlen);
+
+	rc = iscsi_add_hdr(ctask, sizeof(ecdb_ahdr->ahslength) +
+	                   sizeof(ecdb_ahdr->ahstype) + ahslength + pad_len);
+	if (rc)
+		return rc;
+
+	if (pad_len)
+		memset(&ecdb_ahdr->ecdb[rlen], 0, pad_len);
+
+	ecdb_ahdr->ahslength = cpu_to_be16(ahslength);
+	ecdb_ahdr->ahstype = ISCSI_AHSTYPE_CDB;
+	ecdb_ahdr->reserved = 0;
+	memcpy(ecdb_ahdr->ecdb, cmd->cmnd + ISCSI_CDB_SIZE, rlen);
+
+	debug_scsi("iscsi_prep_ecdb_ahs: varlen_cdb_len %d "
+		   "rlen %d pad_len %d ahs_length %d iscsi_headers_size %u\n",
+		   cmd->cmd_len, rlen, pad_len, ahslength, ctask->hdr_len);
+
+	return 0;
+}
+
 /**
  * iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
  * @ctask: iscsi cmd task
@@ -150,7 +189,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	struct iscsi_session *session = conn->session;
 	struct iscsi_cmd *hdr = ctask->hdr;
 	struct scsi_cmnd *sc = ctask->sc;
-	unsigned hdrlength;
+	unsigned hdrlength, cmd_len;
 	int rc;
 
 	ctask->hdr_len = 0;
@@ -165,10 +204,16 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	hdr->cmdsn = cpu_to_be32(session->cmdsn);
 	session->cmdsn++;
 	hdr->exp_statsn = cpu_to_be32(conn->exp_statsn);
-	memcpy(hdr->cdb, sc->cmnd, sc->cmd_len);
-	if (sc->cmd_len < MAX_COMMAND_SIZE)
-		memset(&hdr->cdb[sc->cmd_len], 0,
-			MAX_COMMAND_SIZE - sc->cmd_len);
+	cmd_len = sc->cmd_len;
+	if (cmd_len < ISCSI_CDB_SIZE)
+		memset(&hdr->cdb[cmd_len], 0, ISCSI_CDB_SIZE - cmd_len);
+	else if (cmd_len > ISCSI_CDB_SIZE) {
+		rc = iscsi_prep_ecdb_ahs(ctask);
+		if (rc)
+			return rc;
+		cmd_len = ISCSI_CDB_SIZE;
+	}
+	memcpy(hdr->cdb, sc->cmnd, cmd_len);
 
 	ctask->imm_count = 0;
 	if (sc->sc_data_direction == DMA_TO_DEVICE) {
diff --git a/include/scsi/iscsi_proto.h b/include/scsi/iscsi_proto.h
index 5ffec8a..e0593bf 100644
--- a/include/scsi/iscsi_proto.h
+++ b/include/scsi/iscsi_proto.h
@@ -112,6 +112,7 @@ struct iscsi_ahs_hdr {
 
 #define ISCSI_AHSTYPE_CDB		1
 #define ISCSI_AHSTYPE_RLENGTH		2
+#define ISCSI_CDB_SIZE			16
 
 /* iSCSI PDU Header */
 struct iscsi_cmd {
@@ -125,7 +126,7 @@ struct iscsi_cmd {
 	__be32 data_length;
 	__be32 cmdsn;
 	__be32 exp_statsn;
-	uint8_t cdb[16];	/* SCSI Command Block */
+	uint8_t cdb[ISCSI_CDB_SIZE];	/* SCSI Command Block */
 	/* Additional Data (Command Dependent) */
 };
 
@@ -154,7 +155,8 @@ struct iscsi_ecdb_ahdr {
 	__be16 ahslength;	/* CDB length - 15, including reserved byte */
 	uint8_t ahstype;
 	uint8_t reserved;
-	uint8_t ecdb[260 - 16];	/* 4-byte aligned extended CDB spillover */
+	/* 4-byte aligned extended CDB spillover */
+	uint8_t ecdb[260 - ISCSI_CDB_SIZE];
 };
 
 /* SCSI Response Header */
-- 
1.5.3.3

[PATCH 2/3 ver2] iscsi: bidi support - libiscsi

[Boaz Harrosh]

  iscsi bidi support at the generic libiscsi level
  - prepare the additional bidi_read rlength header.
  - access the right scsi_in() and/or scsi_out() side of things.
    also for resid.
  - Handle BIDI underflow overflow from target

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
Reviewed-by: Pete Wyckoff <pw@osc.edu>
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/libiscsi.c |   85 ++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index a43b8ee..9c12915 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -176,6 +176,31 @@ static int iscsi_prep_ecdb_ahs(struct iscsi_cmd_task *ctask)
 	return 0;
 }
 
+static int iscsi_prep_bidi_ahs(struct iscsi_cmd_task *ctask)
+{
+	struct scsi_cmnd *sc = ctask->sc;
+	struct iscsi_rlength_ahdr *rlen_ahdr;
+	int rc;
+
+	rlen_ahdr = iscsi_next_hdr(ctask);
+	rc = iscsi_add_hdr(ctask, sizeof(*rlen_ahdr));
+	if (rc)
+		return rc;
+
+	rlen_ahdr->ahslength =
+		cpu_to_be16(sizeof(rlen_ahdr->read_length) +
+						  sizeof(rlen_ahdr->reserved));
+	rlen_ahdr->ahstype = ISCSI_AHSTYPE_RLENGTH;
+	rlen_ahdr->reserved = 0;
+	rlen_ahdr->read_length = cpu_to_be32(scsi_in(sc)->length);
+
+	debug_scsi("bidi-in rlen_ahdr->read_length(%d) "
+		   "rlen_ahdr->ahslength(%d)\n",
+		   be32_to_cpu(rlen_ahdr->read_length),
+		   be16_to_cpu(rlen_ahdr->ahslength));
+	return 0;
+}
+
 /**
  * iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
  * @ctask: iscsi cmd task
@@ -200,7 +225,6 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	hdr->flags = ISCSI_ATTR_SIMPLE;
 	int_to_scsilun(sc->device->lun, (struct scsi_lun *)hdr->lun);
 	hdr->itt = build_itt(ctask->itt, session->age);
-	hdr->data_length = cpu_to_be32(scsi_bufflen(sc));
 	hdr->cmdsn = cpu_to_be32(session->cmdsn);
 	session->cmdsn++;
 	hdr->exp_statsn = cpu_to_be32(conn->exp_statsn);
@@ -216,7 +240,15 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	memcpy(hdr->cdb, sc->cmnd, cmd_len);
 
 	ctask->imm_count = 0;
+	if (scsi_bidi_cmnd(sc)) {
+		hdr->flags |= ISCSI_FLAG_CMD_READ;
+		rc = iscsi_prep_bidi_ahs(ctask);
+		if (rc)
+			return rc;
+	}
 	if (sc->sc_data_direction == DMA_TO_DEVICE) {
+		unsigned out_len = scsi_out(sc)->length;
+		hdr->data_length = cpu_to_be32(out_len);
 		hdr->flags |= ISCSI_FLAG_CMD_WRITE;
 		/*
 		 * Write counters:
@@ -237,19 +269,19 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 		ctask->unsol_datasn = 0;
 
 		if (session->imm_data_en) {
-			if (scsi_bufflen(sc) >= session->first_burst)
+			if (out_len >= session->first_burst)
 				ctask->imm_count = min(session->first_burst,
 							conn->max_xmit_dlength);
 			else
-				ctask->imm_count = min(scsi_bufflen(sc),
+				ctask->imm_count = min(out_len,
 							conn->max_xmit_dlength);
 			hton24(hdr->dlength, ctask->imm_count);
 		} else
 			zero_data(hdr->dlength);
 
 		if (!session->initial_r2t_en) {
-			ctask->unsol_count = min((session->first_burst),
-				(scsi_bufflen(sc))) - ctask->imm_count;
+			ctask->unsol_count = min(session->first_burst, out_len)
+							     - ctask->imm_count;
 			ctask->unsol_offset = ctask->imm_count;
 		}
 
@@ -259,6 +291,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	} else {
 		hdr->flags |= ISCSI_FLAG_CMD_FINAL;
 		zero_data(hdr->dlength);
+		hdr->data_length = cpu_to_be32(scsi_in(sc)->length);
 
 		if (sc->sc_data_direction == DMA_FROM_DEVICE)
 			hdr->flags |= ISCSI_FLAG_CMD_READ;
@@ -277,10 +310,12 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 		return EIO;
 
 	conn->scsicmd_pdus_cnt++;
-	debug_scsi("iscsi prep [%s cid %d sc %p cdb 0x%x itt 0x%x len %d "
-		"cmdsn %d win %d]\n",
-		sc->sc_data_direction == DMA_TO_DEVICE ? "write" : "read",
-		conn->id, sc, sc->cmnd[0], ctask->itt, scsi_bufflen(sc),
+	debug_scsi("iscsi prep [%s cid %d sc %p cdb 0x%x itt 0x%x "
+		"len %d bidi_len %d cmdsn %d win %d]\n",
+		scsi_bidi_cmnd(sc) ? "bidirectional" :
+		     sc->sc_data_direction == DMA_TO_DEVICE ? "write" : "read",
+		conn->id, sc, sc->cmnd[0], ctask->itt,
+		scsi_bufflen(sc), scsi_bidi_cmnd(sc) ? scsi_in(sc)->length : 0,
 		session->cmdsn, session->max_cmdsn - session->exp_cmdsn + 1);
 	return 0;
 }
@@ -343,7 +378,12 @@ static void fail_command(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask,
 		conn->session->tt->cleanup_cmd_task(conn, ctask);
 
 	sc->result = err;
-	scsi_set_resid(sc, scsi_bufflen(sc));
+	if (!scsi_bidi_cmnd(sc))
+		scsi_set_resid(sc, scsi_bufflen(sc));
+	else {
+		scsi_out(sc)->resid = scsi_out(sc)->length;
+		scsi_in(sc)->resid = scsi_in(sc)->length;
+	}
 	if (conn->ctask == ctask)
 		conn->ctask = NULL;
 	/* release ref from queuecommand */
@@ -478,6 +518,18 @@ invalid_datalen:
 			   min_t(uint16_t, senselen, SCSI_SENSE_BUFFERSIZE));
 	}
 
+	if (rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW |
+			   ISCSI_FLAG_CMD_BIDI_OVERFLOW)) {
+		int res_count = be32_to_cpu(rhdr->bi_residual_count);
+
+		if (scsi_bidi_cmnd(sc) && res_count > 0 &&
+				(rhdr->flags & ISCSI_FLAG_CMD_BIDI_OVERFLOW ||
+				 res_count <= scsi_in(sc)->length))
+			scsi_in(sc)->resid = res_count;
+		else
+			sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
+	}
+
 	if (rhdr->flags & (ISCSI_FLAG_CMD_UNDERFLOW |
 	                   ISCSI_FLAG_CMD_OVERFLOW)) {
 		int res_count = be32_to_cpu(rhdr->residual_count);
@@ -485,13 +537,11 @@ invalid_datalen:
 		if (res_count > 0 &&
 		    (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
 		     res_count <= scsi_bufflen(sc)))
+			/* write side for bidi or uni-io set_resid */
 			scsi_set_resid(sc, res_count);
 		else
 			sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
-	} else if (rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW |
-	                          ISCSI_FLAG_CMD_BIDI_OVERFLOW))
-		sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
-
+	}
 out:
 	debug_scsi("done [sc %lx res %d itt 0x%x]\n",
 		   (long)sc, sc->result, ctask->itt);
@@ -1147,7 +1197,12 @@ reject:
 fault:
 	spin_unlock(&session->lock);
 	debug_scsi("iscsi: cmd 0x%x is not queued (%d)\n", sc->cmnd[0], reason);
-	scsi_set_resid(sc, scsi_bufflen(sc));
+	if (!scsi_bidi_cmnd(sc))
+		scsi_set_resid(sc, scsi_bufflen(sc));
+	else {
+		scsi_out(sc)->resid = scsi_out(sc)->length;
+		scsi_in(sc)->resid = scsi_in(sc)->length;
+	}
 	sc->scsi_done(sc);
 	spin_lock(host->host_lock);
 	return 0;
-- 
1.5.3.3

[PATCH 3/3 ver2] iscsi: bidi support - iscsi_tcp

[Boaz Harrosh]

  bidi support for iscsi_tcp
  - access the right scsi_in() and/or scsi_out() side of things.
    also for resid

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
Reviewed-by: Pete Wyckoff <pw@osc.edu>
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/iscsi_tcp.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 8a17867..72b9b2a 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -528,6 +528,7 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 	struct iscsi_session *session = conn->session;
 	struct scsi_cmnd *sc = ctask->sc;
 	int datasn = be32_to_cpu(rhdr->datasn);
+	unsigned total_in_length = scsi_in(sc)->length;
 
 	iscsi_update_cmdsn(session, (struct iscsi_nopin*)rhdr);
 	if (tcp_conn->in.datalen == 0)
@@ -542,10 +543,10 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 	tcp_ctask->exp_datasn++;
 
 	tcp_ctask->data_offset = be32_to_cpu(rhdr->offset);
-	if (tcp_ctask->data_offset + tcp_conn->in.datalen > scsi_bufflen(sc)) {
+	if (tcp_ctask->data_offset + tcp_conn->in.datalen > total_in_length) {
 		debug_tcp("%s: data_offset(%d) + data_len(%d) > total_length_in(%d)\n",
 		          __FUNCTION__, tcp_ctask->data_offset,
-		          tcp_conn->in.datalen, scsi_bufflen(sc));
+		          tcp_conn->in.datalen, total_in_length);
 		return ISCSI_ERR_DATA_OFFSET;
 	}
 
@@ -558,8 +559,8 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 
 			if (res_count > 0 &&
 			    (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
-			     res_count <= scsi_bufflen(sc)))
-				scsi_set_resid(sc, res_count);
+			     res_count <= total_in_length))
+				scsi_in(sc)->resid = res_count;
 			else
 				sc->result = (DID_BAD_TARGET << 16) |
 					rhdr->cmd_status;
@@ -670,11 +671,11 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 			    r2t->data_length, session->max_burst);
 
 	r2t->data_offset = be32_to_cpu(rhdr->data_offset);
-	if (r2t->data_offset + r2t->data_length > scsi_bufflen(ctask->sc)) {
+	if (r2t->data_offset + r2t->data_length > scsi_out(ctask->sc)->length) {
 		iscsi_conn_printk(KERN_ERR, conn,
 				  "invalid R2T with data len %u at offset %u "
 				  "and total length %d\n", r2t->data_length,
-				  r2t->data_offset, scsi_bufflen(ctask->sc));
+				  r2t->data_offset, scsi_out(ctask->sc)->length);
 		__kfifo_put(tcp_ctask->r2tpool.queue, (void*)&r2t,
 			    sizeof(void*));
 		return ISCSI_ERR_DATALEN;
@@ -771,6 +772,7 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 		if (tcp_conn->in.datalen) {
 			struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
 			struct hash_desc *rx_hash = NULL;
+			struct scsi_data_buffer *sdb = scsi_in(ctask->sc);
 
 			/*
 			 * Setup copy of Data-In into the Scsi_Cmnd
@@ -788,8 +790,8 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 				  tcp_ctask->data_offset,
 				  tcp_conn->in.datalen);
 			return iscsi_segment_seek_sg(&tcp_conn->in.segment,
-						     scsi_sglist(ctask->sc),
-						     scsi_sg_count(ctask->sc),
+						     sdb->table.sgl,
+						     sdb->table.nents,
 						     tcp_ctask->data_offset,
 						     tcp_conn->in.datalen,
 						     iscsi_tcp_process_data_in,
@@ -1332,7 +1334,8 @@ iscsi_tcp_ctask_init(struct iscsi_cmd_task *ctask)
 		return 0;
 
 	/* If we have immediate data, attach a payload */
-	err = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc), scsi_sg_count(sc),
+	err = iscsi_tcp_send_data_prep(conn, scsi_out(sc)->table.sgl,
+				       scsi_out(sc)->table.nents,
 				       0, ctask->imm_count);
 	if (err)
 		return err;
@@ -1386,6 +1389,7 @@ iscsi_tcp_ctask_xmit(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 {
 	struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
 	struct scsi_cmnd *sc = ctask->sc;
+	struct scsi_data_buffer *sdb = scsi_out(sc);
 	int rc = 0;
 
 flush:
@@ -1412,9 +1416,8 @@ flush:
 				ctask->itt, tcp_ctask->sent, ctask->data_count);
 
 		iscsi_tcp_send_hdr_prep(conn, hdr, sizeof(*hdr));
-		rc = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc),
-					      scsi_sg_count(sc),
-					      tcp_ctask->sent,
+		rc = iscsi_tcp_send_data_prep(conn, sdb->table.sgl,
+					      sdb->table.nents, tcp_ctask->sent,
 					      ctask->data_count);
 		if (rc)
 			goto fail;
@@ -1460,8 +1463,8 @@ flush:
 		iscsi_tcp_send_hdr_prep(conn, &r2t->dtask.hdr,
 					sizeof(struct iscsi_hdr));
 
-		rc = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc),
-					      scsi_sg_count(sc),
+		rc = iscsi_tcp_send_data_prep(conn, sdb->table.sgl,
+					      sdb->table.nents,
 					      r2t->data_offset + r2t->sent,
 					      r2t->data_count);
 		if (rc)
-- 
1.5.3.3

Re: [PATCH 0/3] iscsi bidi & varlen support

[Boaz Harrosh]

On Tue, Feb 12 2008 at 22:17 +0200, Pete Wyckoff <pw@osc.edu> wrote:
> pw@osc.edu wrote on Tue, 12 Feb 2008 15:12 -0500:
>> bharrosh@panasas.com wrote on Thu, 31 Jan 2008 20:08 +0200:
>>> Cheers after 1.3 years these can go in.
>>>
>>> [PATCH 1/3] iscsi: extended cdb support
>>>    The varlen support is not yet in mainline for
>>>   block and scsi-ml. But the API for drivers will
>>>   not change. All LLD need to do is max_command to
>>>   the it's maximum and be ready for bigger commands.
>>>   This is what's done here. Once these commands start
>>>   coming iscsi will be ready for them.
>>>
>>> [PATCH 2/3] iscsi: bidi support - libiscsi
>>> [PATCH 3/3] iscsi: bidi support - iscsi_tcp
>>>   bidirectional commands support in iscsi.
>>>   iSER is not yet ready, but it will not break.
>>>   There is already a mechanism in libiscsi that will
>>>   return error if bidi commands are sent iSER way.
>>>
>>> Pete please send me the iSER bits so we can port them
>>> to this latest version.
>>>
>>> Mike these patches are ontop of iscs branch of the iscsi
>>> git tree, they will apply but for compilation you will need
>>> to sync with Linus mainline. The patches are for the in-tree
>>> iscsi code. I own you the compat patch for the out-off-tree
>>> code, but this I will only be Sunday.
>> Here's the patch to add bidi support to iSER too.  It works
>> with my setup, but could use more testing.  Note that this does
>> rely on the 3 patches quoted above.
> 
> Similar, for varlen support to iSER.  Probably apply this one before
> the bidi one I just sent.
> 
> 		-- Pete
> 
> 
> From: Pete Wyckoff <pw@osc.edu>
> Subject: [PATCH] iscsi iser: varlen
> 
> Handle variable-length CDBs in iSER.
> 
> Signed-off-by: Pete Wyckoff <pw@osc.edu>
> ---
>  drivers/infiniband/ulp/iser/iscsi_iser.c     |    5 +++--
>  drivers/infiniband/ulp/iser/iscsi_iser.h     |    2 +-
>  drivers/infiniband/ulp/iser/iser_initiator.c |   16 ++++++++++------
>  3 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
> index 5f2284d..9dfc310 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> @@ -401,7 +401,8 @@ iscsi_iser_session_create(struct iscsi_transport *iscsit,
>  		ctask      = session->cmds[i];
>  		iser_ctask = ctask->dd_data;
>  		ctask->hdr = (struct iscsi_cmd *)&iser_ctask->desc.iscsi_header;
> -		ctask->hdr_max = sizeof(iser_ctask->desc.iscsi_header);
> +		ctask->hdr_max = sizeof(iser_ctask->desc.iscsi_header) +
> +				 sizeof(iser_ctask->desc.hdrextbuf);
>  	}
>  
>  	for (i = 0; i < session->mgmtpool_max; i++) {
> @@ -604,7 +605,7 @@ static struct iscsi_transport iscsi_iser_transport = {
>  	.host_template          = &iscsi_iser_sht,
>  	.conndata_size		= sizeof(struct iscsi_conn),
>  	.max_lun                = ISCSI_ISER_MAX_LUN,
> -	.max_cmd_len            = ISCSI_ISER_MAX_CMD_LEN,
> +	.max_cmd_len            = 260,

Same bug I had. .max_cmd_len is still char, before the varlen patch to scsi-ml.
So it must be at most 252, Until that patch is introduced and it can return to
the correct 260 or better yet SCSI_MAX_VARLEN_CDB_SIZE. That also is only 
defined in the scsi-ml varlen patch.

>  	/* session management */
>  	.create_session         = iscsi_iser_session_create,
>  	.destroy_session        = iscsi_session_teardown,
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
> index db8f81a..66905df 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.h
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
> @@ -90,7 +90,6 @@
>  /* FMR space for 1 MB of 4k-page transfers, plus 1 if not page aligned */
>  #define ISCSI_ISER_SG_TABLESIZE         (((1<<20) >> SHIFT_4K) + 1)
>  #define ISCSI_ISER_MAX_LUN		256
> -#define ISCSI_ISER_MAX_CMD_LEN		16
>  
>  /* QP settings */
>  /* Maximal bounds on received asynchronous PDUs */
> @@ -217,6 +216,7 @@ enum iser_desc_type {
>  struct iser_desc {
>  	struct iser_hdr              iser_header;
>  	struct iscsi_hdr             iscsi_header;
> +	char			     hdrextbuf[ISCSI_MAX_AHS_SIZE];
>  	struct iser_regd_buf         hdr_regd_buf;
>  	void                         *data;         /* used by RX & TX_CONTROL */
>  	struct iser_regd_buf         data_regd_buf; /* used by RX & TX_CONTROL */
> diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
> index 83247f1..ea3f5dc 100644
> --- a/drivers/infiniband/ulp/iser/iser_initiator.c
> +++ b/drivers/infiniband/ulp/iser/iser_initiator.c
> @@ -246,9 +246,13 @@ post_rx_kmalloc_failure:
>  	return err;
>  }
>  
> -/* creates a new tx descriptor and adds header regd buffer */
> +/**
> + * iser_create_send_desc - creates a new tx descriptor and adds header regd buffer
> + * @iscsi_hdr_len: length of &struct iscsi_hdr and any AHSes in hdrextbuf
> + */
>  static void iser_create_send_desc(struct iscsi_iser_conn *iser_conn,
> -				  struct iser_desc       *tx_desc)
> +				  struct iser_desc       *tx_desc,
> +				  int iscsi_hdr_len)
>  {
>  	struct iser_regd_buf *regd_hdr = &tx_desc->hdr_regd_buf;
>  	struct iser_dto      *send_dto = &tx_desc->dto;
> @@ -256,7 +260,7 @@ static void iser_create_send_desc(struct iscsi_iser_conn *iser_conn,
>  	memset(regd_hdr, 0, sizeof(struct iser_regd_buf));
>  	regd_hdr->device  = iser_conn->ib_conn->device;
>  	regd_hdr->virt_addr  = tx_desc; /* == &tx_desc->iser_header */
> -	regd_hdr->data_size  = ISER_TOTAL_HEADERS_LEN;
> +	regd_hdr->data_size  = sizeof(struct iser_hdr) + iscsi_hdr_len;
>  
>  	send_dto->ib_conn         = iser_conn->ib_conn;
>  	send_dto->notify_enable   = 1;
> @@ -342,7 +346,7 @@ int iser_send_command(struct iscsi_conn     *conn,
>  	iser_ctask->desc.type = ISCSI_TX_SCSI_COMMAND;
>  	send_dto = &iser_ctask->desc.dto;
>  	send_dto->ctask = iser_ctask;
> -	iser_create_send_desc(iser_conn, &iser_ctask->desc);
> +	iser_create_send_desc(iser_conn, &iser_ctask->desc, ctask->hdr_len);
>  
>  	if (hdr->flags & ISCSI_FLAG_CMD_READ)
>  		data_buf = &iser_ctask->data[ISER_DIR_IN];
> @@ -435,7 +439,7 @@ int iser_send_data_out(struct iscsi_conn     *conn,
>  	/* build the tx desc regd header and add it to the tx desc dto */
>  	send_dto = &tx_desc->dto;
>  	send_dto->ctask = iser_ctask;
> -	iser_create_send_desc(iser_conn, tx_desc);
> +	iser_create_send_desc(iser_conn, tx_desc, ctask->hdr_len);
>  
>  	iser_reg_single(iser_conn->ib_conn->device,
>  			send_dto->regd[0], DMA_TO_DEVICE);
> @@ -492,7 +496,7 @@ int iser_send_control(struct iscsi_conn *conn,
>  	mdesc->type = ISCSI_TX_CONTROL;
>  	send_dto = &mdesc->dto;
>  	send_dto->ctask = NULL;
> -	iser_create_send_desc(iser_conn, mdesc);
> +	iser_create_send_desc(iser_conn, mdesc, sizeof(struct iscsi_hdr));
>  
>  	device = iser_conn->ib_conn->device;
>  

I'm afraid the varlen patches to block and scsi-ml are waiting because of
me. There are more things I need to check, before they can get approved.

Once I do that, and varlen gets accepted, iSER and iscsi_tcp can go up 
to 260 for the .max_cmd_len as they should.

Boaz
 

Re: [PATCH 0/3] iscsi bidi & varlen support

[Pete Wyckoff]

bharrosh@panasas.com wrote on Mon, 18 Feb 2008 17:39 +0200:
> On Tue, Feb 12 2008 at 22:17 +0200, Pete Wyckoff <pw@osc.edu> wrote:
> > From: Pete Wyckoff <pw@osc.edu>
> > Subject: [PATCH] iscsi iser: varlen
> > 
> > Handle variable-length CDBs in iSER.
> > 
> > Signed-off-by: Pete Wyckoff <pw@osc.edu>
> > ---
> >  drivers/infiniband/ulp/iser/iscsi_iser.c     |    5 +++--
> >  drivers/infiniband/ulp/iser/iscsi_iser.h     |    2 +-
> >  drivers/infiniband/ulp/iser/iser_initiator.c |   16 ++++++++++------
> >  3 files changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
> > index 5f2284d..9dfc310 100644
> > --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> > +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> > @@ -401,7 +401,8 @@ iscsi_iser_session_create(struct iscsi_transport *iscsit,
> >  		ctask      = session->cmds[i];
> >  		iser_ctask = ctask->dd_data;
> >  		ctask->hdr = (struct iscsi_cmd *)&iser_ctask->desc.iscsi_header;
> > -		ctask->hdr_max = sizeof(iser_ctask->desc.iscsi_header);
> > +		ctask->hdr_max = sizeof(iser_ctask->desc.iscsi_header) +
> > +				 sizeof(iser_ctask->desc.hdrextbuf);
> >  	}
> >  
> >  	for (i = 0; i < session->mgmtpool_max; i++) {
> > @@ -604,7 +605,7 @@ static struct iscsi_transport iscsi_iser_transport = {
> >  	.host_template          = &iscsi_iser_sht,
> >  	.conndata_size		= sizeof(struct iscsi_conn),
> >  	.max_lun                = ISCSI_ISER_MAX_LUN,
> > -	.max_cmd_len            = ISCSI_ISER_MAX_CMD_LEN,
> > +	.max_cmd_len            = 260,
> 
> Same bug I had. .max_cmd_len is still char, before the varlen patch to scsi-ml.
> So it must be at most 252, Until that patch is introduced and it can return to
> the correct 260 or better yet SCSI_MAX_VARLEN_CDB_SIZE. That also is only 
> defined in the scsi-ml varlen patch.

Ah, that is unfortunate.

> I'm afraid the varlen patches to block and scsi-ml are waiting because of
> me. There are more things I need to check, before they can get approved.
> 
> Once I do that, and varlen gets accepted, iSER and iscsi_tcp can go up 
> to 260 for the .max_cmd_len as they should.

I will sit on these iser changes until we get core varlen resolved,
then.  Or you can just sequence it all cleverly through the various
maintainers.

		-- Pete

Re: [PATCH 0/3 ver2] iscsi bidi & varlen support

[James Bottomley]

On Mon, 2008-02-18 at 17:08 +0200, Boaz Harrosh wrote:
> But ... James? is
> there any chance these can go into scsi-rc-fixes for the 2.6.25
> kernel? The reason they are so late was mainly because of a fallout
> in the merge process and a bug that was introduced because of that,
> but they were intended to go together with bidi into 2.6.25. Also
> as an important client code to the bidi-api that is introduced in
> 2.6.25 kernel.

Well, I think you know the answer to that one under Linus' rules for non
merge window submission.  It's not a bug fix; we haven't even put it
into -mm for testing and it's a pretty invasive change.

James

Re: [PATCH 0/3 ver2] iscsi bidi & varlen support

[Boaz Harrosh]

On Mon, Feb 18 2008 at 19:22 +0200, James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> On Mon, 2008-02-18 at 17:08 +0200, Boaz Harrosh wrote:
>> But ... James? is
>> there any chance these can go into scsi-rc-fixes for the 2.6.25
>> kernel? The reason they are so late was mainly because of a fallout
>> in the merge process and a bug that was introduced because of that,
>> but they were intended to go together with bidi into 2.6.25. Also
>> as an important client code to the bidi-api that is introduced in
>> 2.6.25 kernel.
> 
> Well, I think you know the answer to that one under Linus' rules for non
> merge window submission.  It's not a bug fix; we haven't even put it
> into -mm for testing and it's a pretty invasive change.
> 
> James
> 
> 

It was extensively tested by all iscsi people. It has the Sign-off-by
of the iscsi maintainer. They are not new patches.

But, yes you are right. I now remember the trouble we had with Linus
last time. So it's 2.6.26 then. :-( . People that need it for 2.6.25
will just get it off the git tree.

Boaz

[PATCH 3/3] iscsi: bidi support - iscsi_tcp

[michaelc]

From: Boaz Harrosh <bharrosh@panasas.com>

  bidi support for iscsi_tcp
  - access the right scsi_in() and/or scsi_out() side of things.
    also for resid

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
Reviewed-by: Pete Wyckoff <pw@osc.edu>
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/iscsi_tcp.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 8a17867..72b9b2a 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -528,6 +528,7 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 	struct iscsi_session *session = conn->session;
 	struct scsi_cmnd *sc = ctask->sc;
 	int datasn = be32_to_cpu(rhdr->datasn);
+	unsigned total_in_length = scsi_in(sc)->length;
 
 	iscsi_update_cmdsn(session, (struct iscsi_nopin*)rhdr);
 	if (tcp_conn->in.datalen == 0)
@@ -542,10 +543,10 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 	tcp_ctask->exp_datasn++;
 
 	tcp_ctask->data_offset = be32_to_cpu(rhdr->offset);
-	if (tcp_ctask->data_offset + tcp_conn->in.datalen > scsi_bufflen(sc)) {
+	if (tcp_ctask->data_offset + tcp_conn->in.datalen > total_in_length) {
 		debug_tcp("%s: data_offset(%d) + data_len(%d) > total_length_in(%d)\n",
 		          __FUNCTION__, tcp_ctask->data_offset,
-		          tcp_conn->in.datalen, scsi_bufflen(sc));
+		          tcp_conn->in.datalen, total_in_length);
 		return ISCSI_ERR_DATA_OFFSET;
 	}
 
@@ -558,8 +559,8 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 
 			if (res_count > 0 &&
 			    (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
-			     res_count <= scsi_bufflen(sc)))
-				scsi_set_resid(sc, res_count);
+			     res_count <= total_in_length))
+				scsi_in(sc)->resid = res_count;
 			else
 				sc->result = (DID_BAD_TARGET << 16) |
 					rhdr->cmd_status;
@@ -670,11 +671,11 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 			    r2t->data_length, session->max_burst);
 
 	r2t->data_offset = be32_to_cpu(rhdr->data_offset);
-	if (r2t->data_offset + r2t->data_length > scsi_bufflen(ctask->sc)) {
+	if (r2t->data_offset + r2t->data_length > scsi_out(ctask->sc)->length) {
 		iscsi_conn_printk(KERN_ERR, conn,
 				  "invalid R2T with data len %u at offset %u "
 				  "and total length %d\n", r2t->data_length,
-				  r2t->data_offset, scsi_bufflen(ctask->sc));
+				  r2t->data_offset, scsi_out(ctask->sc)->length);
 		__kfifo_put(tcp_ctask->r2tpool.queue, (void*)&r2t,
 			    sizeof(void*));
 		return ISCSI_ERR_DATALEN;
@@ -771,6 +772,7 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 		if (tcp_conn->in.datalen) {
 			struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
 			struct hash_desc *rx_hash = NULL;
+			struct scsi_data_buffer *sdb = scsi_in(ctask->sc);
 
 			/*
 			 * Setup copy of Data-In into the Scsi_Cmnd
@@ -788,8 +790,8 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 				  tcp_ctask->data_offset,
 				  tcp_conn->in.datalen);
 			return iscsi_segment_seek_sg(&tcp_conn->in.segment,
-						     scsi_sglist(ctask->sc),
-						     scsi_sg_count(ctask->sc),
+						     sdb->table.sgl,
+						     sdb->table.nents,
 						     tcp_ctask->data_offset,
 						     tcp_conn->in.datalen,
 						     iscsi_tcp_process_data_in,
@@ -1332,7 +1334,8 @@ iscsi_tcp_ctask_init(struct iscsi_cmd_task *ctask)
 		return 0;
 
 	/* If we have immediate data, attach a payload */
-	err = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc), scsi_sg_count(sc),
+	err = iscsi_tcp_send_data_prep(conn, scsi_out(sc)->table.sgl,
+				       scsi_out(sc)->table.nents,
 				       0, ctask->imm_count);
 	if (err)
 		return err;
@@ -1386,6 +1389,7 @@ iscsi_tcp_ctask_xmit(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 {
 	struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
 	struct scsi_cmnd *sc = ctask->sc;
+	struct scsi_data_buffer *sdb = scsi_out(sc);
 	int rc = 0;
 
 flush:
@@ -1412,9 +1416,8 @@ flush:
 				ctask->itt, tcp_ctask->sent, ctask->data_count);
 
 		iscsi_tcp_send_hdr_prep(conn, hdr, sizeof(*hdr));
-		rc = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc),
-					      scsi_sg_count(sc),
-					      tcp_ctask->sent,
+		rc = iscsi_tcp_send_data_prep(conn, sdb->table.sgl,
+					      sdb->table.nents, tcp_ctask->sent,
 					      ctask->data_count);
 		if (rc)
 			goto fail;
@@ -1460,8 +1463,8 @@ flush:
 		iscsi_tcp_send_hdr_prep(conn, &r2t->dtask.hdr,
 					sizeof(struct iscsi_hdr));
 
-		rc = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc),
-					      scsi_sg_count(sc),
+		rc = iscsi_tcp_send_data_prep(conn, sdb->table.sgl,
+					      sdb->table.nents,
 					      r2t->data_offset + r2t->sent,
 					      r2t->data_count);
 		if (rc)
-- 
1.5.4.1