On Mon, 2017-06-26 at 11:51 -0700, Arun Easi wrote: > On Sun, 25 Jun 2017, 1:15pm, Nicholas A. Bellinger 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. > > > > 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. > > > > 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. > > Yes, FirstBurstLength is proposed initially in our case. Agree, it looks > ok from our side. > > Just wish these workarounds come with knobs, with a default off setting. Fair point. Will think about how that might look.. -- 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
