diff mbox series

[v2,06/13] qla2xxx: edif: flush stale events and msgs on session down

Message ID 20211021073208.27582-7-njavali@marvell.com (mailing list archive)
State Superseded
Headers show
Series qla2xxx - misc driver and EDIF bug fixes | expand

Commit Message

Nilesh Javali Oct. 21, 2021, 7:32 a.m. UTC 
From: Quinn Tran <qutran@marvell.com>

On session down, driver will flush all stale messages and
doorbell events. This prevents authentication application
from having to process stale data.

Fixes: 4de067e5df12 ("scsi: qla2xxx: edif: Add N2N support for EDIF")
Signed-off-by: Karunakara Merugu <kmerugu@marvell.com>
Signed-off-by: Quinn Tran <qutran@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_edif.c   | 99 ++++++++++++++++++++++++++++++-
 drivers/scsi/qla2xxx/qla_gbl.h    |  2 +
 drivers/scsi/qla2xxx/qla_target.c |  1 +
 3 files changed, 101 insertions(+), 1 deletion(-)

Comments

Himanshu Madhani Oct. 21, 2021, 2:21 p.m. UTC | #1 
> On Oct 21, 2021, at 2:32 AM, Nilesh Javali <njavali@marvell.com> wrote:
> 
> From: Quinn Tran <qutran@marvell.com>
> 
> On session down, driver will flush all stale messages and
> doorbell events. This prevents authentication application
> from having to process stale data.
> 
> Fixes: 4de067e5df12 ("scsi: qla2xxx: edif: Add N2N support for EDIF")
> Signed-off-by: Karunakara Merugu <kmerugu@marvell.com>
> Signed-off-by: Quinn Tran <qutran@marvell.com>
> Signed-off-by: Nilesh Javali <njavali@marvell.com>
> ---
> drivers/scsi/qla2xxx/qla_edif.c   | 99 ++++++++++++++++++++++++++++++-
> drivers/scsi/qla2xxx/qla_gbl.h    |  2 +
> drivers/scsi/qla2xxx/qla_target.c |  1 +
> 3 files changed, 101 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_edif.c b/drivers/scsi/qla2xxx/qla_edif.c
> index 33cdcdf9f511..b4eca966067a 100644
> --- a/drivers/scsi/qla2xxx/qla_edif.c
> +++ b/drivers/scsi/qla2xxx/qla_edif.c
> @@ -1595,6 +1595,41 @@ qla_enode_stop(scsi_qla_host_t *vha)
> 	spin_unlock_irqrestore(&vha->pur_cinfo.pur_lock, flags);
> }
> 
> +static void qla_enode_clear(scsi_qla_host_t *vha, port_id_t portid)
> +{
> +	unsigned    long flags;
> +	struct enode    *e, *tmp;
> +	struct purexevent   *purex;
> +	LIST_HEAD(enode_list);
> +
> +	if (vha->pur_cinfo.enode_flags != ENODE_ACTIVE) {
> +		ql_dbg(ql_dbg_edif, vha, 0x09102,
> +		       "%s enode not active\n", __func__);
> +		return;
> +	}
> +	spin_lock_irqsave(&vha->pur_cinfo.pur_lock, flags);
> +	list_for_each_entry_safe(e, tmp, &vha->pur_cinfo.head, list) {
> +		purex = &e->u.purexinfo;
> +		if (purex->pur_info.pur_sid.b24 == portid.b24) {
> +			ql_dbg(ql_dbg_edif, vha, 0x911d,
> +			    "%s free ELS sid=%x. xchg %x, nb=%xh\n”,
					      ^^  
Shouldn’t this be %06x?
	
> +			    __func__, portid.b24,
> +			    purex->pur_info.pur_rx_xchg_address,
> +			    purex->pur_info.pur_bytes_rcvd);
> +
> +			list_del_init(&e->list);
> +			list_add_tail(&e->list, &enode_list);
> +		}
> +	}
> +

No need for newline above.

> +	spin_unlock_irqrestore(&vha->pur_cinfo.pur_lock, flags);
> +
> +	list_for_each_entry_safe(e, tmp, &enode_list, list) {
> +		list_del_init(&e->list);
> +		qla_enode_free(vha, e);
> +	}
> +}
> +
> /*
>  *  allocate enode struct and populate buffer
>  *  returns: enode pointer with buffers
> @@ -1794,6 +1829,59 @@ qla_edb_node_free(scsi_qla_host_t *vha, struct edb_node *node)
> 	node->ntype = N_UNDEF;
> }
> 
> +static void qla_edb_clear(scsi_qla_host_t *vha, port_id_t portid)
> +{
> +	unsigned long flags;
> +	struct edb_node *e, *tmp;
> +	port_id_t sid;
> +	LIST_HEAD(edb_list);
> +
> +	if (vha->e_dbell.db_flags != EDB_ACTIVE) {
> +		/* doorbell list not enabled */
> +		ql_dbg(ql_dbg_edif, vha, 0x09102,
> +		       "%s doorbell not enabled\n", __func__);
> +		return;
> +	}
> +
> +	/* grab lock so list doesn't move */
> +	spin_lock_irqsave(&vha->e_dbell.db_lock, flags);
> +

