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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html