From: Nicholas Bellinger <[email protected]>
This patch re-introduces part of a long standing login workaround that
was recently dropped by:
commit 1c99de981f30b3e7868b8d20ce5479fa1c0fea46
Author: Nicholas Bellinger <[email protected]>
Date: Sun Apr 2 13:36:44 2017 -0700
iscsi-target: Drop work-around for legacy GlobalSAN initiator
Namely, the workaround for FirstBurstLength ended up being required by
Mellanox Flexboot PXE boot ROMs as reported by Robert.
So this patch re-adds the work-around for FirstBurstLength within
iscsi_check_proposer_for_optional_reply(), and makes the key optional
to respond when the initiator does not propose, nor respond to it.
Also as requested by Arun, this patch introduces a new TPG attribute
named 'login_keys_workaround' that controls the use of both the
FirstBurstLength workaround, as well as the two other existing
workarounds for gPXE iSCSI boot client.
By default, the workaround is enabled with login_keys_workaround=1,
since Mellanox FlexBoot requires it, and Arun has verified the Qlogic
MSFT initiator already proposes FirstBurstLength, so it's uneffected
by this re-adding this part of the original work-around.
Reported-by: Robert LeBlanc <[email protected]>
Cc: Robert LeBlanc <[email protected]>
Cc: Arun Easi <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/iscsi/iscsi_target_configfs.c | 2 ++
drivers/target/iscsi/iscsi_target_nego.c | 6 ++--
drivers/target/iscsi/iscsi_target_parameters.c | 41 ++++++++++++++++++--------
drivers/target/iscsi/iscsi_target_parameters.h | 2 +-
drivers/target/iscsi/iscsi_target_tpg.c | 19 ++++++++++++
drivers/target/iscsi/iscsi_target_tpg.h | 1 +
include/target/iscsi/iscsi_target_core.h | 9 ++++++
7 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
index 535a8e0..0dd4c45 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -781,6 +781,7 @@ static int lio_target_init_nodeacl(struct se_node_acl *se_nacl,
DEF_TPG_ATTRIB(t10_pi);
DEF_TPG_ATTRIB(fabric_prot_type);
DEF_TPG_ATTRIB(tpg_enabled_sendtargets);
+DEF_TPG_ATTRIB(login_keys_workaround);
static struct configfs_attribute *lio_target_tpg_attrib_attrs[] = {
&iscsi_tpg_attrib_attr_authentication,
@@ -796,6 +797,7 @@ static int lio_target_init_nodeacl(struct se_node_acl *se_nacl,
&iscsi_tpg_attrib_attr_t10_pi,
&iscsi_tpg_attrib_attr_fabric_prot_type,
&iscsi_tpg_attrib_attr_tpg_enabled_sendtargets,
+ &iscsi_tpg_attrib_attr_login_keys_workaround,
NULL,
};
diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
index 96df63f..7a6751f 100644
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -864,7 +864,8 @@ static int iscsi_target_handle_csg_zero(
SENDER_TARGET,
login->rsp_buf,
&login->rsp_length,
- conn->param_list);
+ conn->param_list,
+ conn->tpg->tpg_attrib.login_keys_workaround);
if (ret < 0)
return -1;
@@ -934,7 +935,8 @@ static int iscsi_target_handle_csg_one(struct iscsi_conn *conn, struct iscsi_log
SENDER_TARGET,
login->rsp_buf,
&login->rsp_length,
- conn->param_list);
+ conn->param_list,
+ conn->tpg->tpg_attrib.login_keys_workaround);
if (ret < 0) {
iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_INITIATOR_ERR,
ISCSI_LOGIN_STATUS_INIT_ERR);
diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c
index fce6276..caab104 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -765,7 +765,8 @@ static int iscsi_check_for_auth_key(char *key)
return 0;
}
-static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param)
+static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param,
+ bool keys_workaround)
{
if (IS_TYPE_BOOL_AND(param)) {
if (!strcmp(param->value, NO))
@@ -773,19 +774,31 @@ static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param)
} else if (IS_TYPE_BOOL_OR(param)) {
if (!strcmp(param->value, YES))
SET_PSTATE_REPLY_OPTIONAL(param);
- /*
- * Required for gPXE iSCSI boot client
- */
- if (!strcmp(param->name, IMMEDIATEDATA))
- SET_PSTATE_REPLY_OPTIONAL(param);
+
+ if (keys_workaround) {
+ /*
+ * Required for gPXE iSCSI boot client
+ */
+ if (!strcmp(param->name, IMMEDIATEDATA))
+ SET_PSTATE_REPLY_OPTIONAL(param);
+ }
} else if (IS_TYPE_NUMBER(param)) {
if (!strcmp(param->name, MAXRECVDATASEGMENTLENGTH))
SET_PSTATE_REPLY_OPTIONAL(param);
- /*
- * Required for gPXE iSCSI boot client
- */
- if (!strcmp(param->name, MAXCONNECTIONS))
- SET_PSTATE_REPLY_OPTIONAL(param);
+
+ if (keys_workaround) {
+ /*
+ * Required for Mellanox Flexboot PXE boot ROM
+ */
+ if (!strcmp(param->name, FIRSTBURSTLENGTH))
+ SET_PSTATE_REPLY_OPTIONAL(param);
+
+ /*
+ * Required for gPXE iSCSI boot client
+ */
+ if (!strcmp(param->name, MAXCONNECTIONS))
+ SET_PSTATE_REPLY_OPTIONAL(param);
+ }
} else if (IS_PHASE_DECLARATIVE(param))
SET_PSTATE_REPLY_OPTIONAL(param);
}
@@ -1422,7 +1435,8 @@ int iscsi_encode_text_output(
u8 sender,
char *textbuf,
u32 *length,
- struct iscsi_param_list *param_list)
+ struct iscsi_param_list *param_list,
+ bool keys_workaround)
{
char *output_buf = NULL;
struct iscsi_extra_response *er;
@@ -1458,7 +1472,8 @@ int iscsi_encode_text_output(
*length += 1;
output_buf = textbuf + *length;
SET_PSTATE_PROPOSER(param);
- iscsi_check_proposer_for_optional_reply(param);
+ iscsi_check_proposer_for_optional_reply(param,
+ keys_workaround);
pr_debug("Sending key: %s=%s\n",
param->name, param->value);
}
diff --git a/drivers/target/iscsi/iscsi_target_parameters.h b/drivers/target/iscsi/iscsi_target_parameters.h
index 9962ccf..c47b73f 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.h
+++ b/drivers/target/iscsi/iscsi_target_parameters.h
@@ -46,7 +46,7 @@ extern int iscsi_copy_param_list(struct iscsi_param_list **,
extern int iscsi_update_param_value(struct iscsi_param *, char *);
extern int iscsi_decode_text_input(u8, u8, char *, u32, struct iscsi_conn *);
extern int iscsi_encode_text_output(u8, u8, char *, u32 *,
- struct iscsi_param_list *);
+ struct iscsi_param_list *, bool);
extern int iscsi_check_negotiated_keys(struct iscsi_param_list *);
extern void iscsi_set_connection_parameters(struct iscsi_conn_ops *,
struct iscsi_param_list *);
diff --git a/drivers/target/iscsi/iscsi_target_tpg.c b/drivers/target/iscsi/iscsi_target_tpg.c
index abaabba..594d07a 100644
--- a/drivers/target/iscsi/iscsi_target_tpg.c
+++ b/drivers/target/iscsi/iscsi_target_tpg.c
@@ -227,6 +227,7 @@ static void iscsit_set_default_tpg_attribs(struct iscsi_portal_group *tpg)
a->t10_pi = TA_DEFAULT_T10_PI;
a->fabric_prot_type = TA_DEFAULT_FABRIC_PROT_TYPE;
a->tpg_enabled_sendtargets = TA_DEFAULT_TPG_ENABLED_SENDTARGETS;
+ a->login_keys_workaround = TA_DEFAULT_LOGIN_KEYS_WORKAROUND;
}
int iscsit_tpg_add_portal_group(struct iscsi_tiqn *tiqn, struct iscsi_portal_group *tpg)
@@ -895,3 +896,21 @@ int iscsit_ta_tpg_enabled_sendtargets(
return 0;
}
+
+int iscsit_ta_login_keys_workaround(
+ struct iscsi_portal_group *tpg,
+ u32 flag)
+{
+ struct iscsi_tpg_attrib *a = &tpg->tpg_attrib;
+
+ if ((flag != 0) && (flag != 1)) {
+ pr_err("Illegal value %d\n", flag);
+ return -EINVAL;
+ }
+
+ a->login_keys_workaround = flag;
+ pr_debug("iSCSI_TPG[%hu] - TPG enabled bit for login keys workaround: %s ",
+ tpg->tpgt, (a->login_keys_workaround) ? "ON" : "OFF");
+
+ return 0;
+}
diff --git a/drivers/target/iscsi/iscsi_target_tpg.h b/drivers/target/iscsi/iscsi_target_tpg.h
index ceba298..59fd3ca 100644
--- a/drivers/target/iscsi/iscsi_target_tpg.h
+++ b/drivers/target/iscsi/iscsi_target_tpg.h
@@ -48,5 +48,6 @@ extern int iscsit_tpg_del_network_portal(struct iscsi_portal_group *,
extern int iscsit_ta_t10_pi(struct iscsi_portal_group *, u32);
extern int iscsit_ta_fabric_prot_type(struct iscsi_portal_group *, u32);
extern int iscsit_ta_tpg_enabled_sendtargets(struct iscsi_portal_group *, u32);
+extern int iscsit_ta_login_keys_workaround(struct iscsi_portal_group *, u32);
#endif /* ISCSI_TARGET_TPG_H */
diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
index 7948fc6..0ca1fb0 100644
--- a/include/target/iscsi/iscsi_target_core.h
+++ b/include/target/iscsi/iscsi_target_core.h
@@ -66,6 +66,14 @@
#define TA_DEFAULT_FABRIC_PROT_TYPE 0
/* TPG status needs to be enabled to return sendtargets discovery endpoint info */
#define TA_DEFAULT_TPG_ENABLED_SENDTARGETS 1
+/*
+ * Used to control the sending of keys with optional to respond state bit,
+ * as a workaround for non RFC compliant initiators,that do not propose,
+ * nor respond to specific keys required for login to complete.
+ *
+ * See iscsi_check_proposer_for_optional_reply() for more details.
+ */
+#define TA_DEFAULT_LOGIN_KEYS_WORKAROUND 1
#define ISCSI_IOV_DATA_BUFFER 5
@@ -768,6 +776,7 @@ struct iscsi_tpg_attrib {
u8 t10_pi;
u32 fabric_prot_type;
u32 tpg_enabled_sendtargets;
+ u32 login_keys_workaround;
struct iscsi_portal_group *tpg;
};
--
1.9.1
Hey Robert,
Any chance to test this with your Flexboot PXE setup..?
Please give this a spin ASAP to verify it addresses the regression you
reported earlier, wrt FirstBurstLength not being proposed nor responded
to using Flexboot PXE.
Thank you.
On Fri, 2017-07-07 at 22:24 +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <[email protected]>
>
> This patch re-introduces part of a long standing login workaround that
> was recently dropped by:
>
> commit 1c99de981f30b3e7868b8d20ce5479fa1c0fea46
> Author: Nicholas Bellinger <[email protected]>
> Date: Sun Apr 2 13:36:44 2017 -0700
>
> iscsi-target: Drop work-around for legacy GlobalSAN initiator
>
> Namely, the workaround for FirstBurstLength ended up being required by
> Mellanox Flexboot PXE boot ROMs as reported by Robert.
>
> So this patch re-adds the work-around for FirstBurstLength within
> iscsi_check_proposer_for_optional_reply(), and makes the key optional
> to respond when the initiator does not propose, nor respond to it.
>
> Also as requested by Arun, this patch introduces a new TPG attribute
> named 'login_keys_workaround' that controls the use of both the
> FirstBurstLength workaround, as well as the two other existing
> workarounds for gPXE iSCSI boot client.
>
> By default, the workaround is enabled with login_keys_workaround=1,
> since Mellanox FlexBoot requires it, and Arun has verified the Qlogic
> MSFT initiator already proposes FirstBurstLength, so it's uneffected
> by this re-adding this part of the original work-around.
>
> Reported-by: Robert LeBlanc <[email protected]>
> Cc: Robert LeBlanc <[email protected]>
> Cc: Arun Easi <[email protected]>
> Signed-off-by: Nicholas Bellinger <[email protected]>
> ---
> drivers/target/iscsi/iscsi_target_configfs.c | 2 ++
> drivers/target/iscsi/iscsi_target_nego.c | 6 ++--
> drivers/target/iscsi/iscsi_target_parameters.c | 41 ++++++++++++++++++--------
> drivers/target/iscsi/iscsi_target_parameters.h | 2 +-
> drivers/target/iscsi/iscsi_target_tpg.c | 19 ++++++++++++
> drivers/target/iscsi/iscsi_target_tpg.h | 1 +
> include/target/iscsi/iscsi_target_core.h | 9 ++++++
> 7 files changed, 64 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
> index 535a8e0..0dd4c45 100644
> --- a/drivers/target/iscsi/iscsi_target_configfs.c
> +++ b/drivers/target/iscsi/iscsi_target_configfs.c
> @@ -781,6 +781,7 @@ static int lio_target_init_nodeacl(struct se_node_acl *se_nacl,
> DEF_TPG_ATTRIB(t10_pi);
> DEF_TPG_ATTRIB(fabric_prot_type);
> DEF_TPG_ATTRIB(tpg_enabled_sendtargets);
> +DEF_TPG_ATTRIB(login_keys_workaround);
>
> static struct configfs_attribute *lio_target_tpg_attrib_attrs[] = {
> &iscsi_tpg_attrib_attr_authentication,
> @@ -796,6 +797,7 @@ static int lio_target_init_nodeacl(struct se_node_acl *se_nacl,
> &iscsi_tpg_attrib_attr_t10_pi,
> &iscsi_tpg_attrib_attr_fabric_prot_type,
> &iscsi_tpg_attrib_attr_tpg_enabled_sendtargets,
> + &iscsi_tpg_attrib_attr_login_keys_workaround,
> NULL,
> };
>
> diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
> index 96df63f..7a6751f 100644
> --- a/drivers/target/iscsi/iscsi_target_nego.c
> +++ b/drivers/target/iscsi/iscsi_target_nego.c
> @@ -864,7 +864,8 @@ static int iscsi_target_handle_csg_zero(
> SENDER_TARGET,
> login->rsp_buf,
> &login->rsp_length,
> - conn->param_list);
> + conn->param_list,
> + conn->tpg->tpg_attrib.login_keys_workaround);
> if (ret < 0)
> return -1;
>
> @@ -934,7 +935,8 @@ static int iscsi_target_handle_csg_one(struct iscsi_conn *conn, struct iscsi_log
> SENDER_TARGET,
> login->rsp_buf,
> &login->rsp_length,
> - conn->param_list);
> + conn->param_list,
> + conn->tpg->tpg_attrib.login_keys_workaround);
> if (ret < 0) {
> iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_INITIATOR_ERR,
> ISCSI_LOGIN_STATUS_INIT_ERR);
> diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c
> index fce6276..caab104 100644
> --- a/drivers/target/iscsi/iscsi_target_parameters.c
> +++ b/drivers/target/iscsi/iscsi_target_parameters.c
> @@ -765,7 +765,8 @@ static int iscsi_check_for_auth_key(char *key)
> return 0;
> }
>
> -static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param)
> +static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param,
> + bool keys_workaround)
> {
> if (IS_TYPE_BOOL_AND(param)) {
> if (!strcmp(param->value, NO))
> @@ -773,19 +774,31 @@ static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param)
> } else if (IS_TYPE_BOOL_OR(param)) {
> if (!strcmp(param->value, YES))
> SET_PSTATE_REPLY_OPTIONAL(param);
> - /*
> - * Required for gPXE iSCSI boot client
> - */
> - if (!strcmp(param->name, IMMEDIATEDATA))
> - SET_PSTATE_REPLY_OPTIONAL(param);
> +
> + if (keys_workaround) {
> + /*
> + * Required for gPXE iSCSI boot client
> + */
> + if (!strcmp(param->name, IMMEDIATEDATA))
> + SET_PSTATE_REPLY_OPTIONAL(param);
> + }
> } else if (IS_TYPE_NUMBER(param)) {
> if (!strcmp(param->name, MAXRECVDATASEGMENTLENGTH))
> SET_PSTATE_REPLY_OPTIONAL(param);
> - /*
> - * Required for gPXE iSCSI boot client
> - */
> - if (!strcmp(param->name, MAXCONNECTIONS))
> - SET_PSTATE_REPLY_OPTIONAL(param);
> +
> + if (keys_workaround) {
> + /*
> + * Required for Mellanox Flexboot PXE boot ROM
> + */
> + if (!strcmp(param->name, FIRSTBURSTLENGTH))
> + SET_PSTATE_REPLY_OPTIONAL(param);
> +
> + /*
> + * Required for gPXE iSCSI boot client
> + */
> + if (!strcmp(param->name, MAXCONNECTIONS))
> + SET_PSTATE_REPLY_OPTIONAL(param);
> + }
> } else if (IS_PHASE_DECLARATIVE(param))
> SET_PSTATE_REPLY_OPTIONAL(param);
> }
> @@ -1422,7 +1435,8 @@ int iscsi_encode_text_output(
> u8 sender,
> char *textbuf,
> u32 *length,
> - struct iscsi_param_list *param_list)
> + struct iscsi_param_list *param_list,
> + bool keys_workaround)
> {
> char *output_buf = NULL;
> struct iscsi_extra_response *er;
> @@ -1458,7 +1472,8 @@ int iscsi_encode_text_output(
> *length += 1;
> output_buf = textbuf + *length;
> SET_PSTATE_PROPOSER(param);
> - iscsi_check_proposer_for_optional_reply(param);
> + iscsi_check_proposer_for_optional_reply(param,
> + keys_workaround);
> pr_debug("Sending key: %s=%s\n",
> param->name, param->value);
> }
> diff --git a/drivers/target/iscsi/iscsi_target_parameters.h b/drivers/target/iscsi/iscsi_target_parameters.h
> index 9962ccf..c47b73f 100644
> --- a/drivers/target/iscsi/iscsi_target_parameters.h
> +++ b/drivers/target/iscsi/iscsi_target_parameters.h
> @@ -46,7 +46,7 @@ extern int iscsi_copy_param_list(struct iscsi_param_list **,
> extern int iscsi_update_param_value(struct iscsi_param *, char *);
> extern int iscsi_decode_text_input(u8, u8, char *, u32, struct iscsi_conn *);
> extern int iscsi_encode_text_output(u8, u8, char *, u32 *,
> - struct iscsi_param_list *);
> + struct iscsi_param_list *, bool);
> extern int iscsi_check_negotiated_keys(struct iscsi_param_list *);
> extern void iscsi_set_connection_parameters(struct iscsi_conn_ops *,
> struct iscsi_param_list *);
> diff --git a/drivers/target/iscsi/iscsi_target_tpg.c b/drivers/target/iscsi/iscsi_target_tpg.c
> index abaabba..594d07a 100644
> --- a/drivers/target/iscsi/iscsi_target_tpg.c
> +++ b/drivers/target/iscsi/iscsi_target_tpg.c
> @@ -227,6 +227,7 @@ static void iscsit_set_default_tpg_attribs(struct iscsi_portal_group *tpg)
> a->t10_pi = TA_DEFAULT_T10_PI;
> a->fabric_prot_type = TA_DEFAULT_FABRIC_PROT_TYPE;
> a->tpg_enabled_sendtargets = TA_DEFAULT_TPG_ENABLED_SENDTARGETS;
> + a->login_keys_workaround = TA_DEFAULT_LOGIN_KEYS_WORKAROUND;
> }
>
> int iscsit_tpg_add_portal_group(struct iscsi_tiqn *tiqn, struct iscsi_portal_group *tpg)
> @@ -895,3 +896,21 @@ int iscsit_ta_tpg_enabled_sendtargets(
>
> return 0;
> }
> +
> +int iscsit_ta_login_keys_workaround(
> + struct iscsi_portal_group *tpg,
> + u32 flag)
> +{
> + struct iscsi_tpg_attrib *a = &tpg->tpg_attrib;
> +
> + if ((flag != 0) && (flag != 1)) {
> + pr_err("Illegal value %d\n", flag);
> + return -EINVAL;
> + }
> +
> + a->login_keys_workaround = flag;
> + pr_debug("iSCSI_TPG[%hu] - TPG enabled bit for login keys workaround: %s ",
> + tpg->tpgt, (a->login_keys_workaround) ? "ON" : "OFF");
> +
> + return 0;
> +}
> diff --git a/drivers/target/iscsi/iscsi_target_tpg.h b/drivers/target/iscsi/iscsi_target_tpg.h
> index ceba298..59fd3ca 100644
> --- a/drivers/target/iscsi/iscsi_target_tpg.h
> +++ b/drivers/target/iscsi/iscsi_target_tpg.h
> @@ -48,5 +48,6 @@ extern int iscsit_tpg_del_network_portal(struct iscsi_portal_group *,
> extern int iscsit_ta_t10_pi(struct iscsi_portal_group *, u32);
> extern int iscsit_ta_fabric_prot_type(struct iscsi_portal_group *, u32);
> extern int iscsit_ta_tpg_enabled_sendtargets(struct iscsi_portal_group *, u32);
> +extern int iscsit_ta_login_keys_workaround(struct iscsi_portal_group *, u32);
>
> #endif /* ISCSI_TARGET_TPG_H */
> diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
> index 7948fc6..0ca1fb0 100644
> --- a/include/target/iscsi/iscsi_target_core.h
> +++ b/include/target/iscsi/iscsi_target_core.h
> @@ -66,6 +66,14 @@
> #define TA_DEFAULT_FABRIC_PROT_TYPE 0
> /* TPG status needs to be enabled to return sendtargets discovery endpoint info */
> #define TA_DEFAULT_TPG_ENABLED_SENDTARGETS 1
> +/*
> + * Used to control the sending of keys with optional to respond state bit,
> + * as a workaround for non RFC compliant initiators,that do not propose,
> + * nor respond to specific keys required for login to complete.
> + *
> + * See iscsi_check_proposer_for_optional_reply() for more details.
> + */
> +#define TA_DEFAULT_LOGIN_KEYS_WORKAROUND 1
>
> #define ISCSI_IOV_DATA_BUFFER 5
>
> @@ -768,6 +776,7 @@ struct iscsi_tpg_attrib {
> u8 t10_pi;
> u32 fabric_prot_type;
> u32 tpg_enabled_sendtargets;
> + u32 login_keys_workaround;
> struct iscsi_portal_group *tpg;
> };
>
On Fri, 2017-07-07 at 22:24 +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <[email protected]>
>
> This patch re-introduces part of a long standing login workaround that
> was recently dropped by:
>
> commit 1c99de981f30b3e7868b8d20ce5479fa1c0fea46
> Author: Nicholas Bellinger <[email protected]>
> Date: Sun Apr 2 13:36:44 2017 -0700
>
> iscsi-target: Drop work-around for legacy GlobalSAN initiator
>
> Namely, the workaround for FirstBurstLength ended up being required by
> Mellanox Flexboot PXE boot ROMs as reported by Robert.
>
> So this patch re-adds the work-around for FirstBurstLength within
> iscsi_check_proposer_for_optional_reply(), and makes the key optional
> to respond when the initiator does not propose, nor respond to it.
>
> Also as requested by Arun, this patch introduces a new TPG attribute
> named 'login_keys_workaround' that controls the use of both the
> FirstBurstLength workaround, as well as the two other existing
> workarounds for gPXE iSCSI boot client.
>
> By default, the workaround is enabled with login_keys_workaround=1,
> since Mellanox FlexBoot requires it, and Arun has verified the Qlogic
> MSFT initiator already proposes FirstBurstLength, so it's uneffected
> by this re-adding this part of the original work-around.
Hello Nick,
The new configfs attribute ("login_keys_workaround") may confuse users - for
someone who has not followed this e-mail thread it can take a long time
before they figure out that they need to set this configfs attribute. Have
you considered to let the iSCSI target driver figure out whether or not that
variable has to be set, e.g. by looking up the initiator IQN in a list?
Bart.
On Tue, 2017-07-11 at 23:38 +0000, Bart Van Assche wrote:
> On Fri, 2017-07-07 at 22:24 +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <[email protected]>
> >
> > This patch re-introduces part of a long standing login workaround that
> > was recently dropped by:
> >
> > commit 1c99de981f30b3e7868b8d20ce5479fa1c0fea46
> > Author: Nicholas Bellinger <[email protected]>
> > Date: Sun Apr 2 13:36:44 2017 -0700
> >
> > iscsi-target: Drop work-around for legacy GlobalSAN initiator
> >
> > Namely, the workaround for FirstBurstLength ended up being required by
> > Mellanox Flexboot PXE boot ROMs as reported by Robert.
> >
> > So this patch re-adds the work-around for FirstBurstLength within
> > iscsi_check_proposer_for_optional_reply(), and makes the key optional
> > to respond when the initiator does not propose, nor respond to it.
> >
> > Also as requested by Arun, this patch introduces a new TPG attribute
> > named 'login_keys_workaround' that controls the use of both the
> > FirstBurstLength workaround, as well as the two other existing
> > workarounds for gPXE iSCSI boot client.
> >
> > By default, the workaround is enabled with login_keys_workaround=1,
> > since Mellanox FlexBoot requires it, and Arun has verified the Qlogic
> > MSFT initiator already proposes FirstBurstLength, so it's uneffected
> > by this re-adding this part of the original work-around.
>
> Hello Nick,
>
> The new configfs attribute ("login_keys_workaround") may confuse users - for
> someone who has not followed this e-mail thread it can take a long time
> before they figure out that they need to set this configfs attribute.
It's enabled by default, so there is nothing a user has to explicitly
change in order all hosts to 'just work'.
The only reason the attribute was added was by request of Arun, so if
some future initiator doesn't proposed the keys controlled by the
work-around, and still attempts to respond they can at least get
something working w/o code change.
> Have
> you considered to let the iSCSI target driver figure out whether or not that
> variable has to be set, e.g. by looking up the initiator IQN in a list?
>
Given InitiatorName is end user configurable, trying to do a workaround
based on IQN regexs is error prone, at best.
Also, since the FirstBurstLength work-around this patch re-adds had
already been in place for the better part of 8 years, the risk of
interopt issues is almost non existent.