Re: [PATCH v3 11/14] lpfc: Put iocbq on phba->txq when ELS WQ is full or ELS SGL unavailable

Re: [PATCH v3 11/14] lpfc: Put iocbq on phba->txq when ELS WQ is full or ELS SGL unavailable

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incomplete refactoring of __lpfc_sli_issue_iocb_s4 return codes causes unpatched callers to leak IOCBs.
- [High] Early returns on IOCB_FAILED_PUT in patched callers bypass critical state machine updates and counter increments.
- [Medium] The logic in lpfc_drain_txq to queue an IOCB to the head of the txq when the SGL pool is empty is dead code.
--

--- Patch [11]: [PATCH v3 11/14] lpfc: Put iocbq on phba->txq when ELS WQ is full or ELS SGL unavailable ---
commit e664b47bccd196f6d682ce56b8b0fe9c32e4c4a8
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Put iocbq on phba->txq when ELS WQ is full or ELS SGL unavailable

When ELS/CT commands can't be sent due to ELS WQ full, queue the iocbq on
the phba->txq tail for retrying submission later in lpfc_drain_txq.
lpfc_drain_txq is also updated to queue an iocbq back on to the head of
phba->txq when observing ELS WQ full or SGL unavailable events.

> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
> @@ -2766,7 +2780,21 @@ lpfc_issue_els_prli(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
>  	}
>  
>  	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
> -	if (rc == IOCB_ERROR) {
> +	if (rc) {
> +		lpfc_vlog_msg(vport, KERN_NOTICE,
> +			      LOG_ELS | LOG_DISCOVERY | LOG_NODE,
> +			      "0155 PRLI WQE Put returned %d\n",
> +			      rc);
> +
> +		/* Under heavy vpi counts, the driver's host_index can catch up
> +		 * to the hba_index causing a put error. Catch this case and
> +		 * put the IO on phba->txq.
> +		 */
> +		if (rc == IOCB_FAILED_PUT && phba->sli_rev == LPFC_SLI_REV4) {
> +			lpfc_sli4_queue_io_for_retry(phba, elsiocb, false);
> +			return 0;
> +		}

[Severity: High]
Does this early return bypass state machine updates?

By returning 0 here, the function skips setting the NLP_PRLI_SND flag,
incrementing the outstanding PRLI counters (vport->fc_prli_sent and
ndlp->fc4_prli_sent), and looping to send the NVMe PRLI.

When the retried IO eventually completes, the completion handler could decrement
counters that were never incremented, potentially causing an integer underflow
that breaks lpfc_scan_finished(). Also, it appears the second PRLI for NVMe
targets would never be sent.

> +
>  		lpfc_els_free_iocb(phba, elsiocb);
>  		lpfc_nlp_put(ndlp);
>  		return 1;
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -10793,12 +10839,18 @@ __lpfc_sli_issue_iocb_s4(struct lpfc_hba *phba, uint32_t ring_number,
>  			return IOCB_ERROR;
>  	}
>  
> -	if (lpfc_sli4_wq_put(wq, wqe))
> -		return IOCB_ERROR;
> + retry_io:
> +	piocb->cmd_flag &= ~LPFC_IO_IN_RETRY;
>  
> -	lpfc_sli_ringtxcmpl_put(phba, pring, piocb);
> +	/* Push the wqe to the wq.  If the push fails with -EBUSY it means
> +	 * the WQ is full.  Pass this status back to enable a retry.
> +	 */
> +	rc = lpfc_sli4_wq_put(wq, wqe);
> +	if (rc == -EBUSY)
> +		return IOCB_FAILED_PUT;
>  
> -	return 0;
> +	lpfc_sli_ringtxcmpl_put(phba, pring, piocb);
> +	return rc;
>  }

[Severity: High]
Will unpatched callers leak resources by treating IOCB_FAILED_PUT as success?

Several existing ELS/CT callers, such as lpfc_issue_els_logo() and
lpfc_issue_els_adisc(), explicitly check if the return value is exactly
IOCB_ERROR:

drivers/scsi/lpfc/lpfc_els.c:lpfc_issue_els_logo() {
	...
	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
	if (rc == IOCB_ERROR) {
		lpfc_els_free_iocb(phba, elsiocb);
		...
	}
	...
}

Because IOCB_FAILED_PUT (7) is not equal to IOCB_ERROR (1), these functions
might treat the failure as a successful submission. Could this cause the driver
to permanently leak the IOCB buffer/SGL/DMA mappings and hang the ELS state
machine?

> @@ -21277,7 +21329,7 @@ lpfc_drain_txq(struct lpfc_hba *phba)
>  
>  		ret = __lpfc_sli_issue_iocb(phba, pring->ringno, piocbq, 0);

[ ... ]

> +		if (txq_cnt == 0 || ret == IOCB_BUSY ||
> +		    ret == IOCB_FAILED_PUT) {
> +			/* If ELS WQ was full (IOCB_FAILED_PUT), then an SGL has
> +			 * been assigned.  If SGL pool was empty (IOCB_BUSY),
> +			 * then all we need is to put back on phba->txq to try
> +			 * again later.
> +			 */

[ ... ]

> +			switch (ret) {
> +			case IOCB_FAILED_PUT:
> +				piocbq->cmd_flag |= LPFC_IO_IN_RETRY;
> +				__lpfc_sli_ringtx_put(phba, pring, piocbq,
> +						      true);
> +				break;
> +			case IOCB_BUSY:
> +				__lpfc_sli_ringtx_put(phba, pring, piocbq,
> +						      true);
> +				break;

[Severity: Medium]
Is this IOCB_BUSY case dead code?

The call to __lpfc_sli_issue_iocb() above passes 0 for the flag argument.
If an SGLQ cannot be allocated, __lpfc_sli_issue_iocb_s4() evaluates:

	if (!(flag & SLI_IOCB_RET_IOCB)) {
		__lpfc_sli_ringtx_put(phba, pring, piocb, false);
		return IOCB_SUCCESS;
	} else {
		return IOCB_BUSY;
	}

Since flag is 0, it silently requeues the IO to the tail of txq and returns
IOCB_SUCCESS (0). Therefore, lpfc_drain_txq() would never receive IOCB_BUSY,
and the intended optimization to queue the IOCB to the head when the SGL pool
is empty would be bypassed.

> +			}
> +			spin_unlock_irqrestore(&pring->ring_lock, iflags);
>  			break;
> +		}
> +
> +		spin_unlock_irqrestore(&pring->ring_lock, iflags);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=11

Re: [PATCH v3 06/14] lpfc: Fix ndlp use-after-free during repeated RSCN and rediscovery sequence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Race condition between `lpfc_unregister_remote_port` and `lpfc_dev_loss_tmo_callbk` leading to double-put and Use-After-Free.
- [High] Unbounded reference count leak for NVMe Target (NVMET) nodes during repeated RSCN rediscovery.
- [Medium] Lockless check in `lpfc_nlp_unreg_node` introduces a TOCTOU race leading to double unregistration.
--

--- Patch [6]: [PATCH v3 06/14] lpfc: Fix ndlp use-after-free during repeated RSCN and rediscovery sequence ---
commit a4e515a68e8dcd0a05052708ad0dbafb1627b3e9
Author: Justin Tee <justintee8345@gmail.com>
Subject: lpfc: Fix ndlp use-after-free during repeated RSCN and rediscovery sequence

This commit refactors lpfc_fc4_xpt_flags to use separate flags for SCSI
and NVME registrations. It adds wait flags to track unregister requests
during device loss and transport callbacks.

[ ... ]
> --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
> +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
[ ... ]
> @@ -4562,6 +4556,7 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
>  {
>  	struct fc_rport *rport = ndlp->rport;
>  	struct lpfc_vport *vport = ndlp->vport;
> +	unsigned long flags;
>  
>  	if (vport->cfg_enable_fc4_type == LPFC_ENABLE_NVME)
>  		return;
> @@ -4577,6 +4572,11 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
>  			 kref_read(&ndlp->kref));
>  
>  	fc_remote_port_delete(rport);
> +
> +	/* Flag unreg pending and reference released */
> +	spin_lock_irqsave(&ndlp->lock, flags);
> +	ndlp->fc4_xpt_flags |= SCSI_XPT_UNREG_WAIT;
> +	spin_unlock_irqrestore(&ndlp->lock, flags);
>  	lpfc_nlp_put(ndlp);
>  }

[Severity: High]
Can this sequence cause a double-put of the ndlp structure if the transport
callback races with this function?

Because SCSI_XPT_UNREG_WAIT is set after fc_remote_port_delete() is called,
the queued dev_loss_work task can execute asynchronously before the lock is
acquired here to set the wait flag.

If the transport callback wins the race, lpfc_dev_loss_tmo_callbk() will read
!(ndlp->fc4_xpt_flags & SCSI_XPT_UNREG_WAIT) as true, clear SCSI_XPT_REGD,
and call lpfc_nlp_put(). When lpfc_unregister_remote_port() resumes, it sets
the wait flag and unconditionally calls lpfc_nlp_put() again, which could
trigger a use-after-free.

[ ... ]
> @@ -4623,7 +4623,9 @@ lpfc_nlp_reg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>  	lpfc_check_nlp_post_devloss(vport, ndlp);
>  
>  	spin_lock_irqsave(&ndlp->lock, iflags);
> -	if (ndlp->fc4_xpt_flags & NLP_XPT_REGD) {
> +	if ((ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD)) &&
> +	    !(ndlp->fc4_xpt_flags & (SCSI_XPT_UNREG_WAIT |
> +				     NVME_XPT_UNREG_WAIT))) {
>  		/* Already registered with backend, trigger rescan */
>  		spin_unlock_irqrestore(&ndlp->lock, iflags);

[Severity: High]
Does this code leak an ndlp reference for NVMe targets during repeated RSCN
rediscovery events?

In an NVMe target setup (nvmet_support != 0), neither SCSI_XPT_REGD nor
NVME_XPT_REGD is ever set. Instead, the code falls through to the target
handling block at the end of lpfc_nlp_reg_node():

        } else {
                /* Just take an NDLP ref count since the
                 * target does not register rports.
                 */
                lpfc_nlp_get(ndlp);
        }

Without a registration flag set, every subsequent call to lpfc_nlp_reg_node()
during repeated RSCNs will bypass this early return check, falling through to
execute lpfc_nlp_get() again and permanently leaking a reference each time.

