When the link goes down and comes up, FDMI requests are not sent out anymore. Fix bug by turning off FNIC_FDMI_ACTIVE when the link goes down.
Fixes: 09c1e6ab4ab2 ("scsi: fnic: Add and integrate support for FDMI") Reviewed-by: Sesidhar Baddela sebaddel@cisco.com Reviewed-by: Arulprabhu Ponnusamy arulponn@cisco.com Reviewed-by: Gian Carlo Boffa gcboffa@cisco.com Reviewed-by: Arun Easi aeasi@cisco.com Tested-by: Karan Tilak Kumar kartilak@cisco.com Cc: stable@vger.kernel.org Signed-off-by: Karan Tilak Kumar kartilak@cisco.com --- Changes between v3 and v4: - Incorporate review comments from Dan: - Remove comments from Cc tag
Changes between v2 and v3: - Incorporate review comments from Dan: - Add Cc to stable
Changes between v1 and v2: - Incorporate review comments from Dan: - Add Fixes tag --- drivers/scsi/fnic/fdls_disc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/fnic/fdls_disc.c b/drivers/scsi/fnic/fdls_disc.c index 9e9939d41fa8..14691db4d5f9 100644 --- a/drivers/scsi/fnic/fdls_disc.c +++ b/drivers/scsi/fnic/fdls_disc.c @@ -5078,9 +5078,12 @@ void fnic_fdls_link_down(struct fnic_iport_s *iport) fdls_delete_tport(iport, tport); }
- if ((fnic_fdmi_support == 1) && (iport->fabric.fdmi_pending > 0)) { - timer_delete_sync(&iport->fabric.fdmi_timer); - iport->fabric.fdmi_pending = 0; + if (fnic_fdmi_support == 1) { + if (iport->fabric.fdmi_pending > 0) { + timer_delete_sync(&iport->fabric.fdmi_timer); + iport->fabric.fdmi_pending = 0; + } + iport->flags &= ~FNIC_FDMI_ACTIVE; }
FNIC_FCS_DBG(KERN_INFO, fnic->host, fnic->fnic_num,
Hi Karan.
You've got two patches in this series with the same Fixes: tag.
[PATCH v4 2/5] scsi: fnic: Fix crash in fnic_wq_cmpl_handler when FDMI times out Fixes: 09c1e6ab4ab2 ("scsi: fnic: Add and integrate support for FDMI")
[PATCH v4 4/5] scsi: fnic: Turn off FDMI ACTIVE flags on link down Fixes: 09c1e6ab4ab2 ("scsi: fnic: Add and integrate support for FDMI")
both of these patches modify the same file:
drivers/scsi/fnic/fdls_disc.c
So I recommend you squash patch 4/5 and patch 2/5 into one.
Thanks,
/John
On 6/12/25 6:18 PM, Karan Tilak Kumar wrote:
When the link goes down and comes up, FDMI requests are not sent out anymore. Fix bug by turning off FNIC_FDMI_ACTIVE when the link goes down.
Fixes: 09c1e6ab4ab2 ("scsi: fnic: Add and integrate support for FDMI") Reviewed-by: Sesidhar Baddela sebaddel@cisco.com Reviewed-by: Arulprabhu Ponnusamy arulponn@cisco.com Reviewed-by: Gian Carlo Boffa gcboffa@cisco.com Reviewed-by: Arun Easi aeasi@cisco.com Tested-by: Karan Tilak Kumar kartilak@cisco.com Cc: stable@vger.kernel.org Signed-off-by: Karan Tilak Kumar kartilak@cisco.com
Changes between v3 and v4: - Incorporate review comments from Dan:
- Remove comments from Cc tag
Changes between v2 and v3: - Incorporate review comments from Dan:
- Add Cc to stable
Changes between v1 and v2: - Incorporate review comments from Dan:
- Add Fixes tag
drivers/scsi/fnic/fdls_disc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/fnic/fdls_disc.c b/drivers/scsi/fnic/fdls_disc.c index 9e9939d41fa8..14691db4d5f9 100644 --- a/drivers/scsi/fnic/fdls_disc.c +++ b/drivers/scsi/fnic/fdls_disc.c @@ -5078,9 +5078,12 @@ void fnic_fdls_link_down(struct fnic_iport_s *iport) fdls_delete_tport(iport, tport); }
- if ((fnic_fdmi_support == 1) && (iport->fabric.fdmi_pending > 0)) {
timer_delete_sync(&iport->fabric.fdmi_timer);iport->fabric.fdmi_pending = 0;
- if (fnic_fdmi_support == 1) {
if (iport->fabric.fdmi_pending > 0) {timer_delete_sync(&iport->fabric.fdmi_timer);iport->fabric.fdmi_pending = 0;}iport->flags &= ~FNIC_FDMI_ACTIVE;FNIC_FCS_DBG(KERN_INFO, fnic->host, fnic->fnic_num,
On Friday, June 13, 2025 1:29 PM, John Meneghini jmeneghi@redhat.com wrote:
Hi Karan.
You've got two patches in this series with the same Fixes: tag.
[PATCH v4 2/5] scsi: fnic: Fix crash in fnic_wq_cmpl_handler when FDMI times out Fixes: 09c1e6ab4ab2 ("scsi: fnic: Add and integrate support for FDMI")
[PATCH v4 4/5] scsi: fnic: Turn off FDMI ACTIVE flags on link down Fixes: 09c1e6ab4ab2 ("scsi: fnic: Add and integrate support for FDMI")
both of these patches modify the same file:
drivers/scsi/fnic/fdls_disc.c
So I recommend you squash patch 4/5 and patch 2/5 into one.
Thanks,
/John
Thanks for your proposal, John.
I'm a bit hesitant to squash them into one since each patch fixes unrelated bugs. The reason they have the same fixes tag is because we found two separate issues in the same commit.
I get your concern, however. What do you think about the following?
Move this patch to the beginning and increment driver version to 1.8.0.1: [PATCH v4 2/5] scsi: fnic: Fix crash in fnic_wq_cmpl_handler when FDMI times out
Move this patch to the next and increment driver version to 1.8.0.2: [PATCH v4 4/5] scsi: fnic: Turn off FDMI ACTIVE flags on link down
Move this patch to the next: [PATCH v4 3/5] scsi: fnic: Add and improve logs in FDMI and FDMI ABTS paths
Ideally, I would have liked to squash this above patch with the first patch in this series, since it's easier to debug if and when an issue occurs in that path. I separated them since it would be easier to review. I could add a Fixes tag here as well, but I'm not sure about that since they are just log messages, and they are not really fixes.
Move this patch to the end: [PATCH v4 1/5] scsi: fnic: Set appropriate logging level for log message
Regards, Karan
Sounds good to me. Please make these changes in your v5 patch series and I'll approve.
/John
On 6/13/25 5:59 PM, Karan Tilak Kumar (kartilak) wrote:
On Friday, June 13, 2025 1:29 PM, John Meneghini jmeneghi@redhat.com wrote:
Hi Karan.
You've got two patches in this series with the same Fixes: tag.
[PATCH v4 2/5] scsi: fnic: Fix crash in fnic_wq_cmpl_handler when FDMI times out Fixes: 09c1e6ab4ab2 ("scsi: fnic: Add and integrate support for FDMI")
[PATCH v4 4/5] scsi: fnic: Turn off FDMI ACTIVE flags on link down Fixes: 09c1e6ab4ab2 ("scsi: fnic: Add and integrate support for FDMI")
both of these patches modify the same file:
drivers/scsi/fnic/fdls_disc.c
So I recommend you squash patch 4/5 and patch 2/5 into one.
Thanks,
/John
Thanks for your proposal, John.
I'm a bit hesitant to squash them into one since each patch fixes unrelated bugs. The reason they have the same fixes tag is because we found two separate issues in the same commit.
I get your concern, however. What do you think about the following?
Move this patch to the beginning and increment driver version to 1.8.0.1: [PATCH v4 2/5] scsi: fnic: Fix crash in fnic_wq_cmpl_handler when FDMI times out
Move this patch to the next and increment driver version to 1.8.0.2: [PATCH v4 4/5] scsi: fnic: Turn off FDMI ACTIVE flags on link down
Move this patch to the next: [PATCH v4 3/5] scsi: fnic: Add and improve logs in FDMI and FDMI ABTS paths
Ideally, I would have liked to squash this above patch with the first patch in this series, since it's easier to debug if and when an issue occurs in that path. I separated them since it would be easier to review. I could add a Fixes tag here as well, but I'm not sure about that since they are just log messages, and they are not really fixes.
Move this patch to the end: [PATCH v4 1/5] scsi: fnic: Set appropriate logging level for log message
Regards, Karan
On Monday, June 16, 2025 7:36 AM, John Meneghini jmeneghi@redhat.com wrote:
Sounds good to me. Please make these changes in your v5 patch series and I'll approve.
/John
Thanks John. I've refactored the patches based on the outlined plan and posted V5.
Regards, Karan
linux-stable-mirror@lists.linaro.org
-
John Meneghini -
Karan Tilak Kumar -
Karan Tilak Kumar (kartilak)