Re: Possible Bug? LIO does not support SCSI commands with Immediate Bit Set?

[Lee Duncan <lduncan@xxxxxxxx>]

On 11/26/23 16:39, Mike Christie wrote:
> On 11/26/23 6:01 AM, Hannes Reinecke wrote:
> > On 11/25/23 19:28, Lee Duncan wrote:
> > > Hi Martin/list:
> > >
> > > I am trying to track down an issue I am seeing when trying to boot using iSCSI root using the fastlinq qedi converged network adapter initiator and an LIO target.
> > >
> > > In this configuration, but using a non-LIO (hardware) iSCSI target, everything "just works". But when I switch to using an LIO software target, I get this error when booting:
> > >
> > > > 2023-09-20T14:10:32.835358-0500 worker5 kernel: Illegally set Immediate Bit in iSCSI Initiator Scsi Command PDU.
> > > > ... > 2023-09-20T14:10:32.835422-0500 worker5 kernel: Got unknown iSCSI
> > > OpCode: 0x52
> > >
> > > It looks like the fastlinq adapter is sending a SCSI write (0x12) with the Immediate bit set (0x40).
> > >
> > > The code in iscsi_target.c:iscsit_setup_scsi_cmd(): looks like this:
> > >
> > > >      if (hdr->opcode & ISCSI_OP_IMMEDIATE) {
> > > >          pr_err("Illegally set Immediate Bit in iSCSI Initiator"
> > > >                  " Scsi Command PDU.\n");
> > > >          return iscsit_add_reject_cmd(cmd,
> > > >                           ISCSI_REASON_BOOKMARK_INVALID, buf);
> > > >      }
> > >
> > > But I looked at rfc3720, and it looks like SCSI PDUs can have the Immediate Bit set:
> > >
> > > > 10.3.  SCSI Command
> > > >     The format of the SCSI Command PDU is:
> > > >     Byte/     0       |       1       |       2       |       3       |
> > > >        /              |               |               |               |
> > > >       |0 1 2 3 4 5 6 7|0 1 2 3 4 5 6 7|0 1 2 3 4 5 6 7|0 1 2 3 4 5 6 7|
> > > >       +---------------+---------------+---------------+---------------+
> > > >      0|.|I| 0x01      |F|R|W|. .|ATTR | Reserved                      |
> > > >       +---------------+---------------+---------------+---------------+
> > > >      4|TotalAHSLength | DataSegmentLength                             |
> > > >       +---------------+---------------+---------------+---------------+
> > > >      8| Logical Unit Number (LUN)                                     |
> > > >       +                                                               +
> > > >     12|                                                               |
> > > >       +---------------+---------------+---------------+---------------+
> > > > ...
> > >
> > > Where "I" is the immediate bit.
> > >
> > > Frankly, I'm never sure I read the SPEC correctly, so I'm not sure if I'm mistaken here, but it looks like LIO does not allow the Immediate bit (and hence Immediate data), even though the SPEC does allow it. But it also looks like, during negotiation phase, LIO negotiates ImmediateData like any other parameter, allowing "YES".
> > >
> > > In our testing, if we set ImmediateData=No in the target, then our problem goes away, i.e. we can now boot from an LIO target. This is because Immediate Data is negotiated off, of course.
> > >
> > > Is this a bug, or is this adapter doing something wrong? I would appreciate other viewpoints.
> > >
> > > Thank you.
> >
> > Sounds like a bug.
> > Can you check if this helps?
> >
> > diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> > index b516c2893420..ad68706ad9f7 100644
> > --- a/drivers/target/iscsi/iscsi_target.c
> > +++ b/drivers/target/iscsi/iscsi_target.c
> > @@ -1060,7 +1060,8 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
> >
> > ISCSI_REASON_BOOKMARK_INVALID, buf);
> >          }
> >
> > -       if (hdr->opcode & ISCSI_OP_IMMEDIATE) {
> > +       if ((hdr->opcode & ISCSI_OP_IMMEDIATE) &&
> > +           !conn->sess->sess_ops->ImmediateData) {
> >                  pr_err("Illegally set Immediate Bit in iSCSI Initiator"
> >                                  " Scsi Command PDU.\n");
> >                  return iscsit_add_reject_cmd(cmd,
>
>
> These are different things.
>
> Immediate data means you can have data after the header in the PDU.
>
> The immediate bit on the header has CmdSN rules which allows you to
> send commands without having to increment the CmdSN. It's used for
> things like TMFs because at that time, the window might be closed due
> to SCSI commands having filled it.

Mike: it sounds like that implies that setting the immediate bit in the 
header is actually not allowed for regular SCSI PDUs (like Read, Write), 
correct?

--
Lee