diff mbox

QLogic 57840S iSCSI Offload Engine causes unknown OpCode 0x43 error

Message ID 1490936567.6977.41.camel@haakon3.risingtidesystems.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nicholas A. Bellinger March 31, 2017, 5:02 a.m. UTC
Hi Martin & Co,

On Thu, 2017-03-23 at 15:44 +0100, Martin Svec wrote:
> Hello Himanshu,
> 
> Dne 22.3.2017 v 2:19 Madhani, Himanshu napsal(a):
> > Hello Nic, Martin, 
> >
> 
> <SNIP>
> 
> > Here’s response i got from our windows driver team
> >
> > ——
> > There is a known issue with iscsi login in tcm/lio side and we have a driver workaround for
> > handling this target issue.
> >
> > Please try with registry key ‘wkflg=1’. See attached snapshot
> >
> > Thanks,
> > - Himanshu
> >
> I tried the workaround in "qeois" key and also in "bxois" key which seem to be the right service
> in our case, but with no luck. Then, I downgraded iSCSI Adapter driver to the version recommended
> by Dell and upgraded back to the latest version found by Windows Driver Update (ver. 7.14.1.1).
> None of it fixed the issue. Do the OEM-branded BCM57840S drivers support this workaround?
> 
> Used registry keys:
> 
> [HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\bxois\Parameters\Device]
> "DriverParameter"="wkflg=1"
> 
> [HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\qeois\Parameters\Device]
> "DriverParameter"="wkflg=1"
> 

After reviewing the packet capture, it's clear what's going on..

So the initiator is not proposing DefaultTime2Retain and
DefaultTime2Wait, so the target proposes them itself and then
transitions to full feature phase operation by setting
ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT.

