On 08/14/2009 01:42 AM, Hannes Reinecke wrote:
> Mike Christie wrote:
>> Hannes Reinecke wrote:
>>> Whenever we send a Data-Out response to an affected LUN during
>>> LU Reset, we should be setting the 'FINAL' bit. This will
>>> indicate to the target that we consider this transfer finished.
>>>
>>> Signed-off-by: Hannes Reinecke<h...@suse.de>
>>> ---
>>>   drivers/scsi/libiscsi.c |    4 ++++
>>>   1 files changed, 4 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
>>> index a0d1217..16d35f0 100644
>>> --- a/drivers/scsi/libiscsi.c
>>> +++ b/drivers/scsi/libiscsi.c
>>> @@ -321,6 +321,10 @@ static int iscsi_check_tmf_restrictions(struct
>>> iscsi_task *task, int opcode)
>>>                         task->itt, task->hdr_itt, hdr_lun);
>>>               return -EACCES;
>>>           }
>>> +        /*
>>> +         * Set FINAL flag to terminate data transfer.
>>> +         */
>>> +        task->hdr->flags |= ISCSI_FLAG_CMD_FINAL;
>>
>> Wasn't this in the first patch before?
>>
> Yes. But I split it off as it's a different issue.
> And this one apparently requires more discussion :-)
>
>> The data-out hdr is not prepd yet.  The entire thing gets memset'd, so
>> ORing it does not help.
>>
> Yes, and no.
> We're hitting this section for Data-Out PDUs only, ie for tasks on the
> 'requeue' list. All other tasks (for which iscsi_prep_scsi_cmd() would
> be called) are filtered out here:
>
>               if (opcode != ISCSI_OP_SCSI_DATA_OUT) {
>                       iscsi_conn_printk(KERN_INFO, conn,
>
> as 'opcode' is the parameter passed during invocation, not the actual
> opcode of the task.
> So the 'FINAL' bit will only be set when we're here:

Yes, it is set there at this time, but we have to prep the data-out pdus 
still.


>
>       list_for_each_entry_safe(task, tmptask,&conn->requeue, running) {
>               /*
>                * we always do fastlogout - conn stop code will clean up.
>                */
>               if (conn->session->state == ISCSI_STATE_LOGGING_OUT)
>                       break;
>
>               if (iscsi_check_tmf_restrictions(task, ISCSI_OP_SCSI_DATA_OUT))
>                       continue;
>
>               conn->task = task;
>               list_del_init(&conn->task->running);
>               if (conn->task->state == ISCSI_TASK_PENDING)
>                       conn->task->state = ISCSI_TASK_RUNNING;
>               if (conn->conn_debug)
>                       iscsi_conn_printk(KERN_INFO, conn,
>                                         "requeue cmdpdu [itt 0x%x cmdsn %u "
>                                         "lun %u] xmit\n", conn->task->itt,
>                                         be32_to_cpu(conn->task->cmdsn),
>                                         conn->task->sc ? 
> conn->task->sc->device->lun : -1);
>               rc = iscsi_xmit_task(conn);
>
> and there we don't do any header prep()ing.
>


We actually do. We have to prep the data-out pdu/s that get sent. For 
tasks on the requeue list the iscsi_xmit_task above we do:

iscsi_xmit_task->iscsi_tcp_task_xmit->iscsi_prep_data_out_pdu

iscsi_prep_data_out_pdu can then get called multiple times depending on 
how many data-outs it takes to send all the requested data. We might 
have to send more than one data-out because we can only send up to 
max_xmit_dlength in each data-out, but the target can request that we 
send more than that. The target can ask us to send a total of max burst 
bytes in the sequence of data-outs.

There is also the unsolicited data-out case and the burst/dlength values 
are a little different. We have the first burst value to deal with and 
we have immediate data. However, we can end up sending more than one 
data-out there too.


iscsi_prep_data_out_pdu does this:


void iscsi_prep_data_out_pdu(struct iscsi_task *task, struct 
iscsi_r2t_info *r2t,
                            struct iscsi_data *hdr)
{
......

         memset(hdr, 0, sizeof(struct iscsi_data));



>> Plus you only set the F bit for the final PDU. This would send multiple
>> PDUs with the final bit set. I think the point of sending a pdu with the
>> F bit set is that you do not have to send all the data-outs for the
>> sequence (if a tmf was executing then it does not have to wait for MB of
>> data). Just send one with the F bit set.
>>
> Yes, that was the plan. But I'm not _that_ firm with the R2T code, so
> the following argumentation might be wrong.
> So, I was under the impression that we're only ever send 1 Data-Out
> PDU in response to a R2T. However, there might be more than one
> R2T/Data-Out pairs exchanged until the entire data is transferred.

Yeah, we actually send multiple data-outs for a r2t if needed. In each 
pdu we can only send max_xmit_dlength bytes, so if the target asked us 
for more than that we send multiple pdus.


iscsi_tcp_task_xmit is a Fugly loop. We basically do:

flush:
        if (r2t needs more data to be sent)
                iscsi_prep_data_out_pdu
                update how many bytes were sent in data-out
                goto flush;


> The idea here was that we're setting the 'FINAL' flag for a Data-Out
> PDU whenever the data transfer should be stopped.
> This _should_ induce the Target to stop sending us R2Ts.
> If it does, the transfer is terminated and everyone's happy.
> If it doesn't (ie it'll ignore the FINAL flag) we would still
> have to send a Data-Out Response.
>
>> You want to modify iscsi_prep_data_out_pdu to check if a tmf would
>> affect that task, and if so then hit the final bit on it. We probably
>> also want to indicate to the caller that we ended it early so it it can
>> stop sending more data-outs for that r2t.
>>
> Does it make a difference?
> When changing iscsi_prep_data_out_pdu() we would be modifying the
> header when it's being prep()ed, ie after the SCSI cmd PDU is sent.
> But then the task is being rescheduled on the 'requeue' list after
> a R2T has been received, and my patch sends the flag just before
> it's being sent.
> Or am I wrong?


iscsi_prep_data_out_pdu can be called multiple times in different contexts.

1. If InitialR2T=No we can end up sending a scsi cmd pdu, then 
unsolicited data-outs. Depending on the settings we can send the scsi 
cmd pdu, immediated data and then also unsolicited data-outs (that is 
what iscsi_prep_scsi_cmd_pdu is trying to figure out in the if 
DMA_TO_DEVICE code).


In this case we do:

iscsi_data_xmit
        loop cmdqueue
                check tmf state
                iscsi_xmit_task


iscsi_xmit_task
        iscsi_tcp_task_xmit

                // Note: we could do this loop multiple times,
                // if more than one data-out is needed to
                // send all the data that was requested.
flush:
                xmit_pdu (this basically calls sendpage/sendmsg to send the 
scsi cmd pdu)

                // this handles unsolicited and solicted r2ts
                // so with InitialR2T=No this will handle the unsol
                //
                // it will also figure out how much data to send and
                // where we are in the data-out sequence (for example
                // we could send have sent one data-out and then got
                // a sendpage failure, and later when the iscsi_tcp
                // write space function is called we will run
                // this function again to either complete the
                // pdu that was being sent or start a new one if
                // needed.
                iscsi_tcp_get_curr_r2t()

                // this does a memset of the hdr, so it will
                // clear the flags you set.
                iscsi_prep_data_out_pdu()
                goto flush;



2. Then for the solicted r2t case we have
iscsi_data_xmit
        loop requeue
                check tmf state
                iscsi_xmit_task


iscsi_xmit_task

        iscsi_tcp_task_xmit

                // Note: we could do this loop multiple times,
                // if more than one data-out is needed to
                // send all the data that was requested.
flush:
                xmit_pdu (this basically calls sendpage/sendmsg to send the 
scsi cmd pdu)

                iscsi_tcp_get_curr_r2t()

                // this does a memset of the hdr, so it will
                // clear the flags you set.
                iscsi_prep_data_out_pdu()
                goto flush;



You know too, unless this is actually helping your target or any target, 
you can probably bury this on the TODO list.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~----------~----~----~----~------~----~------~--~---

Reply via email to