[stgt] [PATCH 14/15] Fix iSCSI Data-Out solicitation

[Arne Redlich]

Am Dienstag, den 16.06.2009, 06:24 -0400 schrieb Pete Wyckoff:
> arne.redlich at googlemail.com wrote on Tue, 09 Jun 2009 18:22 +0200:
> > Currently the iSCSI driver only solicits MaxXmitDSL(!) bytes with each R2T,
> > although it could solicit MaxBurstLength bytes.
> 
> This fix makes sense.  R2T should ask for a whole burst.
> 
> > This was introduced with the iSER driver and a RDMA_TRANSFER_SIZE constant to
> > limit the size of RDMA transfers. However, MaxBurstLength can and should be
> > used for that.
> 
> But RDMA should not necessarily use MAX_BURST for its data transfer
> sizes.  This size is completely controlled by the target and not
> even visible to the initiator.  The initiator should not be involved
> in the negotiation of the size.

I don't agree. MaxBurstLength is the upper limit for the amount of data
an R2T can solicit, so it's also the upper limit for the RDMA read the
R2T is translated to. You can certainly request smaller sizes, but
soliciting / RDMA reading data sizes > MaxBurstLength is a violation of
the iSCSI spec.

> There was a longish discussion of this value in the archive
> somewhere around the time iser was getting added to tgt.
> 
> Unfortunately I don't have gear to test this anymore.  One risk is
> that iser initiators will negotiate a small MaxBurstLength and get
> bad performance.

See above - if this is negotiated it has to be used.

>   Another is they negotiate something bigger than
> the mempool_size constant and break the iser code, if someone tries
> to set target's MaxBurstLength bigger hoping for faster transfers.

I agree.

> If you want to be safe, you can leave data_inout_max_length alone.
> Just change iscsi_r2t_build to "if (rdma)" to select max_burst or
> data_in_out_len.
> 
> If I know Fujita code style, he won't like the "if (rdma)" you added
> here or the one I'm suggesting.  In which case you might add a
> second conn parameter, data_r2t_max_length, that gets set appropriately.
> And rename data_inout_max_length to remove the "in" part now that
> you've figured out the R2T situation.

Hmm, it would actually seem cleanest to me if the iSER driver would just
limit MaxBurstLength (and other parameters if required) before login
negotiation - iser_accept_connection() looks like the right place for
that.

Thanks for the feedback,
Arne

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