The problem is, a hack was made to allow the GlobalSAN iSCSI initiator
for MacOSX (which didn't follow RFC) to work waaay back in 2009, to
allow the response of these two keys (along with two other keys) to be
optional.

https://groups.google.com/forum/#!topic/linux-iscsi-target-dev/7mtXSSwGR98

The result is that since the QLogic MSFT initiator doesn't propose them,
LIO proposes them itself, and then immediately transitions to full
feature phase.  However, the QLogic MSFT side still attempts to respond
to DefaultTime2Retain and DefaultTime2Wait, even though
ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT have been set
in the last login response by LIO.

AFAIK, this is the only initiator I've seen that doesn't propose
DefaultTime2Retain + DefaultTime2Wait, also doesn't honor the target's
request to transition to full feature phase, but then still attempts to
respond to the keys.

So really this is a grey area.  The original hack to support GlobalSAN
is definitely not RFC, but at the same time Qlogic MSFT should really be
sending DefaultTime2Retain + DefaultTime2Wait, and should be honoring
LIO's ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT to
transition to full feature phase.

That said, here is a patch to disable the GlobalSAN hack that should get
you up and running.

Preferably, I'd like to drop the old GlobalSAN hack to avoid this
situation all together, but I don't know if it still suffers from the
same bug or not.

Give this a shot, and let's see if we can find someone with a recent
version of GlobalSAN to determine if it suffers from the same breakage.


--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Easi, Arun March 31, 2017, 5:35 a.m. UTC | #1
Hi Nic,

On Thu, 30 Mar 2017, 10:02pm, Nicholas A. Bellinger wrote:

> Hi Martin & Co,
> 
> On Thu, 2017-03-23 at 15:44 +0100, Martin Svec wrote:
> > Hello Himanshu,
> > 
> > Dne 22.3.2017 v 2:19 Madhani, Himanshu napsal(a):
> > > Hello Nic, Martin, 
> > >
> > 
> > <SNIP>
> > 
> > > Here’s response i got from our windows driver team
> > >
> > > ——
> > > There is a known issue with iscsi login in tcm/lio side and we have a driver workaround for
> > > handling this target issue.
> > >
> > > Please try with registry key ‘wkflg=1’. See attached snapshot
> > >
> > > Thanks,
> > > - Himanshu
> > >
> > I tried the workaround in "qeois" key and also in "bxois" key which seem to be the right service
> > in our case, but with no luck. Then, I downgraded iSCSI Adapter driver to the version recommended
> > by Dell and upgraded back to the latest version found by Windows Driver Update (ver. 7.14.1.1).
> > None of it fixed the issue. Do the OEM-branded BCM57840S drivers support this workaround?
> > 
> > Used registry keys:
> > 
> > [HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\bxois\Parameters\Device]
> > "DriverParameter"="wkflg=1"
> > 
> > [HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\qeois\Parameters\Device]
> > "DriverParameter"="wkflg=1"
> > 
> 
> After reviewing the packet capture, it's clear what's going on..
> 
> So the initiator is not proposing DefaultTime2Retain and
> DefaultTime2Wait, so the target proposes them itself and then
> transitions to full feature phase operation by setting
> ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT.
> 
> The problem is, a hack was made to allow the GlobalSAN iSCSI initiator
> for MacOSX (which didn't follow RFC) to work waaay back in 2009, to
> allow the response of these two keys (along with two other keys) to be
> optional.
> 
> https://groups.google.com/forum/#!topic/linux-iscsi-target-dev/7mtXSSwGR98

Thanks for the background, I was under the impression that this was just a 
plain bug in TCM target.

> 
> The result is that since the QLogic MSFT initiator doesn't propose them,
> LIO proposes them itself, and then immediately transitions to full
> feature phase.  However, the QLogic MSFT side still attempts to respond
> to DefaultTime2Retain and DefaultTime2Wait, even though
> ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT have been set
> in the last login response by LIO.
> 
> AFAIK, this is the only initiator I've seen that doesn't propose
> DefaultTime2Retain + DefaultTime2Wait, also doesn't honor the target's
> request to transition to full feature phase, but then still attempts to
> respond to the keys.

Interestingly, I was suggesting the same as a workaround to our Windows 
driver team earlier Today.

> 
> So really this is a grey area.  The original hack to support GlobalSAN
> is definitely not RFC, but at the same time Qlogic MSFT should really be
> sending DefaultTime2Retain + DefaultTime2Wait, and should be honoring
> LIO's ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT to
> transition to full feature phase.

IMHO, not proposing the keys (and taking defaults, instead) is better, and 
more RFC compliant, than not waiting for a response to the proposed keys. 
This would not be so bad because most initiators proposes main keys in the 
initial request itself anyway.

This is what RFC has to say about the response to proposals:

    Responses are REQUIRED in all other cases, and the value chosen and
    sent by the acceptor becomes the outcome of the negotiation.

The exception was given only to boolean keys; given that these keys are 
not boolean keys, initiators are required by the RFC to respond.

..and this about using defaults:

    All negotiations are explicit (i.e., the result MUST only be based on
    newly exchanged or declared values).  There are no implicit
    proposals.  If a proposal is not made, then a reply cannot be
    expected.  Conservative design also requires that default values
    should not be relied upon when use of some other value has serious
    consequences.

Just my 2c.

Regards,
-Arun

> 
> That said, here is a patch to disable the GlobalSAN hack that should get
> you up and running.
> 
> Preferably, I'd like to drop the old GlobalSAN hack to avoid this
> situation all together, but I don't know if it still suffers from the
> same bug or not.
> 
> Give this a shot, and let's see if we can find someone with a recent
> version of GlobalSAN to determine if it suffers from the same breakage.
> 
> diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_param
> index e65bf78..ecde825 100644
> --- a/drivers/target/iscsi/iscsi_target_parameters.c
> +++ b/drivers/target/iscsi/iscsi_target_parameters.c
> @@ -793,10 +793,12 @@ static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param)
>                         SET_PSTATE_REPLY_OPTIONAL(param);
>                 if (!strcmp(param->name, FIRSTBURSTLENGTH))
>                         SET_PSTATE_REPLY_OPTIONAL(param);
> +#if 0
>                 if (!strcmp(param->name, DEFAULTTIME2WAIT))
>                         SET_PSTATE_REPLY_OPTIONAL(param);
>                 if (!strcmp(param->name, DEFAULTTIME2RETAIN))
>                         SET_PSTATE_REPLY_OPTIONAL(param);
> +#endif
>                 /*
>                  * Required for gPXE iSCSI boot client
>                  */
>
Nicholas A. Bellinger March 31, 2017, 7:09 a.m. UTC | #2
Hey Arun!

Long time no see.  Hope that you are well.

On Thu, 2017-03-30 at 22:35 -0700, Arun Easi wrote:
> Hi Nic,
> 
> On Thu, 30 Mar 2017, 10:02pm, Nicholas A. Bellinger wrote:
> 
> > Hi Martin & Co,
> > 
> > On Thu, 2017-03-23 at 15:44 +0100, Martin Svec wrote:
> > > Hello Himanshu,
> > > 
> > > Dne 22.3.2017 v 2:19 Madhani, Himanshu napsal(a):
> > > > Hello Nic, Martin, 

<SNIP>

> > 
> > After reviewing the packet capture, it's clear what's going on..
> > 
> > So the initiator is not proposing DefaultTime2Retain and
> > DefaultTime2Wait, so the target proposes them itself and then
> > transitions to full feature phase operation by setting
> > ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT.
> > 
> > The problem is, a hack was made to allow the GlobalSAN iSCSI initiator
> > for MacOSX (which didn't follow RFC) to work waaay back in 2009, to
> > allow the response of these two keys (along with two other keys) to be
> > optional.
> > 
> > https://groups.google.com/forum/#!topic/linux-iscsi-target-dev/7mtXSSwGR98
> 
> Thanks for the background, I was under the impression that this was just a 
> plain bug in TCM target.
> 
> > 
> > The result is that since the QLogic MSFT initiator doesn't propose them,
> > LIO proposes them itself, and then immediately transitions to full
> > feature phase.  However, the QLogic MSFT side still attempts to respond
> > to DefaultTime2Retain and DefaultTime2Wait, even though
> > ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT have been set
> > in the last login response by LIO.
> > 
> > AFAIK, this is the only initiator I've seen that doesn't propose
> > DefaultTime2Retain + DefaultTime2Wait, also doesn't honor the target's
> > request to transition to full feature phase, but then still attempts to
> > respond to the keys.
> 
> Interestingly, I was suggesting the same as a workaround to our Windows 
> driver team earlier Today.
> 

:-)

> > 
> > So really this is a grey area.  The original hack to support GlobalSAN
> > is definitely not RFC, but at the same time Qlogic MSFT should really be
> > sending DefaultTime2Retain + DefaultTime2Wait, and should be honoring
> > LIO's ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT to
> > transition to full feature phase.
> 
> IMHO, not proposing the keys (and taking defaults, instead) is better, and 
> more RFC compliant, than not waiting for a response to the proposed keys. 
> This would not be so bad because most initiators proposes main keys in the 
> initial request itself anyway.
> 
> This is what RFC has to say about the response to proposals:
> 
>     Responses are REQUIRED in all other cases, and the value chosen and
>     sent by the acceptor becomes the outcome of the negotiation.
> 
> The exception was given only to boolean keys; given that these keys are 
> not boolean keys, initiators are required by the RFC to respond.
> 
> ..and this about using defaults:
> 
>     All negotiations are explicit (i.e., the result MUST only be based on
>     newly exchanged or declared values).  There are no implicit
>     proposals.  If a proposal is not made, then a reply cannot be
>     expected.  Conservative design also requires that default values
>     should not be relied upon when use of some other value has serious
>     consequences.
> 
> Just my 2c.
> 

In retrospect, I agree not proposing the keys and using RFC defaults
instead is a better approach than the original hack.  It would involve a
larger change to existing code to work though..

In the case DefaultTime2Retain and DefaultTime2Wait having either sides
use (potentially) different values is harmless, considering they don't
have anything to do with I/O.

However, the original GlobalSAN initiator also didn't propose nor
respond to FirstBurstLength or MaxBurstLength..

So taking the approach to just use defaults and don't propose anything
for the non I/O related keys would be OK, but for the I/O related keys
that may not be as safe an assumption.

That said, looking around it appears at least some version of GlobalSAN
are now proposing the keys the hack in question originally addressed:

https://forums.freebsd.org/threads/51006/

Albeit, the log output is from discovery but I think it's a good sign
the RFC breakage may finally be addressed.

If that is the case, I'd much proper to drop this old hack all-together.

So I'll go steal a MacOSX laptop from someone tomorrow and give it a
shot with a modern GlobalSAN version.

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicholas A. Bellinger April 2, 2017, 9:18 p.m. UTC | #3
On Fri, 2017-03-31 at 00:09 -0700, Nicholas A. Bellinger wrote:
> On Thu, 2017-03-30 at 22:35 -0700, Arun Easi wrote:

<SNIP>

> > > 
> > > The result is that since the QLogic MSFT initiator doesn't propose them,
> > > LIO proposes them itself, and then immediately transitions to full
> > > feature phase.  However, the QLogic MSFT side still attempts to respond
> > > to DefaultTime2Retain and DefaultTime2Wait, even though
> > > ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT have been set
> > > in the last login response by LIO.
> > > 
> > > AFAIK, this is the only initiator I've seen that doesn't propose
> > > DefaultTime2Retain + DefaultTime2Wait, also doesn't honor the target's
> > > request to transition to full feature phase, but then still attempts to
> > > respond to the keys.
> > 
> > Interestingly, I was suggesting the same as a workaround to our Windows 
> > driver team earlier Today.
> > 
> 
> :-)
> 
> > > 
> > > So really this is a grey area.  The original hack to support GlobalSAN
> > > is definitely not RFC, but at the same time Qlogic MSFT should really be
> > > sending DefaultTime2Retain + DefaultTime2Wait, and should be honoring
> > > LIO's ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT to
> > > transition to full feature phase.
> > 
> > IMHO, not proposing the keys (and taking defaults, instead) is better, and 
> > more RFC compliant, than not waiting for a response to the proposed keys. 
> > This would not be so bad because most initiators proposes main keys in the 
> > initial request itself anyway.
> > 
> > This is what RFC has to say about the response to proposals:
> > 
> >     Responses are REQUIRED in all other cases, and the value chosen and
> >     sent by the acceptor becomes the outcome of the negotiation.
> > 
> > The exception was given only to boolean keys; given that these keys are 
> > not boolean keys, initiators are required by the RFC to respond.
> > 
> > ..and this about using defaults:
> > 
> >     All negotiations are explicit (i.e., the result MUST only be based on
> >     newly exchanged or declared values).  There are no implicit
> >     proposals.  If a proposal is not made, then a reply cannot be
> >     expected.  Conservative design also requires that default values
> >     should not be relied upon when use of some other value has serious
> >     consequences.
> > 
> > Just my 2c.
> > 
> 
> In retrospect, I agree not proposing the keys and using RFC defaults
> instead is a better approach than the original hack.  It would involve a
> larger change to existing code to work though..
> 
> In the case DefaultTime2Retain and DefaultTime2Wait having either sides
> use (potentially) different values is harmless, considering they don't
> have anything to do with I/O.
> 
> However, the original GlobalSAN initiator also didn't propose nor
> respond to FirstBurstLength or MaxBurstLength..
> 
> So taking the approach to just use defaults and don't propose anything
> for the non I/O related keys would be OK, but for the I/O related keys
> that may not be as safe an assumption.
> 
> That said, looking around it appears at least some version of GlobalSAN
> are now proposing the keys the hack in question originally addressed:
> 
> https://forums.freebsd.org/threads/51006/
> 
> Albeit, the log output is from discovery but I think it's a good sign
> the RFC breakage may finally be addressed.
> 
> If that is the case, I'd much proper to drop this old hack all-together.
> 
> So I'll go steal a MacOSX laptop from someone tomorrow and give it a
> shot with a modern GlobalSAN version.
> 

