[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250611183033.4205-2-kartilak@cisco.com>
Date: Wed, 11 Jun 2025 11:30:30 -0700
From: Karan Tilak Kumar <kartilak@...co.com>
To: sebaddel@...co.com
Cc: arulponn@...co.com,
djhawar@...co.com,
gcboffa@...co.com,
mkai2@...co.com,
satishkh@...co.com,
aeasi@...co.com,
jejb@...ux.ibm.com,
martin.petersen@...cle.com,
linux-scsi@...r.kernel.org,
linux-kernel@...r.kernel.org,
jmeneghi@...hat.com,
revers@...hat.com,
dan.carpenter@...aro.org,
Karan Tilak Kumar <kartilak@...co.com>
Subject: [PATCH 2/5] scsi: fnic: Fix crash in fnic_wq_cmpl_handler when FDMI times out
When both the RHBA and RPA FDMI requests time out, fnic reuses a frame
to send ABTS for each of them. On send completion, this causes an
attempt to free the same frame twice that leads to a crash.
Fix crash by allocating separate frames for RHBA and RPA,
and modify ABTS logic accordingly.
Tested by checking MDS for FDMI information.
Tested by using instrumented driver to:
Drop PLOGI response
Drop RHBA response
Drop RPA response
Drop RHBA and RPA response
Drop PLOGI response + ABTS response
Drop RHBA response + ABTS response
Drop RPA response + ABTS response
Drop RHBA and RPA response + ABTS response for both of them
Reviewed-by: Sesidhar Baddela <sebaddel@...co.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@...co.com>
Reviewed-by: Gian Carlo Boffa <gcboffa@...co.com>
Tested-by: Arun Easi <aeasi@...co.com>
Co-developed-by: Arun Easi <aeasi@...co.com>
Signed-off-by: Arun Easi <aeasi@...co.com>
Tested-by: Karan Tilak Kumar <kartilak@...co.com>
Signed-off-by: Karan Tilak Kumar <kartilak@...co.com>
---
drivers/scsi/fnic/fdls_disc.c | 113 +++++++++++++++++++++++++---------
drivers/scsi/fnic/fnic_fdls.h | 1 +
2 files changed, 86 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/fnic/fdls_disc.c b/drivers/scsi/fnic/fdls_disc.c
index c2b6f4eb338e..0ee1b74967b9 100644
--- a/drivers/scsi/fnic/fdls_disc.c
+++ b/drivers/scsi/fnic/fdls_disc.c
@@ -763,47 +763,69 @@ static void fdls_send_fabric_abts(struct fnic_iport_s *iport)
iport->fabric.timer_pending = 1;
}
-static void fdls_send_fdmi_abts(struct fnic_iport_s *iport)
+static uint8_t *fdls_alloc_init_fdmi_abts_frame(struct fnic_iport_s *iport,
+ uint16_t oxid)
{
- uint8_t *frame;
+ struct fc_frame_header *pfdmi_abts;
uint8_t d_id[3];
+ uint8_t *frame;
struct fnic *fnic = iport->fnic;
- struct fc_frame_header *pfabric_abts;
- unsigned long fdmi_tov;
- uint16_t oxid;
- uint16_t frame_size = FNIC_ETH_FCOE_HDRS_OFFSET +
- sizeof(struct fc_frame_header);
frame = fdls_alloc_frame(iport);
if (frame == NULL) {
FNIC_FCS_DBG(KERN_ERR, fnic->host, fnic->fnic_num,
"Failed to allocate frame to send FDMI ABTS");
- return;
+ return NULL;
}
- pfabric_abts = (struct fc_frame_header *) (frame + FNIC_ETH_FCOE_HDRS_OFFSET);
+ pfdmi_abts = (struct fc_frame_header *) (frame + FNIC_ETH_FCOE_HDRS_OFFSET);
fdls_init_fabric_abts_frame(frame, iport);
hton24(d_id, FC_FID_MGMT_SERV);
- FNIC_STD_SET_D_ID(*pfabric_abts, d_id);
+ FNIC_STD_SET_D_ID(*pfdmi_abts, d_id);
+ FNIC_STD_SET_OX_ID(*pfdmi_abts, oxid);
+
+ return frame;
+}
+
+static void fdls_send_fdmi_abts(struct fnic_iport_s *iport)
+{
+ uint8_t *frame;
+ unsigned long fdmi_tov;
+ uint16_t frame_size = FNIC_ETH_FCOE_HDRS_OFFSET +
+ sizeof(struct fc_frame_header);
if (iport->fabric.fdmi_pending & FDLS_FDMI_PLOGI_PENDING) {
- oxid = iport->active_oxid_fdmi_plogi;
- FNIC_STD_SET_OX_ID(*pfabric_abts, oxid);
+ frame = fdls_alloc_init_fdmi_abts_frame(iport,
+ iport->active_oxid_fdmi_plogi);
+ if (frame == NULL)
+ return;
+
fnic_send_fcoe_frame(iport, frame, frame_size);
} else {
if (iport->fabric.fdmi_pending & FDLS_FDMI_REG_HBA_PENDING) {
- oxid = iport->active_oxid_fdmi_rhba;
- FNIC_STD_SET_OX_ID(*pfabric_abts, oxid);
+ frame = fdls_alloc_init_fdmi_abts_frame(iport,
+ iport->active_oxid_fdmi_rhba);
+ if (frame == NULL)
+ return;
+
fnic_send_fcoe_frame(iport, frame, frame_size);
}
if (iport->fabric.fdmi_pending & FDLS_FDMI_RPA_PENDING) {
- oxid = iport->active_oxid_fdmi_rpa;
- FNIC_STD_SET_OX_ID(*pfabric_abts, oxid);
+ frame = fdls_alloc_init_fdmi_abts_frame(iport,
+ iport->active_oxid_fdmi_rpa);
+ if (frame == NULL) {
+ if (iport->fabric.fdmi_pending & FDLS_FDMI_REG_HBA_PENDING)
+ goto arm_timer;
+ else
+ return;
+ }
+
fnic_send_fcoe_frame(iport, frame, frame_size);
}
}
+arm_timer:
fdmi_tov = jiffies + msecs_to_jiffies(2 * iport->e_d_tov);
mod_timer(&iport->fabric.fdmi_timer, round_jiffies(fdmi_tov));
iport->fabric.fdmi_pending |= FDLS_FDMI_ABORT_PENDING;
@@ -2244,6 +2266,21 @@ void fdls_fabric_timer_callback(struct timer_list *t)
spin_unlock_irqrestore(&fnic->fnic_lock, flags);
}
+void fdls_fdmi_retry_plogi(struct fnic_iport_s *iport)
+{
+ struct fnic *fnic = iport->fnic;
+
+ iport->fabric.fdmi_pending = 0;
+ /* If max retries not exhausted, start over from fdmi plogi */
+ if (iport->fabric.fdmi_retry < FDLS_FDMI_MAX_RETRY) {
+ iport->fabric.fdmi_retry++;
+ FNIC_FCS_DBG(KERN_INFO, fnic->host, fnic->fnic_num,
+ "Retry FDMI PLOGI. FDMI retry: %d",
+ iport->fabric.fdmi_retry);
+ fdls_send_fdmi_plogi(iport);
+ }
+}
+
void fdls_fdmi_timer_callback(struct timer_list *t)
{
struct fnic_fdls_fabric_s *fabric = from_timer(fabric, t, fdmi_timer);
@@ -2289,14 +2326,7 @@ void fdls_fdmi_timer_callback(struct timer_list *t)
FNIC_FCS_DBG(KERN_INFO, fnic->host, fnic->fnic_num,
"fdmi timer callback : 0x%x\n", iport->fabric.fdmi_pending);
- iport->fabric.fdmi_pending = 0;
- /* If max retries not exhaused, start over from fdmi plogi */
- if (iport->fabric.fdmi_retry < FDLS_FDMI_MAX_RETRY) {
- iport->fabric.fdmi_retry++;
- FNIC_FCS_DBG(KERN_INFO, fnic->host, fnic->fnic_num,
- "retry fdmi timer %d", iport->fabric.fdmi_retry);
- fdls_send_fdmi_plogi(iport);
- }
+ fdls_fdmi_retry_plogi(iport);
FNIC_FCS_DBG(KERN_INFO, fnic->host, fnic->fnic_num,
"fdmi timer callback : 0x%x\n", iport->fabric.fdmi_pending);
spin_unlock_irqrestore(&fnic->fnic_lock, flags);
@@ -3714,11 +3744,32 @@ static void fdls_process_fdmi_abts_rsp(struct fnic_iport_s *iport,
switch (FNIC_FRAME_TYPE(oxid)) {
case FNIC_FRAME_TYPE_FDMI_PLOGI:
fdls_free_oxid(iport, oxid, &iport->active_oxid_fdmi_plogi);
+
+ iport->fabric.fdmi_pending &= ~FDLS_FDMI_PLOGI_PENDING;
+ iport->fabric.fdmi_pending &= ~FDLS_FDMI_ABORT_PENDING;
break;
case FNIC_FRAME_TYPE_FDMI_RHBA:
+ iport->fabric.fdmi_pending &= ~FDLS_FDMI_REG_HBA_PENDING;
+
+ /* If RPA is still pending, don't turn off ABORT PENDING.
+ * We count on the timer to detect the ABTS timeout and take
+ * corrective action.
+ */
+ if (!(iport->fabric.fdmi_pending & FDLS_FDMI_RPA_PENDING))
+ iport->fabric.fdmi_pending &= ~FDLS_FDMI_ABORT_PENDING;
+
fdls_free_oxid(iport, oxid, &iport->active_oxid_fdmi_rhba);
break;
case FNIC_FRAME_TYPE_FDMI_RPA:
+ iport->fabric.fdmi_pending &= ~FDLS_FDMI_RPA_PENDING;
+
+ /* If RHBA is still pending, don't turn off ABORT PENDING.
+ * We count on the timer to detect the ABTS timeout and take
+ * corrective action.
+ */
+ if (!(iport->fabric.fdmi_pending & FDLS_FDMI_REG_HBA_PENDING))
+ iport->fabric.fdmi_pending &= ~FDLS_FDMI_ABORT_PENDING;
+
fdls_free_oxid(iport, oxid, &iport->active_oxid_fdmi_rpa);
break;
default:
@@ -3728,10 +3779,16 @@ static void fdls_process_fdmi_abts_rsp(struct fnic_iport_s *iport,
break;
}
- timer_delete_sync(&iport->fabric.fdmi_timer);
- iport->fabric.fdmi_pending &= ~FDLS_FDMI_ABORT_PENDING;
-
- fdls_send_fdmi_plogi(iport);
+ /*
+ * Only if ABORT PENDING is off, delete the timer, and if no other
+ * operations are pending, retry FDMI.
+ * Otherwise, let the timer pop and take the appropriate action.
+ */
+ if (!(iport->fabric.fdmi_pending & FDLS_FDMI_ABORT_PENDING)) {
+ timer_delete_sync(&iport->fabric.fdmi_timer);
+ if (!iport->fabric.fdmi_pending)
+ fdls_fdmi_retry_plogi(iport);
+ }
}
static void
diff --git a/drivers/scsi/fnic/fnic_fdls.h b/drivers/scsi/fnic/fnic_fdls.h
index 8e610b65ad57..531d0b37e450 100644
--- a/drivers/scsi/fnic/fnic_fdls.h
+++ b/drivers/scsi/fnic/fnic_fdls.h
@@ -394,6 +394,7 @@ void fdls_send_tport_abts(struct fnic_iport_s *iport,
bool fdls_delete_tport(struct fnic_iport_s *iport,
struct fnic_tport_s *tport);
void fdls_fdmi_timer_callback(struct timer_list *t);
+void fdls_fdmi_retry_plogi(struct fnic_iport_s *iport);
/* fnic_fcs.c */
void fnic_fdls_init(struct fnic *fnic, int usefip);
--
2.47.1
Powered by blists - more mailing lists