All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport
@ 2014-01-15  5:42 Nicholas A. Bellinger
  2014-01-15 16:06 ` Giridhar Malavali
  2014-01-15 22:14 ` Quinn Tran
  0 siblings, 2 replies; 7+ messages in thread
From: Nicholas A. Bellinger @ 2014-01-15  5:42 UTC (permalink / raw)
  To: target-devel
  Cc: linux-scsi, Saurav Kashyap, Quinn Tran, Sawan Chandak,
	Giridhar Malavali, Andrew Vasquez, Nicholas Bellinger

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi Saurav & Co,

Here is the updated NPIV patch on top of the original here:

  [PATCH] qla2xxx: Enhancements to enable NPIV support for QLOGIC ISPs with TCM/LIO.
  http://marc.info/?l=linux-scsi&m=138930044930212&w=2
   
As previously dicussed, it moves fc_vport creation into tcm_qla2xxx
control plane code thus allowing everything required for NPIV port
creation to be encapsulated within configfs.

There are still some bugs to be resolved.  Namely:

 * Determine how to save 'point-to-point' NVRAM settings across target restart
 * Figure out why the initiator logs in, but doesn't perform LUN scan on
   the first login attempt
 * Determine if it's possible to collapse the name formatting further,
   by generating $NPIV_WWNN internally
 * Determine proper NPIV format for EVPD=0x83 SCSI name string

Aside from these remaining issues, multiple NPIV ports can be configured
on a single physical FC port, and initial I/O appears to be functioning
as expected.

Please review.

Thank you,

--nab

--------------------------------------------------------------------

This patch changes qla2xxx qlt_lport_register() code to accept
target_lport_ptr + npiv_wwpn + npiv_wwnn parameters, and updates
tcm_qla2xxx to use the new tcm_qla2xxx_lport_register_npiv_cb()
callback for invoking fc_vport_create() from configfs context
via tcm_qla2xxx_npiv_make_lport() code.

In order for this to work, the qlt_lport_register() callback is
now called without holding qla_tgt_mutex, as the fc_vport creation
process will call qlt_vport_create() -> qlt_add_target(), which
already expects to acquire it.

It enforces /sys/kernel/config/target/qla2xxx_npiv/$NPIV_WWPN
naming in the following format:

     $PHYSICAL_WWPN@$NPIV_WWPN:$NPIV_WWNN

and assumes the $PHYSICAL_WWPN in question has already had been
configured for target mode in non NPIV mode.

Finally, it updates existing tcm_qla2xxx_lport_register_cb() logic
to setup the non NPIV assignments that have now been moved out of
qlt_lport_register() code.

