Hey Arun & Robert, On Thu, 2017-06-29 at 23:07 -0700, Nicholas A. Bellinger wrote: > 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.. > Ok, just postd the following patch to re-add the FirstBurstLength work-around for FlexBoot, and make it configurable via a new 'login_keys_workaround' TPG attribute: iscsi-target: Add login_keys_workaround attribute for non RFC initiators https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git/commit/?h=for-next&id=6bdef5928b40d0225b47b055bbe41b3101f0babf However, after further consideration I did end up enabling this work-around by default, since it doesn't have any effect on QLogic MSFT, and so it 'just works' for existing Flexboot users. So that said, Arun & Robert, please grab this patch and test it ASAP, so it can be pushed in the upcoming v4.13-rc1 PULL request. Thank you. -- 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
