diff mbox

[01/15] lpfc: Add nvme initiator devloss support

Message ID 20170531060439.2073-2-jsmart2021@gmail.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

James Smart May 31, 2017, 6:04 a.m. UTC
Add nvme initiator devloss support

The existing implementation was based on no devloss behavior
in the transport (e.g. immediate teardown) so code didn't
properly handle delayed nvme rport device unregister calls.
In addition, the driver was not correctly cycling the rport
port role for each register-unregister-reregister process.

This patch does the following:
Rework the code to properly handle rport device unregister
calls and potential re-allocation of the remoteport structure
if the port comes back in under dev_loss_tmo.

Correct code that was incorrectly cycling the rport
port role for each register-unregister-reregister process.

Prep the code to enable calling the nvme_fc transport api
to dynamically update dev_loss_tmo when the scsi sysfs interface
changes it.

Memset the rpinfo structure in the registration call to enforce
"accept nvme transport defaults" in the registration call.  Driver
parameters do influence the dev_loss_tmo transport setting
dynamically.

Simplifies the register function: the driver was incorrectly
searching its local rport list to determine resume or new semantics,
which is not valid as the transport already handles this.  The rport
was resumed if the rport handed back matches the ndlp->nrport pointer.
Otherwise, devloss fired and the ndlp's nrport is NULL.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <james.smart@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_attr.c |   7 ++-
 drivers/scsi/lpfc/lpfc_nvme.c | 141 ++++++++++++++++--------------------------
 2 files changed, 57 insertions(+), 91 deletions(-)
diff mbox

Patch

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index bb2d9e238225..37f258fcf6d4 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -3198,9 +3198,12 @@  lpfc_update_rport_devloss_tmo(struct lpfc_vport *vport)
 
 	shost = lpfc_shost_from_vport(vport);
 	spin_lock_irq(shost->host_lock);
-	list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp)
-		if (NLP_CHK_NODE_ACT(ndlp) && ndlp->rport)
+	list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
+		if (!NLP_CHK_NODE_ACT(ndlp))
+			continue;
+		if (ndlp->rport)
 			ndlp->rport->dev_loss_tmo = vport->cfg_devloss_tmo;
+	}
 	spin_unlock_irq(shost->host_lock);
 }
 
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 8008c8205fb6..70675fd7d884 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -186,13 +186,13 @@  lpfc_nvme_remoteport_delete(struct nvme_fc_remote_port *remoteport)
 
 	/* Remove this rport from the lport's list - memory is owned by the
 	 * transport. Remove the ndlp reference for the NVME transport before
-	 * calling state machine to remove the node, this is devloss = 0
-	 * semantics.
+	 * calling state machine to remove the node.
 	 */
 	lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_DISC,
 			"6146 remoteport delete complete %p\n",
 			remoteport);
 	list_del(&rport->list);
+	ndlp->nrport = NULL;
 	lpfc_nlp_put(ndlp);
 
  rport_err:
@@ -1466,7 +1466,7 @@  lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
 
 	/* The remote node has to be ready to send an abort. */
 	if ((ndlp->nlp_state != NLP_STE_MAPPED_NODE) &&
-	    !(ndlp->nlp_type & NLP_NVME_TARGET)) {
+	    (ndlp->nlp_type & NLP_NVME_TARGET)) {
 		lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
 				 "6048 rport %p, DID x%06x not ready for "
 				 "IO. State x%x, Type x%x\n",
@@ -2340,69 +2340,44 @@  lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	localport = vport->localport;
 	lport = (struct lpfc_nvme_lport *)localport->private;
 
-	if (ndlp->nlp_type & (NLP_NVME_TARGET | NLP_NVME_INITIATOR)) {
-
-		/* The driver isn't expecting the rport wwn to change
-		 * but it might get a different DID on a different
-		 * fabric.
+	/* NVME rports are not preserved across devloss.
+	 * Just register this instance.  Note, rpinfo->dev_loss_tmo
+	 * is left 0 to indicate accept transport defaults.  The
+	 * driver communicates port role capabilities consistent
+	 * with the PRLI response data.
+	 */
+	memset(&rpinfo, 0, sizeof(struct nvme_fc_port_info));
+	rpinfo.port_id = ndlp->nlp_DID;
+	if (ndlp->nlp_type & NLP_NVME_TARGET)
+		rpinfo.port_role |= FC_PORT_ROLE_NVME_TARGET;
+	if (ndlp->nlp_type & NLP_NVME_INITIATOR)
+		rpinfo.port_role |= FC_PORT_ROLE_NVME_INITIATOR;
+
+	if (ndlp->nlp_type & NLP_NVME_DISCOVERY)
+		rpinfo.port_role |= FC_PORT_ROLE_NVME_DISCOVERY;
+
+	rpinfo.port_name = wwn_to_u64(ndlp->nlp_portname.u.wwn);
+	rpinfo.node_name = wwn_to_u64(ndlp->nlp_nodename.u.wwn);
+	ret = nvme_fc_register_remoteport(localport, &rpinfo, &remote_port);
+	if (!ret) {
+		/* If the ndlp already has an nrport, this is just
+		 * a resume of the existing rport.  Else this is a
+		 * new rport.
 		 */
-		list_for_each_entry(rport, &lport->rport_list, list) {
-			if (rport->remoteport->port_name !=
-			    wwn_to_u64(ndlp->nlp_portname.u.wwn))
-				continue;
-			lpfc_printf_vlog(ndlp->vport, KERN_INFO, LOG_NVME_DISC,
-					 "6035 lport %p, found matching rport "
-					 "at wwpn 0x%llx, Data: x%x x%x x%x "
-					 "x%06x\n",
-					 lport,
-					 rport->remoteport->port_name,
-					 rport->remoteport->port_id,
-					 rport->remoteport->port_role,
+		rport = remote_port->private;
+		if (ndlp->nrport == rport) {
+			lpfc_printf_vlog(ndlp->vport, KERN_INFO,
+					 LOG_NVME_DISC,
+					 "6014 Rebinding lport to "
+					 "rport wwpn 0x%llx, "
+					 "Data: x%x x%x x%x x%06x\n",
+					 remote_port->port_name,
+					 remote_port->port_id,
+					 remote_port->port_role,
 					 ndlp->nlp_type,
 					 ndlp->nlp_DID);
-			remote_port = rport->remoteport;
-			if ((remote_port->port_id == 0) &&
-			    (remote_port->port_role ==
-			     FC_PORT_ROLE_NVME_DISCOVERY)) {
-				remote_port->port_id = ndlp->nlp_DID;
-				remote_port->port_role &=
-					~FC_PORT_ROLE_NVME_DISCOVERY;
-				if (ndlp->nlp_type & NLP_NVME_TARGET)
-					remote_port->port_role |=
-						FC_PORT_ROLE_NVME_TARGET;
-				if (ndlp->nlp_type & NLP_NVME_INITIATOR)
-					remote_port->port_role |=
-						FC_PORT_ROLE_NVME_INITIATOR;
-
-				lpfc_printf_vlog(ndlp->vport, KERN_INFO,
-						 LOG_NVME_DISC,
-						 "6014 Rebinding lport to "
-						 "rport wwpn 0x%llx, "
-						 "Data: x%x x%x x%x x%06x\n",
-						 remote_port->port_name,
-						 remote_port->port_id,
-						 remote_port->port_role,
-						 ndlp->nlp_type,
-						 ndlp->nlp_DID);
-			}
-			return 0;
-		}
-
-		/* NVME rports are not preserved across devloss.
-		 * Just register this instance.
-		 */
-		rpinfo.port_id = ndlp->nlp_DID;
-		rpinfo.port_role = 0;
-		if (ndlp->nlp_type & NLP_NVME_TARGET)
-			rpinfo.port_role |= FC_PORT_ROLE_NVME_TARGET;
-		if (ndlp->nlp_type & NLP_NVME_INITIATOR)
-			rpinfo.port_role |= FC_PORT_ROLE_NVME_INITIATOR;
-		rpinfo.port_name = wwn_to_u64(ndlp->nlp_portname.u.wwn);
-		rpinfo.node_name = wwn_to_u64(ndlp->nlp_nodename.u.wwn);
-		ret = nvme_fc_register_remoteport(localport, &rpinfo,
-						  &remote_port);
-		if (!ret) {
-			rport = remote_port->private;
+		} else {
+			/* New rport. */
 			rport->remoteport = remote_port;
 			rport->lport = lport;
 			rport->ndlp = lpfc_nlp_get(ndlp);
@@ -2413,26 +2388,22 @@  lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 			list_add_tail(&rport->list, &lport->rport_list);
 			lpfc_printf_vlog(vport, KERN_INFO,
 					 LOG_NVME_DISC | LOG_NODE,
-					 "6022 Binding new rport to lport %p "
-					 "Rport WWNN 0x%llx, Rport WWPN 0x%llx "
-					 "DID x%06x Role x%x\n",
+					 "6022 Binding new rport to "
+					 "lport %p Rport WWNN 0x%llx, "
+					 "Rport WWPN 0x%llx DID "
+					 "x%06x Role x%x\n",
 					 lport,
 					 rpinfo.node_name, rpinfo.port_name,
 					 rpinfo.port_id, rpinfo.port_role);
-		} else {
-			lpfc_printf_vlog(vport, KERN_ERR,
-					 LOG_NVME_DISC | LOG_NODE,
-					 "6031 RemotePort Registration failed "
-					 "err: %d, DID x%06x\n",
-					 ret, ndlp->nlp_DID);
 		}
 	} else {
-		ret = -EINVAL;
-		lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_DISC,
-				 "6027 Unknown nlp_type x%x on DID x%06x "
-				 "ndlp %p.  Not Registering nvme rport\n",
-				 ndlp->nlp_type, ndlp->nlp_DID, ndlp);
+		lpfc_printf_vlog(vport, KERN_ERR,
+				 LOG_NVME_DISC | LOG_NODE,
+				 "6031 RemotePort Registration failed "
+				 "err: %d, DID x%06x\n",
+				 ret, ndlp->nlp_DID);
 	}
+
 	return ret;
 #else
 	return 0;
@@ -2460,7 +2431,6 @@  lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	struct lpfc_nvme_lport *lport;
 	struct lpfc_nvme_rport *rport;
 	struct nvme_fc_remote_port *remoteport;
-	unsigned long wait_tmo;
 
 	localport = vport->localport;
 
@@ -2491,6 +2461,10 @@  lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	 */
 	if (ndlp->nlp_type & (NLP_NVME_TARGET | NLP_NVME_INITIATOR)) {
 		init_completion(&rport->rport_unreg_done);
+
+		/* No concern about the role change on the nvme remoteport.
+		 * The transport will update it.
+		 */
 		ret = nvme_fc_unregister_remoteport(remoteport);
 		if (ret != 0) {
 			lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_DISC,
@@ -2499,17 +2473,6 @@  lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 					 ret, remoteport->port_state);
 		}
 
-		/* Wait for the driver's delete completion routine to finish
-		 * before proceeding.  This guarantees the transport and driver
-		 * have completed the unreg process.
-		 */
-		wait_tmo = msecs_to_jiffies(5000);
-		ret = wait_for_completion_timeout(&rport->rport_unreg_done,
-						  wait_tmo);
-		if (ret == 0) {
-			lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_DISC,
-					 "6169 Unreg nvme wait timeout\n");
-		}
 	}
 	return;