[ ... ]
> @@ -4678,7 +4675,7 @@ lpfc_nlp_unreg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>  	unsigned long iflags;
>  
>  	spin_lock_irqsave(&ndlp->lock, iflags);
> -	if (!(ndlp->fc4_xpt_flags & NLP_XPT_REGD)) {
> +	if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD))) {
>  		spin_unlock_irqrestore(&ndlp->lock, iflags);
[ ... ]
>  	}
> -
> -	ndlp->fc4_xpt_flags &= ~NLP_XPT_REGD;
>  	spin_unlock_irqrestore(&ndlp->lock, iflags);
>  
>  	if (ndlp->rport &&
> -	    ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
> +	    ((ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | SCSI_XPT_UNREG_WAIT)) ==
> +	     SCSI_XPT_REGD)) {
>  		vport->phba->nport_event_cnt++;
>  		lpfc_unregister_remote_port(ndlp);

[Severity: Medium]
Could this lockless check introduce a time-of-check to time-of-use race?

The original code cleared NLP_XPT_REGD inside the ndlp->lock critical
section. By replacing it with a lockless check of the flag combination outside
the lock, two concurrent threads entering lpfc_nlp_unreg_node() could both
read the state as SCSI_XPT_REGD without SCSI_XPT_UNREG_WAIT.

If that happens, both threads would proceed to call
lpfc_unregister_remote_port(ndlp), leading to a double unregistration and
potential double-put of the ndlp structure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=6

Re: [PATCH v3 01/14] lpfc: Fix use-after-free in lpfc_cmpl_ct_cmd_vmid

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Zeroing hash table elements before their removal causes a guaranteed kernel panic, and unsafe hash table iteration breaks list cleanup.
- [High] Multiple severe locking violations in the VMID hash table and array accesses, including using a read lock for destructive writes and lockless modifications.
- [High] The SLI_CTAS_DALLAPP_ID handler leaks per-cpu memory by memsetting the lpfc_vmid structure to zero without freeing `last_io_time`.
- [High] In `lpfc_vmid_vport_cleanup`, `vport->vmid` is freed via `kfree` before the code iterates over `vport->hash_table` to remove entries, resulting in a Use-After-Free.
--

--- Patch [1]: [PATCH v3 01/14] lpfc: Fix use-after-free in lpfc_cmpl_ct_cmd_vmid ---
commit ade52a05f937db7fd95e3ae568536fda43aeccdf
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Fix use-after-free in lpfc_cmpl_ct_cmd_vmid

This commit removes an early call to lpfc_ct_free_iocb() in
lpfc_cmpl_ct_cmd_vmid() for SLI_CTAS_DALLAPP_ID commands. This prevents a
use-after-free of cmdiocb->rsp_dmabuf during CT LS_RJT response handling,
relying instead on the existing free_res label for cleanup.

> diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
> index c7853e7fe0719..e14170550e697 100644
> --- a/drivers/scsi/lpfc/lpfc_ct.c
> +++ b/drivers/scsi/lpfc/lpfc_ct.c
> @@ -3595,8 +3595,6 @@ lpfc_cmpl_ct_cmd_vmid(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
>  	int i;
>  
>  	cmd = be16_to_cpu(ctcmd->CommandResponse.bits.CmdRsp);
> -	if (cmd == SLI_CTAS_DALLAPP_ID)
> -		lpfc_ct_free_iocb(phba, cmdiocb);
>  
>  	if (lpfc_els_chk_latt(vport) || get_job_ulpstatus(phba, rspiocb)) {
>  		if (cmd != SLI_CTAS_DALLAPP_ID)

[Severity: Critical]
This is a pre-existing issue, but looking further down in
lpfc_cmpl_ct_cmd_vmid() under the SLI_CTAS_DALLAPP_ID case:

    memset(vmp, 0, sizeof(struct lpfc_vmid));

This zeroes the embedded hlist_node, including its pprev and next pointers.
Right after this, the code attempts to remove these items:

    if (!hash_empty(vport->hash_table))
        hash_for_each(vport->hash_table, bucket, cur, hnode)
            hash_del(&cur->hnode);

Does calling hash_del() on these zeroed elements cause a NULL pointer
dereference panic when it tries to write to *n->pprev?

Also, since this iteration modifies the list, shouldn't it use
hash_for_each_safe() to prevent breaking the iteration chain?

[Severity: High]
This isn't a bug introduced by this patch, but there appear to be multiple
locking violations in lpfc_cmpl_ct_cmd_vmid() that could lead to data races
and corruption.

First, under the SLI_CTAS_DALLAPP_ID case, the memset() is performed under a
read lock:

    read_lock(&vport->vmid_lock);
    for (i = 0; i < phba->cfg_max_vmid; i++) {
        vmp = &vport->vmid[i];
        if (vmp->flag != LPFC_VMID_SLOT_FREE)
            memset(vmp, 0, sizeof(struct lpfc_vmid));
    }
    read_unlock(&vport->vmid_lock);

Since read_lock() doesn't provide mutual exclusion against concurrent readers,
does this destructive write race with other readers?

Additionally, the hash table iteration and deletion:

    if (!hash_empty(vport->hash_table))
        hash_for_each(vport->hash_table, bucket, cur, hnode)
            hash_del(&cur->hnode);

is performed entirely locklessly.

Furthermore, earlier in the SLI_CTAS_RAPP_IDENT case:

    vmp = lpfc_get_vmid_from_hashtable(vport, hash,
                                       app->obj.entity_id);

This lookup is also performed without holding any lock.

Can these lockless operations and the write-under-read-lock cause list
corruption?

[Severity: High]
This is a pre-existing issue, but the same memset() in lpfc_cmpl_ct_cmd_vmid()
that zeroes the hlist_node:

    memset(vmp, 0, sizeof(struct lpfc_vmid));

also overwrites the entire lpfc_vmid struct. This struct contains a dynamically
allocated per-cpu pointer:

    u64 __percpu *last_io_time;

Does this zeroing cause an unrecoverable leak of the per-cpu memory allocated
for last_io_time?

[Severity: High]
This is also a pre-existing issue, but related to the VMID hash table cleanup
seen above. In drivers/scsi/lpfc/lpfc_scsi.c:lpfc_vmid_vport_cleanup():

    kfree(vport->vmid);

    if (!hash_empty(vport->hash_table))
        hash_for_each(vport->hash_table, bucket, cur, hnode)
            hash_del(&cur->hnode);

The hash table nodes (hnode) are embedded within the lpfc_vmid structures in
the vport->vmid array.

Since kfree(vport->vmid) is called before iterating the hash table, doesn't
the subsequent hash_for_each() loop read from freed memory?

Furthermore, shouldn't hash_for_each_safe() be used here as well since
hash_del() modifies the list?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=1

Re: [PATCH v3 02/14] lpfc: Early return out of lpfc_els_abort when HBA_SETUP flag is not set

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The lockless check of the `HBA_SETUP` flag introduced to prevent a use-after-free is insufficient and suffers from a TOCTOU race, leaving the UAF vulnerability unmitigated.
- [High] The iocb list entry is removed from the shared `txcmplq` without holding the required lock on an error path, leading to list corruption.
--

--- Patch [2]: [PATCH v3 02/14] lpfc: Early return out of lpfc_els_abort when HBA_SETUP flag is not set ---
commit 686b3cec858238b11486ee8f0c3a182653eb8d41
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Early return out of lpfc_els_abort when HBA_SETUP flag is not set
    
It is possible that a dev_loss_tmo callback fires during an hba reset.
The ELS pring structure is cleared by the hba reset path and the
dev_loss_tmo callback executing lpfc_els_abort could be using a stale ELS
pring pointer.  To prevent such a condition, check if HBA_SETUP flag is set
before proceeding to use the ELS pring pointer in lpfc_els_abort. There is
no point to issue aborts when the sli port is not setup anyways.

> diff --git a/drivers/scsi/lpfc/lpfc_nportdisc.c b/drivers/scsi/lpfc/lpfc_nportdisc.c
> index 9c449055a55e6..2c8d995a45bf8 100644
> --- a/drivers/scsi/lpfc/lpfc_nportdisc.c
> +++ b/drivers/scsi/lpfc/lpfc_nportdisc.c
> @@ -227,6 +227,11 @@ lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
>  	struct lpfc_iocbq *iocb, *next_iocb;
>  	int retval = 0;
>  
> +	/* Exit early to prevent race with queue teardown. */
> +	if (unlikely(phba->sli_rev == LPFC_SLI_REV4 &&
> +		     !test_bit(HBA_SETUP, &phba->hba_flag)))
> +		return;
> +

[Severity: High]
This is a pre-existing issue, but does this lockless check of HBA_SETUP
still leave a time-of-check to time-of-use race window open?

A concurrent HBA reset thread can clear HBA_SETUP and destroy the work
queues:

lpfc_wq_destroy()
    kfree(wq->pring);
    wq->pring = NULL;

If this happens immediately after the lockless check in lpfc_els_abort()
but before the pointer is used, could we still fetch a pointer that is
about to be freed, and subsequently dereference it later when acquiring
pring->ring_lock?

>  	pring = lpfc_phba_elsring(phba);
>  
>  	/* In case of error recovery path, we might have a NULL pring here */

[Severity: High]
This is a pre-existing issue, but does the error path later in
lpfc_els_abort() remove the iocb from the shared txcmplq without holding
the required locks?

	spin_unlock_irq(&phba->hbalock);

	if (test_bit(FC_UNLOADING, &phba->pport->load_flag) &&
	    retval && retval != IOCB_ABORTING) {
		list_del_init(&iocb->list);

Since elements are iterated on pring->txcmplq using their list node and
added to a local abort_list using their dlist node, the iocb->list is
still linked into txcmplq.

Could modifying iocb->list here without pring->ring_lock or hbalock lead
to list corruption if the shared list is concurrently accessed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=2

Re: [PATCH v3 10/14] lpfc: Remove slowpath cqe process limiter in slow ring event handler

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Removing the 64 CQE process limiter creates a structurally verified infinite loop when processing MDS Loopback or unsolicited data frames under resource exhaustion, leading to CPU soft lockups.
- [Medium] Mismatch between the commit message's stated log threshold (64) and the actual implemented code (256).
--

--- Patch [10]: [PATCH v3 10/14] lpfc: Remove slowpath cqe process limiter in slow ring event handler ---
commit 69f6839c38e91e160d808d1e850edd0295006605
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Remove slowpath cqe process limiter in slow ring event handler

This patch removes the 64 CQE processing limit in the slow ring event
handler to prevent delayed processing penalties during large SAN
rediscoveries. It replaces the hard limit with an unbounded loop that 
drains the queue and adds diagnostic logging.

> Remove the slowpath cqe process limiter in
> lpfc_sli_handle_slow_ring_event_s4 to ensure the slow path CQ is drained.
> Add log messages to notify when the 64 cqe count is reached.

[Severity: Medium]
The commit message states the log messages notify when the 64 CQE count
is reached, but the implementation uses LPFC_WQE_DEF_COUNT (which is 256).

Could the commit message be updated to match the code, or should the code
be using 64?

> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -4527,7 +4527,7 @@ lpfc_sli_handle_slow_ring_event_s4(struct lpfc_hba *phba,
>  	struct hbq_dmabuf *dmabuf;
>  	struct lpfc_cq_event *cq_event;
>  	unsigned long iflag;
> -	int count = 0;
> +	u32 count = 0;
>  
>  	clear_bit(HBA_SP_QUEUE_EVT, &phba->hba_flag);
>  	while (!list_empty(&phba->sli4_hba.sp_queue_event)) {

[Severity: High]
Could removing the event limit here lead to an infinite loop and a CPU soft
lockup under resource exhaustion?

If an MDS loopback frame or unsolicited data frame is received while the
driver's IOCB pool is exhausted, lpfc_sli4_handle_mds_loopback() fails
to allocate an IOCB and puts the event back on the tail of the exact queue
being drained:

lpfc_sli4_handle_mds_loopback() {
    ...
    list_add_tail(&dmabuf->cq_event.list,
                  &phba->sli4_hba.sp_queue_event);
    ...
}

Since the loop in lpfc_sli_handle_slow_ring_event_s4() no longer breaks at 64
events and doesn't yield, it seems the requeued event will be immediately
dequeued again. Because the kthread never yields to allow IOs to complete,
does the IOCB pool remain permanently exhausted while it infinitely requeues
the same event?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=10

Re: [PATCH v3 08/14] lpfc: Improve PLOGI retry handling for large SAN configurations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Non-atomic test-and-set and post-submission setting of NLP_PLOGI_SND causes permanent target block or bypassed serialization.
- [High] New state validation check in `lpfc_issue_els_plogi` silently drops legitimate PLOGIs, stalling discovery.
- [High] TOCTOU race in delayed PLOGI timer and unreg completion permanently drops deferred PLOGIs.
- [Medium] The `NLP_PLOGI_SND` flag is completely bypassed when a PLOGI IOCB is queued for retry upon `IOCB_FAILED_PUT`.
--

--- Patch [8]: [PATCH v3 08/14] lpfc: Improve PLOGI retry handling for large SAN configurations ---
commit 1ce21a9bd9de0c81a785643032705d301dd6dccb
Author: Justin Tee <justintee8345@gmail.com>
Date:   Tue Jul 14 01:18:06 2026 +0000

lpfc: Improve PLOGI retry handling for large SAN configurations

In large SAN configurations with link perturbations, rediscovery of target
ports is problematic due to PLOGI retry race conditions.

This patch improves target rediscovery by ensuring PLOGI retries are
serialized in unregistration and retry handler paths.

> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 6720fd5ec5232..16b10377a0953 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
[ ... ]
> @@ -2329,17 +2331,43 @@ lpfc_issue_els_plogi(struct lpfc_vport *vport, uint32_t did, uint8_t retry)
[ ... ]
> +	if (test_bit(NLP_PLOGI_SND, &ndlp->nlp_flag)) {

[Severity: Critical]
Can NLP_PLOGI_SND become permanently stuck?

The flag is tested here, but it is not set until after the I/O is submitted
to the HBA at the end of the function. If the hardware completes the PLOGI
quickly, the completion handler lpfc_cmpl_els_plogi() might run before the
issuing thread sets the bit.

The completion handler would clear the bit, achieving nothing, and then the
issuing thread would execute set_bit(), leaving the flag permanently set.

Could this permanently reject future PLOGIs? Would test_and_set_bit() be
more appropriate here to prevent this regression?

> +		lpfc_printf_vlog(vport, KERN_INFO,
> +				 LOG_ELS | LOG_NODE | LOG_DISCOVERY,
> +				 "4113 Reject PLOGI issue, PLOGI in-flight "
> +				 "x%px, DID x%x nflag x%lx\n",
> +				 ndlp, ndlp->nlp_DID, ndlp->nlp_flag);
> +		return 1;
> +	}
> +
> +	if (ndlp->nlp_state > NLP_STE_PLOGI_ISSUE &&
> +	    ndlp->nlp_state <= NLP_STE_MAPPED_NODE) {

[Severity: High]
Does this block legitimate PLOGIs during discovery?

For example, lpfc_rcv_plogi_adisc_issue() calls lpfc_issue_els_plogi() while
the node is in NLP_STE_ADISC_ISSUE. The PLOGI would be silently rejected
here, but the caller proceeds to force the state to NLP_STE_PLOGI_ISSUE
anyway.

Could this cause the port state machine to wait forever for a PLOGI
completion that will never arrive?

> +		lpfc_printf_vlog(vport, KERN_INFO,
> +				 LOG_ELS | LOG_NODE | LOG_DISCOVERY,
> +				 "4114 Reject PLOGI issue, Node in "
> +				 "unexpected state x%px, DID x%x nflag x%lx "
> +				 "in State x%x\n",
> +				 ndlp, ndlp->nlp_DID,
> +				 ndlp->nlp_flag, ndlp->nlp_state);
> +		return 1;
>  	}
>  
>  	cmdsize = (sizeof(uint32_t) + sizeof(struct serv_parm));
> @@ -2415,6 +2443,7 @@ lpfc_issue_els_plogi(struct lpfc_vport *vport, uint32_t did, uint8_t retry)
[ ... ]
>  		/* Under heavy vpi counts, the driver's host_index can catch up
>  		 * to the hba_index causing a put error. Catch this case and
>  		 * put the IO on phba->txq.
>  		 */
>  		if (ret == IOCB_FAILED_PUT && phba->sli_rev == LPFC_SLI_REV4) {
>  			lpfc_sli4_queue_io_for_retry(phba, elsiocb, false);
>  			return 0;
>  		}

[Severity: Medium]
If the IO is queued for retry here and returns early, doesn't it bypass
setting NLP_PLOGI_SND at the bottom of this function?

Since the IO is queued but the flag isn't set, wouldn't concurrent callers
pass the test_bit() check and issue duplicate PLOGIs, bypassing the
intended serialization?

>  		lpfc_els_free_iocb(phba, elsiocb);
>  		lpfc_nlp_put(ndlp);
>  		return 1;
>  	}
>  
> +	set_bit(NLP_PLOGI_SND, &ndlp->nlp_flag);
>  	return 0;
>  }
>  
> @@ -4614,6 +4643,31 @@ lpfc_els_retry_delay_handler(struct lpfc_nodelist *ndlp)
[ ... ]
>  	case ELS_CMD_PLOGI:
> +		/* The driver delayed a PLOGI via the nlp_delayfunc, but
> +		 * it's possible the PLOGI is already on a deferred retry.
> +		 * Catch this case and skip this delayed PLOGI. This prevents
> +		 * multiple PLOGIs in flight. The defer code flow cleans
> +		 * up.
> +		 */
> +		if ((test_bit(NLP_IGNR_REG_CMPL, &ndlp->nlp_flag) ||
> +		     test_bit(NLP_UNREG_INP, &ndlp->nlp_flag)) &&

[Severity: High]
Is there a race condition here with the unreg completion interrupt?

In lpfc_els_retry_delay_handler(), nlp_last_elscmd is temporarily cleared
before reaching this point. If this timer thread decides to skip issuing the
PLOGI because NLP_UNREG_INP is set:

CPU1 (lpfc_els_retry_delay_handler)
    cmd = ndlp->nlp_last_elscmd;
    ndlp->nlp_last_elscmd = 0;
    ...
    if (... test_bit(NLP_UNREG_INP, &ndlp->nlp_flag) ...) {

If CPU2 processes the unreg completion interrupt at this exact moment:

CPU2 (lpfc_sli4_unreg_rpi_cmpl_clr)
    if (!test_bit(NLP_DELAY_TMO, &ndlp->nlp_flag) &&
        ndlp->nlp_last_elscmd == ELS_CMD_PLOGI) {

CPU2 will see nlp_last_elscmd as 0 and skip issuing the PLOGI.

Then CPU1 resumes:

CPU1
    ndlp->nlp_last_elscmd = ELS_CMD_PLOGI;
    break;

Could this result in both paths skipping the PLOGI, permanently stalling
recovery?

> +		    ndlp->nlp_defer_did != NLP_EVT_NOTHING_PENDING &&
> +		    ((ndlp->nlp_DID & Fabric_DID_MASK) != Fabric_DID_MASK) &&
> +		    !test_bit(FC_OFFLINE_MODE, &vport->fc_flag)) {
> +			/* When UNREG_RPI completes we need to have the
> +			 * nlp_last_elscmd set.
> +			 */
> +			ndlp->nlp_last_elscmd = ELS_CMD_PLOGI;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=8

Re: [PATCH v3 04/14] lpfc: Check fc4_xpt_flags before decrementing ndlp kref on FDISC error

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing vulnerabilities in `lpfc_cmpl_els_flogi` and `lpfc_sli4_seq_abort_rsp` due to missing NVMe checks and non-atomic `NLP_DROPPED` flag operations.
--

--- Patch [4]: [PATCH v3 04/14] lpfc: Check fc4_xpt_flags before decrementing ndlp kref on FDISC error ---
commit 0f11d01d4dc626992ac78f74f8c706c25c1fb010
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Check fc4_xpt_flags before decrementing ndlp kref on FDISC error

This patch modifies FDISC error handling to ensure we check whether a node
has been registered with a transport layer before decrementing the initial
kref count. If previously registered, it delegates the kref drop to the
transport's respective dev_loss_tmo callback to avoid a potential
use-after-free.

> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 52fc5058976d9..b756de9986ebe 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
> @@ -11427,11 +11426,22 @@ lpfc_cmpl_els_fdisc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
>  			goto out;
>  		/* Warn FDISC status */
>  		lpfc_vlog_msg(vport, KERN_WARNING, LOG_ELS,
> -			      "0126 FDISC cmpl status: x%x/x%x)\n",
> -			      ulp_status, ulp_word4);
> +			      "0126 FDISC cmpl status: (x%x/x%x) ndlp x%px "
> +			      "Data: x%lx x%x x%x x%x x%x x%x x%x x%x x%x\n",
> +			      ulp_status, ulp_word4, ndlp, ndlp->nlp_flag,
> +			      ndlp->nlp_DID, ndlp->nlp_last_elscmd,
> +			      ndlp->nlp_type, ndlp->nlp_rpi, ndlp->nlp_state,
> +			      ndlp->nlp_prev_state, ndlp->fc4_xpt_flags,
> +			      kref_read(&ndlp->kref));
>  
> -		/* drop initial reference */
> -		if (!test_and_set_bit(NLP_DROPPED, &ndlp->nlp_flag))
> +		/* If have not previously registered with transport layer and no
> +		 * LPFC_EVT_DEV_LOSS work pending, then drop initial reference.
> +		 * Otherwise, let the dev_loss_tmo_callbk drop the initial
> +		 * reference.
> +		 */
> +		if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD)) &&
> +		    !test_bit(NLP_IN_DEV_LOSS, &ndlp->nlp_flag) &&
> +		    !test_and_set_bit(NLP_DROPPED, &ndlp->nlp_flag))
>  			lpfc_nlp_put(ndlp);

[Severity: High]
This is a pre-existing issue, but since this patch updates the logic here to
use an atomic test_and_set_bit() for the NLP_DROPPED flag and checks for both
SCSI and NVMe transport registrations, it highlights similar vulnerable code
blocks elsewhere.

For instance, in the same file, lpfc_cmpl_els_flogi() uses a non-atomic
check and set:

drivers/scsi/lpfc/lpfc_els.c:lpfc_cmpl_els_flogi() {
    ...
	if (!test_bit(NLP_IN_DEV_LOSS, &ndlp->nlp_flag) &&
	    !test_bit(NLP_DROPPED, &ndlp->nlp_flag) &&
	    !(ndlp->fc4_xpt_flags & SCSI_XPT_REGD)) {
		set_bit(NLP_DROPPED, &ndlp->nlp_flag);
		lpfc_nlp_put(ndlp);
	}
    ...
}

Can concurrent threads (like a link flap triggering lpfc_dev_loss_tmo_callbk)
evaluate test_bit(NLP_DROPPED, ...) as false and concurrently double-put the
ndlp kref here?

Also, does this FLOGI path miss the NVME_XPT_REGD check entirely, potentially
allowing premature kref drops for NVMe-only registrations?

A similar non-atomic pattern appears in sequence abort handling:

drivers/scsi/lpfc/lpfc_sli.c:lpfc_sli4_seq_abort_rsp() {
    ...
	if (ndlp->nlp_state == NLP_STE_UNUSED_NODE &&
	    !test_bit(NLP_DROPPED, &ndlp->nlp_flag) &&
	    !(ndlp->fc4_xpt_flags & (NVME_XPT_REGD | SCSI_XPT_REGD))) {
		set_bit(NLP_DROPPED, &ndlp->nlp_flag);
		lpfc_nlp_put(ndlp);
	}
    ...
}

Would it be worth updating these other paths to use the atomic
test_and_set_bit() and the unified transport checks as you did in this patch?

>  
>  		goto fdisc_failed;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=4

Re: [PATCH v3 03/14] lpfc: Fix kernel oops when unmapping scsi dma buffers for an aborted cmd

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch fails to fully resolve the dma_unmap_sg_attrs oops race because the validation logic skips counting the I/O during the critical window before scsi_done() finishes.
- [High] Unprotected concurrent modifications to the bitfield `cmd_flag` cause a Read-Modify-Write (RMW) data race, which can lead to the loss of the `LPFC_DRIVER_ABORTED` flag and completely break the synchronization fix.
--

--- Patch [3]: [PATCH v3 03/14] lpfc: Fix kernel oops when unmapping scsi dma buffers for an aborted cmd ---
commit 6bc9dc760f9e39b87bc318946c28662acdff4498
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Fix kernel oops when unmapping scsi dma buffers for an aborted cmd

This commit updates the inflight I/O check in lpfc_sli_sum_iocb() to count
I/Os that have either LPFC_IO_ON_TXCMPLQ or LPFC_DRIVER_ABORTED set. This
is intended to prevent a race with scsi_done() during aborts.

> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> index 62a30a92b7927..10db07771ccff 100644
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -12725,8 +12725,12 @@ lpfc_sli_sum_iocb(struct lpfc_vport *vport, uint16_t tgt_id, uint64_t lun_id,
>  
>  		if (!iocbq || iocbq->vport != vport)
>  			continue;
> -		if (!(iocbq->cmd_flag & LPFC_IO_FCP) ||
> -		    !(iocbq->cmd_flag & LPFC_IO_ON_TXCMPLQ))
> +		/* Only count FCP i/o */
> +		if (!(iocbq->cmd_flag & LPFC_IO_FCP))
> +			continue;
> +		/* Count i/o whilst LLDD retains an interest in the scsi_cmnd */
> +		if (!(iocbq->cmd_flag &
> +				(LPFC_IO_ON_TXCMPLQ | LPFC_DRIVER_ABORTED)))
>  			continue;

[Severity: High]
This isn't a bug introduced by this patch, but does this fully resolve the
dma_unmap_sg_attrs oops if the validation logic still skips the I/O when
pCmd is NULL?

In the completion paths, pCmd is set to NULL prior to calling scsi_done():

drivers/scsi/lpfc/lpfc_scsi.c:lpfc_scsi_cmd_iocb_cmpl() {
    ...
	lpfc_cmd->pCmd = NULL;
	spin_unlock(&lpfc_cmd->buf_lock);
    ...
	scsi_done(cmd);
}

When the error handling path calls lpfc_sli_sum_iocb(), it appears to rely
on lpfc_sli_validate_fcp_iocb() which excludes the I/O if pCmd is NULL:

drivers/scsi/lpfc/lpfc_sli.c:lpfc_sli_validate_fcp_iocb() {
    ...
	if (lpfc_cmd->pCmd == NULL)
		return rc;
    ...
}

Could this cause the I/O to be ignored during the critical window before
scsi_done() finishes, allowing the counter to reach zero prematurely?

[Severity: High]
This is also a pre-existing issue, but can the LPFC_DRIVER_ABORTED flag be
lost due to a read-modify-write data race on cmd_flag?

The abort path sets LPFC_DRIVER_ABORTED non-atomically under hbalock, but
explicitly drops ring_lock:

drivers/scsi/lpfc/lpfc_scsi.c:lpfc_abort_handler() {
    ...
		spin_unlock(&pring_s4->ring_lock);
		ret_val = lpfc_sli4_issue_abort_iotag(phba, iocb,
						      lpfc_sli_abort_fcp_cmpl);
    ...
}

drivers/scsi/lpfc/lpfc_sli.c:lpfc_sli4_issue_abort_iotag() {
    ...
	cmdiocb->cmd_flag |= LPFC_DRIVER_ABORTED;
    ...
}

Meanwhile, the completion path clears LPFC_IO_ON_TXCMPLQ non-atomically
under ring_lock but without hbalock:

drivers/scsi/lpfc/lpfc_sli.c:lpfc_sli_iocbq_lookup_by_tag() {
    ...
			cmd_iocb->cmd_flag &= ~LPFC_IO_ON_TXCMPLQ;
    ...
}

If an I/O completes exactly as the error handler issues an abort, could one
update overwrite the other, causing LPFC_DRIVER_ABORTED to be unset here
and breaking the synchronization fix?

>  
>  		/* Include counting outstanding aborts */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=3

Re: [PATCH v3 07/14] lpfc: Rework I/O flush ordering when unloading driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unsafe lockless modification of `txcmplq` via `list_del_init(&iocb->list)` leads to list corruption.
- [High] Use-After-Free on `lpfc_queue` structures caused by failing to disable hardware interrupts before destroying the queues.
--

--- Patch [7]: [PATCH v3 07/14] lpfc: Rework I/O flush ordering when unloading driver ---
commit c00c06170572ea9d93301e5572a551c25be7b877
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Rework I/O flush ordering when unloading driver

This patch captures the IOCB_ABORTING return value in lpfc_els_abort to
prevent adding it to the list of iocbs for cancelling, allowing it to
complete naturally. It also reorders lpfc_free_iocb_list and adds a call
to flush_workqueue in lpfc_pci_remove_one_s4 to flush I/O before freeing.

> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index 23355f12fbffa..8682f5a445d29 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
[ ... ]
> @@ -14948,11 +14951,20 @@ lpfc_pci_remove_one_s4(struct pci_dev *pdev)
>  
>  	/* Perform scsi free before driver resource_unset since scsi
>  	 * buffers are released to their corresponding pools here.
> +	 * lpfc_sli4_hba_unset() issues aborts via lpfc_sli_hba_iocb_abort(),
> +	 * which allocates abort IOCBs from phba->lpfc_iocb_list; the pool
> +	 * must still exist, so lpfc_free_iocb_list() runs only after unset.
>  	 */
>  	lpfc_io_free(phba);
> -	lpfc_free_iocb_list(phba);
> -	lpfc_sli4_hba_unset(phba);
>  
> +	/* Flush the PHBA WQ - there could be a race with ELS IOs while lpfc
> +	 * is unloading.  This stops a race between completions, aborts and
> +	 * resource recovery.
> +	 */
> +	if (phba->wq)
> +		flush_workqueue(phba->wq);
> +
> +	lpfc_sli4_hba_unset(phba);

[Severity: High]
This is a pre-existing issue, but does flushing the workqueue here before
calling lpfc_sli4_hba_unset() leave a window where new work items could be
queued?

In lpfc_pci_remove_one_s4(), the call to lpfc_sli4_hba_unset() will issue
aborts via lpfc_sli_hba_iocb_abort(). Because interrupts are still enabled at
this point, hardware completions from these aborts could trigger interrupts
that add new work items to phba->wq.

Later in lpfc_sli4_hba_unset(), interrupts are disabled and
lpfc_sli4_queue_destroy() frees the queue memory. However, since
lpfc_unset_driver_resource_phase2() eventually calls destroy_workqueue(), it
will drain the workqueue and execute those pending work items on the
already freed queue objects. Could this result in a use-after-free when the
driver is unloaded while IO is in flight?

>  	lpfc_unset_driver_resource_phase2(phba);
>  	lpfc_sli4_driver_resource_unset(phba);
>  
> diff --git a/drivers/scsi/lpfc/lpfc_nportdisc.c b/drivers/scsi/lpfc/lpfc_nportdisc.c
> index 2c8d995a45bf8..f917a5bcfd025 100644
> --- a/drivers/scsi/lpfc/lpfc_nportdisc.c
> +++ b/drivers/scsi/lpfc/lpfc_nportdisc.c
[ ... ]
> @@ -271,7 +272,13 @@ lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
>  		retval = lpfc_sli_issue_abort_iotag(phba, pring, iocb, NULL);
>  		spin_unlock_irq(&phba->hbalock);
>  
> -		if (retval && test_bit(FC_UNLOADING, &phba->pport->load_flag)) {
> +		/* An abort that fails here is just cancelled when the driver is
> +		 * going offline.  However, if the abort failure is because the
> +		 * IOCB is already getting aborted, don't cancel.  Just let it
> +		 * complete.
> +		 */
> +		if (test_bit(FC_UNLOADING, &phba->pport->load_flag) &&
> +		    retval && retval != IOCB_ABORTING) {
>  			list_del_init(&iocb->list);
>  			list_add_tail(&iocb->list, &drv_cmpl_list);
>  		}

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to modify iocb->list
here without holding the ring lock?

In lpfc_els_abort(), the loop operates on abort_list using iocb->dlist. The
original iocb->list nodes remain on the active pring->txcmplq. After releasing
phba->hbalock, if lpfc_sli_issue_abort_iotag() fails while unloading,
list_del_init(&iocb->list) is called on txcmplq without acquiring any locks.

Could concurrent hardware completion handlers, such as
lpfc_sli_process_cmp_ring() traversing txcmplq under ring_lock or hbalock,
encounter corrupted list pointers if this list modification races with them?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=7

Re: [PATCH v3 13/14] lpfc: Refactor calls on fc_disctmo to lpfc_set_disctmo in RSCN handler

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing memory barrier in open-coded lock causes NULL pointer dereference.
--

--- Patch [13]: [PATCH v3 13/14] lpfc: Refactor calls on fc_disctmo to lpfc_set_disctmo in RSCN handler ---
commit 1b8710fdf36e956919fff9a52e4a58021de35371
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Refactor calls on fc_disctmo to lpfc_set_disctmo in RSCN handler

This commit refactors discovery timeout timer handling in the RSCN handler
to use lpfc_set_disctmo() instead of modifying the timer directly. This
ensures discovery timer actions are properly logged and visible.

> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 0a7e69dcb2dbe..0c5524c613e57 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c

[ ... ]

> @@ -8497,11 +8494,9 @@ lpfc_els_rcv_rscn(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
>  		set_bit(FC_RSCN_DEFERRED, &vport->fc_flag);
>  
>  		/* Restart disctmo if its already running */
> -		if (test_bit(FC_DISC_TMO, &vport->fc_flag)) {
> -			tmo = ((phba->fc_ratov * 3) + 3);
> -			mod_timer(&vport->fc_disctmo,
> -				  jiffies + secs_to_jiffies(tmo));
> -		}
> +		if (test_bit(FC_DISC_TMO, &vport->fc_flag))
> +			lpfc_set_disctmo(vport);
> +
>  		if ((rscn_cnt < FC_MAX_HOLD_RSCN) &&
>  		    !test_bit(FC_RSCN_DISCOVERY, &vport->fc_flag)) {
>  			set_bit(FC_RSCN_MODE, &vport->fc_flag);

[Severity: High]
This is a pre-existing issue, but could the lockless updates to
fc_rscn_id_cnt and fc_rscn_flush result in a null pointer dereference?

In lpfc_els_rcv_rscn(), new rscn events are added to the list and the flush
flag is cleared without memory barriers. This happens in both the defer path:

vport->fc_rscn_id_list[rscn_cnt] = pcmd;
vport->fc_rscn_id_cnt++;
...
vport->fc_rscn_flush = 0;

And at the end of the function:

vport->fc_rscn_id_list[vport->fc_rscn_id_cnt++] = pcmd;
...
vport->fc_rscn_flush = 0;

If the compiler or cpu reorders these stores, the updated fc_rscn_id_cnt and
fc_rscn_flush = 0 could become visible to other threads before the new
pointer is stored in fc_rscn_id_list.

A concurrent thread running lpfc_rscn_payload_check() might then see the
incremented count and cleared flush flag, but read a stale null pointer from
the array:

for (i = 0; i < vport->fc_rscn_id_cnt; i++) {
        lp = vport->fc_rscn_id_list[i]->virt;

Would it be safer to use smp_store_release() when clearing fc_rscn_flush,
or add an smp_wmb() before updating the flag?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=13

Re: [PATCH v3 05/14] lpfc: Add handling for when PLOGI or PRLI is dropped during link failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The vport-wide counter fc_prli_sent is modified under a node-specific spinlock (ndlp->lock), leading to data races during concurrent PRLI operations.
- [High] The patch only cleans up state flags for PLOGI and PRLI in the link down completion handler, completely missing identical cleanup requirements for LOGO and ADISC, which causes the session to remain irrecoverably stuck.
--

--- Patch [5]: [PATCH v3 05/14] lpfc: Add handling for when PLOGI or PRLI is dropped during link failure ---
commit ef308ec0ae24b6ee74db89f761bac77e41718c55
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Add handling for when PLOGI or PRLI is dropped during link failure

This commit modifies the link down completion handler to clear the 
NLP_PLOGI_SND and NLP_PRLI_SND flags and decrement the fc_prli_sent 
counters. This ensures that normal cleanup actions are not missed when 
PLOGI or PRLI commands are aborted due to a link failure. The fix 
helps prevent recovery failures for login sessions.

> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c

[ ... ]

> @@ -1237,17 +1239,40 @@ lpfc_cmpl_els_link_down(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
>  	ulp_status = get_job_ulpstatus(phba, rspiocb);
>  	ulp_word4 = get_job_word4(phba, rspiocb);
>  

[ ... ]

> +	if (cmd == ELS_CMD_PLOGI) {
> +		/* A PLOGI ELS IO needs to clear the PLOGI_SND flag to
> +		 * acknowledge the ELS completion and allow recovery. Otherwise
> +		 * a subsequent PLOGI gets rejected as a duplicate.
> +		 */
> +		clear_bit(NLP_PLOGI_SND, &ndlp->nlp_flag);
> +	} else if (cmd == ELS_CMD_PRLI || cmd == ELS_CMD_NVMEPRLI) {

[Severity: High]
This isn't a bug introduced by this patch, but does this handler also need to
clean up state flags for LOGO and ADISC?

If a LOGO or ADISC ELS command is aborted by a link down event and routed to
lpfc_cmpl_els_link_down(), it appears the NLP_LOGO_SND and NLP_ADISC_SND
flags are not cleared.

Could this permanently block subsequent connection or management commands to
this node? For example, would lpfc_rcv_plogi() silently ignore incoming PLOGIs 
if NLP_LOGO_SND remains set?

> +		/* A PRLI ELS IO needs to decrement the fc4_prli_sent count
> +		 * added by the lpfc_issue_els_prli function.  A nonzero count
> +		 * stops transport registrations.
> +		 */
> +		clear_bit(NLP_PRLI_SND, &ndlp->nlp_flag);
> +		spin_lock_irq(&ndlp->lock);
> +		vport->fc_prli_sent--;
> +		ndlp->fc4_prli_sent--;
> +		spin_unlock_irq(&ndlp->lock);

[Severity: High]
This is a pre-existing issue, but can modifying the shared vport-wide counter
vport->fc_prli_sent under a localized node-specific lock (ndlp->lock) lead to
data races?

If concurrent ELS PRLI completions or issues on different nodes within the same
virtual port occur, each thread might acquire its own localized ndlp->lock and
perform a read-modify-write on vport->fc_prli_sent without mutual exclusion.

Could this lost update leave the counter permanently non-zero, causing
lpfc_scan_finished() to hang indefinitely and block SCSI device discovery?

>  	}
>  
>  	if (cmdiocb->cmd_flag & LPFC_IO_FABRIC) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=5

Re: [PATCH v3 12/14] lpfc: Update ELS ACC logging for diagnostic troubleshooting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Reordering lpfc_els_rsp_acc() introduces a Use-After-Free window for the RSCN payload buffer.

Pre-existing issues:
- [Critical] Integer overflow in `lpfc_els_rsp_rpl_acc` leads to a massive heap buffer overflow.
--

--- Patch [12]: [PATCH v3 12/14] lpfc: Update ELS ACC logging for diagnostic troubleshooting ---
commit 8be36e657b7a29a5634f38457734d0cc4e29c5eb
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Update ELS ACC logging for diagnostic troubleshooting

This commit adds debug log messages to ELS ACC routines in the lpfc driver
to indicate when ACC frame transmission fails. It also updates the routines
to return distinct error codes for different transmission issues and updates
the corresponding function comment headers.

> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 73552ce4bc64b..0a7e69dcb2dbe 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c

[ ... ]

> @@ -6039,6 +6049,17 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
>  			 ndlp->nlp_DID, ndlp->nlp_flag, ndlp->nlp_state,
>  			 ndlp->nlp_rpi, vport->fc_flag, kref_read(&ndlp->kref));
>  	return 0;
> +
> +err_out:
> +	if (oldiocb->cmd_dmabuf && oldiocb->cmd_dmabuf->virt)
> +		old_opcode = *(uint32_t *)oldiocb->cmd_dmabuf->virt;
> +
> +	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
> +			 "1027 Xmit ELS ACC Unsuccessful: "
> +			 "cmd: x%x, error_code: %d "
> +			 "S_ID: x%x\n", old_opcode, err,
> +			 vport->fc_myDID);
> +	return err;
>  }

[ ... ]

> @@ -8473,14 +8554,14 @@ lpfc_els_rcv_rscn(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
>  	vport->fc_rscn_id_list[vport->fc_rscn_id_cnt++] = pcmd;
>  	/* Indicate we are done walking fc_rscn_id_list on this vport */
>  	vport->fc_rscn_flush = 0;
> +	/* Send back ACC */
> +	lpfc_els_rsp_acc(vport, ELS_CMD_ACC, cmdiocb, ndlp, NULL);

[Severity: High]
Does this reordering introduce a race condition with concurrent freeing?

By moving lpfc_els_rsp_acc() here, it now executes after vport->fc_rscn_flush
is set to 0, but before cmdiocb->cmd_dmabuf is set to NULL.

If a concurrent thread observes fc_rscn_flush == 0, it can process the list
and free the buffer.

If lpfc_els_rsp_acc() subsequently fails, the newly added err_out path shown
above would dereference the freed oldiocb->cmd_dmabuf->virt, potentially
causing a use-after-free.

Should cmdiocb->cmd_dmabuf be set to NULL before calling lpfc_els_rsp_acc(),
or should the flush flag be cleared after?

>  	/*
>  	 * If we zero, cmdiocb->cmd_dmabuf, the calling routine will
>  	 * not try to free it.
>  	 */
>  	cmdiocb->cmd_dmabuf = NULL;
>  	lpfc_set_disctmo(vport);
> -	/* Send back ACC */
> -	lpfc_els_rsp_acc(vport, ELS_CMD_ACC, cmdiocb, ndlp, NULL);
>  	/* send RECOVERY event for ALL nodes that match RSCN payload */
>  	lpfc_rscn_recovery_check(vport);
>  	return lpfc_els_handle_rscn(vport);

[ ... ]

> @@ -9280,7 +9361,9 @@ lpfc_send_rrq(struct lpfc_hba *phba, struct lpfc_node_rrq *rrq)
>   *
>   * Return code
>   *   0 - Successfully issued ACC RPL ELS command
> - *   1 - Failed to issue ACC RPL ELS command
> + *   -ENOMEM - IOCB not prepped successfully
> + *   -EIO - The IOCB failed to issue successfully
> + *   -ENODEV - No associated node for IOCB
>   **/
>  static int
>  lpfc_els_rsp_rpl_acc(struct lpfc_vport *vport, uint16_t cmdsize,

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential integer overflow
leading to a massive heap buffer overflow in this function.

The cmdsize parameter is a uint16_t. When called from lpfc_els_rcv_rpl(),
cmdsize is calculated using an unvalidated maxsize from the remote request:

lpfc_els_rcv_rpl() {
    ...
    cmdsize = sizeof(uint32_t) + maxsize * sizeof(uint32_t);
    ...
    lpfc_els_rsp_rpl_acc(vport, cmdsize, cmdiocb, ndlp);
}

If an attacker sends an RPL request with maxsize = 16383, cmdsize evaluates
to 65536, which truncates to 0 in the uint16_t argument.

Later in lpfc_els_rsp_rpl_acc(), the following memcpy is executed:

    memcpy(pcmd, &rpl_rsp, cmdsize - sizeof(uint32_t));

Since cmdsize is 0, the subtraction underflows, resulting in an enormous
copy size. Could maxsize be validated in lpfc_els_rcv_rpl() to prevent this?

> @@ -9294,12 +9377,15 @@ lpfc_els_rsp_rpl_acc(struct lpfc_vport *vport, uint16_t cmdsize,
>  	struct lpfc_iocbq *elsiocb;
>  	uint8_t *pcmd;
>  	u32 ulp_context;
> +	int err;
>  
>  	elsiocb = lpfc_prep_els_iocb(vport, 0, cmdsize, oldiocb->retry, ndlp,
>  				     ndlp->nlp_DID, ELS_CMD_ACC);
>  
> -	if (!elsiocb)
> -		return 1;
> +	if (!elsiocb) {
> +		err = -ENOMEM;
> +		goto err_out;
> +	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=12

Re: [PATCH v3 09/14] lpfc: Send inhibited ABORT_WQE when PLOGI CQE SEQUENCE_TMO is received

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Issuing an unconditional abort on an already-completed ELS command introduces a tag-reuse race condition that can inadvertently abort a newly allocated command.

Pre-existing issues:
- [High] Deadlock in `lpfc_sli_abort_iocb_ring` due to non-recursive double acquisition of `pring->ring_lock`.
- [High] Data race and list corruption from unlocked iteration of `pring->txcmplq` in `lpfc_sli_host_down`.
--

--- Patch [9]: [PATCH v3 09/14] lpfc: Send inhibited ABORT_WQE when PLOGI CQE SEQUENCE_TMO is received ---
commit faea8f503d6a5b239d2ea58d59fefef913f41aeb
Author: Justin Tee <justintee8345@gmail.com>

lpfc: Send inhibited ABORT_WQE when PLOGI CQE SEQUENCE_TMO is received

This commit updates the driver to send an inhibited ABORT_WQE when a PLOGI
CQE SEQUENCE_TIMEOUT is received. This allows the aborted CQE to return
immediately instead of waiting for a BA_ACC.

> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 16b10377a0953..11c36968a2bed 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
[ ... ]
> @@ -4893,7 +4897,38 @@ lpfc_els_retry(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
>  				/* Reset the Link */
>  				link_reset = 1;
>  				break;
> +			} else if (cmd == ELS_CMD_PLOGI) {
> +
> +				/* if invalid ndlp, do not retry */
> +				if (unlikely(!ndlp)) {
> +					retry = 0;
> +					break;
> +				}
> +
> +				lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
> +						 "0159 PLOGI Sequence TMO for "
> +						 "ndlp x%px x%lx x%x x%x x%x "
> +						 "x%x x%x %u x%x\n",
> +						 ndlp, ndlp->nlp_flag,
> +						 ndlp->nlp_DID, ndlp->nlp_type,
> +						 ndlp->nlp_fc4_type,
> +						 ndlp->nlp_rpi, ndlp->nlp_state,
> +						 kref_read(&ndlp->kref),
> +						 ndlp->fc4_xpt_flags);
> +
> +				/* Abort all outstanding ELS and auto-ABTS.  If
> +				 * no response for E_D_TOV, then it is unlikely
> +				 * auto-ABTS would receive a response too.  So,
> +				 * inhibit the abort for faster XRI release.
> +				 * However, still proceed with delayed retry.
> +				 */
> +				pring = lpfc_phba_elsring(phba);
> +				spin_lock_irqsave(&phba->hbalock, iflags);
> +				lpfc_sli_issue_abort_iotag(phba, pring, cmdiocb,
> +							   true, NULL);

[Severity: High]
Could this unconditional abort introduce a tag reuse race condition?

When a PLOGI command fails with a sequence timeout, this new logic issues
an asynchronous abort and then returns 1 to retry.

Because lpfc_els_retry() returns 1, the caller lpfc_cmpl_els_plogi()
immediately jumps to out_freeiocb and calls lpfc_els_free_iocb(), freeing
the command.

The command is returned to the free list since the exchange busy flag is
only set on the response, not the command itself. Its tag then becomes
available for reallocation. Since the abort is processed asynchronously by
the HBA, could the firmware erroneously abort a newly allocated ELS command
if the tag is reused quickly?

> +				spin_unlock_irqrestore(&phba->hbalock, iflags);
>  			}
> +
>  			retry = 1;
>  			delay = 100;
>  			break;
[ ... ]
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> index 54bc5d8984fc2..0a300efc92401 100644
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -4608,7 +4608,8 @@ lpfc_sli_abort_iocb_ring(struct lpfc_hba *phba, struct lpfc_sli_ring *pring)
>  	} else {
>  		/* Issue ABTS for everything on the txcmplq */
>  		list_for_each_entry_safe(iocb, next_iocb, &pring->txcmplq, list)
> -			lpfc_sli_issue_abort_iotag(phba, pring, iocb, NULL);
> +			lpfc_sli_issue_abort_iotag(phba, pring, iocb, false,
> +						   NULL);

[Severity: High]
This is a pre-existing issue, but does this lead to a deadlock on the ring
lock?

In lpfc_sli_abort_iocb_ring(), the ring lock is acquired before this loop:

spin_lock_irq(plock);
list_for_each_entry_safe(iocb, next_iocb, &pring->txcmplq, list)

Inside the loop, lpfc_sli_issue_abort_iotag() is called, which then
evaluates the ring and unconditionally acquires the exact same lock for
SLI-4:

spin_lock_irqsave(&pring->ring_lock, iflags);

Since these spinlocks are not recursive, won't this cause an immediate
deadlock?

>  	}
>  	spin_unlock_irq(plock);
>  
[ ... ]
> @@ -11998,7 +11999,7 @@ lpfc_sli_host_down(struct lpfc_vport *vport)
>  				if (iocb->vport != vport)
>  					continue;
>  				lpfc_sli_issue_abort_iotag(phba, pring, iocb,
> -							   NULL);
> +							   false, NULL);

[Severity: High]
This is a pre-existing issue, but does this unlocked traversal of the
completion queue invite data races or list corruption?

In lpfc_sli_host_down(), the code explicitly drops the ring lock for SLI-4
just before this loop:

spin_unlock(&pring->ring_lock);
list_for_each_entry_safe(iocb, next_iocb, &pring->txcmplq, list) {
    ...
}

However, the completion queue is actively modified by concurrent completion
handlers like lpfc_sli_sp_handle_rspiocb() which rely on the ring lock for
synchronization. Is it safe to iterate this list while it could be
modified concurrently?

>  			}
>  			pring->flag = prev_pring_flag;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714011812.106753-1-justintee8345@gmail.com?part=9

[PATCH v3 00/14] Update lpfc to revision 15.0.0.1

[Justin Tee]

Update lpfc to revision 15.0.0.1

This patch set contains bug fixes related to cleanup handling in both
normal and error paths, discovery rework for large SAN configurations, and
refactoring of duplicate code.

The patches were cut against Martin's 7.3/scsi-queue tree.

Justin Tee (14):
  lpfc: Fix use-after-free in lpfc_cmpl_ct_cmd_vmid
  lpfc: Early return out of lpfc_els_abort when HBA_SETUP flag is not
    set
  lpfc: Fix kernel oops when unmapping scsi dma buffers for an aborted
    cmd
  lpfc: Check fc4_xpt_flags before decrementing ndlp kref on FDISC error
  lpfc: Add handling for when PLOGI or PRLI is dropped during link
    failure
  lpfc: Fix ndlp use-after-free during repeated RSCN and rediscovery
    sequence
  lpfc: Rework I/O flush ordering when unloading driver
  lpfc: Improve PLOGI retry handling for large SAN configurations
  lpfc: Send inhibited ABORT_WQE when PLOGI CQE SEQUENCE_TMO is received
  lpfc: Remove slowpath cqe process limiter in slow ring event handler
  lpfc: Put iocbq on phba->txq when ELS WQ is full or ELS SGL
    unavailable
  lpfc: Update ELS ACC logging for diagnostic troubleshooting
  lpfc: Refactor calls on fc_disctmo to lpfc_set_disctmo in RSCN handler
  lpfc: Update lpfc version to 15.0.0.1

 drivers/scsi/lpfc/lpfc_bsg.c       |   5 +-
 drivers/scsi/lpfc/lpfc_crtn.h      |  12 +-
 drivers/scsi/lpfc/lpfc_ct.c        |  19 +-
 drivers/scsi/lpfc/lpfc_disc.h      |   2 +-
 drivers/scsi/lpfc/lpfc_els.c       | 427 +++++++++++++++++++++++------
 drivers/scsi/lpfc/lpfc_hbadisc.c   | 100 +++----
 drivers/scsi/lpfc/lpfc_init.c      |  16 +-
 drivers/scsi/lpfc/lpfc_nportdisc.c |  86 +++++-
 drivers/scsi/lpfc/lpfc_nvme.c      |   2 +-
 drivers/scsi/lpfc/lpfc_scsi.c      |   2 +-
 drivers/scsi/lpfc/lpfc_sli.c       | 245 ++++++++++++-----
 drivers/scsi/lpfc/lpfc_sli.h       |   4 +-
 drivers/scsi/lpfc/lpfc_version.h   |   2 +-
 13 files changed, 706 insertions(+), 216 deletions(-)

-- 
2.38.0

[PATCH v3 01/14] lpfc: Fix use-after-free in lpfc_cmpl_ct_cmd_vmid

[Justin Tee]

In lpfc_cmpl_ct_cmd_vmid, there is an early call to lpfc_ct_free_iocb when
cmd is SLI_CTAS_DALLAPP_ID.  Within lpfc_ct_free_iocb the
cmdiocb->rsp_dmabuf will be freed.  This means any ctrsp ptr dereference
for SLI_CT_RESPONSE_FS_RJT or even ctrsp->ReasonCode and ctrsp->Explanation
when handling a CT LS_RJT response is a use-after-free.

Remove the early lpfc_ct_free_iocb call for SLI_CTAS_DALLAPP_ID.  There
already is a free_res label that calls lpfc_ct_free_iocb so there doesn't
need to be an early lpfc_ct_free_iocb at the start of
lpfc_cmpl_ct_cmd_vmid.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_ct.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
index c7853e7fe071..e14170550e69 100644
--- a/drivers/scsi/lpfc/lpfc_ct.c
+++ b/drivers/scsi/lpfc/lpfc_ct.c
@@ -3595,8 +3595,6 @@ lpfc_cmpl_ct_cmd_vmid(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 	int i;
 
 	cmd = be16_to_cpu(ctcmd->CommandResponse.bits.CmdRsp);
-	if (cmd == SLI_CTAS_DALLAPP_ID)
-		lpfc_ct_free_iocb(phba, cmdiocb);
 
 	if (lpfc_els_chk_latt(vport) || get_job_ulpstatus(phba, rspiocb)) {
 		if (cmd != SLI_CTAS_DALLAPP_ID)
-- 
2.38.0

[PATCH v3 02/14] lpfc: Early return out of lpfc_els_abort when HBA_SETUP flag is not set

[Justin Tee]

It is possible that a dev_loss_tmo callback fires during an hba reset.
The ELS pring structure is cleared by the hba reset path and the
dev_loss_tmo callback executing lpfc_els_abort could be using a stale ELS
pring pointer.  To prevent such a condition, check if HBA_SETUP flag is set
before proceeding to use the ELS pring pointer in lpfc_els_abort. There is
no point to issue aborts when the sli port is not setup anyways.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_nportdisc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_nportdisc.c b/drivers/scsi/lpfc/lpfc_nportdisc.c
index 9c449055a55e..2c8d995a45bf 100644
--- a/drivers/scsi/lpfc/lpfc_nportdisc.c
+++ b/drivers/scsi/lpfc/lpfc_nportdisc.c
@@ -227,6 +227,11 @@ lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
 	struct lpfc_iocbq *iocb, *next_iocb;
 	int retval = 0;
 
+	/* Exit early to prevent race with queue teardown. */
+	if (unlikely(phba->sli_rev == LPFC_SLI_REV4 &&
+		     !test_bit(HBA_SETUP, &phba->hba_flag)))
+		return;
+
 	pring = lpfc_phba_elsring(phba);
 
 	/* In case of error recovery path, we might have a NULL pring here */
-- 
2.38.0

[PATCH v3 03/14] lpfc: Fix kernel oops when unmapping scsi dma buffers for an aborted cmd

[Justin Tee]

A kernel oops at dma_unmap_sg_attrs may occur due to a race between an
aborted scsi I/O completion and a scsi error handler issued TUR using the
same repurposed scsi_cmnd structure.

The LPFC_DRIVER_ABORTED cmd_flag is set when inflight I/Os are aborted
via lpfc_sli_abort_taskmgmt, and this flag is not cleared until after
scsi_done is called.  The inflight I/O test is changed to check scsi I/O
for either LFPC_IO_ON_TXCMPLQ or LPFC_DRIVER_ABORTED.  If either cmd_flag
is set, then the I/O should still be counted.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_sli.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 62a30a92b792..10db07771ccf 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -12725,8 +12725,12 @@ lpfc_sli_sum_iocb(struct lpfc_vport *vport, uint16_t tgt_id, uint64_t lun_id,
 
 		if (!iocbq || iocbq->vport != vport)
 			continue;
-		if (!(iocbq->cmd_flag & LPFC_IO_FCP) ||
-		    !(iocbq->cmd_flag & LPFC_IO_ON_TXCMPLQ))
+		/* Only count FCP i/o */
+		if (!(iocbq->cmd_flag & LPFC_IO_FCP))
+			continue;
+		/* Count i/o whilst LLDD retains an interest in the scsi_cmnd */
+		if (!(iocbq->cmd_flag &
+				(LPFC_IO_ON_TXCMPLQ | LPFC_DRIVER_ABORTED)))
 			continue;
 
 		/* Include counting outstanding aborts */
-- 
2.38.0

[PATCH v3 04/14] lpfc: Check fc4_xpt_flags before decrementing ndlp kref on FDISC error

[Justin Tee]

The current initial kref count drop logic for an ndlp that fails FDISC
assumes that the ndlp has never registered with transport layer and thus
the lpfc_dev_loss_tmo_callbk never called.  However, a failed FDISC can
occur after a successful transport layer registration too.  So,
lpfc_dev_loss_tmo_callbk can occur and there is a potential use-after-free
on the ndlp.

Check ndlp->fc4_xpt_flags if previously registered with an upper layer
transport and check ndlp->nlp_flags if there is a LPFC_EVT_DEV_LOSS work
pending.  If not previously registered nor LPFC_EVT_DEV_LOSS work pending,
then set the NLP_DROPPED flag as before and decrement the initial kref on
FDISC error.  However, if ndlp has been previously registered, then let the
pre-existing logic for each transport's respective dev_loss_tmo_callbk
perform the initial kref decrement.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_els.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 52fc5058976d..b756de9986eb 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -11416,7 +11416,6 @@ lpfc_cmpl_els_fdisc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 		ulp_status, ulp_word4, vport->fc_prevDID);
 
 	if (ulp_status) {
-
 		if (lpfc_fabric_login_reqd(phba, cmdiocb, rspiocb)) {
 			lpfc_retry_pport_discovery(phba);
 			goto out;
@@ -11427,11 +11426,22 @@ lpfc_cmpl_els_fdisc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 			goto out;
 		/* Warn FDISC status */
 		lpfc_vlog_msg(vport, KERN_WARNING, LOG_ELS,
-			      "0126 FDISC cmpl status: x%x/x%x)\n",
-			      ulp_status, ulp_word4);
+			      "0126 FDISC cmpl status: (x%x/x%x) ndlp x%px "
+			      "Data: x%lx x%x x%x x%x x%x x%x x%x x%x x%x\n",
+			      ulp_status, ulp_word4, ndlp, ndlp->nlp_flag,
+			      ndlp->nlp_DID, ndlp->nlp_last_elscmd,
+			      ndlp->nlp_type, ndlp->nlp_rpi, ndlp->nlp_state,
+			      ndlp->nlp_prev_state, ndlp->fc4_xpt_flags,
+			      kref_read(&ndlp->kref));
 
-		/* drop initial reference */
-		if (!test_and_set_bit(NLP_DROPPED, &ndlp->nlp_flag))
+		/* If have not previously registered with transport layer and no
+		 * LPFC_EVT_DEV_LOSS work pending, then drop initial reference.
+		 * Otherwise, let the dev_loss_tmo_callbk drop the initial
+		 * reference.
+		 */
+		if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD)) &&
+		    !test_bit(NLP_IN_DEV_LOSS, &ndlp->nlp_flag) &&
+		    !test_and_set_bit(NLP_DROPPED, &ndlp->nlp_flag))
 			lpfc_nlp_put(ndlp);
 
 		goto fdisc_failed;
-- 
2.38.0

[PATCH v3 05/14] lpfc: Add handling for when PLOGI or PRLI is dropped during link failure

[Justin Tee]

PLOGI and PRLI typically complete via the lpfc_cmpl_els_plogi and
lpfc_cmpl_els_prli handler respectively, but when the link drops they
complete via the lpfc_cmpl_els_link_down handler.  When this occurs, normal
cleanup completion actions are missed and we may fail to recover the login
session due to ndlp logistical mixups.  Fix by clearing the NLP_PLOGI_SND
or NLP_PRLI_SND flag and decrement the outstanding prli_sent counters in
the lpfc_cmpl_els_link_down handler.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_els.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index b756de9986eb..6720fd5ec523 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -1230,6 +1230,8 @@ lpfc_cmpl_els_link_down(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 	uint32_t *pcmd;
 	uint32_t cmd;
 	u32 ulp_status, ulp_word4;
+	struct lpfc_vport *vport = cmdiocb->vport;
+	struct lpfc_nodelist *ndlp = cmdiocb->ndlp;
 
 	pcmd = (uint32_t *)cmdiocb->cmd_dmabuf->virt;
 	cmd = *pcmd;
@@ -1237,17 +1239,40 @@ lpfc_cmpl_els_link_down(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 	ulp_status = get_job_ulpstatus(phba, rspiocb);
 	ulp_word4 = get_job_word4(phba, rspiocb);
 
-	lpfc_printf_log(phba, KERN_INFO, LOG_ELS,
-			"6445 ELS completes after LINK_DOWN: "
-			" Status %x/%x cmd x%x flg x%x iotag x%x\n",
-			ulp_status, ulp_word4, cmd,
-			cmdiocb->cmd_flag, cmdiocb->iotag);
+	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
+			 "6445 ELS completes after LINK_DOWN: "
+			 "Status %x/%x cmd x%x data: x%x x%x x%x x%px x%px\n",
+			 ulp_status, ulp_word4, cmd,
+			 cmdiocb->cmd_flag, cmdiocb->iotag,
+			 ndlp->nlp_state, vport, ndlp);
+
+	if (cmd == ELS_CMD_PLOGI) {
+		/* A PLOGI ELS IO needs to clear the PLOGI_SND flag to
+		 * acknowledge the ELS completion and allow recovery. Otherwise
+		 * a subsequent PLOGI gets rejected as a duplicate.
+		 */
+		clear_bit(NLP_PLOGI_SND, &ndlp->nlp_flag);
+	} else if (cmd == ELS_CMD_PRLI || cmd == ELS_CMD_NVMEPRLI) {
+		/* A PRLI ELS IO needs to decrement the fc4_prli_sent count
+		 * added by the lpfc_issue_els_prli function.  A nonzero count
+		 * stops transport registrations.
+		 */
+		clear_bit(NLP_PRLI_SND, &ndlp->nlp_flag);
+		spin_lock_irq(&ndlp->lock);
+		vport->fc_prli_sent--;
+		ndlp->fc4_prli_sent--;
+		spin_unlock_irq(&ndlp->lock);
+	}
 
 	if (cmdiocb->cmd_flag & LPFC_IO_FABRIC) {
 		cmdiocb->cmd_flag &= ~LPFC_IO_FABRIC;
 		atomic_dec(&phba->fabric_iocb_count);
 	}
+
 	lpfc_els_free_iocb(phba, cmdiocb);
+
+	/* lpfc took a reference in the issue.  Release it now. */
+	lpfc_nlp_put(ndlp);
 }
 
 /**
-- 
2.38.0

[PATCH v3 06/14] lpfc: Fix ndlp use-after-free during repeated RSCN and rediscovery sequence

[Justin Tee]

In large SAN configurations when a target port fails over, RSCNs may be
spammed triggering a repeat of restarting discovery events for an ndlp
object.

In the case when discovery reaches PRLI state, but the PRLI operation is
interrupted, this leaves the nlp_fc4_type and nlp_type flags cleared.  And,
on the next cycle through lpfc_nlp_reg_node, the NLP_XPT_REGD flag is set
but registraton with the fc transport is bypassed because
lpfc_valid_xpt_node returns false.

This sets up a condition whereby the next call to lpfc_nlp_unreg_node
results in a premature release of the ndlp, and a callback from the
transport results in a use-after-free condition.

To address this issue, refactor lpfc_fc4_xpt_flags such that both SCSI and
NVME have separate flags indicating registration with their respective
transport.  The flags also indicate a request to unregister had been made.
In dev-loss or transport callback processing, the SCSI_XPT_UNREG_WAIT and
NVME_XPT_UNREG_WAIT flags indicate whether the ndlp reference has already
been released.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_disc.h    |  2 +-
 drivers/scsi/lpfc/lpfc_hbadisc.c | 97 ++++++++++++++++----------------
 2 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_disc.h b/drivers/scsi/lpfc/lpfc_disc.h
index a377e97cbe65..afc5e84bf0fa 100644
--- a/drivers/scsi/lpfc/lpfc_disc.h
+++ b/drivers/scsi/lpfc/lpfc_disc.h
@@ -83,7 +83,7 @@ struct lpfc_enc_info {
 };
 
 enum lpfc_fc4_xpt_flags {
-	NLP_XPT_REGD		= 0x1,
+	SCSI_XPT_UNREG_WAIT	= 0x1,
 	SCSI_XPT_REGD		= 0x2,
 	NVME_XPT_REGD		= 0x4,
 	NVME_XPT_UNREG_WAIT	= 0x8,
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index c9b02d2c6305..6337dae4f1cd 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -200,26 +200,18 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 		/* The scsi_transport is done with the rport so lpfc cannot
 		 * call to unregister.
 		 */
-		if (ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
+		if ((ndlp->fc4_xpt_flags & SCSI_XPT_REGD) &&
+		    !(ndlp->fc4_xpt_flags & SCSI_XPT_UNREG_WAIT)) {
+			/* Reference held since no unreg call made */
 			ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
+			spin_unlock_irqrestore(&ndlp->lock, iflags);
 
-			/* If NLP_XPT_REGD was cleared in lpfc_nlp_unreg_node,
-			 * unregister calls were made to the scsi and nvme
-			 * transports and refcnt was already decremented. Clear
-			 * the NLP_XPT_REGD flag only if the NVME nrport is
-			 * confirmed unregistered.
-			 */
-			if (ndlp->fc4_xpt_flags & NLP_XPT_REGD) {
-				if (!(ndlp->fc4_xpt_flags & NVME_XPT_REGD))
-					ndlp->fc4_xpt_flags &= ~NLP_XPT_REGD;
-				spin_unlock_irqrestore(&ndlp->lock, iflags);
-
-				/* Release scsi transport reference */
-				lpfc_nlp_put(ndlp);
-			} else {
-				spin_unlock_irqrestore(&ndlp->lock, iflags);
-			}
+			/* Release scsi transport reference */
+			lpfc_nlp_put(ndlp);
 		} else {
+			/* Clear scsi xpt flags */
+			ndlp->fc4_xpt_flags &= ~(SCSI_XPT_REGD |
+						 SCSI_XPT_UNREG_WAIT);
 			spin_unlock_irqrestore(&ndlp->lock, iflags);
 		}
 
@@ -270,7 +262,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 	 * The backend does not expect any more calls associated with this
 	 * rport. Remove the association between rport and ndlp.
 	 */
-	ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
+	ndlp->fc4_xpt_flags &= ~(SCSI_XPT_REGD | SCSI_XPT_UNREG_WAIT);
 	((struct lpfc_rport_data *)rport->dd_data)->pnode = NULL;
 	ndlp->rport = NULL;
 	spin_unlock_irqrestore(&ndlp->lock, iflags);
@@ -606,7 +598,7 @@ lpfc_dev_loss_tmo_handler(struct lpfc_nodelist *ndlp)
 		return fcf_inuse;
 	}
 
-	if (!(ndlp->fc4_xpt_flags & NVME_XPT_REGD))
+	if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD)))
 		lpfc_disc_state_machine(vport, ndlp, NULL, NLP_EVT_DEVICE_RM);
 
 	return fcf_inuse;
@@ -4346,7 +4338,8 @@ lpfc_mbx_cmpl_ns_reg_login(struct lpfc_hba *phba, LPFC_MBOXQ_t *pmb)
 		 */
 		if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD))) {
 			clear_bit(NLP_NPR_2B_DISC, &ndlp->nlp_flag);
-			lpfc_nlp_put(ndlp);
+			if (!test_and_set_bit(NLP_DROPPED, &ndlp->nlp_flag))
+				lpfc_nlp_put(ndlp);
 		}
 
 		if (phba->fc_topology == LPFC_TOPOLOGY_LOOP) {
@@ -4527,6 +4520,7 @@ lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	}
 
 	spin_lock_irqsave(&ndlp->lock, flags);
