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 Mon, 2015-01-26 at 12:49 +0200, Sagi Grimberg wrote:
> In case sendtargets response is larger than initiator MRDSL, we
> send a partial sendtargets response (setting F=0, C=1, TTT!=0xffffffff),
> accept a consecutive empty text message and send the rest of the payload.
> In case we are done, we set F=1, C=0, TTT=0xffffffff.
> We do that by storing the sendtargets response bytes done under
> the session.
> 
> Note, we restrict this sequence only for discovery session at the
> moment. Adding this type of functionality for any text command will
> be added when we need it.
> 
> Signed-off-by: Sagi Grimberg <sagig@xxxxxxxxxxxx>
> ---
>  drivers/target/iscsi/iscsi_target.c      |   68 +++++++++++++++++++++++-------
>  include/target/iscsi/iscsi_target_core.h |    3 +
>  2 files changed, 56 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 0a1c3c0..b17691d 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -2007,9 +2007,16 @@ iscsit_process_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
>  	int cmdsn_ret;
>  
>  	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..?

>  	}
>  	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, and then re-queuing the original iscsi_cmd
to send the text response...

> @@ -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.

--nab

--
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