Ok, I've been able to confirm with GlobalSAN iSCSI v5.3.0.541 that it
does correctly propose the four keys in question, and makes the original
hack in iscsi_check_proposer_for_optional_reply() that is causing
problems here, now unnecessary.

Sending out a patch for this shortly.

Martin + Arun + QLogic folks, please verify on your end.

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Martin Svec April 3, 2017, 1:03 p.m. UTC | #4
Dne 2.4.2017 v 23:18 Nicholas A. Bellinger napsal(a):
> On Fri, 2017-03-31 at 00:09 -0700, Nicholas A. Bellinger wrote:
>> On Thu, 2017-03-30 at 22:35 -0700, Arun Easi wrote:
> <SNIP>
>
>>>> The result is that since the QLogic MSFT initiator doesn't propose them,
>>>> LIO proposes them itself, and then immediately transitions to full
>>>> feature phase.  However, the QLogic MSFT side still attempts to respond
>>>> to DefaultTime2Retain and DefaultTime2Wait, even though
>>>> ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT have been set
>>>> in the last login response by LIO.
>>>>
>>>> AFAIK, this is the only initiator I've seen that doesn't propose
>>>> DefaultTime2Retain + DefaultTime2Wait, also doesn't honor the target's
>>>> request to transition to full feature phase, but then still attempts to
>>>> respond to the keys.
>>> Interestingly, I was suggesting the same as a workaround to our Windows 
>>> driver team earlier Today.
>>>
>> :-)
>>
>>>> So really this is a grey area.  The original hack to support GlobalSAN
>>>> is definitely not RFC, but at the same time Qlogic MSFT should really be
>>>> sending DefaultTime2Retain + DefaultTime2Wait, and should be honoring
>>>> LIO's ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT to
>>>> transition to full feature phase.
>>> IMHO, not proposing the keys (and taking defaults, instead) is better, and 
>>> more RFC compliant, than not waiting for a response to the proposed keys. 
>>> This would not be so bad because most initiators proposes main keys in the 
>>> initial request itself anyway.
>>>
>>> This is what RFC has to say about the response to proposals:
>>>
>>>     Responses are REQUIRED in all other cases, and the value chosen and
>>>     sent by the acceptor becomes the outcome of the negotiation.
>>>
>>> The exception was given only to boolean keys; given that these keys are 
>>> not boolean keys, initiators are required by the RFC to respond.
>>>
>>> ..and this about using defaults:
>>>
>>>     All negotiations are explicit (i.e., the result MUST only be based on
>>>     newly exchanged or declared values).  There are no implicit
>>>     proposals.  If a proposal is not made, then a reply cannot be
>>>     expected.  Conservative design also requires that default values
>>>     should not be relied upon when use of some other value has serious
>>>     consequences.
>>>
>>> Just my 2c.
>>>
>> In retrospect, I agree not proposing the keys and using RFC defaults
>> instead is a better approach than the original hack.  It would involve a
>> larger change to existing code to work though..
>>
>> In the case DefaultTime2Retain and DefaultTime2Wait having either sides
>> use (potentially) different values is harmless, considering they don't
>> have anything to do with I/O.
>>
>> However, the original GlobalSAN initiator also didn't propose nor
>> respond to FirstBurstLength or MaxBurstLength..
>>
>> So taking the approach to just use defaults and don't propose anything
>> for the non I/O related keys would be OK, but for the I/O related keys
>> that may not be as safe an assumption.
>>
>> That said, looking around it appears at least some version of GlobalSAN
>> are now proposing the keys the hack in question originally addressed:
>>
>> https://forums.freebsd.org/threads/51006/
>>
>> Albeit, the log output is from discovery but I think it's a good sign
>> the RFC breakage may finally be addressed.
>>
>> If that is the case, I'd much proper to drop this old hack all-together.
>>
>> So I'll go steal a MacOSX laptop from someone tomorrow and give it a
>> shot with a modern GlobalSAN version.
>>
> Ok, I've been able to confirm with GlobalSAN iSCSI v5.3.0.541 that it
> does correctly propose the four keys in question, and makes the original
> hack in iscsi_check_proposer_for_optional_reply() that is causing
> problems here, now unnecessary.
>
> Sending out a patch for this shortly.
>
> Martin + Arun + QLogic folks, please verify on your end.
>
Verified, the patch fixes unknown opcode error during QLogic MSFT login. 