+	ndlp->fc4_xpt_flags &= ~SCSI_XPT_UNREG_WAIT;
 	ndlp->fc4_xpt_flags |= SCSI_XPT_REGD;
 	spin_unlock_irqrestore(&ndlp->lock, flags);
 
@@ -4562,6 +4556,7 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
 {
 	struct fc_rport *rport = ndlp->rport;
 	struct lpfc_vport *vport = ndlp->vport;
+	unsigned long flags;
 
 	if (vport->cfg_enable_fc4_type == LPFC_ENABLE_NVME)
 		return;
@@ -4577,6 +4572,11 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
 			 kref_read(&ndlp->kref));
 
 	fc_remote_port_delete(rport);
+
+	/* Flag unreg pending and reference released */
+	spin_lock_irqsave(&ndlp->lock, flags);
+	ndlp->fc4_xpt_flags |= SCSI_XPT_UNREG_WAIT;
+	spin_unlock_irqrestore(&ndlp->lock, flags);
 	lpfc_nlp_put(ndlp);
 }
 
@@ -4623,7 +4623,9 @@ lpfc_nlp_reg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	lpfc_check_nlp_post_devloss(vport, ndlp);
 
 	spin_lock_irqsave(&ndlp->lock, iflags);
-	if (ndlp->fc4_xpt_flags & NLP_XPT_REGD) {
+	if ((ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD)) &&
+	    !(ndlp->fc4_xpt_flags & (SCSI_XPT_UNREG_WAIT |
+				     NVME_XPT_UNREG_WAIT))) {
 		/* Already registered with backend, trigger rescan */
 		spin_unlock_irqrestore(&ndlp->lock, iflags);
 
@@ -4633,16 +4635,11 @@ lpfc_nlp_reg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 		}
 		return;
 	}