No need for newline above.

> +	list_for_each_entry_safe(e, tmp, &vha->e_dbell.head, list) {
> +		switch (e->ntype) {
> +		case VND_CMD_AUTH_STATE_NEEDED:
> +		case VND_CMD_AUTH_STATE_SESSION_SHUTDOWN:
> +			sid = e->u.plogi_did;
> +			break;
> +		case VND_CMD_AUTH_STATE_ELS_RCVD:
> +			sid = e->u.els_sid;
> +			break;
> +		case VND_CMD_AUTH_STATE_SAUPDATE_COMPL:
> +			/* app wants to see this  */
> +			continue;
> +		default:
> +			ql_log(ql_log_warn, vha, 0x09102,
> +			       "%s unknown type: %x\n", __func__, e->ntype);
					   ^^^^
Please add “node type" for ease of reading messages during debugging 
					
> +			sid.b24 = 0;
> +			break;
> +		}
> +		if (sid.b24 == portid.b24) {
> +			ql_dbg(ql_dbg_edif, vha, 0x910f,
> +			       "%s Doorbell free : type=%x %p\n”,
					^^^^
Please have meaningful prints for ease of debugging 

> +			       __func__, e->ntype, e);
> +			list_del_init(&e->list);
> +			list_add_tail(&e->list, &edb_list);
> +		}
> +	}
> +

No need for newline above. 