Note that in the mean time we downgraded the initiator to Windows 2012 R2
but the opcode error was the same as with Windows 2016. On target side, 
"No response for proposed key" messages are logged for DefaultTime2Retain +
DefaultTime2Wait now. Though, the session is successfully established. Is it
an expected behavior or does it mean that Windows 2012 R2 QLogic driver behaves
slightly different compared to Windows 2016?

Thank you,

Martin


--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Easi, Arun April 3, 2017, 9:26 p.m. UTC | #5
On Mon, 3 Apr 2017, 6:03am, Martin Svec wrote:

> Dne 2.4.2017 v 23:18 Nicholas A. Bellinger napsal(a):
> > On Fri, 2017-03-31 at 00:09 -0700, Nicholas A. Bellinger wrote:
> >> On Thu, 2017-03-30 at 22:35 -0700, Arun Easi wrote:
> > <SNIP>
> >
> >>>> The result is that since the QLogic MSFT initiator doesn't propose them,
> >>>> LIO proposes them itself, and then immediately transitions to full
> >>>> feature phase.  However, the QLogic MSFT side still attempts to respond
> >>>> to DefaultTime2Retain and DefaultTime2Wait, even though
> >>>> ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT have been set
> >>>> in the last login response by LIO.
> >>>>
> >>>> AFAIK, this is the only initiator I've seen that doesn't propose
> >>>> DefaultTime2Retain + DefaultTime2Wait, also doesn't honor the target's
> >>>> request to transition to full feature phase, but then still attempts to
> >>>> respond to the keys.
> >>> Interestingly, I was suggesting the same as a workaround to our Windows 
> >>> driver team earlier Today.
> >>>
> >> :-)
> >>
> >>>> So really this is a grey area.  The original hack to support GlobalSAN
> >>>> is definitely not RFC, but at the same time Qlogic MSFT should really be
> >>>> sending DefaultTime2Retain + DefaultTime2Wait, and should be honoring
> >>>> LIO's ISCSI_FLAG_LOGIN_NEXT_STAGE3 and ISCSI_FLAG_LOGIN_TRANSIT to
> >>>> transition to full feature phase.
> >>> IMHO, not proposing the keys (and taking defaults, instead) is better, and 
> >>> more RFC compliant, than not waiting for a response to the proposed keys. 
> >>> This would not be so bad because most initiators proposes main keys in the 
> >>> initial request itself anyway.
> >>>
> >>> This is what RFC has to say about the response to proposals:
> >>>
> >>>     Responses are REQUIRED in all other cases, and the value chosen and
> >>>     sent by the acceptor becomes the outcome of the negotiation.
> >>>
> >>> The exception was given only to boolean keys; given that these keys are 
> >>> not boolean keys, initiators are required by the RFC to respond.
> >>>
> >>> ..and this about using defaults:
> >>>
> >>>     All negotiations are explicit (i.e., the result MUST only be based on
> >>>     newly exchanged or declared values).  There are no implicit
> >>>     proposals.  If a proposal is not made, then a reply cannot be
> >>>     expected.  Conservative design also requires that default values
> >>>     should not be relied upon when use of some other value has serious
> >>>     consequences.
> >>>
> >>> Just my 2c.
> >>>
> >> In retrospect, I agree not proposing the keys and using RFC defaults
> >> instead is a better approach than the original hack.  It would involve a
> >> larger change to existing code to work though..
> >>
> >> In the case DefaultTime2Retain and DefaultTime2Wait having either sides
> >> use (potentially) different values is harmless, considering they don't
> >> have anything to do with I/O.
> >>
> >> However, the original GlobalSAN initiator also didn't propose nor
> >> respond to FirstBurstLength or MaxBurstLength..
> >>
> >> So taking the approach to just use defaults and don't propose anything
> >> for the non I/O related keys would be OK, but for the I/O related keys
> >> that may not be as safe an assumption.
> >>
> >> That said, looking around it appears at least some version of GlobalSAN
> >> are now proposing the keys the hack in question originally addressed:
> >>
> >> https://forums.freebsd.org/threads/51006/
> >>
> >> Albeit, the log output is from discovery but I think it's a good sign
> >> the RFC breakage may finally be addressed.
> >>
> >> If that is the case, I'd much proper to drop this old hack all-together.
> >>
> >> So I'll go steal a MacOSX laptop from someone tomorrow and give it a
> >> shot with a modern GlobalSAN version.
> >>
> > Ok, I've been able to confirm with GlobalSAN iSCSI v5.3.0.541 that it
> > does correctly propose the four keys in question, and makes the original
> > hack in iscsi_check_proposer_for_optional_reply() that is causing
> > problems here, now unnecessary.
> >
> > Sending out a patch for this shortly.
> >
> > Martin + Arun + QLogic folks, please verify on your end.
> >
> Verified, the patch fixes unknown opcode error during QLogic MSFT login. 
> 
> Note that in the mean time we downgraded the initiator to Windows 2012 R2
> but the opcode error was the same as with Windows 2016. On target side, 
> "No response for proposed key" messages are logged for DefaultTime2Retain +
> DefaultTime2Wait now. Though, the session is successfully established. Is it
> an expected behavior or does it mean that Windows 2012 R2 QLogic driver behaves
> slightly different compared to Windows 2016?
> 

