On Sun, Jun 25, 2017 at 2:15 PM, Nicholas A. Bellinger <nab@xxxxxxxxxxxxxxx> wrote: > Hi Robert, > > Adding Or Gerlitz CC', as he can direct this to the relevant Mellanox > folks responsible for Flexboot. > > Also adding Arun from Cavium CC', to ensure any additional change don't > break the QLogic/MSFT iscsi environment we addressed by dropping the > legacy work-around. > > On Fri, 2017-06-16 at 14:14 -0600, Robert LeBlanc wrote: >> 281e36cbaf : iscsi-target: Drop work-around for legacy GlobalSAN initiator >> > > For reference, here is the email thread: > > http://www.spinics.net/lists/target-devel/msg14938.html > >> Causes "No response for proposed key "FirstBurstLength"." when the >> target is running 4.9.32. We tested both FlexBoot 3.5.109 and 3.5.110. >> If we revert 281e36cbaf then FlexBoot 3.5.109 boots fine, but FlexBoot >> 3.5.110 fails with messages "cmd exceeds last lba 64 (lba 64, sectors >> 1)". >> >> It seems that FlexBoot has relied upon the workaround that now has been removed. >> > > Ok, so it looks like Flexboot is not proposing nor responding to > FirstBurstLength.. > > Robert, please send along a packet capture to verify this is the only > key not being proposed. I'm OOO at the moment, I'll see if someone can get that capture. > Mellanox folks, can you confirm Flexboot is not proposing nor responding > to FirstBurstLength. > >> We have had to revert the patch and downgrade the firmware to get >> things working again. >> >> # flint -d /dev/mst/mt4115_pciconf0 q >> Image type: FS3 >> FW Version: 12.18.1000 >> FW Release Date: 25.1.2017 >> Product Version: rel-12_18_1000 >> Rom Info: type=PXE version=3.5.109 devid=4115 >> Description: UID GuidsNumber >> Base GUID: 248a0703001e7cc8 4 >> Base MAC: 0000248a071e7cc8 4 >> Image VSD: >> Device VSD: >> PSID: MT_2150110033 >> > > So the Qlogic/MSFT environment was not proposing the DefaultTime2Wait + > DefaultTime2Retain keys, which triggered the original work-around which > proposed the keys but did not wait for a response before transitioning > to full feature phase. > > Note the original work-around did this for DefaultTime2Wait, > DefaultTime2Retain, FirstBurstLength, and MaxBurstLength. > > However, the original work-around to propose the missing keys but not > wait for the response was in retrospect, not the right approach per > RFC-3720. > > That said, we've got two options to consider: > > 1) Get the Mellanox/Flexboot folks to fix their ROM to propose, > and/or respond to FirstBurstLength to follow RFC-3720. > 2) Re-add a similar iscsi-target work-around for FirstBurstLength. > > So #1 should really be fixed, as not proposing nor responding to > FirstBurstLength seems like a broken initiator to me, and would likely > run into problems with other targets as well. I agree, I'd prefer not to implement a non-RFC workaround as well for the same reasons. We will test the patch just to know for sure, but it may take a while. > For #2, since the work-around is not RFC I'm reluctant to re-add it. > However, since Qlogic/MFST was tripping over DefaultTime2Wait + > DefaultTime2Retain and not FirstBurstLength, in practice a work-around > for FirstBurstLength only would probably allow FlexBoot to 'just work', > and not break Qlogic/MFST or any other host environments. > > That said, here's a quick patch to do that. Robert, please verify on > your end with Flexboot. > > diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_targ > index fce6276..52b1d44 100644 > --- a/drivers/target/iscsi/iscsi_target_parameters.c > +++ b/drivers/target/iscsi/iscsi_target_parameters.c > @@ -786,6 +786,11 @@ static void iscsi_check_proposer_for_optional_reply(struct iscsi_param * > */ > if (!strcmp(param->name, MAXCONNECTIONS)) > SET_PSTATE_REPLY_OPTIONAL(param); > + /* > + * Required for Mellanox Flexboot iscsi ROM > + */ > + if (!strcmp(param->name, FIRSTBURSTLENGTH)) > + SET_PSTATE_REPLY_OPTIONAL(param); > } else if (IS_PHASE_DECLARATIVE(param)) > SET_PSTATE_REPLY_OPTIONAL(param); > } > > Arun, from looking at the original QLogic/MSFT packet captures > FirstBurstLength is being proposed, so I don't think re-adding this > work-around would have any effect on your end. > > Please verify. > ---------------- Robert LeBlanc PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1 -- 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