On Sun, Apr 6, 2014, Michael Christie <micha...@cs.wisc.edu> wrote:
> On Apr 6, 2014, Or Gerlitz <ogerl...@mellanox.com> wrote:

> > the kernel code path of iscsi_complete_pdu--> __iscsi_complete_pdu -->  
> > iscsi_scsi_cmd_rsp
> > uses a "datalen" value which should account for the length of the data 
> > received from the target.
> >
> > In iser/iscsi_iser_recv() we don't skip the AHS in case the target sent it 
> > and I'd like to fix it.
> > I wasn't fully surewhat is done today in iscsi_tcp... I see few hits in 
> > libiscsi_tcp.c for ahslen but didn't manageto spot the a place where the 
> > ahs is being skipped, is it?
>
> You saw iscsi_tcp_hdr_recv_done in libiscsi_tcp.c right? We just have a 
> preallocated header buffer that we copy to. That function just detects the 
> ahs then sets things up so iscsi_tcp_recv_skb reads it into that buffer.


Yes, I saw iscsi_tcp_hdr_recv_done() but I wasn't sure to fully follow
on what's exactly done there w.r.t to the AHS, it reads the ahs len
and adds it to two size fields, and I was under the impression that
the AHS is skipped before the data pointer is provided to the
iscsi_complete_pdu--> __iscsi_complete_pdu -->  iscsi_scsi_cmd_rsp
call chain but couldn't prove it to myself, so the question, is this
really the case? and I can align iser to do the same practice.

> > Another related issue is with this code in iscsi_scsi_cmd_rsp():
> >
> > invalid_datalen:
> >                        iscsi_conn_printk(KERN_ERR,  conn,
> >                                         "Got CHECK_CONDITION but invalid 
> > data "
> >                                         "buffer size of %d\n", datalen);
> >                        sc->result = DID_BAD_TARGET << 16;
> >                        goto out;
> >                }
> >
> >                senselen = get_unaligned_be16(data);
> >                if (datalen < senselen)
> >                        goto invalid_datalen;
> >
> >
> > Partial sense can come into play e.g with target (e.g non-Linux one) 
> > sending sense
> > which is bigger than SCSI_SENSE_BUFFERSIZE or b/c the initiator RDSL 
> > doesn't let them
> > send the whole sense, etc.
> >
> > This code isn't willing to provide the SCSI midlayer with partial sense, 
> > why not do
> > allow that?
>
> We have never seen a target do that, so you are the first person to request 
> support. Send a patch.

If the iser initiator RDSL is small enough and the target correctly
takes it into account it can happen. Also we saw it with non-Linux
iser target that was attempting to send sense > Linux
SCSI_SENSE_BUFFERSIZE, for the time being we told them to chop @ their
side, but moving fwd, I think the correct behaviour is what described
above, so yes, patch is needed here.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Reply via email to