The issue is applicable to the newer 579xx series (officially QLogic 45000 
/ 41000 Series) as well. I tested this patch with that and it works. The 
"No response.." message appears for me as well.

Log excerpt with dynamic debugging turned on for iscsi_target_parameters.c.

--8<--
[ 2563.528845] Sending key: MaxOutstandingR2T=1
[ 2563.528847] Sending key: ErrorRecoveryLevel=0
[ 2563.528850] No response for proposed key "DefaultTime2Wait".
[ 2563.528851] No response for proposed key "DefaultTime2Retain".
[ 2563.529074] Got key: DefaultTime2Retain=20
[ 2563.529079] iSCSI Parameter updated to DefaultTime2Retain=20
[ 2563.529081] Got key: DefaultTime2Wait=2
[ 2563.529083] iSCSI Parameter updated to DefaultTime2Wait=2
[ 2563.529313] ------------------------------------------------------------------
[ 2563.529316] AuthMethod:                   None
[ 2563.529317] HeaderDigest:                 None
[ 2563.529318] DataDigest:                   None
[ 2563.529322] MaxXmitDataSegmentLength:     262144
[ 2563.529324] MaxRecvDataSegmentLength:     131072
[ 2563.529331] ------------------------------------------------------------------
--8<--

Thanks Nic!

Regards,
-Arun
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_param
index e65bf78..ecde825 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -793,10 +793,12 @@  static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param)
                        SET_PSTATE_REPLY_OPTIONAL(param);
                if (!strcmp(param->name, FIRSTBURSTLENGTH))
                        SET_PSTATE_REPLY_OPTIONAL(param);
+#if 0
                if (!strcmp(param->name, DEFAULTTIME2WAIT))
                        SET_PSTATE_REPLY_OPTIONAL(param);
                if (!strcmp(param->name, DEFAULTTIME2RETAIN))
                        SET_PSTATE_REPLY_OPTIONAL(param);
+#endif
                /*
                 * Required for gPXE iSCSI boot client
                 */