[ofa-general] Re: [Ips] Calculating the VA in iSER header

[Eli Dorfman]

On Tue, Apr 29, 2008 at 8:05 PM, Pete Wyckoff <pw at osc.edu> wrote:
> dorfman.eli at gmail.com wrote on Thu, 17 Apr 2008 14:13 +0300:
>
> > On Wed, Apr 16, 2008 at 6:46 PM, Roland Dreier <rdreier at cisco.com> wrote:
>  > >  > Agree with the interpretation of the spec, and it's probably a bit
>  > >   > clearer that way too.  But we have working initiators and targets
>  > >   > that do it the "wrong" way.
>  > >
>  > >  Yes... I guess the key question is whether there are any initiators that
>  > >  do things the "right" way.
>  > >
>  > >
>  > >   > 1. Flag day: all initiators and targets change at the same time.
>  > >   > Will see data corruption if someone unluckily runs one or the other
>  > >   > using old non-fixed code.
>  > >
>  > >  Seems unacceptable to me... it doesn't make sense at all to break every
>  > >  setup in the world just to be "right" according to the spec.
>  >
>  > This will break only when both initiator and target will use
>  > InitialR2T=No, which means allow unsolicited data.
>  > As far as I know, STGT is not very common (and its version in RHEL5.1
>  > is considered experimental). Its default is also InitialR2T=Yes.
>  > Voltaire's iSCSI over iSER target also uses default InitialR2T=Yes.
>  > So it seems that nothing will break.
>
>  I finally got a chance to look at this just now.  I think you mean
>  default is InitialR2T=No above, which means no unsolicited data.
>  That is the default case, and true, the two different meanings
>  of the initiator-supplied VA coincide.

InitialR2T=Yes means that R2T is required, hence no unsolicited data.
Only is both sides, initiator and target agree on InitialR2T=No then
first data burst is unsolicited.

>
>  But you missed the impact of immediate data.  We run with the
>  defaults (I think) that say the first write request packet should be
>  filled with a bit of the coming data stream.  From iscsid.conf:
>
>     # To enable immediate data (i.e., the initiator sends unsolicited data
>     # with the iSCSI command packet), uncomment the following line:
>     #
>     # The default is Yes
>     node.session.iscsi.ImmediateData = Yes
>
>  Looking at the offset printed out by your patch, it is indeed
>  non-zero for the first RDMA read.  Please correct me if I am
>  mistaken about this---you must have tested all four variations of
>  with and without the patches on initiator and target side, but I did
>  not.

You are right about the ImmediateData=Yes.
I really missed that, so after all this patch will break current
target implementation and
cause data corruption.
I suggest to postpone this patch till we implement the iSER HELLO
message and then add this patch with the corresponding target patch.
This will allow current initiator to work with current target and new
initiator work with new target.
I still think we should do that since future iser implementation will
probably rely on the spec.

>
>  Hence I am still a bit unhappy about having to deal with the
>  fallout, with no way to detect it.  For our local use, I'll keep an
>  older version of stgt in use until we switch to a new kernel, then
>  merge up the target side change.  It is a bother, but I can deal
>  with it.  For other institutions, this lockstep upgrade requirement
>  will not be obvious until they debug the resulting data corruption.
>
>  Still, I do understand why it would be nice to conform to the spec,
>  and it is maybe a bit cleaner that way too.  Maybe you can help with
>  the bug reports on stgt-devel during the transition, and maintain
>  and publish a patch to let it work with old kernels.
>
>                 -- Pete
>