-
-	ndlp->fc4_xpt_flags |= NLP_XPT_REGD;
 	spin_unlock_irqrestore(&ndlp->lock, iflags);
 
 	if (lpfc_valid_xpt_node(ndlp)) {
 		vport->phba->nport_event_cnt++;
-		/*
-		 * Tell the fc transport about the port, if we haven't
-		 * already. If we have, and it's a scsi entity, be
-		 */
+		/* Tell the fc transport about the port */
 		lpfc_register_remote_port(vport, ndlp);
 	}
 
@@ -4650,24 +4647,24 @@ lpfc_nlp_reg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	if (!(ndlp->nlp_fc4_type & NLP_FC4_NVME))
 		return;
 
+	if (vport->phba->sli_rev < LPFC_SLI_REV4)
+		return;
+
 	/* Notify the NVME transport of this new rport. */
-	if (vport->phba->sli_rev >= LPFC_SLI_REV4 &&
-			ndlp->nlp_fc4_type & NLP_FC4_NVME) {
-		if (vport->phba->nvmet_support == 0) {
-			/* Register this rport with the transport.
-			 * Only NVME Target Rports are registered with
-			 * the transport.
-			 */
-			if (ndlp->nlp_type & NLP_NVME_TARGET) {
-				vport->phba->nport_event_cnt++;
-				lpfc_nvme_register_port(vport, ndlp);
-			}
-		} else {
-			/* Just take an NDLP ref count since the
-			 * target does not register rports.
-			 */
-			lpfc_nlp_get(ndlp);
+	if (vport->phba->nvmet_support == 0) {
+		/* Register this rport with the transport.
+		 * Only NVME Target Rports are registered with
+		 * the transport.
+		 */
+		if (ndlp->nlp_type & NLP_NVME_TARGET) {
+			vport->phba->nport_event_cnt++;
+			lpfc_nvme_register_port(vport, ndlp);
 		}
+	} else {
+		/* Just take an NDLP ref count since the
+		 * target does not register rports.
+		 */
+		lpfc_nlp_get(ndlp);
 	}
 }
 
@@ -4678,7 +4675,7 @@ lpfc_nlp_unreg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	unsigned long iflags;
 
 	spin_lock_irqsave(&ndlp->lock, iflags);
-	if (!(ndlp->fc4_xpt_flags & NLP_XPT_REGD)) {
+	if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD))) {
 		spin_unlock_irqrestore(&ndlp->lock, iflags);
 		lpfc_printf_vlog(vport, KERN_INFO,
 				 LOG_ELS | LOG_NODE | LOG_DISCOVERY,
@@ -4688,12 +4685,11 @@ lpfc_nlp_unreg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 				  ndlp->nlp_flag, ndlp->fc4_xpt_flags);
 		return;
 	}