Cc: Sawan Chandak <sawan.chandak@qlogic.com>
Cc: Quinn Tran <quinn.tran@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/scsi/qla2xxx/qla_target.c  |   25 ++++-----
 drivers/scsi/qla2xxx/qla_target.h  |    4 +-
 drivers/scsi/qla2xxx/tcm_qla2xxx.c |  106 ++++++++++++++++++++++++++----------
 3 files changed, 88 insertions(+), 47 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 34ed246..b596f8b 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -4235,8 +4235,9 @@ static void qlt_lport_dump(struct scsi_qla_host *vha, u64 wwpn,
  * @callback:  lport initialization callback for tcm_qla2xxx code
  * @target_lport_ptr: pointer for tcm_qla2xxx specific lport data
  */
-int qlt_lport_register(struct qla_tgt_func_tmpl *qla_tgt_ops, u64 wwpn,
-	int (*callback)(struct scsi_qla_host *), void *target_lport_ptr)
+int qlt_lport_register(void *target_lport_ptr, u64 phys_wwpn,
+		       u64 npiv_wwpn, u64 npiv_wwnn,
+		       int (*callback)(struct scsi_qla_host *, void *, u64, u64))
 {
 	struct qla_tgt *tgt;
 	struct scsi_qla_host *vha;
@@ -4259,7 +4260,7 @@ int qlt_lport_register(struct qla_tgt_func_tmpl *qla_tgt_ops, u64 wwpn,
 			continue;
 
 		spin_lock_irqsave(&ha->hardware_lock, flags);
-		if (host->active_mode & MODE_TARGET) {
+		if ((!npiv_wwpn || !npiv_wwnn) && host->active_mode & MODE_TARGET) {
 			pr_debug("MODE_TARGET already active on qla2xxx(%d)\n",
 			    host->host_no);
 			spin_unlock_irqrestore(&ha->hardware_lock, flags);
@@ -4273,24 +4274,18 @@ int qlt_lport_register(struct qla_tgt_func_tmpl *qla_tgt_ops, u64 wwpn,
 			    " qla2xxx scsi_host\n");
 			continue;
 		}
-		qlt_lport_dump(vha, wwpn, b);
+		qlt_lport_dump(vha, phys_wwpn, b);
 
 		if (memcmp(vha->port_name, b, WWN_SIZE)) {
 			scsi_host_put(host);
 			continue;
 		}
-		/*
-		 * Setup passed parameters ahead of invoking callback
-		 */
-		ha->tgt.tgt_ops = qla_tgt_ops;
-		vha->vha_tgt.target_lport_ptr = target_lport_ptr;
-		rc = (*callback)(vha);
-		if (rc != 0) {
-			ha->tgt.tgt_ops = NULL;
-			vha->vha_tgt.target_lport_ptr = NULL;
-			scsi_host_put(host);
-		}
 		mutex_unlock(&qla_tgt_mutex);
+
+		rc = (*callback)(vha, target_lport_ptr, npiv_wwpn, npiv_wwnn);
+		if (rc != 0)
+			scsi_host_put(host);
+
 		return rc;
 	}
 	mutex_unlock(&qla_tgt_mutex);
diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index b33e411..1d10eec 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -932,8 +932,8 @@ void qlt_disable_vha(struct scsi_qla_host *);
  */
 extern int qlt_add_target(struct qla_hw_data *, struct scsi_qla_host *);
 extern int qlt_remove_target(struct qla_hw_data *, struct scsi_qla_host *);
-extern int qlt_lport_register(struct qla_tgt_func_tmpl *, u64,
-			int (*callback)(struct scsi_qla_host *), void *);
+extern int qlt_lport_register(void *, u64, u64, u64,
+			int (*callback)(struct scsi_qla_host *, void *, u64, u64));
 extern void qlt_lport_deregister(struct scsi_qla_host *);
 extern void qlt_unreg_sess(struct qla_tgt_sess *);
 extern void qlt_fc_port_added(struct scsi_qla_host *, fc_port_t *);
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 113ca95..75a141b 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1559,14 +1559,18 @@ static int tcm_qla2xxx_init_lport(struct tcm_qla2xxx_lport *lport)
 	return 0;
 }
 
-static int tcm_qla2xxx_lport_register_cb(struct scsi_qla_host *vha)
+static int tcm_qla2xxx_lport_register_cb(struct scsi_qla_host *vha,
+					 void *target_lport_ptr,
+					 u64 npiv_wwpn, u64 npiv_wwnn)
 {
-	struct tcm_qla2xxx_lport *lport;
+	struct qla_hw_data *ha = vha->hw;
+	struct tcm_qla2xxx_lport *lport =
+			(struct tcm_qla2xxx_lport *)target_lport_ptr;
 	/*
-	 * Setup local pointer to vha, NPIV VP pointer (if present) and
-	 * vha->tcm_lport pointer
+	 * Setup tgt_ops, local pointer to vha and target_lport_ptr
 	 */
-	lport = (struct tcm_qla2xxx_lport *)vha->vha_tgt.target_lport_ptr;
+	ha->tgt.tgt_ops = &tcm_qla2xxx_template;
+	vha->vha_tgt.target_lport_ptr = target_lport_ptr;
 	lport->qla_vha = vha;
 
 	return 0;
@@ -1598,8 +1602,8 @@ static struct se_wwn *tcm_qla2xxx_make_lport(
 	if (ret != 0)
 		goto out;
 
-	ret = qlt_lport_register(&tcm_qla2xxx_template, wwpn,
-				tcm_qla2xxx_lport_register_cb, lport);
+	ret = qlt_lport_register(lport, wwpn, 0, 0,
+				 tcm_qla2xxx_lport_register_cb);
 	if (ret != 0)
 		goto out_lport;
 
@@ -1637,20 +1641,70 @@ static void tcm_qla2xxx_drop_lport(struct se_wwn *wwn)
 	kfree(lport);
 }
 
+static int tcm_qla2xxx_lport_register_npiv_cb(struct scsi_qla_host *base_vha,
+					      void *target_lport_ptr,
+					      u64 npiv_wwpn, u64 npiv_wwnn)
+{
+	struct fc_vport *vport;
+	struct Scsi_Host *sh = base_vha->host;
+	struct scsi_qla_host *npiv_vha;
+	struct tcm_qla2xxx_lport *lport =
+			(struct tcm_qla2xxx_lport *)target_lport_ptr;
+	struct fc_vport_identifiers vport_id;
+
+	if (!qla_tgt_mode_enabled(base_vha)) {
+		pr_err("qla2xxx base_vha not enabled for target mode\n");
+		return -EPERM;
+	}
+
+	memset(&vport_id, 0, sizeof(vport_id));
+	vport_id.port_name = npiv_wwpn;
+	vport_id.node_name = npiv_wwnn;
+	vport_id.roles = FC_PORT_ROLE_FCP_INITIATOR;
+	vport_id.vport_type = FC_PORTTYPE_NPIV;
+	vport_id.disable = false;
+
+	vport = fc_vport_create(sh, 0, &vport_id);
+	if (!vport) {
+		pr_err("fc_vport_create failed for qla2xxx_npiv\n");
+		return -ENODEV;
+	}
+	/*
+	 * Setup local pointer to NPIV vhba + target_lport_ptr
+	 */
+	npiv_vha = (struct scsi_qla_host *)vport->dd_data;
+	npiv_vha->vha_tgt.target_lport_ptr = target_lport_ptr;
+	lport->qla_vha = npiv_vha;
+
+	scsi_host_get(npiv_vha->host);
+	return 0;
+}
+
+
 static struct se_wwn *tcm_qla2xxx_npiv_make_lport(
 	struct target_fabric_configfs *tf,
 	struct config_group *group,
 	const char *name)
 {
 	struct tcm_qla2xxx_lport *lport;
-	u64 npiv_wwpn, npiv_wwnn;
+	u64 phys_wwpn, npiv_wwpn, npiv_wwnn;
+	char *p, tmp[128];
 	int ret;
-	struct scsi_qla_host *vha = NULL;
-	struct qla_hw_data *ha = NULL;
-	scsi_qla_host_t *base_vha = NULL;
 
-	if (tcm_qla2xxx_npiv_parse_wwn(name, strlen(name)+1,
-				&npiv_wwpn, &npiv_wwnn) < 0)
+	snprintf(tmp, 128, "%s", name);
+
+	p = strchr(tmp, '@');
+	if (!p) {
+		pr_err("Unable to locate NPIV '@' seperator\n");
+		return ERR_PTR(-EINVAL);
+	}
+	*p++ = '\0';
+
+	if (tcm_qla2xxx_parse_wwn(tmp, &phys_wwpn, 1) < 0)
+		return ERR_PTR(-EINVAL);
+
+	if (tcm_qla2xxx_npiv_parse_wwn(p, strlen(p)+1,
+				       &npiv_wwpn, &npiv_wwnn) < 0)
 		return ERR_PTR(-EINVAL);
 
 	lport = kzalloc(sizeof(struct tcm_qla2xxx_lport), GFP_KERNEL);
@@ -1668,21 +1722,11 @@ static struct se_wwn *tcm_qla2xxx_npiv_make_lport(
 	if (ret != 0)
 		goto out;
 
-	ret = qlt_lport_register(&tcm_qla2xxx_template, npiv_wwpn,
-				tcm_qla2xxx_lport_register_cb, lport);
-
+	ret = qlt_lport_register(lport, phys_wwpn, npiv_wwpn, npiv_wwnn,
+				 tcm_qla2xxx_lport_register_npiv_cb);
 	if (ret != 0)
 		goto out_lport;
 
-	vha = lport->qla_vha;
-	ha = vha->hw;
-	base_vha = pci_get_drvdata(ha->pdev);
-
-	if (!qla_tgt_mode_enabled(base_vha)) {
-		ret = -EPERM;
-		goto out_lport;
-	}
-
 	return &lport->lport_wwn;
 out_lport:
 	vfree(lport->lport_loopid_map);
@@ -1696,14 +1740,16 @@ static void tcm_qla2xxx_npiv_drop_lport(struct se_wwn *wwn)
 {
 	struct tcm_qla2xxx_lport *lport = container_of(wwn,
 			struct tcm_qla2xxx_lport, lport_wwn);
-	struct scsi_qla_host *vha = lport->qla_vha;
-	struct Scsi_Host *sh = vha->host;
+	struct scsi_qla_host *npiv_vha = lport->qla_vha;
+	struct qla_hw_data *ha = npiv_vha->hw;
+	scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev);
+
+	scsi_host_put(npiv_vha->host);
 	/*
 	 * Notify libfc that we want to release the vha->fc_vport
 	 */
-	fc_vport_terminate(vha->fc_vport);
-
-	scsi_host_put(sh);
+	fc_vport_terminate(npiv_vha->fc_vport);
+	scsi_host_put(base_vha->host);
 	kfree(lport);
 }
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport
  2014-01-15  5:42 [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport Nicholas A. Bellinger
@ 2014-01-15 16:06 ` Giridhar Malavali
  2014-01-15 21:45   ` Nicholas A. Bellinger
  2014-01-15 22:14 ` Quinn Tran
  1 sibling, 1 reply; 7+ messages in thread
From: Giridhar Malavali @ 2014-01-15 16:06 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: target-devel, linux-scsi, Saurav Kashyap, Quinn Tran,
	Sawan Chandak, Andrew Vasquez, Nicholas Bellinger

[-- Attachment #1: Type: text/plain, Size: 11896 bytes --]



Sent from my iPhone. Please excuse any typos.

> On Jan 14, 2014, at 10:02 PM, "Nicholas A. Bellinger" <nab@daterainc.com> wrote:
> 
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Hi Saurav & Co,
> 
> Here is the updated NPIV patch on top of the original here:
> 
>  [PATCH] qla2xxx: Enhancements to enable NPIV support for QLOGIC ISPs with TCM/LIO.
>  http://marc.info/?l=linux-scsi&m=138930044930212&w=2
> 
Thanks. We will review and get back.


> As previously dicussed, it moves fc_vport creation into tcm_qla2xxx
> control plane code thus allowing everything required for NPIV port
> creation to be encapsulated within configfs.
> 
> There are still some bugs to be resolved.  Namely:
> 
> * Determine how to save 'point-to-point' NVRAM settings across target restart

We will look into this. 

> * Figure out why the initiator logs in, but doesn't perform LUN scan on
>   the first login attempt
> * Determine if it's possible to collapse the name formatting further,
>   by generating $NPIV_WWNN internally
> * Determine proper NPIV format for EVPD=0x83 SCSI name string
> 
> Aside from these remaining issues, multiple NPIV ports can be configured
> on a single physical FC port, and initial I/O appears to be functioning
> as expected.
> 

That's good.
> Please review.
> 
> Thank you,
> 
> --nab
> 
> --------------------------------------------------------------------
> 
> This patch changes qla2xxx qlt_lport_register() code to accept
> target_lport_ptr + npiv_wwpn + npiv_wwnn parameters, and updates
> tcm_qla2xxx to use the new tcm_qla2xxx_lport_register_npiv_cb()
> callback for invoking fc_vport_create() from configfs context
> via tcm_qla2xxx_npiv_make_lport() code.
> 
> In order for this to work, the qlt_lport_register() callback is
> now called without holding qla_tgt_mutex, as the fc_vport creation
> process will call qlt_vport_create() -> qlt_add_target(), which
> already expects to acquire it.
> 
> It enforces /sys/kernel/config/target/qla2xxx_npiv/$NPIV_WWPN
> naming in the following format:
> 
>     $PHYSICAL_WWPN@$NPIV_WWPN:$NPIV_WWNN
> 
> and assumes the $PHYSICAL_WWPN in question has already had been
> configured for target mode in non NPIV mode.
> 
> Finally, it updates existing tcm_qla2xxx_lport_register_cb() logic
> to setup the non NPIV assignments that have now been moved out of
> qlt_lport_register() code.
> 
> Cc: Sawan Chandak <sawan.chandak@qlogic.com>
> Cc: Quinn Tran <quinn.tran@qlogic.com>
> Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
> drivers/scsi/qla2xxx/qla_target.c  |   25 ++++-----
> drivers/scsi/qla2xxx/qla_target.h  |    4 +-
> drivers/scsi/qla2xxx/tcm_qla2xxx.c |  106 ++++++++++++++++++++++++++----------
> 3 files changed, 88 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
> index 34ed246..b596f8b 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -4235,8 +4235,9 @@ static void qlt_lport_dump(struct scsi_qla_host *vha, u64 wwpn,
>  * @callback:  lport initialization callback for tcm_qla2xxx code
>  * @target_lport_ptr: pointer for tcm_qla2xxx specific lport data
>  */
> -int qlt_lport_register(struct qla_tgt_func_tmpl *qla_tgt_ops, u64 wwpn,
> -    int (*callback)(struct scsi_qla_host *), void *target_lport_ptr)
> +int qlt_lport_register(void *target_lport_ptr, u64 phys_wwpn,
> +               u64 npiv_wwpn, u64 npiv_wwnn,
> +               int (*callback)(struct scsi_qla_host *, void *, u64, u64))
> {
>    struct qla_tgt *tgt;
>    struct scsi_qla_host *vha;
> @@ -4259,7 +4260,7 @@ int qlt_lport_register(struct qla_tgt_func_tmpl *qla_tgt_ops, u64 wwpn,
>            continue;
> 
>        spin_lock_irqsave(&ha->hardware_lock, flags);
> -        if (host->active_mode & MODE_TARGET) {
> +        if ((!npiv_wwpn || !npiv_wwnn) && host->active_mode & MODE_TARGET) {
>            pr_debug("MODE_TARGET already active on qla2xxx(%d)\n",
>                host->host_no);
>            spin_unlock_irqrestore(&ha->hardware_lock, flags);
> @@ -4273,24 +4274,18 @@ int qlt_lport_register(struct qla_tgt_func_tmpl *qla_tgt_ops, u64 wwpn,
>                " qla2xxx scsi_host\n");
>            continue;
>        }
> -        qlt_lport_dump(vha, wwpn, b);
> +        qlt_lport_dump(vha, phys_wwpn, b);
> 
>        if (memcmp(vha->port_name, b, WWN_SIZE)) {
>            scsi_host_put(host);
>            continue;
>        }
> -        /*
> -         * Setup passed parameters ahead of invoking callback
> -         */
> -        ha->tgt.tgt_ops = qla_tgt_ops;
> -        vha->vha_tgt.target_lport_ptr = target_lport_ptr;
> -        rc = (*callback)(vha);
> -        if (rc != 0) {
> -            ha->tgt.tgt_ops = NULL;
> -            vha->vha_tgt.target_lport_ptr = NULL;
> -            scsi_host_put(host);
> -        }
>        mutex_unlock(&qla_tgt_mutex);
> +
> +        rc = (*callback)(vha, target_lport_ptr, npiv_wwpn, npiv_wwnn);
> +        if (rc != 0)
> +            scsi_host_put(host);
> +
>        return rc;
>    }
>    mutex_unlock(&qla_tgt_mutex);
> diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
> index b33e411..1d10eec 100644
> --- a/drivers/scsi/qla2xxx/qla_target.h
> +++ b/drivers/scsi/qla2xxx/qla_target.h
> @@ -932,8 +932,8 @@ void qlt_disable_vha(struct scsi_qla_host *);
>  */
> extern int qlt_add_target(struct qla_hw_data *, struct scsi_qla_host *);
> extern int qlt_remove_target(struct qla_hw_data *, struct scsi_qla_host *);
> -extern int qlt_lport_register(struct qla_tgt_func_tmpl *, u64,
> -            int (*callback)(struct scsi_qla_host *), void *);
> +extern int qlt_lport_register(void *, u64, u64, u64,
> +            int (*callback)(struct scsi_qla_host *, void *, u64, u64));
> extern void qlt_lport_deregister(struct scsi_qla_host *);
> extern void qlt_unreg_sess(struct qla_tgt_sess *);
> extern void qlt_fc_port_added(struct scsi_qla_host *, fc_port_t *);
> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> index 113ca95..75a141b 100644
> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> @@ -1559,14 +1559,18 @@ static int tcm_qla2xxx_init_lport(struct tcm_qla2xxx_lport *lport)
>    return 0;
> }
> 
> -static int tcm_qla2xxx_lport_register_cb(struct scsi_qla_host *vha)
> +static int tcm_qla2xxx_lport_register_cb(struct scsi_qla_host *vha,
> +                     void *target_lport_ptr,
> +                     u64 npiv_wwpn, u64 npiv_wwnn)
> {
> -    struct tcm_qla2xxx_lport *lport;
> +    struct qla_hw_data *ha = vha->hw;
> +    struct tcm_qla2xxx_lport *lport =
> +            (struct tcm_qla2xxx_lport *)target_lport_ptr;
>    /*
> -     * Setup local pointer to vha, NPIV VP pointer (if present) and
> -     * vha->tcm_lport pointer
> +     * Setup tgt_ops, local pointer to vha and target_lport_ptr
>     */
> -    lport = (struct tcm_qla2xxx_lport *)vha->vha_tgt.target_lport_ptr;
> +    ha->tgt.tgt_ops = &tcm_qla2xxx_template;
> +    vha->vha_tgt.target_lport_ptr = target_lport_ptr;
>    lport->qla_vha = vha;
> 
>    return 0;
> @@ -1598,8 +1602,8 @@ static struct se_wwn *tcm_qla2xxx_make_lport(
>    if (ret != 0)
>        goto out;
> 
> -    ret = qlt_lport_register(&tcm_qla2xxx_template, wwpn,
> -                tcm_qla2xxx_lport_register_cb, lport);
> +    ret = qlt_lport_register(lport, wwpn, 0, 0,
> +                 tcm_qla2xxx_lport_register_cb);
>    if (ret != 0)
>        goto out_lport;
> 
> @@ -1637,20 +1641,70 @@ static void tcm_qla2xxx_drop_lport(struct se_wwn *wwn)
>    kfree(lport);
> }
> 
> +static int tcm_qla2xxx_lport_register_npiv_cb(struct scsi_qla_host *base_vha,
> +                          void *target_lport_ptr,
> +                          u64 npiv_wwpn, u64 npiv_wwnn)
> +{
> +    struct fc_vport *vport;
> +    struct Scsi_Host *sh = base_vha->host;
> +    struct scsi_qla_host *npiv_vha;
> +    struct tcm_qla2xxx_lport *lport =
> +            (struct tcm_qla2xxx_lport *)target_lport_ptr;
> +    struct fc_vport_identifiers vport_id;
> +
> +    if (!qla_tgt_mode_enabled(base_vha)) {
> +        pr_err("qla2xxx base_vha not enabled for target mode\n");
> +        return -EPERM;
> +    }
> +
> +    memset(&vport_id, 0, sizeof(vport_id));
> +    vport_id.port_name = npiv_wwpn;
> +    vport_id.node_name = npiv_wwnn;
> +    vport_id.roles = FC_PORT_ROLE_FCP_INITIATOR;
> +    vport_id.vport_type = FC_PORTTYPE_NPIV;
> +    vport_id.disable = false;
> +
> +    vport = fc_vport_create(sh, 0, &vport_id);
> +    if (!vport) {
> +        pr_err("fc_vport_create failed for qla2xxx_npiv\n");
> +        return -ENODEV;
> +    }
> +    /*
> +     * Setup local pointer to NPIV vhba + target_lport_ptr
> +     */
> +    npiv_vha = (struct scsi_qla_host *)vport->dd_data;
> +    npiv_vha->vha_tgt.target_lport_ptr = target_lport_ptr;
> +    lport->qla_vha = npiv_vha;
> +
> +    scsi_host_get(npiv_vha->host);
> +    return 0;
> +}
> +
> +
> static struct se_wwn *tcm_qla2xxx_npiv_make_lport(
>    struct target_fabric_configfs *tf,
>    struct config_group *group,
>    const char *name)
> {
>    struct tcm_qla2xxx_lport *lport;
> -    u64 npiv_wwpn, npiv_wwnn;
> +    u64 phys_wwpn, npiv_wwpn, npiv_wwnn;
> +    char *p, tmp[128];
>    int ret;
> -    struct scsi_qla_host *vha = NULL;
> -    struct qla_hw_data *ha = NULL;
> -    scsi_qla_host_t *base_vha = NULL;
> 
> -    if (tcm_qla2xxx_npiv_parse_wwn(name, strlen(name)+1,
> -                &npiv_wwpn, &npiv_wwnn) < 0)
> +    snprintf(tmp, 128, "%s", name);
> +
> +    p = strchr(tmp, '@');
> +    if (!p) {
> +        pr_err("Unable to locate NPIV '@' seperator\n");
> +        return ERR_PTR(-EINVAL);
> +    }
> +    *p++ = '\0';
> +
> +    if (tcm_qla2xxx_parse_wwn(tmp, &phys_wwpn, 1) < 0)
> +        return ERR_PTR(-EINVAL);
> +
> +    if (tcm_qla2xxx_npiv_parse_wwn(p, strlen(p)+1,
> +                       &npiv_wwpn, &npiv_wwnn) < 0)
>        return ERR_PTR(-EINVAL);
> 
>    lport = kzalloc(sizeof(struct tcm_qla2xxx_lport), GFP_KERNEL);
> @@ -1668,21 +1722,11 @@ static struct se_wwn *tcm_qla2xxx_npiv_make_lport(
>    if (ret != 0)
>        goto out;
> 
> -    ret = qlt_lport_register(&tcm_qla2xxx_template, npiv_wwpn,
> -                tcm_qla2xxx_lport_register_cb, lport);
> -
> +    ret = qlt_lport_register(lport, phys_wwpn, npiv_wwpn, npiv_wwnn,
> +                 tcm_qla2xxx_lport_register_npiv_cb);
>    if (ret != 0)
>        goto out_lport;
> 
> -    vha = lport->qla_vha;
> -    ha = vha->hw;
> -    base_vha = pci_get_drvdata(ha->pdev);
> -
> -    if (!qla_tgt_mode_enabled(base_vha)) {
> -        ret = -EPERM;
> -        goto out_lport;
> -    }
> -
>    return &lport->lport_wwn;
> out_lport:
>    vfree(lport->lport_loopid_map);
> @@ -1696,14 +1740,16 @@ static void tcm_qla2xxx_npiv_drop_lport(struct se_wwn *wwn)
> {
>    struct tcm_qla2xxx_lport *lport = container_of(wwn,
>            struct tcm_qla2xxx_lport, lport_wwn);
> -    struct scsi_qla_host *vha = lport->qla_vha;
> -    struct Scsi_Host *sh = vha->host;
> +    struct scsi_qla_host *npiv_vha = lport->qla_vha;
> +    struct qla_hw_data *ha = npiv_vha->hw;
> +    scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev);
> +
> +    scsi_host_put(npiv_vha->host);
>    /*
>     * Notify libfc that we want to release the vha->fc_vport
>     */
> -    fc_vport_terminate(vha->fc_vport);
> -
> -    scsi_host_put(sh);
> +    fc_vport_terminate(npiv_vha->fc_vport);
> +    scsi_host_put(base_vha->host);
>    kfree(lport);
> }
> 
> -- 
> 1.7.10.4
> 

[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 7677 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport
  2014-01-15 16:06 ` Giridhar Malavali
@ 2014-01-15 21:45   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 7+ messages in thread
From: Nicholas A. Bellinger @ 2014-01-15 21:45 UTC (permalink / raw)
  To: Giridhar Malavali
  Cc: Nicholas A. Bellinger, target-devel, linux-scsi, Saurav Kashyap,
	Quinn Tran, Sawan Chandak, Andrew Vasquez

Hi Giridhar & Co,

A few more questions below..

On Wed, 2014-01-15 at 16:06 +0000, Giridhar Malavali wrote:
>
> > On Jan 14, 2014, at 10:02 PM, "Nicholas A. Bellinger" <nab@daterainc.com> wrote:
> > 
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > Hi Saurav & Co,
> > 
> > Here is the updated NPIV patch on top of the original here:
> > 
> >  [PATCH] qla2xxx: Enhancements to enable NPIV support for QLOGIC ISPs with TCM/LIO.
> >  http://marc.info/?l=linux-scsi&m=138930044930212&w=2
> > 
> Thanks. We will review and get back.
> 
> 
> > As previously dicussed, it moves fc_vport creation into tcm_qla2xxx
> > control plane code thus allowing everything required for NPIV port
> > creation to be encapsulated within configfs.
> > 
> > There are still some bugs to be resolved.  Namely:
> > 
> > * Determine how to save 'point-to-point' NVRAM settings across target restart
> 
> We will look into this. 
> 
> > * Figure out why the initiator logs in, but doesn't perform LUN scan on
> >   the first login attempt

Not sure what's happening here, but the same issue with the original
patch too.

> > * Determine if it's possible to collapse the name formatting further,
> >   by generating $NPIV_WWNN internally

Wrt to this bit..  There is never a case where the trailing bytes for
$NPIV_WWPN + $NPIV_WWNN would be different, right..?

If that's the case, then collapsing the /sys/kernel/config/target/qla2xxx/
formatting to $PHYSICAL_WWPN:$NPIV_WWPN makes sense here, and simply
generate the $NPIV_WWNN required for fc_vport_create() internally in
tcm_qla2xxx code.

> > * Determine proper NPIV format for EVPD=0x83 SCSI name string
> > 

AFAICT this should be the $NPIV_WWN + port_id, correct..?

> > Aside from these remaining issues, multiple NPIV ports can be configured
> > on a single physical FC port, and initial I/O appears to be functioning
> > as expected.
> > 
> 
> That's good.
> > 

:-)

--nab

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport
  2014-01-15  5:42 [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport Nicholas A. Bellinger
  2014-01-15 16:06 ` Giridhar Malavali
@ 2014-01-15 22:14 ` Quinn Tran
  2014-01-16  2:05   ` Nicholas A. Bellinger
  1 sibling, 1 reply; 7+ messages in thread
From: Quinn Tran @ 2014-01-15 22:14 UTC (permalink / raw)
  To: Nicholas A. Bellinger, target-devel
  Cc: linux-scsi, Saurav Kashyap, Sawan Chandak, Giridhar Malavali,
	Andrew Vasquez

Nicholas,

Answer for the following is below.

<SNIP>
There are still some bugs to be resolved.  Namely:

* Determine how to save 'point-to-point' NVRAM settings across target
restart
* Figure out why the initiator logs in, but doesn't perform LUN scan on
   the first login attempt
* Determine if it's possible to collapse the name formatting further,
   by generating $NPIV_WWNN internally
* Determine proper NPIV format for EVPD=0x83 SCSI name string

Aside from these remaining issues, multiple NPIV ports can be configured
on a single physical FC port, and initial I/O appears to be functioning
as expected.

<SNIP>
---

Bullet 1) After additional analysis by the team of the problem, what you
have bumped into is a driver design decision and firmware design decision.
 At the time of the problem, the problem signature did not match with what
we saw in house and we forgot to reiterate during code submission that you
must follow the step listed in the setup script.

The original problem is the driver have not had a chance to get the switch
capabilities (i.e. NPIV Support bit).  When you attempt to create an NPIV
host, NPIV creation failed due to a prelim check of the capability.

         if (!(ha->switch_cap & FLOGI_MID_SUPPORT))
                return VPCERR_NO_FABRIC_SUPP;


When we suggested the "qaucli -n <host #> CO 1", that allows you to
proceed further.  What happened there is a chip reset was happening behind
the scene, follow with link up and get the switch capabilities.  However,
the problem persist on driver unload/load.  When reload, you need another
nudge to get moving again.

Driver design: When you load the driver, it most likely that you've used
driver param "qlini_mode=disabled" (i.e. target mode=on) for all physical
ports.  In this case, qla driver will not "Initialize the firmware" (i.e.
not Link up) & will not get the switch capabilities from firmware.  The
reason for not link up as target port at load time is due to TCM resources
are not available yet.  If we do linkup as target mode, then remote
initiator will start LUN discovery and ends up with time out problem due
to TCM not there to handle the commands.  Because the link is not up, the
switch capability flags is not read (i.e. NPIV creation failed)

Firmware design:  Qlogic firmware requires the physical port(i.e. Virtual
Port 0) to be enabled 1st, before NPIV creation (other VP's).

This imply that TCM configuration have to enable the physical port 1st
also (echo 1 > enable), before any NPIV creation can happen. This will be
the procedure in exercising  this feature.



Bullet 2) As to why the LUN scan did not happen on the initial login,
could be due to the mode the port was told to come up as.   Initiator will
not scan the lun, if the remote port is also an initiator.  We'll keep an
eye on this during testing.  Please let us know the steps to recreate.


Bullet 3) Generating NPIV WWPN id:  Qlogic overall strategy for this
question is: Qla2xxx driver is a common driver being used by various
hardware vendors(HP, IBM, Dell, etc..).  Each vendors have a different
scheme on how to generate the WWPN value.  QLogic's design decision is to
push that up to the user app to generate these values rather having
QLA2xxx generating it dynamically.


Bullet 4) VPD Page 83h, SCSI name string.  If you implying to use the WWPN
as part of the "SCSI name string", the answer is the same as previous
bullet.  In this case, the setup up script would have to extract the data
from configfs in order to generate the "SCSI name string"



Regards,
Quinn Tran



________________________________

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport
  2014-01-15 22:14 ` Quinn Tran
@ 2014-01-16  2:05   ` Nicholas A. Bellinger
  2014-01-16 20:54     ` Quinn Tran
  0 siblings, 1 reply; 7+ messages in thread
From: Nicholas A. Bellinger @ 2014-01-16  2:05 UTC (permalink / raw)
  To: Quinn Tran
  Cc: Nicholas A. Bellinger, target-devel, linux-scsi, Saurav Kashyap,
	Sawan Chandak, Giridhar Malavali, Andrew Vasquez

Hi Quinn,

Thanks for the response.  Comments are inline below..

On Wed, 2014-01-15 at 22:14 +0000, Quinn Tran wrote:
> Nicholas,
> 
> Answer for the following is below.
> 
> <SNIP>
> There are still some bugs to be resolved.  Namely:
> 
> * Determine how to save 'point-to-point' NVRAM settings across target
> restart
> * Figure out why the initiator logs in, but doesn't perform LUN scan on
>    the first login attempt
> * Determine if it's possible to collapse the name formatting further,
>    by generating $NPIV_WWNN internally
> * Determine proper NPIV format for EVPD=0x83 SCSI name string
> 
> Aside from these remaining issues, multiple NPIV ports can be configured
> on a single physical FC port, and initial I/O appears to be functioning
> as expected.
> 
> <SNIP>
> ---
> 
> Bullet 1) After additional analysis by the team of the problem, what you
> have bumped into is a driver design decision and firmware design decision.
>  At the time of the problem, the problem signature did not match with what
> we saw in house and we forgot to reiterate during code submission that you
> must follow the step listed in the setup script.
> 
> The original problem is the driver have not had a chance to get the switch
> capabilities (i.e. NPIV Support bit).  When you attempt to create an NPIV
> host, NPIV creation failed due to a prelim check of the capability.
> 
>          if (!(ha->switch_cap & FLOGI_MID_SUPPORT))
>                 return VPCERR_NO_FABRIC_SUPP;
> 
> 
> When we suggested the "qaucli -n <host #> CO 1", that allows you to
> proceed further.  What happened there is a chip reset was happening behind
> the scene, follow with link up and get the switch capabilities.  However,
> the problem persist on driver unload/load.  When reload, you need another
> nudge to get moving again.
> 
> Driver design: When you load the driver, it most likely that you've used
> driver param "qlini_mode=disabled" (i.e. target mode=on) for all physical
> ports.  In this case, qla driver will not "Initialize the firmware" (i.e.
> not Link up) & will not get the switch capabilities from firmware.  The
> reason for not link up as target port at load time is due to TCM resources
> are not available yet.  If we do linkup as target mode, then remote
> initiator will start LUN discovery and ends up with time out problem due
> to TCM not there to handle the commands.  Because the link is not up, the
> switch capability flags is not read (i.e. NPIV creation failed)
> 
> Firmware design:  Qlogic firmware requires the physical port(i.e. Virtual
> Port 0) to be enabled 1st, before NPIV creation (other VP's).
> 
> This imply that TCM configuration have to enable the physical port 1st
> also (echo 1 > enable), before any NPIV creation can happen. This will be
> the procedure in exercising  this feature.
> 
> 

Thanks for the detailed explanation.

As you noticed, I managed to miss the fact that the underlying physical
port needs to be enabled in target mode before virtual ports can be
configured correctly.

So that said, scratch this particular issue.

> 
> Bullet 2) As to why the LUN scan did not happen on the initial login,
> could be due to the mode the port was told to come up as.   Initiator will
> not scan the lun, if the remote port is also an initiator.  We'll keep an
> eye on this during testing.  Please let us know the steps to recreate.
> 

This is reproducible each time after an NPIV target port has been
configured + enabled.

On the initiator side, (assuming that qla2xxx is already unloaded), the
first 'modprobe qla2xxx' will result in a qla_target session being
established for the correct FC initiator WWPN, but no subsequent ATIO
packet traffic is generated from a initiator side LUN scan.

Doing a initiator side 'rmmod qla2xxx' + second 'modprobe qla2xxx' will
reestablish the qla_target session, and the LUN scan + ATIO packets
begin to flow as expected.

> 
> Bullet 3) Generating NPIV WWPN id:  Qlogic overall strategy for this
> question is: Qla2xxx driver is a common driver being used by various
> hardware vendors(HP, IBM, Dell, etc..).  Each vendors have a different
> scheme on how to generate the WWPN value.  QLogic's design decision is to
> push that up to the user app to generate these values rather having
> QLA2xxx generating it dynamically.
> 

So question here was not contents of the NPIV WWPN value, but rather if
the NPIV WWPN and NPIV WWNN are always expected to have the same suffix.

Eg, is it safe to assume that the '21' and '20' prefixes for NPIV WWPN +
NPIV WWNN are the only different part of the $NPIV_WWPN:$NPIV_WWNN tuple
passed into fc_vport_create()..?

If so, I'd like to remove $NPIV_WWNN from being passed into configfs,
and simply generate this value internally, eg:

  /sys/kernel/config/target/qla2xxx_npiv/$PHYS_WWPN:$NPIV_WWPN

> Bullet 4) VPD Page 83h, SCSI name string.  If you implying to use the WWPN
> as part of the "SCSI name string", the answer is the same as previous
> bullet.  In this case, the setup up script would have to extract the data
> from configfs in order to generate the "SCSI name string"
> 

This question was geared toward how SCSI name should be represented in
INQUIRY EVPD=0x83 for NPIV ports.

For physical ports, this ends up being the 21:00:00:XX:XX:XX:XX:XX value
matching what is passed to configfs.  For virtual ports the passed
$NPIV_WWPN part would be here in similar format, but the question was if
the port_id should also be appended here similar to existing 'lsscsi
--transport' output, eg:

root@devstack:~# lsscsi --transport
[0:0:0:0]    disk    ata:                            /dev/sda 
[91:0:0:0]   disk    fc:0x21111111222222220x610101   /dev/sdb

Thanks again,

--nab

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport
  2014-01-16  2:05   ` Nicholas A. Bellinger
@ 2014-01-16 20:54     ` Quinn Tran
  2014-01-16 21:50       ` Nicholas A. Bellinger
  0 siblings, 1 reply; 7+ messages in thread
From: Quinn Tran @ 2014-01-16 20:54 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Nicholas A. Bellinger, target-devel, linux-scsi, Saurav Kashyap,
	Sawan Chandak, Giridhar Malavali, Andrew Vasquez

[-- Attachment #1: Type: text/plain, Size: 4793 bytes --]

Nicholas,

Comments inline. Thanks.


<SNIP>
>Bullet 3) Generating NPIV WWPN id:  Qlogic overall strategy for this
>question is: Qla2xxx driver is a common driver being used by various
>hardware vendors(HP, IBM, Dell, etc..).  Each vendors have a different
>scheme on how to generate the WWPN value.  QLogic's design decision is to
>push that up to the user app to generate these values rather having
>QLA2xxx generating it dynamically.

So question here was not contents of the NPIV WWPN value, but rather if
the NPIV WWPN and NPIV WWNN are always expected to have the same suffix.

Eg, is it safe to assume that the '21' and '20' prefixes for NPIV WWPN +
NPIV WWNN are the only different part of the $NPIV_WWPN:$NPIV_WWNN tuple
passed into fc_vport_create()..?

If so, I'd like to remove $NPIV_WWNN from being passed into configfs,
and simply generate this value internally, eg:

  /sys/kernel/config/target/qla2xxx_npiv/$PHYS_WWPN:$NPIV_WWPN
<SNIP>

QT> Mis-read this one.

It's not safe to assume "21" & "20" as the standardize prefixes and the
rest of the $WWPN/$WWNN will be the same. While it is common to see "21" &
"20" (i.e. Network Addr Authority/NAA 2 format) on a lot of adapters.  The
common case that we see only cover 1 NAA format.  I've seen the NAA5
format float around on different adapters.

I don't want to paint us in to a corner, that will require future code
change in this area.  The user that generate the $NPIV_WWPN + $NPIV_WWNN
may decide to use another NAA format where it will not match this
implementation.

By collapsing the $NPIV_WWNN into the driver, its takes away the option
for an administrator of a large SAN to assign/identify storage chassis
using $WWNN.  Even though it is clunky to carry around.

I prefer the format "$PHYS_WWPN@$NPIV_WWPN:$NPIV_WWNN" with the "@" key,
for the case where the user download an older version of RSTLIB that still
follow the older format/backward compatible.  The '@' key inside the
driver provides the distinction of the new configfs format.  No preference
on $PHYS_WWPN appear at the front or back.


Network address authority Format:
NAA1 =10-00-vv-vv-vv-ss-ss-ss
NAA2 =2x-xx-vv-vv-vv-ss-ss-ss
NAA5 =5v-vv-vv-vs-ss-ss-ss-ss
NAA6 =6?-Š (16 bytes/32 hex digits, not applicable for HW adapter)

x=vendor code (0-00=wwnn 1-00=wwpn)
v=vendor id
s=serial number (NAA5: user may decide to use WWPN=N & WWNN=N+1)


----
<SNIP>
>Bullet 4) VPD Page 83h, SCSI name string.  If you implying to use the WWPN
>as part of the "SCSI name string", the answer is the same as previous
>bullet.  In this case, the setup up script would have to extract the data
>from configfs in order to generate the "SCSI name string"

This question was geared toward how SCSI name should be represented in
INQUIRY EVPD=0x83 for NPIV ports.

For physical ports, this ends up being the 21:00:00:XX:XX:XX:XX:XX value
matching what is passed to configfs.  For virtual ports the passed
$NPIV_WWPN part would be here in similar format, but the question was if
the port_id should also be appended here similar to existing 'lsscsi
--transport' output, eg:

root@devstack:~# lsscsi --transport
[0:0:0:0]    disk    ata:                            /dev/sda
[91:0:0:0]   disk    fc:0x21111111222222220x610101   /dev/sdb
<SNIP>


SPC4 rev 36n, section 7.8.6.11 >









                                        The SCSI NAME STRING field starts with either:

...
b) the four UTF-8 characters 'naa.' concatenated with 16 or 32 hexadecimal
digits for an NAA identifier
(see 7.8.6.6). The first hexadecimal digit shall be the most significant
four bits of the first byte (i.e.,
most significant byte) of the NAA identifier; or
...
"If the ASSOCIATION field is set to 01b (i.e., target port), the SCSI NAME
STRING field ends with the five UTF-8
characters ',t,0x' concatenated with two or more hexadecimal digits as
defined in the applicable SCSI
transport protocol standard."
...


QT> If I read SPC4 correctly, the format is as follow (minor tweak).  For
FC, there is only 1 format (i.e. Association field = 1).  Correct the
format if you read it differently.




[91:0:0:0] disk fc:naa.2111111122222222,t,0x610101  /dev/sdb



Scsi name string = "naa.2111111122222222,t,0x610101"

---







Regards,
Quinn Tran



________________________________

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 6673 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport
  2014-01-16 20:54     ` Quinn Tran
@ 2014-01-16 21:50       ` Nicholas A. Bellinger
  0 siblings, 0 replies; 7+ messages in thread
From: Nicholas A. Bellinger @ 2014-01-16 21:50 UTC (permalink / raw)
  To: Quinn Tran
  Cc: Nicholas A. Bellinger, target-devel, linux-scsi, Saurav Kashyap,
	Sawan Chandak, Giridhar Malavali, Andrew Vasquez

On Thu, 2014-01-16 at 20:54 +0000, Quinn Tran wrote:
> Nicholas,
> 
> Comments inline. Thanks.
> 
> 
> <SNIP>
> >Bullet 3) Generating NPIV WWPN id:  Qlogic overall strategy for this
> >question is: Qla2xxx driver is a common driver being used by various
> >hardware vendors(HP, IBM, Dell, etc..).  Each vendors have a different
> >scheme on how to generate the WWPN value.  QLogic's design decision is to
> >push that up to the user app to generate these values rather having
> >QLA2xxx generating it dynamically.
> 
> So question here was not contents of the NPIV WWPN value, but rather if
> the NPIV WWPN and NPIV WWNN are always expected to have the same suffix.
> 
> Eg, is it safe to assume that the '21' and '20' prefixes for NPIV WWPN +
> NPIV WWNN are the only different part of the $NPIV_WWPN:$NPIV_WWNN tuple
> passed into fc_vport_create()..?
> 
> If so, I'd like to remove $NPIV_WWNN from being passed into configfs,
> and simply generate this value internally, eg:
> 
>   /sys/kernel/config/target/qla2xxx_npiv/$PHYS_WWPN:$NPIV_WWPN
> <SNIP>
> 
> QT> Mis-read this one.
> 
> It's not safe to assume "21" & "20" as the standardize prefixes and the
> rest of the $WWPN/$WWNN will be the same. While it is common to see "21" &
> "20" (i.e. Network Addr Authority/NAA 2 format) on a lot of adapters.  The
> common case that we see only cover 1 NAA format.  I've seen the NAA5
> format float around on different adapters.
> 
> I don't want to paint us in to a corner, that will require future code
> change in this area.  The user that generate the $NPIV_WWPN + $NPIV_WWNN
> may decide to use another NAA format where it will not match this
> implementation.
> 
> By collapsing the $NPIV_WWNN into the driver, its takes away the option
> for an administrator of a large SAN to assign/identify storage chassis
> using $WWNN.  Even though it is clunky to carry around.
> 

Fair enough.  I'm currently planning for targetcli to (by default)
generate $NPIV_WWPN + $NPIV_WWNN values for end users in NAA2 format.

Optionally, advanced versions will be able to provide their own
$NPIV_WWPN + $NPIV_WWNN values at port creation time if necessary.

> I prefer the format "$PHYS_WWPN@$NPIV_WWPN:$NPIV_WWNN" with the "@" key,
> for the case where the user download an older version of RSTLIB that still
> follow the older format/backward compatible.  The '@' key inside the
> driver provides the distinction of the new configfs format.  No preference
> on $PHYS_WWPN appear at the front or back.
> 

Note that rtslib/targetcli can always use 'free form' names, for which
user can pass any value into /sys/kernel/config/target/$FABRIC/$WWPN.

> 
> Network address authority Format:
> NAA1 =10-00-vv-vv-vv-ss-ss-ss
> NAA2 =2x-xx-vv-vv-vv-ss-ss-ss
> NAA5 =5v-vv-vv-vs-ss-ss-ss-ss
> NAA6 =6?-Š (16 bytes/32 hex digits, not applicable for HW adapter)
> 
> x=vendor code (0-00=wwnn 1-00=wwpn)
> v=vendor id
> s=serial number (NAA5: user may decide to use WWPN=N & WWNN=N+1)
> 

Ok, so we've been using NAA2 formatting for all FC/FCoE ports
represented in configfs thus far.

> 
> ----
> <SNIP>
> >Bullet 4) VPD Page 83h, SCSI name string.  If you implying to use the WWPN
> >as part of the "SCSI name string", the answer is the same as previous
> >bullet.  In this case, the setup up script would have to extract the data
> >from configfs in order to generate the "SCSI name string"
> 
> This question was geared toward how SCSI name should be represented in
> INQUIRY EVPD=0x83 for NPIV ports.
> 
> For physical ports, this ends up being the 21:00:00:XX:XX:XX:XX:XX value
> matching what is passed to configfs.  For virtual ports the passed
> $NPIV_WWPN part would be here in similar format, but the question was if
> the port_id should also be appended here similar to existing 'lsscsi
> --transport' output, eg:
> 
> root@devstack:~# lsscsi --transport
> [0:0:0:0]    disk    ata:                            /dev/sda
> [91:0:0:0]   disk    fc:0x21111111222222220x610101   /dev/sdb
> <SNIP>
> 
> 
> SPC4 rev 36n, section 7.8.6.11 >
>                                         The SCSI NAME STRING field starts with either:
> 
> ...
> b) the four UTF-8 characters 'naa.' concatenated with 16 or 32 hexadecimal
> digits for an NAA identifier
> (see 7.8.6.6). The first hexadecimal digit shall be the most significant
> four bits of the first byte (i.e.,
> most significant byte) of the NAA identifier; or
> ...
> "If the ASSOCIATION field is set to 01b (i.e., target port), the SCSI NAME
> STRING field ends with the five UTF-8
> characters ',t,0x' concatenated with two or more hexadecimal digits as
> defined in the applicable SCSI
> transport protocol standard."
> ...
> 
> 
> QT> If I read SPC4 correctly, the format is as follow (minor tweak).  For
> FC, there is only 1 format (i.e. Association field = 1).  Correct the
> format if you read it differently.
> 
> 
> 
> 
> [91:0:0:0] disk fc:naa.2111111122222222,t,0x610101  /dev/sdb
> 
> 
> 
> Scsi name string = "naa.2111111122222222,t,0x610101"
> 

Mmmm, so target core is exposing the SCSI name string prefix matching
what's in /sys/kernel/config/target/$FABRIC/WWPN.

For non NPIV qla2xxx + tcm_fc(FCoE), this ends up being:

    21:00:XX:XX:XX:XX:XX:XX,t,0x0001

So this will need to be changed for the FC/FCoE cases to follow the
'naa.' prefixed format mentioned above.  AFAIK this is only used for
information purposes, so it's fairly low priority atm.

--nab

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-01-16 21:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-15  5:42 [PATCH] qla2xxx: Configure NPIV fc_vport via tcm_qla2xxx_npiv_make_lport Nicholas A. Bellinger
2014-01-15 16:06 ` Giridhar Malavali
2014-01-15 21:45   ` Nicholas A. Bellinger
2014-01-15 22:14 ` Quinn Tran
2014-01-16  2:05   ` Nicholas A. Bellinger
2014-01-16 20:54     ` Quinn Tran
2014-01-16 21:50       ` Nicholas A. Bellinger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.