> +	spin_unlock_irqrestore(&vha->e_dbell.db_lock, flags);
> +
> +	list_for_each_entry_safe(e, tmp, &edb_list, list) {
> +		qla_edb_node_free(vha, e);
> +		list_del_init(&e->list);
> +		kfree(e);
> +	}
> +}
> +
> /* function called when app is stopping */
> 
> void
> @@ -2380,7 +2468,7 @@ void qla24xx_auth_els(scsi_qla_host_t *vha, void **pkt, struct rsp_que **rsp)
> 	ql_dbg(ql_dbg_edif, host, 0x0910c,
> 	    "%s COMPLETE purex->pur_info.pur_bytes_rcvd =%xh s:%06x -> d:%06x xchg=%xh\n",
> 	    __func__, purex->pur_info.pur_bytes_rcvd, purex->pur_info.pur_sid.b24,
> -	    purex->pur_info.pur_did.b24, p->rx_xchg_addr);
> +	    purex->pur_info.pur_did.b24, purex->pur_info.pur_rx_xchg_address);
> 
> 	qla_edb_eventcreate(host, VND_CMD_AUTH_STATE_ELS_RCVD, sid, 0, NULL);
> }
> @@ -3403,3 +3491,12 @@ void qla_edif_sess_down(struct scsi_qla_host *vha, struct fc_port *sess)
> 		qla2x00_post_aen_work(vha, FCH_EVT_PORT_OFFLINE, sess->d_id.b24);
> 	}
> }
> +
> +void qla_edif_clear_appdata(struct scsi_qla_host *vha, struct fc_port *fcport)
> +{
> +	if (!(fcport->flags & FCF_FCSP_DEVICE))
> +		return;
> +
> +	qla_edb_clear(vha, fcport->d_id);
> +	qla_enode_clear(vha, fcport->d_id);
> +}
> diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
> index 8faaa0ec595d..76b89bd297dc 100644
> --- a/drivers/scsi/qla2xxx/qla_gbl.h
> +++ b/drivers/scsi/qla2xxx/qla_gbl.h
> @@ -142,6 +142,8 @@ void qlt_chk_edif_rx_sa_delete_pending(scsi_qla_host_t *vha, fc_port_t *fcport,
> void qla2x00_release_all_sadb(struct scsi_qla_host *vha, struct fc_port *fcport);
> int qla_edif_process_els(scsi_qla_host_t *vha, struct bsg_job *bsgjob);
> void qla_edif_sess_down(struct scsi_qla_host *vha, struct fc_port *sess);
> +void qla_edif_clear_appdata(struct scsi_qla_host *vha,
> +			    struct fc_port *fcport);
> const char *sc_to_str(uint16_t cmd);
> 
> /*
> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
> index b3478ed9b12e..edc34e69d75b 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -1003,6 +1003,7 @@ void qlt_free_session_done(struct work_struct *work)
> 					"%s bypassing release_all_sadb\n",
> 					__func__);
> 			}
> +			qla_edif_clear_appdata(vha, sess);
> 			qla_edif_sess_down(vha, sess);
> 		}
> 		qla2x00_mark_device_lost(vha, sess, 0);
> -- 
> 2.19.0.rc0
> 

Once you fix small nits, feel free to add

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	 Oracle Linux Engineering
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_edif.c b/drivers/scsi/qla2xxx/qla_edif.c
index 33cdcdf9f511..b4eca966067a 100644
--- a/drivers/scsi/qla2xxx/qla_edif.c
+++ b/drivers/scsi/qla2xxx/qla_edif.c
@@ -1595,6 +1595,41 @@  qla_enode_stop(scsi_qla_host_t *vha)
 	spin_unlock_irqrestore(&vha->pur_cinfo.pur_lock, flags);
 }
 
+static void qla_enode_clear(scsi_qla_host_t *vha, port_id_t portid)
+{
+	unsigned    long flags;
+	struct enode    *e, *tmp;
+	struct purexevent   *purex;
+	LIST_HEAD(enode_list);
+
+	if (vha->pur_cinfo.enode_flags != ENODE_ACTIVE) {
+		ql_dbg(ql_dbg_edif, vha, 0x09102,
+		       "%s enode not active\n", __func__);
+		return;
+	}
+	spin_lock_irqsave(&vha->pur_cinfo.pur_lock, flags);
+	list_for_each_entry_safe(e, tmp, &vha->pur_cinfo.head, list) {
+		purex = &e->u.purexinfo;
+		if (purex->pur_info.pur_sid.b24 == portid.b24) {
+			ql_dbg(ql_dbg_edif, vha, 0x911d,
+			    "%s free ELS sid=%x. xchg %x, nb=%xh\n",
+			    __func__, portid.b24,
+			    purex->pur_info.pur_rx_xchg_address,
+			    purex->pur_info.pur_bytes_rcvd);
+
+			list_del_init(&e->list);
+			list_add_tail(&e->list, &enode_list);
+		}
+	}
+
+	spin_unlock_irqrestore(&vha->pur_cinfo.pur_lock, flags);
+
+	list_for_each_entry_safe(e, tmp, &enode_list, list) {
+		list_del_init(&e->list);
+		qla_enode_free(vha, e);
+	}
+}
+
 /*
  *  allocate enode struct and populate buffer
  *  returns: enode pointer with buffers
@@ -1794,6 +1829,59 @@  qla_edb_node_free(scsi_qla_host_t *vha, struct edb_node *node)
 	node->ntype = N_UNDEF;
 }
 
+static void qla_edb_clear(scsi_qla_host_t *vha, port_id_t portid)
+{
+	unsigned long flags;
+	struct edb_node *e, *tmp;
+	port_id_t sid;
+	LIST_HEAD(edb_list);
+
+	if (vha->e_dbell.db_flags != EDB_ACTIVE) {
+		/* doorbell list not enabled */
+		ql_dbg(ql_dbg_edif, vha, 0x09102,
+		       "%s doorbell not enabled\n", __func__);
+		return;
+	}
+
+	/* grab lock so list doesn't move */
+	spin_lock_irqsave(&vha->e_dbell.db_lock, flags);
+
+	list_for_each_entry_safe(e, tmp, &vha->e_dbell.head, list) {
+		switch (e->ntype) {
+		case VND_CMD_AUTH_STATE_NEEDED:
+		case VND_CMD_AUTH_STATE_SESSION_SHUTDOWN:
+			sid = e->u.plogi_did;
+			break;
+		case VND_CMD_AUTH_STATE_ELS_RCVD:
+			sid = e->u.els_sid;
+			break;
+		case VND_CMD_AUTH_STATE_SAUPDATE_COMPL:
+			/* app wants to see this  */
+			continue;
+		default:
+			ql_log(ql_log_warn, vha, 0x09102,
+			       "%s unknown type: %x\n", __func__, e->ntype);
+			sid.b24 = 0;
+			break;
+		}
+		if (sid.b24 == portid.b24) {
+			ql_dbg(ql_dbg_edif, vha, 0x910f,
+			       "%s Doorbell free : type=%x %p\n",
+			       __func__, e->ntype, e);
+			list_del_init(&e->list);
+			list_add_tail(&e->list, &edb_list);
+		}
+	}
+
+	spin_unlock_irqrestore(&vha->e_dbell.db_lock, flags);
+
+	list_for_each_entry_safe(e, tmp, &edb_list, list) {
+		qla_edb_node_free(vha, e);
+		list_del_init(&e->list);
+		kfree(e);
+	}
+}
+
 /* function called when app is stopping */
 
 void
@@ -2380,7 +2468,7 @@  void qla24xx_auth_els(scsi_qla_host_t *vha, void **pkt, struct rsp_que **rsp)
 	ql_dbg(ql_dbg_edif, host, 0x0910c,
 	    "%s COMPLETE purex->pur_info.pur_bytes_rcvd =%xh s:%06x -> d:%06x xchg=%xh\n",
 	    __func__, purex->pur_info.pur_bytes_rcvd, purex->pur_info.pur_sid.b24,
-	    purex->pur_info.pur_did.b24, p->rx_xchg_addr);
+	    purex->pur_info.pur_did.b24, purex->pur_info.pur_rx_xchg_address);
 
 	qla_edb_eventcreate(host, VND_CMD_AUTH_STATE_ELS_RCVD, sid, 0, NULL);
 }
@@ -3403,3 +3491,12 @@  void qla_edif_sess_down(struct scsi_qla_host *vha, struct fc_port *sess)
 		qla2x00_post_aen_work(vha, FCH_EVT_PORT_OFFLINE, sess->d_id.b24);
 	}
 }
+
+void qla_edif_clear_appdata(struct scsi_qla_host *vha, struct fc_port *fcport)
+{
+	if (!(fcport->flags & FCF_FCSP_DEVICE))
+		return;
+
+	qla_edb_clear(vha, fcport->d_id);
+	qla_enode_clear(vha, fcport->d_id);
+}
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index 8faaa0ec595d..76b89bd297dc 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -142,6 +142,8 @@  void qlt_chk_edif_rx_sa_delete_pending(scsi_qla_host_t *vha, fc_port_t *fcport,
 void qla2x00_release_all_sadb(struct scsi_qla_host *vha, struct fc_port *fcport);
 int qla_edif_process_els(scsi_qla_host_t *vha, struct bsg_job *bsgjob);
 void qla_edif_sess_down(struct scsi_qla_host *vha, struct fc_port *sess);
+void qla_edif_clear_appdata(struct scsi_qla_host *vha,
+			    struct fc_port *fcport);
 const char *sc_to_str(uint16_t cmd);
 
 /*
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index b3478ed9b12e..edc34e69d75b 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1003,6 +1003,7 @@  void qlt_free_session_done(struct work_struct *work)
 					"%s bypassing release_all_sadb\n",
 					__func__);
 			}
+			qla_edif_clear_appdata(vha, sess);
 			qla_edif_sess_down(vha, sess);
 		}
 		qla2x00_mark_device_lost(vha, sess, 0);