tomof at acm.org wrote on Sat, 18 Aug 2007 14:10 +0900:
> From: Pete Wyckoff <pw at osc.edu>
> Subject: Re: [Stgt-devel] [PATCH] iscsi fix xmit param
> Date: Wed, 15 Aug 2007 11:15:59 -0500
>
> > True. There are a few cases.
> >
> > 1. ini sends mrdsl > target mxdsl. Apply minimum, use target max.
> > 2. ini sends mrdsl < target mxdsl. Apply minimum, use ini max.
> > 3. ini does not mention mrdsl. Target should set its mxdsl to the
> > standard default (8192), not use its default max.
> >
> > This last case was broken before my earlier patch "iscsi param
> > cleanup". Now it is fixed but I changed the target default to 8k.
> > I want the target to agree to use a larger value if the initiator is
> > willing, but still get case #3 correct if the initiator does not
> > specify.
>
> Really?
>
> I think that if ini does not mention mrdsl and target doesn't use the
> standard default mxdsl value (a target admin sets non default value),
> text_check_param() sets xmdsl to the default value.
It really was broken. But this is an odd case as open-iscsi at
least always sends mrdsl, even if it is set to use the default.
If you roll back to 92d767a711ed82791b171ce7c9b722263076c519 or so,
just before "iscsi param cleanup" you'll see:
iscsi/param.c: mxdsl = 262144
iscsi/target.c: mxdsl = 8192
and this code in iscsi/iscsid.c:
if (p[i].state == KEY_STATE_START && p[i].val != session_keys[i].def) {
if (conn->state == STATE_LOGIN) {
if (i == ISCSI_PARAM_MAX_XMIT_DLENGTH) {
if (p[i].val > session_keys[i].def)
p[i].val = session_keys[i].def;
p[i].state = KEY_STATE_DONE;
continue;
}
memset(buf, 0, sizeof(buf));
param_val_to_str(session_keys, i, p[i].val,
buf);
text_key_add(conn, session_keys[i].name, buf);
p[i].state = KEY_STATE_REQUEST;
}
cnt++;
}
(p[] is conn->session_param, which is initialized from target params
then modified by negotiation.)
If target admin does not change target mxdsl, and initiator does not
send mrdsl, this code will see 8192 != 262144 and use 8192. Fine,
that's the default.
If target admin changes target mxdsl to 16384 using tgtadm, and
initiator does not send mrdsl, this code will see 16384 != 262144
and use 16384 without negotiation. Bad, should use the default.
With the "iscsi param cleanup" patch and this new "iscsi fix xmit
param", I think everything works as it should.
-- Pete
|
|