On Sat, Jan 21, 2017 at 05:52:44AM +0530, Bart Van Assche wrote:
>
> Hello Varun,
>
> Sorry but now that I have had some more time to think about this I doubt that
> the above patch is a correct interpretation of the spec. How about the patch
> below?
>
> Thanks,
>
> Bart.
>
>
> From: Varun Prakash <varun@xxxxxxxxxxx>
> Subject: [PATCH] target/iscsi: Fix seq_end_offset calculation
>
> In case of unsolicited data for the first sequence
> seq_end_offset must be set to minimum of total data length
> and FirstBurstLength, so do not add cmd->write_data_done
> to the min of total data length and FirstBurstLength.
>
> Signed-off-by: Varun Prakash <varun@xxxxxxxxxxx>
> [ bvanassche: fixed seq_end_offset calculation ]
> Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
> ---
> drivers/target/iscsi/iscsi_target_erl0.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target_erl0.c b/drivers/target/iscsi/iscsi_target_erl0.c
> index b54e72c7ab0f..243326fabf23 100644
> --- a/drivers/target/iscsi/iscsi_target_erl0.c
> +++ b/drivers/target/iscsi/iscsi_target_erl0.c
> @@ -44,10 +44,9 @@ void iscsit_set_dataout_sequence_values(
> */
> if (cmd->unsolicited_data) {
> cmd->seq_start_offset = cmd->write_data_done;
> - cmd->seq_end_offset = (cmd->write_data_done +
> - ((cmd->se_cmd.data_length >
> - conn->sess->sess_ops->FirstBurstLength) ?
> - conn->sess->sess_ops->FirstBurstLength : cmd->se_cmd.data_length));
> + cmd->seq_end_offset =
> + min(conn->sess->sess_ops->FirstBurstLength,
> + cmd->write_data_done + cmd->se_cmd.data_length);
> return;
> }
Hi Bart,
This patch does not look correct to me because with this patch
seq_end_offset can get a value which is greater than IO size, for example-
Set following parameters on the target
ImmediateData=Yes
InitialR2T=No
FirstBurstLength=64k
MaxXmitDataSegmentLength=8k
Login from open iSCSI initiator and issue a WRITE cmd
dd if=/dev/zero of=/dev/sdb bs=32k count=1 oflag=direct
After first iSCSI PDU(WRITE cmd + immediate data(8k))
cmd->write_data_done = 8192
cmd->seq_start_offset= 8192
With this patch
cmd->seq_end_offset = 40k min(64k, 8k + 32k)
40k is not correct as IO size is 32k.
cmd->seq_end_offset should be 32k min(64k, 32k) in this case.
Thanks
Varun
--
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
