On 2/3/2015 12:55 PM, Sagi Grimberg wrote:
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.
Nic,
Ping on the open questions I had here?
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