-
-	ndlp->fc4_xpt_flags &= ~NLP_XPT_REGD;
 	spin_unlock_irqrestore(&ndlp->lock, iflags);
 
 	if (ndlp->rport &&
-	    ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
+	    ((ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | SCSI_XPT_UNREG_WAIT)) ==
+	     SCSI_XPT_REGD)) {
 		vport->phba->nport_event_cnt++;
 		lpfc_unregister_remote_port(ndlp);
 	} else if (!ndlp->rport) {
@@ -4706,7 +4702,12 @@ lpfc_nlp_unreg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 				 kref_read(&ndlp->kref));
 	}
 
-	if (ndlp->fc4_xpt_flags & NVME_XPT_REGD) {
+	/* If no remote NVME node is indicated, just exit */
+	if (!(ndlp->nlp_fc4_type & NLP_FC4_NVME))
+		return;
+
+	if ((ndlp->fc4_xpt_flags & (NVME_XPT_REGD | NVME_XPT_UNREG_WAIT)) ==
+	    NVME_XPT_REGD) {
 		vport->phba->nport_event_cnt++;
 		if (vport->phba->nvmet_support == 0) {
 			lpfc_nvme_unregister_port(vport, ndlp);
-- 
2.38.0

[PATCH v3 07/14] lpfc: Rework I/O flush ordering when unloading driver

[Justin Tee]

The lpfc_els_abort routine has a code path that cancels outstanding
I/Os on the ELS ring when attempted aborts fail.  The failed aborts are
queued to a drv_cmpl_list and then cancelled after the ELS pring->txcmplq
is fully traversed.  However if the abort failure returns IOCB_ABORTING,
then the driver should not have cancelled it.  Doing so starts two threads
working on the same iocb and ndlp, leading to unintended race conditions.

Fix by capturing the IOCB_ABORTING return value in lpfc_els_abort and not
adding it to the list of iocbs for cancelling.  We should allow the iocb
scheduled for abort to complete naturally.  This avoids simultaneous
threads acting on the same iocb and ndlp objects.

The lpfc_free_iocb_list is moved to execute after lpfc_sli4_hba_unset
allowing the routine to flush I/O before freeing it.  And, in
lpfc_pci_remove_one_s4 a call to flush the phba->wq is added.  This makes
the unload logic consistent with offline handling logic.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_init.c      | 16 ++++++++++++++--
 drivers/scsi/lpfc/lpfc_nportdisc.c | 11 +++++++++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 82af59c913e9..c23bc3d059a1 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -13515,6 +13515,9 @@ lpfc_sli4_hba_unset(struct lpfc_hba *phba)
 	/* Stop the SLI4 device port */
 	if (phba->pport)
 		phba->pport->work_port_events = 0;
+
+	/* All IO completed and queues released. Free the IOCBs. */
+	lpfc_free_iocb_list(phba);
 }
 
 /*
@@ -14949,11 +14952,20 @@ lpfc_pci_remove_one_s4(struct pci_dev *pdev)
 
 	/* Perform scsi free before driver resource_unset since scsi
 	 * buffers are released to their corresponding pools here.
+	 * lpfc_sli4_hba_unset() issues aborts via lpfc_sli_hba_iocb_abort(),
+	 * which allocates abort IOCBs from phba->lpfc_iocb_list; the pool
+	 * must still exist, so lpfc_free_iocb_list() runs only after unset.
 	 */
 	lpfc_io_free(phba);
-	lpfc_free_iocb_list(phba);
-	lpfc_sli4_hba_unset(phba);
 
+	/* Flush the PHBA WQ - there could be a race with ELS IOs while lpfc
+	 * is unloading.  This stops a race between completions, aborts and
+	 * resource recovery.
+	 */
+	if (phba->wq)
+		flush_workqueue(phba->wq);
+
+	lpfc_sli4_hba_unset(phba);
 	lpfc_unset_driver_resource_phase2(phba);
 	lpfc_sli4_driver_resource_unset(phba);
 
diff --git a/drivers/scsi/lpfc/lpfc_nportdisc.c b/drivers/scsi/lpfc/lpfc_nportdisc.c
index 2c8d995a45bf..f917a5bcfd02 100644
--- a/drivers/scsi/lpfc/lpfc_nportdisc.c
+++ b/drivers/scsi/lpfc/lpfc_nportdisc.c
@@ -255,8 +255,9 @@ lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
 	spin_lock_irq(&phba->hbalock);
 	if (phba->sli_rev == LPFC_SLI_REV4)
 		spin_lock(&pring->ring_lock);
+
 	list_for_each_entry_safe(iocb, next_iocb, &pring->txcmplq, list) {
-	/* Add to abort_list on on NDLP match. */
+		/* Add to abort_list on NDLP match. */
 		if (lpfc_check_sli_ndlp(phba, pring, iocb, ndlp))
 			list_add_tail(&iocb->dlist, &abort_list);
 	}
@@ -271,7 +272,13 @@ lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
 		retval = lpfc_sli_issue_abort_iotag(phba, pring, iocb, NULL);
 		spin_unlock_irq(&phba->hbalock);
 
-		if (retval && test_bit(FC_UNLOADING, &phba->pport->load_flag)) {
+		/* An abort that fails here is just cancelled when the driver is
+		 * going offline.  However, if the abort failure is because the
+		 * IOCB is already getting aborted, don't cancel.  Just let it
+		 * complete.
+		 */
+		if (test_bit(FC_UNLOADING, &phba->pport->load_flag) &&
+		    retval && retval != IOCB_ABORTING) {
 			list_del_init(&iocb->list);
 			list_add_tail(&iocb->list, &drv_cmpl_list);
 		}
-- 
2.38.0

[PATCH v3 08/14] lpfc: Improve PLOGI retry handling for large SAN configurations

[Justin Tee]

In large SAN configurations with link perturbations, rediscovery of target
ports is problematic due to PLOGI retry race conditions.

This patch improves target rediscovery by ensuring PLOGI retries are
serialized in unregistration and retry handler paths.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_els.c       | 62 +++++++++++++++++++--
 drivers/scsi/lpfc/lpfc_nportdisc.c | 58 ++++++++++++++++++++
 drivers/scsi/lpfc/lpfc_sli.c       | 87 +++++++++++++++++-------------
 3 files changed, 167 insertions(+), 40 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 6720fd5ec523..16b10377a095 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -2159,6 +2159,8 @@ lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 		goto out_freeiocb;
 	}
 
+	clear_bit(NLP_PLOGI_SND, &ndlp->nlp_flag);
+
 	/* Since ndlp can be freed in the disc state machine, note if this node
 	 * is being used during discovery.
 	 */
@@ -2329,17 +2331,43 @@ lpfc_issue_els_plogi(struct lpfc_vport *vport, uint32_t did, uint8_t retry)
 	     test_bit(NLP_UNREG_INP, &ndlp->nlp_flag)) &&
 	    ((ndlp->nlp_DID & Fabric_DID_MASK) != Fabric_DID_MASK) &&
 	    !test_bit(FC_OFFLINE_MODE, &vport->fc_flag)) {
-		lpfc_printf_vlog(vport, KERN_INFO, LOG_DISCOVERY,
+		lpfc_printf_vlog(vport, KERN_INFO,
+				 LOG_ELS | LOG_NODE | LOG_DISCOVERY,
 				 "4110 Issue PLOGI x%x deferred "
 				 "on NPort x%x rpi x%x flg x%lx Data:"
 				 " x%px\n",
 				 ndlp->nlp_defer_did, ndlp->nlp_DID,
 				 ndlp->nlp_rpi, ndlp->nlp_flag, ndlp);
 
-		/* We can only defer 1st PLOGI */
-		if (ndlp->nlp_defer_did == NLP_EVT_NOTHING_PENDING)
+		/* Don't defer a PLOGI that is already in that condition.
+		 * Also set the nlp_last_elscmd to PLOGI to get the retry.
+		 */
+		if (ndlp->nlp_defer_did == NLP_EVT_NOTHING_PENDING) {
 			ndlp->nlp_defer_did = did;
-		return 0;
+			ndlp->nlp_last_elscmd = ELS_CMD_PLOGI;
+		}
+		return 1;
+	}
+
+	if (test_bit(NLP_PLOGI_SND, &ndlp->nlp_flag)) {
+		lpfc_printf_vlog(vport, KERN_INFO,
+				 LOG_ELS | LOG_NODE | LOG_DISCOVERY,
+				 "4113 Reject PLOGI issue, PLOGI in-flight "
+				 "x%px, DID x%x nflag x%lx\n",
+				 ndlp, ndlp->nlp_DID, ndlp->nlp_flag);
+		return 1;
+	}
+
+	if (ndlp->nlp_state > NLP_STE_PLOGI_ISSUE &&
+	    ndlp->nlp_state <= NLP_STE_MAPPED_NODE) {
+		lpfc_printf_vlog(vport, KERN_INFO,
+				 LOG_ELS | LOG_NODE | LOG_DISCOVERY,
+				 "4114 Reject PLOGI issue, Node in "
+				 "unexpected state x%px, DID x%x nflag x%lx "
+				 "in State x%x\n",
+				 ndlp, ndlp->nlp_DID,
+				 ndlp->nlp_flag, ndlp->nlp_state);
+		return 1;
 	}
 
 	cmdsize = (sizeof(uint32_t) + sizeof(struct serv_parm));
@@ -2415,6 +2443,7 @@ lpfc_issue_els_plogi(struct lpfc_vport *vport, uint32_t did, uint8_t retry)
 		return 1;
 	}
 
+	set_bit(NLP_PLOGI_SND, &ndlp->nlp_flag);
 	return 0;
 }
 
@@ -4614,6 +4643,31 @@ lpfc_els_retry_delay_handler(struct lpfc_nodelist *ndlp)
 		lpfc_issue_els_flogi(vport, ndlp, retry);
 		break;
 	case ELS_CMD_PLOGI:
+		/* The driver delayed a PLOGI via the nlp_delayfunc, but
+		 * it's possible the PLOGI is already on a deferred retry.
+		 * Catch this case and skip this delayed PLOGI. This prevents
+		 * multiple PLOGIs in flight. The defer code flow cleans
+		 * up.
+		 */
+		if ((test_bit(NLP_IGNR_REG_CMPL, &ndlp->nlp_flag) ||
+		     test_bit(NLP_UNREG_INP, &ndlp->nlp_flag)) &&
+		    ndlp->nlp_defer_did != NLP_EVT_NOTHING_PENDING &&
+		    ((ndlp->nlp_DID & Fabric_DID_MASK) != Fabric_DID_MASK) &&
+		    !test_bit(FC_OFFLINE_MODE, &vport->fc_flag)) {
+			/* When UNREG_RPI completes we need to have the
+			 * nlp_last_elscmd set.
+			 */
+			ndlp->nlp_last_elscmd = ELS_CMD_PLOGI;
+			lpfc_printf_vlog(vport, KERN_INFO,
+					 LOG_ELS | LOG_NODE | LOG_DISCOVERY,
+					 "4112 Skip delayed PLOGI x%x deferred "
+					 "on NPort x%x rpi x%x flg x%lx Data:"
+					 " x%px\n",
+					 ndlp->nlp_defer_did, ndlp->nlp_DID,
+					 ndlp->nlp_rpi, ndlp->nlp_flag, ndlp);
+			break;
+		}
+
 		if (!lpfc_issue_els_plogi(vport, ndlp->nlp_DID, retry)) {
 			ndlp->nlp_prev_state = ndlp->nlp_state;
 			lpfc_nlp_set_state(vport, ndlp, NLP_STE_PLOGI_ISSUE);
diff --git a/drivers/scsi/lpfc/lpfc_nportdisc.c b/drivers/scsi/lpfc/lpfc_nportdisc.c
index f917a5bcfd02..0270ab7e602f 100644
--- a/drivers/scsi/lpfc/lpfc_nportdisc.c
+++ b/drivers/scsi/lpfc/lpfc_nportdisc.c
@@ -920,6 +920,64 @@ lpfc_rcv_logo(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
 				 ndlp->nlp_DID, ndlp->nlp_state,
 				 ndlp->nlp_type, vport->fc_flag);
 
+		/* The driver wants to schedule a delayed PLOGI to recover
+		 * the remote Nport.  However, there are two cases that
+		 * stop this so that multiple PLOGI are not inflight to
+		 * the same NPortID
+		 *
+		 * Do not schedule a delayed PLOGI if the deferred PLOGI
+		 * code already set up a PLOGI retry after an UNREG_RPI
+		 * mailbox completes.
+		 */
+		if (test_bit(NLP_UNREG_INP, &ndlp->nlp_flag) &&
+		    ndlp->nlp_defer_did == ndlp->nlp_DID &&
+		    ndlp->nlp_last_elscmd == ELS_CMD_PLOGI) {
+			lpfc_printf_vlog(vport, KERN_INFO,
+					 LOG_NODE | LOG_ELS | LOG_DISCOVERY,
+					 "3206 No PLOGI delay, defer PLOGI "
+					 "waiting on DID x%06x UNREG_RPI "
+					 "nflag x%lx state x%x lastels x%x "
+					 "defer_did x%x\n",
+					 ndlp->nlp_DID, ndlp->nlp_flag,
+					 ndlp->nlp_state, ndlp->nlp_last_elscmd,
+					 ndlp->nlp_defer_did);
+			goto out;
+		}
+
+		/* A delayed PLOGI retry is not required if the ndlp's delay
+		 * timer is running and the last command was PLOGI.
+		 */
+		if (test_bit(NLP_DELAY_TMO, &ndlp->nlp_flag) &&
+		    ndlp->nlp_last_elscmd == ELS_CMD_PLOGI) {
+			lpfc_printf_vlog(vport, KERN_INFO,
+					 LOG_NODE | LOG_ELS | LOG_DISCOVERY,
+					 "3207 No PLOGI delay, PLOGI_DELAY_TMO "
+					 "active on DID x%06x "
+					 "nflag x%lx state x%x lastels x%x "
+					 "defer_did x%x\n",
+					 ndlp->nlp_DID, ndlp->nlp_flag,
+					 ndlp->nlp_state, ndlp->nlp_last_elscmd,
+					 ndlp->nlp_defer_did);
+			goto out;
+		}
+
+		/* Do not schedule a PLOGI retry if the ndlp state is NPR
+		 * and vport has received an RSCN
+		 */
+		if (ndlp->nlp_state == NLP_STE_NPR_NODE &&
+		    test_bit(FC_RSCN_MODE, &vport->fc_flag)) {
+			lpfc_printf_vlog(vport, KERN_INFO,
+					 LOG_NODE | LOG_ELS | LOG_DISCOVERY,
+					 "3939 No PLOGI delay, RSCN in "
+					 "progress for NPR DID x%06x "
+					 "nflag x%lx state x%x last_els x%x "
+					 "defer_did x%06x\n",
+					 ndlp->nlp_DID, ndlp->nlp_flag,
+					 ndlp->nlp_state, ndlp->nlp_last_elscmd,
+					 ndlp->nlp_defer_did);
+			goto out;
+		}
+
 		/* Special cases for rports that recover post LOGO. */
 		if ((!(ndlp->nlp_type == NLP_FABRIC) &&
 		     (ndlp->nlp_type & (NLP_FCP_TARGET | NLP_NVME_TARGET) ||
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 10db07771ccf..54bc5d8984fc 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -2914,7 +2914,19 @@ lpfc_sli_def_mbox_cmpl(struct lpfc_hba *phba, LPFC_MBOXQ_t *pmb)
 			    ndlp->nlp_defer_did != NLP_EVT_NOTHING_PENDING) {
 				clear_bit(NLP_UNREG_INP, &ndlp->nlp_flag);
 				ndlp->nlp_defer_did = NLP_EVT_NOTHING_PENDING;
-				lpfc_issue_els_plogi(vport, ndlp->nlp_DID, 0);
+
+				if (!test_bit(NLP_DELAY_TMO, &ndlp->nlp_flag) &&
+				    ndlp->nlp_last_elscmd == ELS_CMD_PLOGI) {
+					rc = lpfc_issue_els_plogi(vport,
+								  ndlp->nlp_DID,
+								  0);
+					if (!rc) {
+						ndlp->nlp_prev_state =
+							ndlp->nlp_state;
+						lpfc_nlp_set_state(vport, ndlp,
+							   NLP_STE_PLOGI_ISSUE);
+					}
+				}
 			} else {
 				clear_bit(NLP_UNREG_INP, &ndlp->nlp_flag);
 			}
@@ -2967,52 +2979,55 @@ lpfc_sli4_unreg_rpi_cmpl_clr(struct lpfc_hba *phba, LPFC_MBOXQ_t *pmb)
 	bool unreg_inp;
 
 	ndlp = pmb->ctx_ndlp;
-	if (pmb->u.mb.mbxCommand == MBX_UNREG_LOGIN) {
+	if (pmb->u.mb.mbxCommand == MBX_UNREG_LOGIN && ndlp) {
 		if (phba->sli_rev == LPFC_SLI_REV4 &&
 		    (bf_get(lpfc_sli_intf_if_type,
 		     &phba->sli4_hba.sli_intf) >=
 		     LPFC_SLI_INTF_IF_TYPE_2)) {
-			if (ndlp) {
-				lpfc_printf_vlog(
-					 vport, KERN_INFO,
+			lpfc_printf_vlog(vport, KERN_INFO,
 					 LOG_MBOX | LOG_SLI | LOG_NODE,
-					 "0010 UNREG_LOGIN vpi:x%x "
-					 "rpi:%x DID:%x defer x%x flg x%lx "
-					 "x%px\n",
+					 "0010 UNREG_LOGIN vpi:x%x rpi:%x "
+					 "DID:%x defer x%x flg x%lx x%px\n",
 					 vport->vpi, ndlp->nlp_rpi,
 					 ndlp->nlp_DID, ndlp->nlp_defer_did,
-					 ndlp->nlp_flag,
-					 ndlp);
+					 ndlp->nlp_flag, ndlp);
 
-				/* Cleanup the nlp_flag now that the UNREG RPI
-				 * has completed.
-				 */
-				unreg_inp = test_and_clear_bit(NLP_UNREG_INP,
-							       &ndlp->nlp_flag);
-				clear_bit(NLP_LOGO_ACC, &ndlp->nlp_flag);
+			/* Cleanup the nlp_flag now that the UNREG RPI
+			 * has completed.
+			 */
+			unreg_inp = test_and_clear_bit(NLP_UNREG_INP,
+						       &ndlp->nlp_flag);
+			clear_bit(NLP_LOGO_ACC, &ndlp->nlp_flag);
 
-				/* Check to see if there are any deferred
-				 * events to process
-				 */
-				if (unreg_inp &&
-				    ndlp->nlp_defer_did !=
-				    NLP_EVT_NOTHING_PENDING) {
-					lpfc_printf_vlog(
-						vport, KERN_INFO,
-						LOG_MBOX | LOG_SLI | LOG_NODE,
-						"4111 UNREG cmpl deferred "
-						"clr x%x on "
-						"NPort x%x Data: x%x x%px\n",
-						ndlp->nlp_rpi, ndlp->nlp_DID,
-						ndlp->nlp_defer_did, ndlp);
-					ndlp->nlp_defer_did =
-						NLP_EVT_NOTHING_PENDING;
-					lpfc_issue_els_plogi(
-						vport, ndlp->nlp_DID, 0);
-				}
+			/* Check to see if there are any deferred
+			 * events to process
+			 */
+			if (unreg_inp &&
+			    ndlp->nlp_defer_did != NLP_EVT_NOTHING_PENDING) {
+				lpfc_printf_vlog(vport, KERN_INFO,
+						 LOG_MBOX | LOG_SLI | LOG_NODE,
+						 "4111 UNREG cmpl deferred "
+						 "clr x%x on  NPort x%x "
+						 "Data: x%x x%x x%px\n",
+						 ndlp->nlp_rpi, ndlp->nlp_DID,
+						 ndlp->nlp_defer_did,
+						 ndlp->nlp_last_elscmd, ndlp);
+				ndlp->nlp_defer_did = NLP_EVT_NOTHING_PENDING;
 
-				lpfc_nlp_put(ndlp);
+				if (!test_bit(NLP_DELAY_TMO, &ndlp->nlp_flag) &&
+				    ndlp->nlp_last_elscmd == ELS_CMD_PLOGI) {
+					if (lpfc_issue_els_plogi(vport,
+								 ndlp->nlp_DID,
+								 0))
+						goto out;
+
+					ndlp->nlp_prev_state = ndlp->nlp_state;
+					lpfc_nlp_set_state(vport, ndlp,
+							   NLP_STE_PLOGI_ISSUE);
+				}
 			}
+out:
+			lpfc_nlp_put(ndlp);
 		}
 	}
 
-- 
2.38.0

[PATCH v3 09/14] lpfc: Send inhibited ABORT_WQE when PLOGI CQE SEQUENCE_TMO is received

[Justin Tee]

It is unlikely that an BA_ACC will be received for a sent ABTS knowing
that a previously sent PLOGI E_D_TOV timed out.

By sending an ABORT_WQE with IA=1, the XRI_ABORTED CQE for the PLOGI CQE
SEQUENCE_TIMEOUT with XB=1 will return immediately compared to waiting
E_D_TOV for a BA_ACC.

Add a new bool ia argument variable to lpfc_sli_issue_abort_iotag, to
explicitly set the IA bit when filling out ABORT_WQE.  When the ia argument
is false, we fall back to the old logic of implicitly setting the IA bit
under previous conditions.  Setting the ia argument to true, is currently
only used for PLOGI CQE LOCAL_REJECT/SEQUENCE_TIMEOUT.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_bsg.c       |  5 ++--
 drivers/scsi/lpfc/lpfc_crtn.h      |  5 ++--
 drivers/scsi/lpfc/lpfc_els.c       | 48 ++++++++++++++++++++++++++----
 drivers/scsi/lpfc/lpfc_hbadisc.c   |  3 +-
 drivers/scsi/lpfc/lpfc_nportdisc.c |  3 +-
 drivers/scsi/lpfc/lpfc_nvme.c      |  2 +-
 drivers/scsi/lpfc/lpfc_scsi.c      |  2 +-
 drivers/scsi/lpfc/lpfc_sli.c       | 17 ++++++-----
 8 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_bsg.c b/drivers/scsi/lpfc/lpfc_bsg.c
index c95165905483..56f55f818cce 100644
--- a/drivers/scsi/lpfc/lpfc_bsg.c
+++ b/drivers/scsi/lpfc/lpfc_bsg.c
@@ -1,7 +1,7 @@
 /*******************************************************************
  * This file is part of the Emulex Linux Device Driver for         *
  * Fibre Channel Host Bus Adapters.                                *
- * Copyright (C) 2017-2024 Broadcom. All Rights Reserved. The term *
+ * Copyright (C) 2017-2026 Broadcom. All Rights Reserved. The term *
  * “Broadcom” refers to Broadcom Inc. and/or its subsidiaries.     *
  * Copyright (C) 2009-2015 Emulex.  All rights reserved.           *
  * EMULEX and SLI are trademarks of Emulex.                        *
@@ -5857,7 +5857,8 @@ lpfc_bsg_timeout(struct bsg_job *job)
 			}
 		}
 		if (list_empty(&completions))
-			lpfc_sli_issue_abort_iotag(phba, pring, cmdiocb, NULL);
+			lpfc_sli_issue_abort_iotag(phba, pring, cmdiocb, false,
+						   NULL);
 		spin_unlock_irqrestore(&phba->hbalock, flags);
 		if (!list_empty(&completions)) {
 			lpfc_sli_cancel_iocbs(phba, &completions,
diff --git a/drivers/scsi/lpfc/lpfc_crtn.h b/drivers/scsi/lpfc/lpfc_crtn.h
index 8a5b76bdea06..2ca5f0229ca1 100644
--- a/drivers/scsi/lpfc/lpfc_crtn.h
+++ b/drivers/scsi/lpfc/lpfc_crtn.h
@@ -402,8 +402,9 @@ int lpfc_sli_hbq_count(void);
 int lpfc_sli_hbqbuf_add_hbqs(struct lpfc_hba *, uint32_t);
 void lpfc_sli_hbqbuf_free_all(struct lpfc_hba *);
 int lpfc_sli_hbq_size(void);
-int lpfc_sli_issue_abort_iotag(struct lpfc_hba *, struct lpfc_sli_ring *,
-			       struct lpfc_iocbq *, void *);
+int lpfc_sli_issue_abort_iotag(struct lpfc_hba *phba,
+			       struct lpfc_sli_ring *pring,
+			       struct lpfc_iocbq *cmdiocb, bool ia, void *cmpl);
 int lpfc_sli_sum_iocb(struct lpfc_vport *, uint16_t, uint64_t, lpfc_ctx_cmd);
 int lpfc_sli_abort_iocb(struct lpfc_vport *vport, u16 tgt_id, u64 lun_id,
 			lpfc_ctx_cmd abort_cmd);
diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 16b10377a095..11c36968a2be 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -1556,7 +1556,7 @@ lpfc_els_abort_flogi(struct lpfc_hba *phba)
 					iocb->fabric_cmd_cmpl =
 						lpfc_ignore_els_cmpl;
 				lpfc_sli_issue_abort_iotag(phba, pring, iocb,
-							   NULL);
+							   false, NULL);
 			}
 		}
 	}
@@ -2129,7 +2129,7 @@ lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 	struct lpfc_dmabuf *prsp;
 	bool disc;
 	struct serv_parm *sp = NULL;
-	u32 ulp_status, ulp_word4, did, iotag;
+	u32 ulp_status, ulp_word4, did, iotag, word3;
 	bool release_node = false;
 
 	/* we pass cmdiocb to state machine which needs rspiocb as well */
@@ -2141,9 +2141,11 @@ lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 
 	if (phba->sli_rev == LPFC_SLI_REV4) {
 		iotag = get_wqe_reqtag(cmdiocb);
+		word3 = rspiocb->wcqe_cmpl.word3;
 	} else {
 		irsp = &rspiocb->iocb;
 		iotag = irsp->ulpIoTag;
+		word3 = 0;
 	}
 
 	lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_ELS_CMD,
@@ -2169,10 +2171,10 @@ lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 	/* PLOGI completes to NPort <nlp_DID> */
 	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
 			 "0102 PLOGI completes to NPort x%06x "
-			 "IoTag x%x Data: x%x x%x x%x x%x x%x\n",
+			 "IoTag x%x Data: x%x x%x x%x x%x x%x x%x\n",
 			 ndlp->nlp_DID, iotag,
 			 ndlp->nlp_fc4_type,
-			 ulp_status, ulp_word4,
+			 ulp_status, ulp_word4, word3,
 			 disc, vport->num_disc_nodes);
 
 	/* Check to see if link went down during discovery */
@@ -4785,6 +4787,7 @@ lpfc_els_retry(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 	union lpfc_wqe128 *irsp = &rspiocb->wqe;
 	struct lpfc_nodelist *ndlp = cmdiocb->ndlp;
 	struct lpfc_dmabuf *pcmd = cmdiocb->cmd_dmabuf;
+	struct lpfc_sli_ring *pring;
 	uint32_t *elscmd;
 	struct ls_rjt stat;
 	int retry = 0, maxretry = lpfc_max_els_tries, delay = 0;
@@ -4792,6 +4795,7 @@ lpfc_els_retry(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 	uint32_t cmd = 0;
 	uint32_t did;
 	int link_reset = 0, rc;
+	unsigned long iflags;
 	u32 ulp_status = get_job_ulpstatus(phba, rspiocb);
 	u32 ulp_word4 = get_job_word4(phba, rspiocb);
 	u8 rsn_code_exp = 0;
@@ -4893,7 +4897,38 @@ lpfc_els_retry(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 				/* Reset the Link */
 				link_reset = 1;
 				break;
+			} else if (cmd == ELS_CMD_PLOGI) {
+
+				/* if invalid ndlp, do not retry */
+				if (unlikely(!ndlp)) {
+					retry = 0;
+					break;
+				}
+
+				lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
+						 "0159 PLOGI Sequence TMO for "
+						 "ndlp x%px x%lx x%x x%x x%x "
+						 "x%x x%x %u x%x\n",
+						 ndlp, ndlp->nlp_flag,
+						 ndlp->nlp_DID, ndlp->nlp_type,
+						 ndlp->nlp_fc4_type,
+						 ndlp->nlp_rpi, ndlp->nlp_state,
+						 kref_read(&ndlp->kref),
+						 ndlp->fc4_xpt_flags);
+
+				/* Abort all outstanding ELS and auto-ABTS.  If
+				 * no response for E_D_TOV, then it is unlikely
+				 * auto-ABTS would receive a response too.  So,
+				 * inhibit the abort for faster XRI release.
+				 * However, still proceed with delayed retry.
+				 */
+				pring = lpfc_phba_elsring(phba);
+				spin_lock_irqsave(&phba->hbalock, iflags);
+				lpfc_sli_issue_abort_iotag(phba, pring, cmdiocb,
+							   true, NULL);
+				spin_unlock_irqrestore(&phba->hbalock, iflags);
 			}
+
 			retry = 1;
 			delay = 100;
 			break;
@@ -9763,7 +9798,7 @@ lpfc_els_timeout_handler(struct lpfc_vport *vport)
 
 		spin_lock_irq(&phba->hbalock);
 		list_del_init(&piocb->dlist);
-		lpfc_sli_issue_abort_iotag(phba, pring, piocb, NULL);
+		lpfc_sli_issue_abort_iotag(phba, pring, piocb, false, NULL);
 		spin_unlock_irq(&phba->hbalock);
 	}
 
@@ -9881,7 +9916,8 @@ lpfc_els_flush_cmd(struct lpfc_vport *vport)
 		if (mbx_tmo_err || !(phba->sli.sli_flag & LPFC_SLI_ACTIVE))
 			list_move_tail(&piocb->list, &cancel_list);
 		else
-			lpfc_sli_issue_abort_iotag(phba, pring, piocb, NULL);
+			lpfc_sli_issue_abort_iotag(phba, pring, piocb, false,
+						   NULL);
 
 		spin_unlock_irqrestore(&phba->hbalock, iflags);
 	}
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 6337dae4f1cd..6669932e77c8 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -5993,7 +5993,8 @@ lpfc_free_tx(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
 
 		if (ulp_command == CMD_ELS_REQUEST64_CR ||
 		    ulp_command == CMD_XMIT_ELS_RSP64_CX) {
-			lpfc_sli_issue_abort_iotag(phba, pring, iocb, NULL);
+			lpfc_sli_issue_abort_iotag(phba, pring, iocb, false,
+						   NULL);
 		}
 	}
 	spin_unlock_irq(&phba->hbalock);
diff --git a/drivers/scsi/lpfc/lpfc_nportdisc.c b/drivers/scsi/lpfc/lpfc_nportdisc.c
index 0270ab7e602f..e7ade735a97c 100644
--- a/drivers/scsi/lpfc/lpfc_nportdisc.c
+++ b/drivers/scsi/lpfc/lpfc_nportdisc.c
@@ -269,7 +269,8 @@ lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
 	list_for_each_entry_safe(iocb, next_iocb, &abort_list, dlist) {
 		spin_lock_irq(&phba->hbalock);
 		list_del_init(&iocb->dlist);
-		retval = lpfc_sli_issue_abort_iotag(phba, pring, iocb, NULL);
+		retval = lpfc_sli_issue_abort_iotag(phba, pring, iocb, false,
+						    NULL);
 		spin_unlock_irq(&phba->hbalock);
 
 		/* An abort that fails here is just cancelled when the driver is
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 71714ea390d9..45b5966d9e6b 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -743,7 +743,7 @@ __lpfc_nvme_ls_abort(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
 	spin_unlock(&pring->ring_lock);
 
 	if (foundit)
-		lpfc_sli_issue_abort_iotag(phba, pring, wqe, NULL);
+		lpfc_sli_issue_abort_iotag(phba, pring, wqe, false, NULL);
 	spin_unlock_irq(&phba->hbalock);
 
 	if (foundit)
diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index f2cab134af7f..ce9d7de21590 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -5622,7 +5622,7 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
 						      lpfc_sli_abort_fcp_cmpl);
 	} else {
 		pring = &phba->sli.sli3_ring[LPFC_FCP_RING];
-		ret_val = lpfc_sli_issue_abort_iotag(phba, pring, iocb,
+		ret_val = lpfc_sli_issue_abort_iotag(phba, pring, iocb, false,
 						     lpfc_sli_abort_fcp_cmpl);
 	}
 
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 54bc5d8984fc..0a300efc9240 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -4608,7 +4608,8 @@ lpfc_sli_abort_iocb_ring(struct lpfc_hba *phba, struct lpfc_sli_ring *pring)
 	} else {
 		/* Issue ABTS for everything on the txcmplq */
 		list_for_each_entry_safe(iocb, next_iocb, &pring->txcmplq, list)
-			lpfc_sli_issue_abort_iotag(phba, pring, iocb, NULL);
+			lpfc_sli_issue_abort_iotag(phba, pring, iocb, false,
+						   NULL);
 	}
 	spin_unlock_irq(plock);
 
@@ -11998,7 +11999,7 @@ lpfc_sli_host_down(struct lpfc_vport *vport)
 				if (iocb->vport != vport)
 					continue;
 				lpfc_sli_issue_abort_iotag(phba, pring, iocb,
-							   NULL);
+							   false, NULL);
 			}
 			pring->flag = prev_pring_flag;
 		}
@@ -12026,7 +12027,7 @@ lpfc_sli_host_down(struct lpfc_vport *vport)
 				if (iocb->vport != vport)
 					continue;
 				lpfc_sli_issue_abort_iotag(phba, pring, iocb,
-							   NULL);
+							   false, NULL);
 			}
 			pring->flag = prev_pring_flag;
 		}
@@ -12443,6 +12444,7 @@ lpfc_ignore_els_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
  * @phba: Pointer to HBA context object.
  * @pring: Pointer to driver SLI ring object.
  * @cmdiocb: Pointer to driver command iocb object.
+ * @ia: Flag to explicitly or implicitly inhibit abort.
  * @cmpl: completion function.
  *
  * This function issues an abort iocb for the provided command iocb. In case
