Re: [PATCH v1 3/4] iscsi-target: Support multi-sequence sendtargets text response

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

 



On 1/30/2015 11:50 PM, Nicholas A. Bellinger wrote:

  	if (!text_in) {
-		pr_err("Unable to locate text_in buffer for sendtargets"
-		       " discovery\n");
-		goto reject;
+		if (conn->sess->sess_ops->SessionType) {
+			/* Assume this is a consecutive sendtargets */
+			cmd->cmd_flags |= IFC_SENDTARGETS_ALL;
+			cmd->targ_xfer_tag = be32_to_cpu(hdr->ttt);
+			goto empty_sendtargets;
+		} else {
+			pr_err("Unable to locate text_in buffer for sendtargets"
+			       " discovery\n");
+			goto reject;
+		}

Btw, why is this restricted to SessionType=Discovery..?  AFAICT it
should work as expected for SessionType=Normal as well, no..?

I guess it can, but is the initiator allowed to send us sendtargets
request in a normal session? I assume this is IFC_SENDTARGETS_ALL
(although I guess I don't need to assume that if I locate the cmd from
the hdr->ttt).


  	}
  	if (strncmp("SendTargets", text_in, 11) != 0) {
  		pr_err("Received Text Data that is not"
@@ -2032,6 +2039,7 @@ iscsit_process_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
  		goto reject;
  	}

+empty_sendtargets:
  	spin_lock_bh(&conn->cmd_lock);
  	list_add_tail(&cmd->i_conn_node, &conn->conn_cmd_list);
  	spin_unlock_bh(&conn->cmd_lock);

Mmmm, I don't think this is right per the RFC..  (Adding MNC CC')

 From section 10.10.3:

"If the command is sent as part of a sequence of text requests and
  responses, the Initiator Task Tag MUST be the same for all the
  requests within the sequence (similar to linked SCSI commands)."

It looks like the code here adds the empty sendtargets text requests to
conn_cmd_list, with a new iscsi_cmd descriptor but using the same ITT as
the original text request.

I think the subsequent empty text requests should be locating the
original iscsi_cmd via ITT,

Don't you mean "via TTT"?

and then re-queuing the original iscsi_cmd
to send the text response...

I agree this can happen, but how should I prevent the transports
(TCP/iSER) from removing the command from the commands list when they
are done with it? I need this command to be kept in the list for the
next text request....

Should the transports also be aware this is a partial response? Can't say I like that very much...


@@ -3519,14 +3544,25 @@ iscsit_build_text_rsp(struct iscsi_cmd *cmd, struct iscsi_conn *conn,
  		      struct iscsi_text_rsp *hdr,
  		      enum iscsit_transport_type network_transport)
  {
+	struct iscsi_session *session = conn->sess;
  	int text_length, padding;
+	bool completed = true;

-	text_length = iscsit_build_sendtargets_response(cmd, network_transport);
+	text_length = iscsit_build_sendtargets_response(cmd, network_transport,
+							session->st_rsp_bytes,
+							&completed);
  	if (text_length < 0)
  		return text_length;

+	if (completed) {
+		hdr->flags |= ISCSI_FLAG_CMD_FINAL;
+	} else {
+		hdr->flags |= ISCSI_FLAG_TEXT_CONTINUE;
+		session->st_rsp_bytes += text_length;
+		if (cmd->targ_xfer_tag == 0xFFFFFFFF)
+			cmd->targ_xfer_tag = session_get_next_ttt(session);
+	}
  	hdr->opcode = ISCSI_OP_TEXT_RSP;
-	hdr->flags |= ISCSI_FLAG_CMD_FINAL;
  	padding = ((-text_length) & 3);
  	hton24(hdr->dlength, text_length);
  	hdr->itt = cmd->init_task_tag;

<SNIP>

diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
index 5f41a17..c7b5adc 100644
--- a/include/target/iscsi/iscsi_target_core.h
+++ b/include/target/iscsi/iscsi_target_core.h
@@ -651,6 +651,9 @@ struct iscsi_session {
  	/* Used for session reference counting */
  	int			session_usage_count;
  	int			session_waiting_on_uc;
+	/* sendtargets text response bytes done */
+	int			st_rsp_bytes;
+
  	atomic_long_t		cmd_pdus;
  	atomic_long_t		rsp_pdus;
  	atomic_long_t		tx_data_octets;

... which would allow the per session ->st_rsp_bytes to disappear, and
just use iscsi_cmd->read_data_done for tracking the offset across text
response CONTINUE bit usage.

Yep, agreed.

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




[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux