[stgt] [PATCH] ISCSI: Honour MaxRecvDataSegmentLength for NORMAL sessions

[FUJITA Tomonori]

On Sun, 19 Aug 2012 06:55:39 +1000
ronnie sahlberg <ronniesahlberg at gmail.com> wrote:

> On Sat, Aug 18, 2012 at 7:10 PM, FUJITA Tomonori
> <fujita.tomonori at lab.ntt.co.jp> wrote:
> > On Sat, 18 Aug 2012 18:56:09 +1000
> > ronnie sahlberg <ronniesahlberg at gmail.com> wrote:
> >
> >> The bug is that IF the initiator does state it can handle larger
> >> DATA-IN PDUs than 8kb,   like open-iscsi does, and all other iniators
> >> also does.
> >
> > btw, I've seen initiators that don't send
> > MaxRecvDataSegmentLength long ago. I can't recall what.
> >
> >
> >> The b\ug in tgtd in processing the login is that tgtd ONLY parses this
> >> "lets use bigger than 8kb pdu" IFF the session is a discovery session,
> >> but not for NORMAL sessions.
> >
> > Can you recall why we handle discovery and normal sessions differently?
> 
> That is an issue. Why should we handle discover and normal sessions
> differently at all.
> 
> Please see the attached patch, it removes the session==discovery
> conditional and makes tgtd honour the
> negotiated maxrecvdatasegmentlength for all sessions, not just discovery.

Looks fine. I thought that we need to check the value that an
initiator sends but we call param_check_val() here so it should be
fine.

Can you resend the patch in the proper format (in-line not an
attachment)? Also please update the description; if you configure
MaxXmit by hand, this is not the issue. It enables admins to configure
the maximum buffer length (accepting MaxRecv blindly isn't a good idea
since allocating very large memory isn't nice). However, I agree that
it's bad to require admins to configure it by hand. So I'm fine by
accepting MaxRecv blindly.


> This should handle both the original issue with discovery   and also
> the issue with large reads being split up in 8k trains
--
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