@@ -12455,7 +12457,7 @@ lpfc_ignore_els_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
  **/
 int
 lpfc_sli_issue_abort_iotag(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
-			   struct lpfc_iocbq *cmdiocb, void *cmpl)
+			   struct lpfc_iocbq *cmdiocb, bool ia, void *cmpl)
 {
 	struct lpfc_vport *vport = cmdiocb->vport;
 	struct lpfc_iocbq *abtsiocbp;
@@ -12464,7 +12466,6 @@ lpfc_sli_issue_abort_iotag(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 	struct lpfc_nodelist *ndlp = NULL;
 	u32 ulp_command = get_job_cmnd(phba, cmdiocb);
 	u16 ulp_context, iotag;
-	bool ia;
 
 	/*
 	 * There are certain command types we don't want to abort.  And we
@@ -12514,7 +12515,7 @@ lpfc_sli_issue_abort_iotag(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 	}
 
 	/* Just close the exchange under certain conditions. */
-	if (test_bit(FC_UNLOADING, &vport->load_flag) ||
+	if (ia || test_bit(FC_UNLOADING, &vport->load_flag) ||
 	    phba->link_state < LPFC_LINK_UP ||
 	    (phba->sli_rev == LPFC_SLI_REV4 &&
 	     phba->sli4_hba.link_state.status == LPFC_FC_LA_TYPE_LINK_DOWN) ||
@@ -12557,7 +12558,7 @@ lpfc_sli_issue_abort_iotag(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 
 abort_iotag_exit:
 
-	lpfc_printf_vlog(vport, KERN_INFO, LOG_SLI,
+	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS | LOG_SLI,
 			 "0339 Abort IO XRI x%x, Original iotag x%x, "
 			 "abort tag x%x Cmdjob : x%px Abortjob : x%px "
 			 "retval x%x : IA %d cmd_cmpl %ps\n",
@@ -12851,7 +12852,7 @@ lpfc_sli_abort_iocb(struct lpfc_vport *vport, u16 tgt_id, u64 lun_id,
 		} else if (phba->sli_rev == LPFC_SLI_REV4) {
 			pring = lpfc_sli4_calc_ring(phba, iocbq);
 		}
-		ret_val = lpfc_sli_issue_abort_iotag(phba, pring, iocbq,
+		ret_val = lpfc_sli_issue_abort_iotag(phba, pring, iocbq, false,
 						     lpfc_sli_abort_fcp_cmpl);
 		spin_unlock_irqrestore(&phba->hbalock, iflags);
 		if (ret_val != IOCB_SUCCESS)
-- 
2.38.0

[PATCH v3 10/14] lpfc: Remove slowpath cqe process limiter in slow ring event handler

[Justin Tee]

There is a cqe process limit of 64 cqes in
lpfc_sli_handle_slow_ring_event_s4.  The HBA_SP_QUEUE_EVT flag is not set
nor the worker thread rescheduled when reaching this 64 limit.  This means
a burst of over 64 cqes can incur a delayed processing penalty, which can
be problematic in large SAN configurations waiting for rediscovery after a
link perturbation.

Remove the slowpath cqe process limiter in
lpfc_sli_handle_slow_ring_event_s4 to ensure the slow path CQ is drained.
Add log messages to notify when the 64 cqe count is reached.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_sli.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 0a300efc9240..424ad15416ef 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -4527,7 +4527,7 @@ lpfc_sli_handle_slow_ring_event_s4(struct lpfc_hba *phba,
 	struct hbq_dmabuf *dmabuf;
 	struct lpfc_cq_event *cq_event;
 	unsigned long iflag;
-	int count = 0;
+	u32 count = 0;
 
 	clear_bit(HBA_SP_QUEUE_EVT, &phba->hba_flag);
 	while (!list_empty(&phba->sli4_hba.sp_queue_event)) {
@@ -4547,22 +4547,39 @@ lpfc_sli_handle_slow_ring_event_s4(struct lpfc_hba *phba,
 			if (irspiocbq)
 				lpfc_sli_sp_handle_rspiocb(phba, pring,
 							   irspiocbq);
-			count++;
 			break;
 		case CQE_CODE_RECEIVE:
 		case CQE_CODE_RECEIVE_V1:
 			dmabuf = container_of(cq_event, struct hbq_dmabuf,
 					      cq_event);
 			lpfc_sli4_handle_received_buffer(phba, dmabuf);
-			count++;
 			break;
 		default:
+			lpfc_printf_log(phba, KERN_INFO, LOG_ELS,
+					"7771 Unknown WCQE completion code "
+					"x%x, ignoring.\n",
+					bf_get(lpfc_wcqe_c_code,
+					       &cq_event->cqe.wcqe_cmpl));
 			break;
 		}
 
-		/* Limit the number of events to 64 to avoid soft lockups */
-		if (count == 64)
-			break;
+		/* This loop runs until the ELS/CT CQ is empty.  Post a one
+		 * time message for debug support when ELS WQ ecount
+		 * completions are processed - this represent 1 full ELS WQ
+		 * wrap.
+		 */
+		if (++count == LPFC_WQE_DEF_COUNT) {
+			lpfc_printf_log(phba, KERN_INFO, LOG_ELS,
+					"7772 %s SP CQE count %d\n",
+					__func__, count);
+		}
+	}
+
+	/* Log a final message to note how many CQEs were processed. */
+	if (count > LPFC_WQE_DEF_COUNT) {
+		lpfc_printf_log(phba, KERN_INFO, LOG_ELS,
+				"7773 %s SP CQEs complete, count %d\n",
+				__func__, count);
 	}
 }
 
-- 
2.38.0

[PATCH v3 11/14] lpfc: Put iocbq on phba->txq when ELS WQ is full or ELS SGL unavailable

[Justin Tee]

When ELS/CT commands can't be sent due to ELS WQ full, queue the iocbq on
the phba->txq tail for retrying submission later in lpfc_drain_txq.

lpfc_drain_txq is flagged to be called by the worker thread when an ELS CQE
completes lpfc_sli4_sp_handle_els_wcqe through the HBA_SP_QUEUE_EVT flag.

lpfc_drain_txq is also updated to queue an iocbq back on to the head of
phba->txq when lpfc_drain_txq itself still observes ELS WQ full or SGL
unavailable events.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_crtn.h      |   7 +-
 drivers/scsi/lpfc/lpfc_ct.c        |  17 ++++-
 drivers/scsi/lpfc/lpfc_els.c       |  58 ++++++++++++++--
 drivers/scsi/lpfc/lpfc_nportdisc.c |   9 ++-
 drivers/scsi/lpfc/lpfc_sli.c       | 104 +++++++++++++++++++++++++----
 drivers/scsi/lpfc/lpfc_sli.h       |   4 +-
 6 files changed, 176 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_crtn.h b/drivers/scsi/lpfc/lpfc_crtn.h
index 2ca5f0229ca1..47a593fe5e69 100644
--- a/drivers/scsi/lpfc/lpfc_crtn.h
+++ b/drivers/scsi/lpfc/lpfc_crtn.h
@@ -536,8 +536,11 @@ int lpfc_bsg_timeout(struct bsg_job *);
 int lpfc_bsg_ct_unsol_event(struct lpfc_hba *, struct lpfc_sli_ring *,
 			     struct lpfc_iocbq *);
 int lpfc_bsg_ct_unsol_abort(struct lpfc_hba *, struct hbq_dmabuf *);
-void __lpfc_sli_ringtx_put(struct lpfc_hba *, struct lpfc_sli_ring *,
-	struct lpfc_iocbq *);
+void lpfc_sli4_queue_io_for_retry(struct lpfc_hba *phba,
+				  struct lpfc_iocbq *iocb,
+				  bool head);
+void __lpfc_sli_ringtx_put(struct lpfc_hba *phba, struct lpfc_sli_ring *ring,
+			   struct lpfc_iocbq *iocb, bool head);
 struct lpfc_iocbq *lpfc_sli_ringtx_get(struct lpfc_hba *,
 	struct lpfc_sli_ring *);
 int __lpfc_sli_issue_iocb(struct lpfc_hba *, uint32_t,
diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
index e14170550e69..16f9cd507834 100644
--- a/drivers/scsi/lpfc/lpfc_ct.c
+++ b/drivers/scsi/lpfc/lpfc_ct.c
@@ -639,11 +639,24 @@ lpfc_gen_req(struct lpfc_vport *vport, struct lpfc_dmabuf *bmp,
 		goto out;
 
 	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, geniocb, 0);
-	if (rc == IOCB_ERROR) {
+	if (rc) {
+		lpfc_vlog_msg(vport, KERN_NOTICE,
+			      LOG_ELS | LOG_DISCOVERY | LOG_NODE,
+			      "0156 %ps WQE Put returned %d\n",
+			      cmpl, rc);
+
+		/* Under heavy vpi counts, the driver's host_index can catch up
+		 * to the hba_index causing a put error. Catch this case and
+		 * put the IO on phba->txq.
+		 */
+		if (rc == IOCB_FAILED_PUT && phba->sli_rev == LPFC_SLI_REV4) {
+			lpfc_sli4_queue_io_for_retry(phba, geniocb, false);
+			return 0;
+		}
+
 		lpfc_nlp_put(ndlp);
 		goto out;
 	}
-
 	return 0;
 out:
 	lpfc_sli_release_iocbq(phba, geniocb);
diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 11c36968a2be..73552ce4bc64 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -2440,6 +2440,20 @@ lpfc_issue_els_plogi(struct lpfc_vport *vport, uint32_t did, uint8_t retry)
 
 	ret = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
 	if (ret) {
+		lpfc_vlog_msg(vport, KERN_NOTICE,
+			      LOG_ELS | LOG_DISCOVERY | LOG_NODE,
+			      "0157 PLOGI WQE Put returned %d\n",
+			      ret);
+
+		/* Under heavy vpi counts, the driver's host_index can catch up
+		 * to the hba_index causing a put error. Catch this case and
+		 * put the IO on phba->txq.
+		 */
+		if (ret == IOCB_FAILED_PUT && phba->sli_rev == LPFC_SLI_REV4) {
+			lpfc_sli4_queue_io_for_retry(phba, elsiocb, false);
+			return 0;
+		}
+
 		lpfc_els_free_iocb(phba, elsiocb);
 		lpfc_nlp_put(ndlp);
 		return 1;
@@ -2766,7 +2780,21 @@ lpfc_issue_els_prli(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
 	}
 
 	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
-	if (rc == IOCB_ERROR) {
+	if (rc) {
+		lpfc_vlog_msg(vport, KERN_NOTICE,
+			      LOG_ELS | LOG_DISCOVERY | LOG_NODE,
+			      "0155 PRLI WQE Put returned %d\n",
+			      rc);
+
+		/* Under heavy vpi counts, the driver's host_index can catch up
+		 * to the hba_index causing a put error. Catch this case and
+		 * put the IO on phba->txq.
+		 */
+		if (rc == IOCB_FAILED_PUT && phba->sli_rev == LPFC_SLI_REV4) {
+			lpfc_sli4_queue_io_for_retry(phba, elsiocb, false);
+			return 0;
+		}
+
 		lpfc_els_free_iocb(phba, elsiocb);
 		lpfc_nlp_put(ndlp);
 		return 1;
@@ -5982,7 +6010,21 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 	}
 
 	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
-	if (rc == IOCB_ERROR) {
+	if (rc) {
+		lpfc_vlog_msg(vport, KERN_NOTICE,
+			      LOG_ELS | LOG_DISCOVERY | LOG_NODE,
+			      "0154 LS_ACC WQE Put returned %d\n",
+			      rc);
+
+		/* Under heavy vpi counts, the driver's host_index can catch up
+		 * to the hba_index causing a put error. Catch this case and
+		 * put the IO on phba->txq.
+		 */
+		if (rc == IOCB_FAILED_PUT && phba->sli_rev == LPFC_SLI_REV4) {
+			lpfc_sli4_queue_io_for_retry(phba, elsiocb, false);
+			return 0;
+		}
+
 		lpfc_els_free_iocb(phba, elsiocb);
 		lpfc_nlp_put(ndlp);
 		return 1;
@@ -10583,6 +10625,7 @@ lpfc_els_unsol_buffer(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 	uint8_t rjt_exp, rjt_err = 0, init_link = 0;
 	struct lpfc_wcqe_complete *wcqe_cmpl = NULL;
 	LPFC_MBOXQ_t *mbox;
+	u16 rcv_oxid;
 
 	if (!vport || !elsiocb->cmd_dmabuf)
 		goto dropit;
@@ -10652,11 +10695,18 @@ lpfc_els_unsol_buffer(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 	if ((cmd & ELS_CMD_MASK) == ELS_CMD_RSCN) {
 		cmd &= ELS_CMD_MASK;
 	}
+
+	/* Fetch the incoming ox_id for SLI4 only - SLI3 can't do this. */
+	rcv_oxid = 0xffff;
+	if (phba->sli_rev == LPFC_SLI_REV4)
+		rcv_oxid = get_job_rcvoxid(phba, elsiocb);
+
 	/* ELS command <elsCmd> received from NPORT <did> */
 	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
 			 "0112 ELS command x%x received from NPORT x%x "
-			 "refcnt %d Data: x%x x%lx x%x x%x\n",
-			 cmd, did, kref_read(&ndlp->kref), vport->port_state,
+			 "refcnt %d rcvoxid x%x Data: x%x x%lx x%x x%x\n",
+			 cmd, did, kref_read(&ndlp->kref),
+			 rcv_oxid, vport->port_state,
 			 vport->fc_flag, vport->fc_myDID, vport->fc_prevDID);
 
 	/* reject till our FLOGI completes or PLOGI assigned DID via PT2PT */
diff --git a/drivers/scsi/lpfc/lpfc_nportdisc.c b/drivers/scsi/lpfc/lpfc_nportdisc.c
index e7ade735a97c..c0b2a678d492 100644
--- a/drivers/scsi/lpfc/lpfc_nportdisc.c
+++ b/drivers/scsi/lpfc/lpfc_nportdisc.c
@@ -1993,6 +1993,7 @@ lpfc_cmpl_reglogin_reglogin_issue(struct lpfc_vport *vport,
 	LPFC_MBOXQ_t *pmb = (LPFC_MBOXQ_t *) arg;
 	MAILBOX_t *mb = &pmb->u.mb;
 	uint32_t did  = mb->un.varWords[1];
+	int rc;
 
 	if (mb->mbxStatus) {
 		/* RegLogin failed */
@@ -2071,7 +2072,13 @@ lpfc_cmpl_reglogin_reglogin_issue(struct lpfc_vport *vport,
 
 		ndlp->nlp_prev_state = NLP_STE_REG_LOGIN_ISSUE;
 		lpfc_nlp_set_state(vport, ndlp, NLP_STE_PRLI_ISSUE);
-		if (lpfc_issue_els_prli(vport, ndlp, 0)) {
+		rc = lpfc_issue_els_prli(vport, ndlp, 0);
+		if (rc) {
+			lpfc_vlog_msg(vport, KERN_NOTICE,
+				      LOG_ELS | LOG_DISCOVERY | LOG_NODE,
+				      "3015 PRLI Issue returning %d to DID "
+				      "x%06x, Send LOGO\n",
+				      rc, ndlp->nlp_DID);
 			lpfc_issue_els_logo(vport, ndlp, 0);
 			ndlp->nlp_prev_state = NLP_STE_REG_LOGIN_ISSUE;
 			lpfc_nlp_set_state(vport, ndlp, NLP_STE_NPR_NODE);
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 424ad15416ef..8bbe269e8a3d 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -243,6 +243,38 @@ lpfc_sli4_pcimem_bcopy(void *srcp, void *destp, uint32_t cnt)
 #define lpfc_sli4_pcimem_bcopy(a, b, c) lpfc_sli_pcimem_bcopy(a, b, c)
 #endif
 
+/**
+ * lpfc_sli4_queue_io_for_retry - Put an ELS/CT IO request on the txq.
+ * @phba: pointer to the hba instance.
+ * @iocb: the IO request to put on the txq.
+ * @head: A boolean to request a put to the head (true) or tail (false) of the
+ *        txq.
+ *
+ * This routine is for SLI4 interfaces only.
+ * When ELS/CT commands can't be sent because of ELS WQ full put status,
+ * queue the iocbq on the phba->txq, at the head or tail as requested by
+ * the caller, for a retry later in lpfc_drain_txq. The lpfc_drain_txq
+ * routine is called per ELS CQE completion.
+ */
+void lpfc_sli4_queue_io_for_retry(struct lpfc_hba *phba,
+				  struct lpfc_iocbq *iocb,
+				  bool head)
+{
+	unsigned long iflags;
+	struct lpfc_sli_ring *pring;
+
+	/* Mark the IO as in RETRY to stop a full reprep of the SGL/XRI. */
+	iocb->cmd_flag |= LPFC_IO_IN_RETRY;
+	pring = phba->sli4_hba.els_wq->pring;
+
+	/* Insert to the head or tail of the txq ring as indicated
+	 * by the caller's head argument.
+	 */
+	spin_lock_irqsave(&pring->ring_lock, iflags);
+	__lpfc_sli_ringtx_put(phba, pring, iocb, head);
+	spin_unlock_irqrestore(&pring->ring_lock, iflags);
+}
+
 /**
  * lpfc_sli4_wq_put - Put a Work Queue Entry on an Work Queue
  * @q: The Work Queue to operate on.
@@ -275,6 +307,10 @@ lpfc_sli4_wq_put(struct lpfc_queue *q, union lpfc_wqe128 *wqe)
 	/* If the host has not yet processed the next entry then we are done */
 	idx = ((q->host_index + 1) % q->entry_count);
 	if (idx == q->hba_index) {
+		lpfc_printf_log(q->phba, KERN_WARNING, LOG_SLI,
+				"9998 No available WQ Slots on "
+				"q_id x%x, host x%x, hba x%x\n",
+				q->queue_id, idx, q->hba_index);
 		q->WQ_overflow++;
 		return -EBUSY;
 	}
@@ -10419,6 +10455,7 @@ lpfc_mbox_api_table_setup(struct lpfc_hba *phba, uint8_t dev_grp)
  * @phba: Pointer to HBA context object.
  * @pring: Pointer to driver SLI ring object.
  * @piocb: Pointer to address of newly added command iocb.
+ * @head: put at head (true) or tail (false)
  *
  * This function is called with hbalock held for SLI3 ports or
  * the ring lock held for SLI4 ports to add a command
@@ -10427,14 +10464,20 @@ lpfc_mbox_api_table_setup(struct lpfc_hba *phba, uint8_t dev_grp)
  **/
 void
 __lpfc_sli_ringtx_put(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
-		    struct lpfc_iocbq *piocb)
+		      struct lpfc_iocbq *piocb, bool head)
 {
 	if (phba->sli_rev == LPFC_SLI_REV4)
 		lockdep_assert_held(&pring->ring_lock);
 	else
 		lockdep_assert_held(&phba->hbalock);
-	/* Insert the caller's iocb in the txq tail for later processing. */
-	list_add_tail(&piocb->list, &pring->txq);
+
+	/* Insert the caller's iocb in the txq head or tail for later
+	 * processing.
+	 */
+	if (head)
+		list_add(&piocb->list, &pring->txq);
+	else
+		list_add_tail(&piocb->list, &pring->txq);
 }
 
 /**
@@ -10587,7 +10630,7 @@ __lpfc_sli_issue_iocb_s3(struct lpfc_hba *phba, uint32_t ring_number,
  out_busy:
 
 	if (!(flag & SLI_IOCB_RET_IOCB)) {
-		__lpfc_sli_ringtx_put(phba, pring, piocb);
+		__lpfc_sli_ringtx_put(phba, pring, piocb, false);
 		return IOCB_SUCCESS;
 	}
 
@@ -10725,6 +10768,7 @@ __lpfc_sli_issue_iocb_s4(struct lpfc_hba *phba, uint32_t ring_number,
 	struct lpfc_queue *wq;
 	struct lpfc_sli_ring *pring;
 	u32 ulp_command = get_job_cmnd(phba, piocb);
+	int rc = IOCB_SUCCESS;
 
 	/* Get the WQ */
 	if ((piocb->cmd_flag & LPFC_IO_FCP) ||
@@ -10740,9 +10784,11 @@ __lpfc_sli_issue_iocb_s4(struct lpfc_hba *phba, uint32_t ring_number,
 	/*
 	 * The WQE can be either 64 or 128 bytes,
 	 */
-
 	lockdep_assert_held(&pring->ring_lock);
 	wqe = &piocb->wqe;
+	if (piocb->cmd_flag & LPFC_IO_IN_RETRY)
+		goto retry_io;
+
 	if (piocb->sli4_xritag == NO_XRI) {
 		if (ulp_command == CMD_ABORT_XRI_CX)
 			sglq = NULL;
@@ -10752,7 +10798,7 @@ __lpfc_sli_issue_iocb_s4(struct lpfc_hba *phba, uint32_t ring_number,
 				if (!(flag & SLI_IOCB_RET_IOCB)) {
 					__lpfc_sli_ringtx_put(phba,
 							pring,
-							piocb);
+							piocb, false);
 					return IOCB_SUCCESS;
 				} else {
 					return IOCB_BUSY;
@@ -10793,12 +10839,18 @@ __lpfc_sli_issue_iocb_s4(struct lpfc_hba *phba, uint32_t ring_number,
 			return IOCB_ERROR;
 	}
 
-	if (lpfc_sli4_wq_put(wq, wqe))
-		return IOCB_ERROR;
+ retry_io:
+	piocb->cmd_flag &= ~LPFC_IO_IN_RETRY;
 
-	lpfc_sli_ringtxcmpl_put(phba, pring, piocb);
+	/* Push the wqe to the wq.  If the push fails with -EBUSY it means
+	 * the WQ is full.  Pass this status back to enable a retry.
+	 */
+	rc = lpfc_sli4_wq_put(wq, wqe);
+	if (rc == -EBUSY)
+		return IOCB_FAILED_PUT;
 
-	return 0;
+	lpfc_sli_ringtxcmpl_put(phba, pring, piocb);
+	return rc;
 }
 
 /*
@@ -21277,7 +21329,7 @@ lpfc_drain_txq(struct lpfc_hba *phba)
 
 		ret = __lpfc_sli_issue_iocb(phba, pring->ringno, piocbq, 0);
 
-		if (ret && ret != IOCB_BUSY) {
+		if (ret && ret != IOCB_BUSY && ret != IOCB_FAILED_PUT) {
 			fail_msg = " - Cannot send IO ";
 			piocbq->cmd_flag &= ~LPFC_DRIVER_ABORTED;
 		}
@@ -21293,9 +21345,35 @@ lpfc_drain_txq(struct lpfc_hba *phba)
 			list_add_tail(&piocbq->list, &completions);
 			fail_msg = NULL;
 		}
-		spin_unlock_irqrestore(&pring->ring_lock, iflags);
-		if (txq_cnt == 0 || ret == IOCB_BUSY)
+
+		if (txq_cnt == 0 || ret == IOCB_BUSY ||
+		    ret == IOCB_FAILED_PUT) {
+			/* If ELS WQ was full (IOCB_FAILED_PUT), then an SGL has
+			 * been assigned.  If SGL pool was empty (IOCB_BUSY),
+			 * then all we need is to put back on phba->txq to try
+			 * again later.
+			 */
+			lpfc_printf_log(phba, KERN_INFO, LOG_SLI,
+					"2820 IOCB iotag x%x xri x%x ret %d "
+					"cmd_flg x%x txq_cnt x%x\n",
+					piocbq->iotag, piocbq->sli4_xritag, ret,
+					piocbq->cmd_flag, txq_cnt);
+			switch (ret) {
+			case IOCB_FAILED_PUT:
+				piocbq->cmd_flag |= LPFC_IO_IN_RETRY;
+				__lpfc_sli_ringtx_put(phba, pring, piocbq,
+						      true);
+				break;
+			case IOCB_BUSY:
+				__lpfc_sli_ringtx_put(phba, pring, piocbq,
+						      true);
+				break;
+			}
+			spin_unlock_irqrestore(&pring->ring_lock, iflags);
 			break;
+		}
+
+		spin_unlock_irqrestore(&pring->ring_lock, iflags);
 	}
 	/* Cancel all the IOCBs that cannot be issued */
 	lpfc_sli_cancel_iocbs(phba, &completions, IOSTAT_LOCAL_REJECT,
diff --git a/drivers/scsi/lpfc/lpfc_sli.h b/drivers/scsi/lpfc/lpfc_sli.h
index cf7c42ec0306..86a7363a771a 100644
--- a/drivers/scsi/lpfc/lpfc_sli.h
+++ b/drivers/scsi/lpfc/lpfc_sli.h
@@ -1,7 +1,7 @@
 /*******************************************************************
  * This file is part of the Emulex Linux Device Driver for         *
  * Fibre Channel Host Bus Adapters.                                *
- * Copyright (C) 2017-2024 Broadcom. All Rights Reserved. The term *
+ * Copyright (C) 2017-2026 Broadcom. All Rights Reserved. The term *
  * “Broadcom” refers to Broadcom Inc. and/or its subsidiaries.     *
  * Copyright (C) 2004-2016 Emulex.  All rights reserved.           *
  * EMULEX and SLI are trademarks of Emulex.                        *
@@ -123,6 +123,7 @@ struct lpfc_iocbq {
 #define LPFC_IO_NVMET		0x800000 /* NVMET command */
 #define LPFC_IO_VMID            0x1000000 /* VMID tagged IO */
 #define LPFC_IO_CMF		0x4000000 /* CMF command */
+#define LPFC_IO_IN_RETRY	0x8000000 /* Caller is retrying IO. */
 
 	uint32_t drvrTimeout;	/* driver timeout in seconds */
 	struct lpfc_vport *vport;/* virtual port pointer */
@@ -160,6 +161,7 @@ struct lpfc_iocbq {
 #define IOCB_ABORTED        4
 #define IOCB_ABORTING	    5
 #define IOCB_NORESOURCE	    6
+#define IOCB_FAILED_PUT     7
 
 #define SLI_WQE_RET_WQE    1    /* Return WQE if cmd ring full */
 
-- 
2.38.0

[PATCH v3 12/14] lpfc: Update ELS ACC logging for diagnostic troubleshooting

[Justin Tee]

Currently, there are ELS ACC routines that lack debug log messages to
indicate when ACC frame transmission run into issues.  The generic
lpfc_els_rsp_acc and more specific ACC routines are updated to log when
there is an issue with transmitting the frame.  The routines are also
updated to return different return codes when encountering various
transmission issues and their function comment header is updated.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_els.c | 187 ++++++++++++++++++++++++++---------
 1 file changed, 141 insertions(+), 46 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 73552ce4bc64..0a7e69dcb2db 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -4037,13 +4037,8 @@ lpfc_els_rcv_rdf(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
 	int rc;
 
 	rc = lpfc_els_rsp_acc(vport, ELS_CMD_RDF, cmdiocb, ndlp, NULL);
-	/* Send LS_ACC */
-	if (rc) {
-		lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS | LOG_CGN_MGMT,
-				 "1623 Failed to RDF_ACC from x%x for x%x Data: %d\n",
-				 ndlp->nlp_DID, vport->fc_myDID, rc);
+	if (rc)
 		return -EIO;
-	}
 
 	rc = lpfc_issue_els_rdf(vport, 0);
 	/* Issue new RDF for reregistering */
@@ -5777,7 +5772,10 @@ lpfc_cmpl_els_rsp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
  *
  * Return code
  *   0 - Successfully issued acc response
- *   1 - Failed to issue acc response
+ *   -ENOMEM - IOCB not prepped successfully
+ *   -EIO - The IOCB failed to issue successfully
+ *   -ENODEV - No associated node for IOCB
+ *   -EACCES - ACC unhandled for this command
  **/
 int
 lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
@@ -5796,6 +5794,8 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 	int rc;
 	ELS_PKT *els_pkt_ptr;
 	struct fc_els_rdf_resp *rdf_resp;
+	int err;
+	uint32_t old_opcode = 0;
 
 	switch (flag) {
 	case ELS_CMD_ACC:
@@ -5804,7 +5804,8 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 					     ndlp, ndlp->nlp_DID, ELS_CMD_ACC);
 		if (!elsiocb) {
 			clear_bit(NLP_LOGO_ACC, &ndlp->nlp_flag);
-			return 1;
+			err = -ENOMEM;
+			goto err_out;
 		}
 
 		if (phba->sli_rev == LPFC_SLI_REV4) {
@@ -5839,8 +5840,10 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 		cmdsize = (sizeof(struct serv_parm) + sizeof(uint32_t));
 		elsiocb = lpfc_prep_els_iocb(vport, 0, cmdsize, oldiocb->retry,
 					     ndlp, ndlp->nlp_DID, ELS_CMD_ACC);
-		if (!elsiocb)
-			return 1;
+		if (!elsiocb) {
+			err = -ENOMEM;
+			goto err_out;
+		}
 
 		if (phba->sli_rev == LPFC_SLI_REV4) {
 			wqe = &elsiocb->wqe;
@@ -5917,8 +5920,10 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 		cmdsize = sizeof(uint32_t) + sizeof(PRLO);
 		elsiocb = lpfc_prep_els_iocb(vport, 0, cmdsize, oldiocb->retry,
 					     ndlp, ndlp->nlp_DID, ELS_CMD_PRLO);
-		if (!elsiocb)
-			return 1;
+		if (!elsiocb) {
+			err = -ENOMEM;
+			goto err_out;
+		}
 
 		if (phba->sli_rev == LPFC_SLI_REV4) {
 			wqe = &elsiocb->wqe;
@@ -5955,8 +5960,10 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 		cmdsize = sizeof(*rdf_resp);
 		elsiocb = lpfc_prep_els_iocb(vport, 0, cmdsize, oldiocb->retry,
 					     ndlp, ndlp->nlp_DID, ELS_CMD_ACC);
-		if (!elsiocb)
-			return 1;
+		if (!elsiocb) {
+			err = -ENOMEM;
+			goto err_out;
+		}
 
 		if (phba->sli_rev == LPFC_SLI_REV4) {
 			wqe = &elsiocb->wqe;
@@ -5991,7 +5998,8 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 		rdf_resp->lsri.rqst_w0.cmd = ELS_RDF;
 		break;
 	default:
-		return 1;
+		err = -EACCES;
+		goto err_out;
 	}
 	if (test_bit(NLP_LOGO_ACC, &ndlp->nlp_flag)) {
 		if (!test_bit(NLP_RPI_REGISTERED, &ndlp->nlp_flag) &&
@@ -6006,7 +6014,8 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 	elsiocb->ndlp = lpfc_nlp_get(ndlp);
 	if (!elsiocb->ndlp) {
 		lpfc_els_free_iocb(phba, elsiocb);
-		return 1;
+		err = -ENODEV;
+		goto err_out;
 	}
 
 	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
@@ -6027,7 +6036,8 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 
 		lpfc_els_free_iocb(phba, elsiocb);
 		lpfc_nlp_put(ndlp);
-		return 1;
+		err = -EIO;
+		goto err_out;
 	}
 
 	/* Xmit ELS ACC response tag <ulpIoTag> */
@@ -6039,6 +6049,17 @@ lpfc_els_rsp_acc(struct lpfc_vport *vport, uint32_t flag,
 			 ndlp->nlp_DID, ndlp->nlp_flag, ndlp->nlp_state,
 			 ndlp->nlp_rpi, vport->fc_flag, kref_read(&ndlp->kref));
 	return 0;
+
+err_out:
+	if (oldiocb->cmd_dmabuf && oldiocb->cmd_dmabuf->virt)
+		old_opcode = *(uint32_t *)oldiocb->cmd_dmabuf->virt;
+
+	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
+			 "1027 Xmit ELS ACC Unsuccessful: "
+			 "cmd: x%x, error_code: %d "
+			 "S_ID: x%x\n", old_opcode, err,
+			 vport->fc_myDID);
+	return err;
 }
 
 /**
@@ -6253,7 +6274,9 @@ lpfc_issue_els_edc_rsp(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
  *
  * Return code
  *   0 - Successfully issued acc adisc response
- *   1 - Failed to issue adisc acc response
+ *   -ENOMEM - IOCB not prepped successfully
+ *   -EIO - The IOCB failed to issue successfully
+ *   -ENODEV - No associated node for IOCB
  **/
 int
 lpfc_els_rsp_adisc_acc(struct lpfc_vport *vport, struct lpfc_iocbq *oldiocb,
@@ -6268,12 +6291,15 @@ lpfc_els_rsp_adisc_acc(struct lpfc_vport *vport, struct lpfc_iocbq *oldiocb,
 	uint16_t cmdsize;
 	int rc;
 	u32 ulp_context;
+	int err;
 
 	cmdsize = sizeof(uint32_t) + sizeof(ADISC);
 	elsiocb = lpfc_prep_els_iocb(vport, 0, cmdsize, oldiocb->retry, ndlp,
 				     ndlp->nlp_DID, ELS_CMD_ACC);
-	if (!elsiocb)
-		return 1;
+	if (!elsiocb) {
+		err = -ENOMEM;
+		goto err_out;
+	}
 
 	if (phba->sli_rev == LPFC_SLI_REV4) {
 		wqe = &elsiocb->wqe;
@@ -6320,17 +6346,26 @@ lpfc_els_rsp_adisc_acc(struct lpfc_vport *vport, struct lpfc_iocbq *oldiocb,
 	elsiocb->ndlp = lpfc_nlp_get(ndlp);
 	if (!elsiocb->ndlp) {
 		lpfc_els_free_iocb(phba, elsiocb);
-		return 1;
+		err = -ENODEV;
+		goto err_out;
 	}
 
 	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
 	if (rc == IOCB_ERROR) {
 		lpfc_els_free_iocb(phba, elsiocb);
 		lpfc_nlp_put(ndlp);
-		return 1;
+		err = -EIO;
+		goto err_out;
 	}
 
 	return 0;
+
+err_out:
+	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
+			 "1025 Xmit ADISC ACC Unsuccessful: "
+			 "error_code: %d S_ID: x%x\n",
+			 err, vport->fc_myDID);
+	return err;
 }
 
 /**
@@ -6350,7 +6385,10 @@ lpfc_els_rsp_adisc_acc(struct lpfc_vport *vport, struct lpfc_iocbq *oldiocb,
  *
  * Return code
  *   0 - Successfully issued acc prli response
- *   1 - Failed to issue acc prli response
+ *   -ENOMEM - IOCB not prepped successfully
+ *   -EIO - The IOCB failed to issue successfully
+ *   -ENODEV - No associated node for IOCB
+ *   -EACCES - Acc not needed for this command
  **/
 int
 lpfc_els_rsp_prli_acc(struct lpfc_vport *vport, struct lpfc_iocbq *oldiocb,
@@ -6370,6 +6408,7 @@ lpfc_els_rsp_prli_acc(struct lpfc_vport *vport, struct lpfc_iocbq *oldiocb,
 	struct lpfc_dmabuf *req_buf;
 	int rc;
 	u32 elsrspcmd, ulp_context;
+	int err;
 
 	/* Need the incoming PRLI payload to determine if the ACC is for an
 	 * FC4 or NVME PRLI type.  The PRLI type is at word 1.
@@ -6391,13 +6430,16 @@ lpfc_els_rsp_prli_acc(struct lpfc_vport *vport, struct lpfc_iocbq *oldiocb,
 		cmdsize = sizeof(uint32_t) + sizeof(struct lpfc_nvme_prli);
 		elsrspcmd = (ELS_CMD_ACC | (ELS_CMD_NVMEPRLI & ~ELS_RSP_MASK));
 	} else {
-		return 1;
+		err = -EACCES;
+		goto err_out;
 	}
 
 	elsiocb = lpfc_prep_els_iocb(vport, 0, cmdsize, oldiocb->retry, ndlp,
 				     ndlp->nlp_DID, elsrspcmd);
-	if (!elsiocb)
-		return 1;
+	if (!elsiocb) {
+		err = -ENOMEM;
+		goto err_out;
+	}
 
 	if (phba->sli_rev == LPFC_SLI_REV4) {
 		wqe = &elsiocb->wqe;
@@ -6512,17 +6554,28 @@ lpfc_els_rsp_prli_acc(struct lpfc_vport *vport, struct lpfc_iocbq *oldiocb,
 	elsiocb->ndlp =  lpfc_nlp_get(ndlp);
 	if (!elsiocb->ndlp) {
 		lpfc_els_free_iocb(phba, elsiocb);
-		return 1;
+		err = -ENODEV;
+		goto err_out;
 	}
 
 	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
 	if (rc == IOCB_ERROR) {
 		lpfc_els_free_iocb(phba, elsiocb);
 		lpfc_nlp_put(ndlp);
-		return 1;
+		err = -EIO;
+		goto err_out;
 	}
 
 	return 0;
+
+err_out:
+	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
+			 "1026 Xmit PRLI ACC Unsuccessful: "
+			 "cmd: x%x, error_code: %d "
+			 "S_ID: x%x\n",
+			 *(uint32_t *)oldiocb->cmd_dmabuf->virt, err,
+			 vport->fc_myDID);
+	return err;
 }
 
 /**
@@ -6543,7 +6596,9 @@ lpfc_els_rsp_prli_acc(struct lpfc_vport *vport, struct lpfc_iocbq *oldiocb,
  *
  * Return code
  *   0 - Successfully issued acc rnid response
- *   1 - Failed to issue acc rnid response
+ *   -ENOMEM - IOCB not prepped successfully
+ *   -EIO - The IOCB failed to issue successfully
+ *   -ENODEV - No associated node for IOCB
  **/
 static int
 lpfc_els_rsp_rnid_acc(struct lpfc_vport *vport, uint8_t format,
@@ -6558,6 +6613,7 @@ lpfc_els_rsp_rnid_acc(struct lpfc_vport *vport, uint8_t format,
 	uint16_t cmdsize;
 	int rc;
 	u32 ulp_context;
+	int err;
 
 	cmdsize = sizeof(uint32_t) + sizeof(uint32_t)
 					+ (2 * sizeof(struct lpfc_name));
@@ -6566,8 +6622,10 @@ lpfc_els_rsp_rnid_acc(struct lpfc_vport *vport, uint8_t format,
 
 	elsiocb = lpfc_prep_els_iocb(vport, 0, cmdsize, oldiocb->retry, ndlp,
 				     ndlp->nlp_DID, ELS_CMD_ACC);
-	if (!elsiocb)
-		return 1;
+	if (!elsiocb) {
+		err = -ENOMEM;
+		goto err_out;
+	}
 
 	if (phba->sli_rev == LPFC_SLI_REV4) {
 		wqe = &elsiocb->wqe;
@@ -6626,17 +6684,26 @@ lpfc_els_rsp_rnid_acc(struct lpfc_vport *vport, uint8_t format,
 	elsiocb->ndlp = lpfc_nlp_get(ndlp);
 	if (!elsiocb->ndlp) {
 		lpfc_els_free_iocb(phba, elsiocb);
-		return 1;
+		err = -ENODEV;
+		goto err_out;
 	}
 
 	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
 	if (rc == IOCB_ERROR) {
 		lpfc_els_free_iocb(phba, elsiocb);
 		lpfc_nlp_put(ndlp);
-		return 1;
+		err = -EIO;
+		goto err_out;
 	}
 
 	return 0;
+
+err_out:
+	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
+			 "1028 Xmit RNID ACC Unsuccessful: "
+			 "error_code: %d S_ID: x%x\n",
+			 err, vport->fc_myDID);
+	return err;
 }
 
 /**
@@ -6696,7 +6763,9 @@ lpfc_els_clear_rrq(struct lpfc_vport *vport,
  *
  * Return code
  *   0 - Successfully issued acc echo response
- *   1 - Failed to issue acc echo response
+ *   -ENOMEM - IOCB not prepped successfully
+ *   -EIO - The IOCB failed to issue successfully
+ *   -ENODEV - No associated node for IOCB
  **/
 static int
 lpfc_els_rsp_echo_acc(struct lpfc_vport *vport, uint8_t *data,
@@ -6710,6 +6779,7 @@ lpfc_els_rsp_echo_acc(struct lpfc_vport *vport, uint8_t *data,
 	uint16_t cmdsize;
 	int rc;
 	u32 ulp_context;
+	int err;
 
 	if (phba->sli_rev == LPFC_SLI_REV4)
 		cmdsize = oldiocb->wcqe_cmpl.total_data_placed;
@@ -6723,8 +6793,10 @@ lpfc_els_rsp_echo_acc(struct lpfc_vport *vport, uint8_t *data,
 		cmdsize = LPFC_BPL_SIZE;
 	elsiocb = lpfc_prep_els_iocb(vport, 0, cmdsize, oldiocb->retry, ndlp,
 				     ndlp->nlp_DID, ELS_CMD_ACC);
-	if (!elsiocb)
-		return 1;
+	if (!elsiocb) {
+		err = -ENOMEM;
+		goto err_out;
+	}
 
 	if (phba->sli_rev == LPFC_SLI_REV4) {
 		wqe = &elsiocb->wqe;
@@ -6760,17 +6832,26 @@ lpfc_els_rsp_echo_acc(struct lpfc_vport *vport, uint8_t *data,
 	elsiocb->ndlp =  lpfc_nlp_get(ndlp);
 	if (!elsiocb->ndlp) {
 		lpfc_els_free_iocb(phba, elsiocb);
-		return 1;
+		err = -ENODEV;
+		goto err_out;
 	}
 
 	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
 	if (rc == IOCB_ERROR) {
 		lpfc_els_free_iocb(phba, elsiocb);
 		lpfc_nlp_put(ndlp);
-		return 1;
+		err = -EIO;
+		goto err_out;
 	}
 
 	return 0;
+
+err_out:
+	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
+			 "1029 Xmit ECHO ACC Unsuccessful: "
+			 "error_code: %d S_ID: x%x\n",
+			 err, vport->fc_myDID);
+	return err;
 }
 
 /**
@@ -8473,14 +8554,14 @@ lpfc_els_rcv_rscn(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
 	vport->fc_rscn_id_list[vport->fc_rscn_id_cnt++] = pcmd;
 	/* Indicate we are done walking fc_rscn_id_list on this vport */
 	vport->fc_rscn_flush = 0;
+	/* Send back ACC */
+	lpfc_els_rsp_acc(vport, ELS_CMD_ACC, cmdiocb, ndlp, NULL);
 	/*
 	 * If we zero, cmdiocb->cmd_dmabuf, the calling routine will
 	 * not try to free it.
 	 */
 	cmdiocb->cmd_dmabuf = NULL;
 	lpfc_set_disctmo(vport);
-	/* Send back ACC */
-	lpfc_els_rsp_acc(vport, ELS_CMD_ACC, cmdiocb, ndlp, NULL);
 	/* send RECOVERY event for ALL nodes that match RSCN payload */
 	lpfc_rscn_recovery_check(vport);
 	return lpfc_els_handle_rscn(vport);
@@ -9280,7 +9361,9 @@ lpfc_send_rrq(struct lpfc_hba *phba, struct lpfc_node_rrq *rrq)
  *
  * Return code
  *   0 - Successfully issued ACC RPL ELS command
- *   1 - Failed to issue ACC RPL ELS command
+ *   -ENOMEM - IOCB not prepped successfully
+ *   -EIO - The IOCB failed to issue successfully
+ *   -ENODEV - No associated node for IOCB
  **/
 static int
 lpfc_els_rsp_rpl_acc(struct lpfc_vport *vport, uint16_t cmdsize,
@@ -9294,12 +9377,15 @@ lpfc_els_rsp_rpl_acc(struct lpfc_vport *vport, uint16_t cmdsize,
 	struct lpfc_iocbq *elsiocb;
 	uint8_t *pcmd;
 	u32 ulp_context;
+	int err;
 
 	elsiocb = lpfc_prep_els_iocb(vport, 0, cmdsize, oldiocb->retry, ndlp,
 				     ndlp->nlp_DID, ELS_CMD_ACC);
 
-	if (!elsiocb)
-		return 1;
+	if (!elsiocb) {
+		err = -ENOMEM;
+		goto err_out;
+	}
 
 	ulp_context = get_job_ulpcontext(phba, elsiocb);
 	if (phba->sli_rev == LPFC_SLI_REV4) {
@@ -9342,17 +9428,26 @@ lpfc_els_rsp_rpl_acc(struct lpfc_vport *vport, uint16_t cmdsize,
 	elsiocb->ndlp = lpfc_nlp_get(ndlp);
 	if (!elsiocb->ndlp) {
 		lpfc_els_free_iocb(phba, elsiocb);
-		return 1;
+		err = -ENODEV;
+		goto err_out;
 	}
 
 	rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
 	if (rc == IOCB_ERROR) {
 		lpfc_els_free_iocb(phba, elsiocb);
 		lpfc_nlp_put(ndlp);
-		return 1;
+		err = -EIO;
+		goto err_out;
 	}
 
 	return 0;
+
+err_out:
+	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
+			 "1030 Xmit ELS RPL ACC Unsuccessful: "
+			 "error_code: %d S_ID: x%x\n",
+			 err, vport->fc_myDID);
+	return err;
 }
 
 /**
@@ -9587,7 +9682,7 @@ lpfc_els_rcv_fan(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
  * @ndlp: pointer to a node-list data structure.
  *
  * Return code
- *   0 - Successfully processed echo iocb (currently always return 0)
+ *   0 - Successfully processed edc iocb (currently always return 0)
  **/
 static int
 lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
-- 
2.38.0

[PATCH v3 13/14] lpfc: Refactor calls on fc_disctmo to lpfc_set_disctmo in RSCN handler

[Justin Tee]

The lpfc_set_disctmo routine is not used for all cases when the driver
needs to restart discovery on the fc_disctmo timer.  Not doing so, makes
discovery timer actions invisible in some cases as they do not get logged.
This patch substitutes calls on fc_disctmo to use lpfc_set_disctmo in
lpfc_els_rcv_rscn.

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_els.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 0a7e69dcb2db..0c5524c613e5 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -8383,7 +8383,7 @@ lpfc_els_rcv_rscn(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
 	uint32_t payload_len, length, nportid, *cmd;
 	int rscn_cnt;
 	int rscn_id = 0, hba_id = 0;
-	int i, tmo;
+	int i;
 
 	pcmd = cmdiocb->cmd_dmabuf;
 	lp = (uint32_t *) pcmd->virt;
@@ -8462,11 +8462,8 @@ lpfc_els_rcv_rscn(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
 			lpfc_els_rsp_acc(vport, ELS_CMD_ACC, cmdiocb,
 				ndlp, NULL);
 			/* Restart disctmo if its already running */
-			if (test_bit(FC_DISC_TMO, &vport->fc_flag)) {
-				tmo = ((phba->fc_ratov * 3) + 3);
-				mod_timer(&vport->fc_disctmo,
-					  jiffies + secs_to_jiffies(tmo));
-			}
+			if (test_bit(FC_DISC_TMO, &vport->fc_flag))
+				lpfc_set_disctmo(vport);
 			return 0;
 		}
 	}
@@ -8497,11 +8494,9 @@ lpfc_els_rcv_rscn(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
 		set_bit(FC_RSCN_DEFERRED, &vport->fc_flag);
 
 		/* Restart disctmo if its already running */
-		if (test_bit(FC_DISC_TMO, &vport->fc_flag)) {
-			tmo = ((phba->fc_ratov * 3) + 3);
-			mod_timer(&vport->fc_disctmo,
-				  jiffies + secs_to_jiffies(tmo));
-		}
+		if (test_bit(FC_DISC_TMO, &vport->fc_flag))
+			lpfc_set_disctmo(vport);
+
 		if ((rscn_cnt < FC_MAX_HOLD_RSCN) &&
 		    !test_bit(FC_RSCN_DISCOVERY, &vport->fc_flag)) {
 			set_bit(FC_RSCN_MODE, &vport->fc_flag);
-- 
2.38.0

[PATCH v3 14/14] lpfc: Update lpfc version to 15.0.0.1

[Justin Tee]

Update lpfc version to 15.0.0.1

Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/scsi/lpfc/lpfc_version.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_version.h b/drivers/scsi/lpfc/lpfc_version.h
index d6e6e436fbfc..7df63d118234 100644
--- a/drivers/scsi/lpfc/lpfc_version.h
+++ b/drivers/scsi/lpfc/lpfc_version.h
@@ -20,7 +20,7 @@
  * included with this package.                                     *
  *******************************************************************/
 
-#define LPFC_DRIVER_VERSION "15.0.0.0"
+#define LPFC_DRIVER_VERSION "15.0.0.1"
 #define LPFC_DRIVER_NAME		"lpfc"
 
 /* Used for SLI 2/3 */
-- 
2.38.0