linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] scsi: qla2xxx: Bug fixes
@ 2019-11-20 22:27 Roman Bolshakov
  2019-11-20 22:27 ` [PATCH v2 01/15] scsi: qla2xxx: Ignore NULL pointer in tcm_qla2xxx_free_mcmd Roman Bolshakov
                   ` (15 more replies)
  0 siblings, 16 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel; +Cc: linux, Roman Bolshakov


Hi Martin,

The patch series contains fixes for qla2xxx and solves two visible
issues:
  - Target port in N2N topology doesn't perform login if it has higher
    WWPN than initiator
  - ABORT TASK TMF leads to crash if it's received shortly after ACL of
    an initiator is deleted and there's active I/O from the initiator

It also contains reliability improvements and cleanups.

Unfortunately, there's still an issue the latest patch. The discard
works but ELS IOCB for LOGO is likely built incorrectly by
qla24xx_els_dcmd_iocb(). The issue can also be exposed when "1" is
written to fc_host/hostN/device/issue_logo file with logging turned on.

Changes since v1 (https://patchwork.kernel.org/cover/11141979/):
- Fixes target port in N2N mode were added (patches 5-11);
- Target port makes explicit LOGO on session teardown in the patch made
  by Quinn. Together with patch 1, it helps to immediately turn
  fc_remote_port to the Blocked stated on client side and avoids visibly
  stuck session;
- The last three patches address violation of FCP specification with
  regards to handling of ABTS-LS from ports that are not currently
  logged in.

Thank you,
Roman

Quinn Tran (1):
  scsi: qla2xxx: Use explicit LOGO in target mode

Roman Bolshakov (14):
  scsi: qla2xxx: Ignore NULL pointer in tcm_qla2xxx_free_mcmd
  scsi: qla2xxx: Initialize free_work before flushing it
  scsi: qla2xxx: Drop superfluous INIT_WORK of del_work
  scsi: qla2xxx: Change discovery state before PLOGI
  scsi: qla2xxx: Allow PLOGI in target mode
  scsi: qla2xxx: Don't call qlt_async_event twice
  scsi: qla2xxx: Fix PLOGI payload and ELS IOCB dump length
  scsi: qla2xxx: Configure local loop for N2N target
  scsi: qla2xxx: Send Notify ACK after N2N PLOGI
  scsi: qla2xxx: Don't defer relogin unconditonally
  scsi: qla2xxx: Ignore PORT UPDATE after N2N PLOGI
  scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
  scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB
  scsi: qla2xxx: Handle ABTS according to FCP spec for logged out ports

 drivers/scsi/qla2xxx/qla_attr.c    |  2 +-
 drivers/scsi/qla2xxx/qla_def.h     |  1 +
 drivers/scsi/qla2xxx/qla_gbl.h     |  2 +-
 drivers/scsi/qla2xxx/qla_init.c    | 21 ++++++---------
 drivers/scsi/qla2xxx/qla_iocb.c    | 42 ++++++++++++++++++++++++------
 drivers/scsi/qla2xxx/qla_isr.c     |  4 ---
 drivers/scsi/qla2xxx/qla_mbx.c     |  3 ++-
 drivers/scsi/qla2xxx/qla_target.c  | 34 ++++++++++++++++--------
 drivers/scsi/qla2xxx/tcm_qla2xxx.c |  3 +++
 9 files changed, 73 insertions(+), 39 deletions(-)

-- 
2.24.0


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2 01/15] scsi: qla2xxx: Ignore NULL pointer in tcm_qla2xxx_free_mcmd
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-20 22:27 ` [PATCH v2 02/15] scsi: qla2xxx: Initialize free_work before flushing it Roman Bolshakov
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Bart Van Assche,
	Thomas Abraham, stable, Himanshu Madhani

If ABTS cannot be completed in target mode, the driver attempts to free
related management command and crashes:

  NIP [d000000019181ee8] tcm_qla2xxx_free_mcmd+0x40/0x80 [tcm_qla2xxx]
  LR [d00000001dc1e6f8] qlt_response_pkt+0x190/0xa10 [qla2xxx]
  Call Trace:
  [c000003fff27bb50] [c000003fff27bc10] 0xc000003fff27bc10 (unreliable)
  [c000003fff27bb70] [d00000001dc1e6f8] qlt_response_pkt+0x190/0xa10 [qla2xxx]
  [c000003fff27bc10] [d00000001dbc2be0] qla24xx_process_response_queue+0x5d8/0xbd0 [qla2xxx]
  [c000003fff27bd50] [d00000001dbc632c] qla24xx_msix_rsp_q+0x64/0x150 [qla2xxx]
  [c000003fff27bde0] [c000000000187200] __handle_irq_event_percpu+0x90/0x310
  [c000003fff27bea0] [c0000000001874b8] handle_irq_event_percpu+0x38/0x90
  [c000003fff27bee0] [c000000000187574] handle_irq_event+0x64/0xb0
  [c000003fff27bf10] [c00000000018cd38] handle_fasteoi_irq+0xe8/0x280
  [c000003fff27bf40] [c000000000185ccc] generic_handle_irq+0x4c/0x70
  [c000003fff27bf60] [c000000000016cec] __do_irq+0x7c/0x1d0
  [c000003fff27bf90] [c00000000002a530] call_do_irq+0x14/0x24
  [c00000207d2cba90] [c000000000016edc] do_IRQ+0x9c/0x130
  [c00000207d2cbae0] [c000000000008bf4] hardware_interrupt_common+0x114/0x120
  --- interrupt: 501 at arch_local_irq_restore+0x74/0x90
      LR = arch_local_irq_restore+0x74/0x90
  [c00000207d2cbdd0] [c0000000001c64fc] tick_broadcast_oneshot_control+0x4c/0x60 (unreliable)
  [c00000207d2cbdf0] [c0000000007ac840] cpuidle_enter_state+0xf0/0x450
  [c00000207d2cbe50] [c00000000016b81c] call_cpuidle+0x4c/0x90
  [c00000207d2cbe70] [c00000000016bc30] do_idle+0x2b0/0x330
  [c00000207d2cbec0] [c00000000016beec] cpu_startup_entry+0x3c/0x50
  [c00000207d2cbef0] [c00000000004a06c] start_secondary+0x63c/0x670
  [c00000207d2cbf90] [c00000000000aa6c] start_secondary_prolog+0x10/0x14

The crash can be triggered by ACL deletion when there's active I/O.

During ACL deletion, qla2xxx performs implicit LOGO that's invisible for
the initiator. Only the driver and firmware are aware of the logout.
Therefore the initiator continues to send SCSI commands and the target
always responds with SAM STATUS BUSY as it can't find the session.

The command times out after a while and initiator invokes ABORT TASK TMF
for the command. The TMF is mapped to ABTS-LS in FCP. The target can't
find session for S_ID originating ABTS-LS so it never allocates mcmd.
And since N_Port handle was deleted after LOGO, it is no longer valid
and ABTS Response IOCB is returned from firmware with status 31. Then
free_mcmd is invoked on NULL pointer and the kernel crashes.

[ 7734.578642] qla2xxx [0000:00:0c.0]-e837:6: ABTS_RECV_24XX: instance 0
[ 7734.578644] qla2xxx [0000:00:0c.0]-f811:6: qla_target(0): task abort (s_id=1:2:0, tag=1209504, param=0)
[ 7734.578645] find_sess_by_s_id: 0x010200
[ 7734.578645] Unable to locate s_id: 0x010200
[ 7734.578646] qla2xxx [0000:00:0c.0]-f812:6: qla_target(0): task abort for non-existent session
[ 7734.578648] qla2xxx [0000:00:0c.0]-e806:6: Sending task mgmt ABTS response (ha=c0000000d5819000, atio=c0000000d3fd4700, status=4
[ 7734.578730] qla2xxx [0000:00:0c.0]-e838:6: ABTS_RESP_24XX: compl_status 31
[ 7734.578732] qla2xxx [0000:00:0c.0]-e863:6: qla_target(0): ABTS_RESP_24XX failed 31 (subcode 19:a)
[ 7734.578740] Unable to handle kernel paging request for data at address 0x00000200

Fixes: 6b0431d6fa20b ("scsi: qla2xxx: Fix out of order Termination and ABTS response")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Thomas Abraham <tabraham@suse.com>
Cc: stable@vger.kernel.org
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Acked-by: Himanshu Madhani <hmadhani@marvell.com>
---
 drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 042a24314edc..bab2073c1f72 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -246,6 +246,8 @@ static void tcm_qla2xxx_complete_mcmd(struct work_struct *work)
  */
 static void tcm_qla2xxx_free_mcmd(struct qla_tgt_mgmt_cmd *mcmd)
 {
+	if (!mcmd)
+		return;
 	INIT_WORK(&mcmd->free_work, tcm_qla2xxx_complete_mcmd);
 	queue_work(tcm_qla2xxx_free_wq, &mcmd->free_work);
 }
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 02/15] scsi: qla2xxx: Initialize free_work before flushing it
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
  2019-11-20 22:27 ` [PATCH v2 01/15] scsi: qla2xxx: Ignore NULL pointer in tcm_qla2xxx_free_mcmd Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-20 22:27 ` [PATCH v2 03/15] scsi: qla2xxx: Drop superfluous INIT_WORK of del_work Roman Bolshakov
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, stable, Himanshu Madhani

Target creation triggers a new BUG_ON introduced in in 4d43d395fed12
("workqueue: Try to catch flush_work() without INIT_WORK().").
The BUG_ON reveals an attempt to flush free_work in qla24xx_do_nack_work
before it's initialized in qlt_unreg_sess:

  WARNING: CPU: 7 PID: 211 at kernel/workqueue.c:3031 __flush_work.isra.38+0x40/0x2e0
  CPU: 7 PID: 211 Comm: kworker/7:1 Kdump: loaded Tainted: G            E     5.3.0-rc7-vanilla+ #2
  Workqueue: qla2xxx_wq qla2x00_iocb_work_fn [qla2xxx]
  NIP:  c000000000159620 LR: c0080000009d91b0 CTR: c0000000001598c0
  REGS: c000000005f3f730 TRAP: 0700   Tainted: G            E      (5.3.0-rc7-vanilla+)
  MSR:  800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24002222  XER: 00000000
  CFAR: c0000000001598d0 IRQMASK: 0
  GPR00: c0080000009d91b0 c000000005f3f9c0 c000000001670a00 c0000003f8655ca8
  GPR04: c0000003f8655c00 000000000000ffff 0000000000000011 ffffffffffffffff
  GPR08: c008000000949228 0000000000000000 0000000000000001 c0080000009e7780
  GPR12: 0000000000002200 c00000003fff6200 c000000000161bc8 0000000000000004
  GPR16: c0000003f9d68280 0000000002000000 0000000000000005 0000000000000003
  GPR20: 0000000000000002 000000000000ffff 0000000000000000 fffffffffffffef7
  GPR24: c000000004f73848 c000000004f73838 c000000004f73f28 c000000005f3fb60
  GPR28: c000000004f73e48 c000000004f73c80 c000000004f73818 c0000003f9d68280
  NIP [c000000000159620] __flush_work.isra.38+0x40/0x2e0
  LR [c0080000009d91b0] qla24xx_do_nack_work+0x88/0x180 [qla2xxx]
  Call Trace:
  [c000000005f3f9c0] [c000000000159644] __flush_work.isra.38+0x64/0x2e0 (unreliable)
  [c000000005f3fa50] [c0080000009d91a0] qla24xx_do_nack_work+0x78/0x180 [qla2xxx]
  [c000000005f3fae0] [c0080000009496ec] qla2x00_do_work+0x604/0xb90 [qla2xxx]
  [c000000005f3fc40] [c008000000949cd8] qla2x00_iocb_work_fn+0x60/0xe0 [qla2xxx]
  [c000000005f3fc80] [c000000000157bb8] process_one_work+0x2c8/0x5b0
  [c000000005f3fd10] [c000000000157f28] worker_thread+0x88/0x660
  [c000000005f3fdb0] [c000000000161d64] kthread+0x1a4/0x1b0
  [c000000005f3fe20] [c00000000000b960] ret_from_kernel_thread+0x5c/0x7c
  Instruction dump:
  3d22001d 892966b1 7d908026 91810008 f821ff71 69290001 0b090000 2e290000
  40920200 e9230018 7d2a0074 794ad182 <0b0a0000> 2fa90000 419e01e8 7c0802a6
  ---[ end trace 5ccf335d4f90fcb8 ]---

Fixes: 1021f0bc2f3d6 ("scsi: qla2xxx: allow session delete to finish before create.")
Cc: Quinn Tran <qutran@marvell.com>
Cc: stable@vger.kernel.org
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Acked-by: Himanshu Madhani <hmadhani@marvell.com>
---
 drivers/scsi/qla2xxx/qla_init.c   | 1 +
 drivers/scsi/qla2xxx/qla_target.c | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 1dbee8800218..4f3da968163e 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -4852,6 +4852,7 @@ qla2x00_alloc_fcport(scsi_qla_host_t *vha, gfp_t flags)
 	}
 
 	INIT_WORK(&fcport->del_work, qla24xx_delete_sess_fn);
+	INIT_WORK(&fcport->free_work, qlt_free_session_done);
 	INIT_WORK(&fcport->reg_work, qla_register_fcport_fn);
 	INIT_LIST_HEAD(&fcport->gnl_entry);
 	INIT_LIST_HEAD(&fcport->list);
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 51b275a575a5..7f470a314b1a 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1160,7 +1160,6 @@ void qlt_unreg_sess(struct fc_port *sess)
 	sess->last_rscn_gen = sess->rscn_gen;
 	sess->last_login_gen = sess->login_gen;
 
-	INIT_WORK(&sess->free_work, qlt_free_session_done);
 	queue_work(sess->vha->hw->wq, &sess->free_work);
 }
 EXPORT_SYMBOL(qlt_unreg_sess);
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 03/15] scsi: qla2xxx: Drop superfluous INIT_WORK of del_work
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
  2019-11-20 22:27 ` [PATCH v2 01/15] scsi: qla2xxx: Ignore NULL pointer in tcm_qla2xxx_free_mcmd Roman Bolshakov
  2019-11-20 22:27 ` [PATCH v2 02/15] scsi: qla2xxx: Initialize free_work before flushing it Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-20 22:27 ` [PATCH v2 04/15] scsi: qla2xxx: Change discovery state before PLOGI Roman Bolshakov
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Hannes Reinecke, Himanshu Madhani

del_work is already initialized inside qla2x00_alloc_fcport, there's no
need to overwrite it. Indeed, it might prevent complete traversal of
workqueue list.

Fixes: a01c77d2cbc45 ("scsi: qla2xxx: Move session delete to driver work queue")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Acked-by: Himanshu Madhani <hmadhani@marvell.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 7f470a314b1a..ab62fcba8ab3 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1264,7 +1264,6 @@ void qlt_schedule_sess_for_deletion(struct fc_port *sess)
 	    "Scheduling sess %p for deletion %8phC\n",
 	    sess, sess->port_name);
 
-	INIT_WORK(&sess->del_work, qla24xx_delete_sess_fn);
 	WARN_ON(!queue_work(sess->vha->hw->wq, &sess->del_work));
 }
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 04/15] scsi: qla2xxx: Change discovery state before PLOGI
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (2 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 03/15] scsi: qla2xxx: Drop superfluous INIT_WORK of del_work Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-20 22:27 ` [PATCH v2 05/15] scsi: qla2xxx: Allow PLOGI in target mode Roman Bolshakov
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, stable, Himanshu Madhani

When a port sends PLOGI, discovery state should be changed to login
pending, otherwise RELOGIN_NEEDED bit is set in
qla24xx_handle_plogi_done_event(). RELOGIN_NEEDED triggers another
PLOGI, and it never goes out of the loop until login timer expires.

Fixes: 8777e4314d397 ("scsi: qla2xxx: Migrate NVME N2N handling into state machine")
Fixes: 8b5292bcfcacf ("scsi: qla2xxx: Fix Relogin to prevent modifying scan_state flag")
Cc: Quinn Tran <qutran@marvell.com>
Cc: stable@vger.kernel.org
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Acked-by: Himanshu Madhani <hmadhani@marvell.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 4f3da968163e..fcb309be50d9 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -533,6 +533,7 @@ static int qla_post_els_plogi_work(struct scsi_qla_host *vha, fc_port_t *fcport)
 
 	e->u.fcport.fcport = fcport;
 	fcport->flags |= FCF_ASYNC_ACTIVE;
+	fcport->disc_state = DSC_LOGIN_PEND;
 	return qla2x00_post_work(vha, e);
 }
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 05/15] scsi: qla2xxx: Allow PLOGI in target mode
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (3 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 04/15] scsi: qla2xxx: Change discovery state before PLOGI Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-20 22:52   ` [EXT] " Quinn Tran
  2019-11-20 22:27 ` [PATCH v2 06/15] scsi: qla2xxx: Don't call qlt_async_event twice Roman Bolshakov
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Himanshu Madhani

According to FC-LS-3 (Fibre Channel Link Services) 6.3.2.4
"N_Port Login - No Fabric present", if both parties in the point-to-point
connection know N_Port_Names of each other, Nx_Port with the highest
N_Port_name shall transmit PLOGI. The specification sets no restrictions
on the port role that should send PLOGI.

However, FCP-4 (Fibre Channel Protocol for SCSI, Fourth Version) 6.2
"Overview of Process Login and Process Logout", instructs that in
point-to-point topology, initiator shall send explicit PRLI ELS.

The change fixes stuck P2P login, when target WWPN is higher than
initiator WWPN.

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index fcb309be50d9..12391815be06 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -1527,10 +1527,6 @@ int qla24xx_fcport_handle_login(struct scsi_qla_host *vha, fc_port_t *fcport)
 		}
 	}
 
-	/* for pure Target Mode. Login will not be initiated */
-	if (vha->host->active_mode == MODE_TARGET)
-		return 0;
-
 	if (fcport->flags & FCF_ASYNC_SENT) {
 		set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
 		return 0;
@@ -1720,6 +1716,10 @@ void qla24xx_handle_relogin_event(scsi_qla_host_t *vha,
 void qla_handle_els_plogi_done(scsi_qla_host_t *vha,
 				      struct event_arg *ea)
 {
+	/* for pure Target Mode, PRLI will not be initiated */
+	if (vha->host->active_mode == MODE_TARGET)
+		return;
+
 	ql_dbg(ql_dbg_disc, vha, 0x2118,
 	    "%s %d %8phC post PRLI\n",
 	    __func__, __LINE__, ea->fcport->port_name);
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 06/15] scsi: qla2xxx: Don't call qlt_async_event twice
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (4 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 05/15] scsi: qla2xxx: Allow PLOGI in target mode Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-21 15:43   ` Himanshu Madhani
  2019-11-20 22:27 ` [PATCH v2 07/15] scsi: qla2xxx: Fix PLOGI payload and ELS IOCB dump length Roman Bolshakov
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Himanshu Madhani

MBA_PORT_UPDATE generates duplicate log lines in target mode because
qlt_async_event is called twice. Drop the calls within the case as
the function will be called right after the switch statement.

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_isr.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 1b8f297449cf..c6274178b0b0 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1061,8 +1061,6 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
 			ql_dbg(ql_dbg_async, vha, 0x5011,
 			    "Asynchronous PORT UPDATE ignored %04x/%04x/%04x.\n",
 			    mb[1], mb[2], mb[3]);
-
-			qlt_async_event(mb[0], vha, mb);
 			break;
 		}
 
@@ -1079,8 +1077,6 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
 		set_bit(LOOP_RESYNC_NEEDED, &vha->dpc_flags);
 		set_bit(LOCAL_LOOP_UPDATE, &vha->dpc_flags);
 		set_bit(VP_CONFIG_OK, &vha->vp_flags);
-
-		qlt_async_event(mb[0], vha, mb);
 		break;
 
 	case MBA_RSCN_UPDATE:		/* State Change Registration */
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 07/15] scsi: qla2xxx: Fix PLOGI payload and ELS IOCB dump length
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (5 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 06/15] scsi: qla2xxx: Don't call qlt_async_event twice Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-21 16:39   ` [EXT] " Himanshu Madhani
  2019-11-20 22:27 ` [PATCH v2 08/15] scsi: qla2xxx: Configure local loop for N2N target Roman Bolshakov
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Himanshu Madhani

The size of the buffer is hardcoded as 0x70 or 112 bytes, while the
size of ELS IOCB is 0x40 and the size of PLOGI payload returned by
Get Parameters command is 0x74.

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_iocb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index b25f87ff8cde..bd8160fddcd3 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2676,7 +2676,8 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
 		ql_dbg(ql_dbg_io + ql_dbg_buffer, vha, 0x3073,
 		    "PLOGI ELS IOCB:\n");
 		ql_dump_buffer(ql_log_info, vha, 0x0109,
-		    (uint8_t *)els_iocb, 0x70);
+		    (uint8_t *)els_iocb,
+		    sizeof(*els_iocb));
 	} else {
 		els_iocb->control_flags = 1 << 13;
 		els_iocb->tx_byte_count =
@@ -2934,7 +2935,8 @@ qla24xx_els_dcmd2_iocb(scsi_qla_host_t *vha, int els_opcode,
 
 	ql_dbg(ql_dbg_disc + ql_dbg_buffer, vha, 0x3073, "PLOGI buffer:\n");
 	ql_dump_buffer(ql_dbg_disc + ql_dbg_buffer, vha, 0x0109,
-	    (uint8_t *)elsio->u.els_plogi.els_plogi_pyld, 0x70);
+	    (uint8_t *)elsio->u.els_plogi.els_plogi_pyld,
+	    sizeof(*elsio->u.els_plogi.els_plogi_pyld));
 
 	rval = qla2x00_start_sp(sp);
 	if (rval != QLA_SUCCESS) {
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 08/15] scsi: qla2xxx: Configure local loop for N2N target
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (6 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 07/15] scsi: qla2xxx: Fix PLOGI payload and ELS IOCB dump length Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-20 23:06   ` [EXT] " Quinn Tran
  2019-11-21 16:39   ` Himanshu Madhani
  2019-11-20 22:27 ` [PATCH v2 09/15] scsi: qla2xxx: Send Notify ACK after N2N PLOGI Roman Bolshakov
                   ` (7 subsequent siblings)
  15 siblings, 2 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Himanshu Madhani

qla2x00_configure_local_loop initializes PLOGI payload for PLOGI ELS
using Get Parameters mailbox command.

In the case when the driver is running in target mode, the topology is
N2N and the target port has higher WWPN, LOCAL_LOOP_UPDATE bit is
cleared too early and PLOGI payload is not initialized by the Get
Parameters command. That causes a failure of ELS IOCB carrying the
PLOGI with 0x15 aka Data Underrun error.

LOCAL_LOOP_UPDATE has to be set to initialize PLOGI payload.

Fixes: 48acad099074 ("scsi: qla2xxx: Fix N2N link re-connect")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 12391815be06..096f41fe17d2 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -4932,14 +4932,8 @@ qla2x00_configure_loop(scsi_qla_host_t *vha)
 		set_bit(RSCN_UPDATE, &flags);
 		clear_bit(LOCAL_LOOP_UPDATE, &flags);
 
-	} else if (ha->current_topology == ISP_CFG_N) {
-		clear_bit(RSCN_UPDATE, &flags);
-		if (qla_tgt_mode_enabled(vha)) {
-			/* allow the other side to start the login */
-			clear_bit(LOCAL_LOOP_UPDATE, &flags);
-			set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
-		}
-	} else if (ha->current_topology == ISP_CFG_NL) {
+	} else if (ha->current_topology == ISP_CFG_NL ||
+		   ha->current_topology == ISP_CFG_N) {
 		clear_bit(RSCN_UPDATE, &flags);
 		set_bit(LOCAL_LOOP_UPDATE, &flags);
 	} else if (!vha->flags.online ||
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 09/15] scsi: qla2xxx: Send Notify ACK after N2N PLOGI
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (7 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 08/15] scsi: qla2xxx: Configure local loop for N2N target Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-20 23:53   ` [EXT] " Quinn Tran
  2019-11-21 16:40   ` Himanshu Madhani
  2019-11-20 22:27 ` [PATCH v2 10/15] scsi: qla2xxx: Don't defer relogin unconditonally Roman Bolshakov
                   ` (6 subsequent siblings)
  15 siblings, 2 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Himanshu Madhani,
	Krishna Kant, Alexei Potashnik

qlt_handle_login schedules session for deletion even if a login is in
progress. That causes login bouncing, i.e. a few logins are made before
it settles down.

Complete the first login by sending Notify Acknowledge IOCB via
qlt_plogi_ack_unref if the session is pending login completion.

Fixes: 9cd883f07a54 ("scsi: qla2xxx: Fix session cleanup for N2N")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Cc: Krishna Kant <krishna.kant@purestorage.com>
Cc: Alexei Potashnik <alexei@purestorage.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index ab62fcba8ab3..853fa187d827 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -4802,6 +4802,7 @@ static int qlt_handle_login(struct scsi_qla_host *vha,
 
 	switch (sess->disc_state) {
 	case DSC_DELETED:
+	case DSC_LOGIN_PEND:
 		qlt_plogi_ack_unref(vha, pla);
 		break;
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 10/15] scsi: qla2xxx: Don't defer relogin unconditonally
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (8 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 09/15] scsi: qla2xxx: Send Notify ACK after N2N PLOGI Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-21  0:03   ` [EXT] " Quinn Tran
  2019-11-20 22:27 ` [PATCH v2 11/15] scsi: qla2xxx: Ignore PORT UPDATE after N2N PLOGI Roman Bolshakov
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Himanshu Madhani

qla2x00_configure_local_loop sets RELOGIN_NEEDED bit and calls
qla24xx_fcport_handle_login to perform the login. This bit triggers
a wake up of DPC later after a successful login.

The deferred call is not needed if login succeeds, and it's set in
qla24xx_fcport_handle_login in case of errors, hence it should be safe
to drop.

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 096f41fe17d2..e984746e7428 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -5050,7 +5050,6 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha)
 				memcpy(&ha->plogi_els_payld.data,
 				    (void *)ha->init_cb,
 				    sizeof(ha->plogi_els_payld.data));
-				set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
 			} else {
 				ql_dbg(ql_dbg_init, vha, 0x00d1,
 				    "PLOGI ELS param read fail.\n");
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 11/15] scsi: qla2xxx: Ignore PORT UPDATE after N2N PLOGI
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (9 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 10/15] scsi: qla2xxx: Don't defer relogin unconditonally Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-20 22:27 ` [PATCH v2 12/15] scsi: qla2xxx: Use explicit LOGO in target mode Roman Bolshakov
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Himanshu Madhani

PORT UPDATE asynchronous event is generated on the host that issues PLOGI
ELS (in the case of higher WWPN). In that case, the event shouldn't be
handled as it sets unwanted DPC flags (i.e. LOOP_RESYNC_NEEDED) that
trigger link flap.

Ignore the event if the host has higher WWPN, but handle otherwise.

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_mbx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 4eb88c3ee08e..bb6811b94683 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -3920,6 +3920,7 @@ qla24xx_report_id_acquisition(scsi_qla_host_t *vha,
 					vha->d_id.b24 = 0;
 					vha->d_id.b.al_pa = 1;
 					ha->flags.n2n_bigger = 1;
+					ha->flags.n2n_ae = 0;
 
 					id.b.al_pa = 2;
 					ql_dbg(ql_dbg_async, vha, 0x5075,
@@ -3930,6 +3931,7 @@ qla24xx_report_id_acquisition(scsi_qla_host_t *vha,
 					    "Format 1: Remote login - Waiting for WWPN %8phC.\n",
 					    rptid_entry->u.f1.port_name);
 					ha->flags.n2n_bigger = 0;
+					ha->flags.n2n_ae = 1;
 				}
 				qla24xx_post_newsess_work(vha, &id,
 				    rptid_entry->u.f1.port_name,
@@ -3941,7 +3943,6 @@ qla24xx_report_id_acquisition(scsi_qla_host_t *vha,
 			/* if our portname is higher then initiate N2N login */
 
 			set_bit(N2N_LOGIN_NEEDED, &vha->dpc_flags);
-			ha->flags.n2n_ae = 1;
 			return;
 			break;
 		case TOPO_FL:
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 12/15] scsi: qla2xxx: Use explicit LOGO in target mode
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (10 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 11/15] scsi: qla2xxx: Ignore PORT UPDATE after N2N PLOGI Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-23  4:58   ` kbuild test robot
  2019-11-20 22:27 ` [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb Roman Bolshakov
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Quinn Tran, Himanshu Madhani, Roman Bolshakov

From: Quinn Tran <qutran@marvell.com>

Target makes implicit LOGO on session teardown. LOGO ELS is not send on
the wire and initiator is not aware that target no longer wants talking
to it. Initiator keeps sending I/O requests, target responds with
BA_RJT, they time out and then initiator sends ABORT TASK (ABTS-LS).

Current behaviour incurs unneeded I/O timeout and can be fixed by making
explicit LOGO on session deletion.

Signed-off-by: Quinn Tran <qutran@marvell.com>
Signed-off-by: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_def.h     |  1 +
 drivers/scsi/qla2xxx/qla_iocb.c    | 16 ++++++++++++----
 drivers/scsi/qla2xxx/qla_target.c  |  1 +
 drivers/scsi/qla2xxx/tcm_qla2xxx.c |  1 +
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 460f443f6471..2edd9f7b3074 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -2401,6 +2401,7 @@ typedef struct fc_port {
 	unsigned int id_changed:1;
 	unsigned int scan_needed:1;
 	unsigned int n2n_flag:1;
+	unsigned int explicit_logout:1;
 
 	struct completion nvme_del_done;
 	uint32_t nvme_prli_service_param;
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index bd8160fddcd3..53ccbd6b71ed 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2405,11 +2405,19 @@ qla2x00_login_iocb(srb_t *sp, struct mbx_entry *mbx)
 static void
 qla24xx_logout_iocb(srb_t *sp, struct logio_entry_24xx *logio)
 {
+	u16 control_flags = LCF_COMMAND_LOGO;
 	logio->entry_type = LOGINOUT_PORT_IOCB_TYPE;
-	logio->control_flags =
-	    cpu_to_le16(LCF_COMMAND_LOGO|LCF_IMPL_LOGO);
-	if (!sp->fcport->keep_nport_handle)
-		logio->control_flags |= cpu_to_le16(LCF_FREE_NPORT);
+
+	if (sp->fcport->explicit_logout) {
+		control_flags |= LCF_EXPL_LOGO|LCF_FREE_NPORT;
+	} else {
+		control_flags |= LCF_IMPL_LOGO;
+
+		if (!sp->fcport->keep_nport_handle)
+			control_flags |= LCF_FREE_NPORT;
+	}
+
+	logio->control_flags = cpu_to_le16(control_flags);
 	logio->nport_handle = cpu_to_le16(sp->fcport->loop_id);
 	logio->port_id[0] = sp->fcport->d_id.b.al_pa;
 	logio->port_id[1] = sp->fcport->d_id.b.area;
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 853fa187d827..68c14143e50e 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1104,6 +1104,7 @@ void qlt_free_session_done(struct work_struct *work)
 		}
 	}
 
+	sess->explicit_logout = 0;
 	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
 	sess->free_pending = 0;
 
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index bab2073c1f72..abe7f79bb789 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -350,6 +350,7 @@ static void tcm_qla2xxx_close_session(struct se_session *se_sess)
 	target_sess_cmd_list_set_waiting(se_sess);
 	spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
 
+	sess->explicit_logout = 1;
 	tcm_qla2xxx_put_sess(sess);
 }
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (11 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 12/15] scsi: qla2xxx: Use explicit LOGO in target mode Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-21 22:50   ` [EXT] " Quinn Tran
  2019-11-20 22:27 ` [PATCH v2 14/15] scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB Roman Bolshakov
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel; +Cc: linux, Roman Bolshakov, Quinn Tran

qla24xx_els_dcmd_iocb() performs LOGO and might be invoked in contexts where it
is prohibited to sleep. The new wait parameter provides a way to use the
function in such context, similarly to qla24xx_els_dcmd2_iocb().

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_attr.c   |  2 +-
 drivers/scsi/qla2xxx/qla_gbl.h    |  2 +-
 drivers/scsi/qla2xxx/qla_iocb.c   | 11 +++++++++--
 drivers/scsi/qla2xxx/qla_target.c |  2 +-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 481c05dbea06..35c703ec59ad 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -828,7 +828,7 @@ qla2x00_issue_logo(struct file *filp, struct kobject *kobj,
 
 	ql_log(ql_log_info, vha, 0x70e4, "%s: %d\n", __func__, type);
 
-	qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did);
+	qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did, true);
 	return count;
 }
 
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index 5b163ad85c34..df0f3421e3bb 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -43,7 +43,7 @@ extern void qla2x00_clear_loop_id(fc_port_t *fcport);
 extern int qla2x00_fabric_login(scsi_qla_host_t *, fc_port_t *, uint16_t *);
 extern int qla2x00_local_device_login(scsi_qla_host_t *, fc_port_t *);
 
-extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t);
+extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t, bool);
 extern int qla24xx_els_dcmd2_iocb(scsi_qla_host_t *, int, fc_port_t *, bool);
 extern void qla2x00_els_dcmd2_free(scsi_qla_host_t *vha,
 				   struct els_plogi *els_plogi);
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 53ccbd6b71ed..12b37b711ae8 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2562,7 +2562,7 @@ static void qla2x00_els_dcmd_sp_done(srb_t *sp, int res)
 
 int
 qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
-    port_id_t remote_did)
+    port_id_t remote_did, bool wait)
 {
 	srb_t *sp;
 	fc_port_t *fcport = NULL;
@@ -2601,6 +2601,8 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
 	elsio->timeout = qla2x00_els_dcmd_iocb_timeout;
 	qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT);
 	init_completion(&sp->u.iocb_cmd.u.els_logo.comp);
+	if (wait)
+		sp->flags = SRB_WAKEUP_ON_COMP;
 	sp->done = qla2x00_els_dcmd_sp_done;
 	sp->free = qla2x00_els_dcmd_sp_free;
 
@@ -2637,9 +2639,14 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
 	    sp->name, sp->handle, fcport->loop_id, fcport->d_id.b.domain,
 	    fcport->d_id.b.area, fcport->d_id.b.al_pa);
 
-	wait_for_completion(&elsio->u.els_logo.comp);
+	if (wait) {
+		wait_for_completion(&elsio->u.els_logo.comp);
+	} else {
+		goto done;
+	}
 
 	sp->free(sp);
+done:
 	return rval;
 }
 
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 68c14143e50e..0f2bc4cd562f 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -932,7 +932,7 @@ qlt_send_first_logo(struct scsi_qla_host *vha, qlt_port_logo_t *logo)
 
 	mutex_unlock(&vha->vha_tgt.tgt_mutex);
 
-	res = qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, logo->id);
+	res = qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, logo->id, true);
 
 	mutex_lock(&vha->vha_tgt.tgt_mutex);
 	list_del(&logo->list);
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 14/15] scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (12 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-21 16:37   ` Himanshu Madhani
  2019-11-21 22:52   ` [EXT] " Quinn Tran
  2019-11-20 22:27 ` [PATCH v2 15/15] scsi: qla2xxx: Handle ABTS according to FCP spec for logged out ports Roman Bolshakov
  2019-11-22  9:14 ` [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Hannes Reinecke
  15 siblings, 2 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Himanshu Madhani

The change adds a way to debug LOGO ELS, likewise PLOGI.

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_iocb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 12b37b711ae8..cd2e0f89e9b5 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2627,6 +2627,10 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
 
 	memcpy(elsio->u.els_logo.els_logo_pyld, &logo_pyld,
 	    sizeof(struct els_logo_payload));
+	ql_dbg(ql_dbg_disc + ql_dbg_buffer, vha, 0x3075, "LOGO buffer:");
+	ql_dump_buffer(ql_dbg_disc + ql_dbg_buffer, vha, 0x010a,
+		       elsio->u.els_logo.els_logo_pyld,
+		       sizeof(*elsio->u.els_logo.els_logo_pyld));
 
 	rval = qla2x00_start_sp(sp);
 	if (rval != QLA_SUCCESS) {
@@ -2704,6 +2708,11 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
 		els_iocb->rx_byte_count = 0;
 		els_iocb->rx_address = 0;
 		els_iocb->rx_len = 0;
+		ql_dbg(ql_dbg_io + ql_dbg_buffer, vha, 0x3076,
+		       "LOGO ELS IOCB:");
+		ql_dump_buffer(ql_log_info, vha, 0x010b,
+			       els_iocb,
+			       sizeof(*els_iocb));
 	}
 
 	sp->vha->qla_stats.control_requests++;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 15/15] scsi: qla2xxx: Handle ABTS according to FCP spec for logged out ports
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (13 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 14/15] scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB Roman Bolshakov
@ 2019-11-20 22:27 ` Roman Bolshakov
  2019-11-23  5:57   ` kbuild test robot
  2019-11-22  9:14 ` [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Hannes Reinecke
  15 siblings, 1 reply; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-20 22:27 UTC (permalink / raw)
  To: linux-scsi, target-devel
  Cc: linux, Roman Bolshakov, Quinn Tran, Himanshu Madhani

Fibre Channel Protocol for SCSI, Fourth Version (FCP-4) 12.3.3, "Target
FCP_Port response to Exchange termination" states:

  When an ABTS-LS is received at the target FCP_Port, it shall abort the
  designated Exchange and return one of the following responses:

    a) the target FCP_Port shall discard the ABTS-LS and transmit a LOGO ELS if
       the Nx_Port issuing the ABTS-LS is not currently logged in (i.e., no
       N_Port Login exists);

However, current implementation tries to send a real BA_RJT in response to ABTS
from logged-out ports. The oparation fails because N_Port handle was freed
shortly after logout and ABTS Recieved IOCB contains 0xFFFF N_Port handle.
Existing implementation doesn't send LOGO at all.

The correct way to discard ABTS-LS frame when N_Port handle wasn't found is to
set Terminate ABTS Exchange bit in control flags of ABTS Response IOCB.
LOGO is sent after that.

To avoid a deadlock in qla2x00_start_sp(), hardware_lock should be released.
The unlocking should be safe because ABTS Recieved IOCB is not needed after
s_id is saved on the stack and qlt_24xx_send_abts_resp() is invoked.

Fixes: 2d70c103fd2a ("[SCSI] qla2xxx: Add LLD target-mode infrastructure for >= 24xx series")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 0f2bc4cd562f..331e7adf4b15 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1800,7 +1800,7 @@ static int qlt_build_abts_resp_iocb(struct qla_tgt_mgmt_cmd *mcmd)
  */
 static void qlt_24xx_send_abts_resp(struct qla_qpair *qpair,
 	struct abts_recv_from_24xx *abts, uint32_t status,
-	bool ids_reversed)
+	bool ids_reversed, bool term_exchange)
 {
 	struct scsi_qla_host *vha = qpair->vha;
 	struct qla_hw_data *ha = vha->hw;
@@ -1825,6 +1825,10 @@ static void qlt_24xx_send_abts_resp(struct qla_qpair *qpair,
 	resp->handle = QLA_TGT_SKIP_HANDLE;
 	resp->entry_count = 1;
 	resp->nport_handle = abts->nport_handle;
+	if (term_exchange)
+		resp->control_flags = cpu_to_le16(ABTS_CONTR_FLG_TERM_EXCHG);
+	else
+		resp->control_flags = 0;
 	resp->vp_index = vha->vp_idx;
 	resp->sof_type = abts->sof_type;
 	resp->exchange_address = abts->exchange_address;
@@ -1940,7 +1944,7 @@ static void qlt_24xx_retry_term_exchange(struct scsi_qla_host *vha,
 		qlt_build_abts_resp_iocb(mcmd);
 	else
 		qlt_24xx_send_abts_resp(qpair,
-		    (struct abts_recv_from_24xx *)entry, FCP_TMF_CMPL, true);
+		    (struct abts_recv_from_24xx *)entry, FCP_TMF_CMPL, true, false);
 
 }
 
@@ -2134,7 +2138,7 @@ static void qlt_24xx_handle_abts(struct scsi_qla_host *vha,
 		    "qla_target(%d): ABTS: Abort Sequence not "
 		    "supported\n", vha->vp_idx);
 		qlt_24xx_send_abts_resp(ha->base_qpair, abts, FCP_TMF_REJECTED,
-		    false);
+		    false, false);
 		return;
 	}
 
@@ -2143,7 +2147,7 @@ static void qlt_24xx_handle_abts(struct scsi_qla_host *vha,
 		    "qla_target(%d): ABTS: Unknown Exchange "
 		    "Address received\n", vha->vp_idx);
 		qlt_24xx_send_abts_resp(ha->base_qpair, abts, FCP_TMF_REJECTED,
-		    false);
+		    false, false);
 		return;
 	}
 
@@ -2163,8 +2167,16 @@ static void qlt_24xx_handle_abts(struct scsi_qla_host *vha,
 		    vha->vp_idx);
 		spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
 
+		/*
+		 * According to FCP-4 12.3.3,
+		 * ABTS-LS frame shall be discarded
+		 */
 		qlt_24xx_send_abts_resp(ha->base_qpair, abts, FCP_TMF_REJECTED,
-			    false);
+			    false, true);
+		spin_unlock_irqrestore(&ha->hardware_lock, flags);
+		/* and LOGO ELS transmitted */
+		qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, be_to_port_id(s_id), false);
+		spin_lock_irqsave(&ha->hardware_lock, flags);
 		return;
 	}
 	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
@@ -2172,7 +2184,7 @@ static void qlt_24xx_handle_abts(struct scsi_qla_host *vha,
 
 	if (sess->deleted) {
 		qlt_24xx_send_abts_resp(ha->base_qpair, abts, FCP_TMF_REJECTED,
-		    false);
+		    false, false);
 		return;
 	}
 
@@ -2182,7 +2194,7 @@ static void qlt_24xx_handle_abts(struct scsi_qla_host *vha,
 		    "qla_target(%d): __qlt_24xx_handle_abts() failed: %d\n",
 		    vha->vp_idx, rc);
 		qlt_24xx_send_abts_resp(ha->base_qpair, abts, FCP_TMF_REJECTED,
-		    false);
+		    false, false);
 		return;
 	}
 }
@@ -6213,7 +6225,7 @@ static void qlt_abort_work(struct qla_tgt *tgt,
 out_term:
 	spin_lock_irqsave(&ha->hardware_lock, flags);
 	qlt_24xx_send_abts_resp(ha->base_qpair, &prm->abts,
-	    FCP_TMF_REJECTED, false);
+	    FCP_TMF_REJECTED, false, false);
 	spin_unlock_irqrestore(&ha->hardware_lock, flags);
 }
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* RE: [EXT] [PATCH v2 05/15] scsi: qla2xxx: Allow PLOGI in target mode
  2019-11-20 22:27 ` [PATCH v2 05/15] scsi: qla2xxx: Allow PLOGI in target mode Roman Bolshakov
@ 2019-11-20 22:52   ` Quinn Tran
  2019-11-21 16:38     ` Himanshu Madhani
  0 siblings, 1 reply; 36+ messages in thread
From: Quinn Tran @ 2019-11-20 22:52 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux, Himanshu Madhani

According to FC-LS-3 (Fibre Channel Link Services) 6.3.2.4 "N_Port Login - No Fabric present", if both parties in the point-to-point connection know N_Port_Names of each other, Nx_Port with the highest N_Port_name shall transmit PLOGI. The specification sets no restrictions on the port role that should send PLOGI.

However, FCP-4 (Fibre Channel Protocol for SCSI, Fourth Version) 6.2 "Overview of Process Login and Process Logout", instructs that in point-to-point topology, initiator shall send explicit PRLI ELS.

The change fixes stuck P2P login, when target WWPN is higher than initiator WWPN.

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index fcb309be50d9..12391815be06 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -1527,10 +1527,6 @@ int qla24xx_fcport_handle_login(struct scsi_qla_host *vha, fc_port_t *fcport)
 		}
 	}
 
-	/* for pure Target Mode. Login will not be initiated */
-	if (vha->host->active_mode == MODE_TARGET)
-		return 0;
-
QT:  Nack.  Leave this hunk.  Instead do this

if (vha->host->active_mode == MODE_TARGET) && !N2N_TOPO(vha->hw)

--


 	if (fcport->flags & FCF_ASYNC_SENT) {
 		set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
 		return 0;
@@ -1720,6 +1716,10 @@ void qla24xx_handle_relogin_event(scsi_qla_host_t *vha,  void qla_handle_els_plogi_done(scsi_qla_host_t *vha,
 				      struct event_arg *ea)
 {
+	/* for pure Target Mode, PRLI will not be initiated */
+	if (vha->host->active_mode == MODE_TARGET)
+		return;
+
QT:  Ack.

 	ql_dbg(ql_dbg_disc, vha, 0x2118,
 	    "%s %d %8phC post PRLI\n",
 	    __func__, __LINE__, ea->fcport->port_name);
--
2.24.0


^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [EXT] [PATCH v2 08/15] scsi: qla2xxx: Configure local loop for N2N target
  2019-11-20 22:27 ` [PATCH v2 08/15] scsi: qla2xxx: Configure local loop for N2N target Roman Bolshakov
@ 2019-11-20 23:06   ` Quinn Tran
  2019-11-21 16:39   ` Himanshu Madhani
  1 sibling, 0 replies; 36+ messages in thread
From: Quinn Tran @ 2019-11-20 23:06 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux, Himanshu Madhani

qla2x00_configure_local_loop initializes PLOGI payload for PLOGI ELS using Get Parameters mailbox command.

In the case when the driver is running in target mode, the topology is N2N and the target port has higher WWPN, LOCAL_LOOP_UPDATE bit is cleared too early and PLOGI payload is not initialized by the Get Parameters command. That causes a failure of ELS IOCB carrying the PLOGI with 0x15 aka Data Underrun error.

LOCAL_LOOP_UPDATE has to be set to initialize PLOGI payload.

Fixes: 48acad099074 ("scsi: qla2xxx: Fix N2N link re-connect")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 12391815be06..096f41fe17d2 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -4932,14 +4932,8 @@ qla2x00_configure_loop(scsi_qla_host_t *vha)
 		set_bit(RSCN_UPDATE, &flags);
 		clear_bit(LOCAL_LOOP_UPDATE, &flags);
 
-	} else if (ha->current_topology == ISP_CFG_N) {
-		clear_bit(RSCN_UPDATE, &flags);
-		if (qla_tgt_mode_enabled(vha)) {
-			/* allow the other side to start the login */
-			clear_bit(LOCAL_LOOP_UPDATE, &flags);
-			set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
-		}
-	} else if (ha->current_topology == ISP_CFG_NL) {
+	} else if (ha->current_topology == ISP_CFG_NL ||
+		   ha->current_topology == ISP_CFG_N) {
 		clear_bit(RSCN_UPDATE, &flags);
 		set_bit(LOCAL_LOOP_UPDATE, &flags);
 	} else if (!vha->flags.online ||
--
2.24.0

QT: ACK.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [EXT] [PATCH v2 09/15] scsi: qla2xxx: Send Notify ACK after N2N PLOGI
  2019-11-20 22:27 ` [PATCH v2 09/15] scsi: qla2xxx: Send Notify ACK after N2N PLOGI Roman Bolshakov
@ 2019-11-20 23:53   ` Quinn Tran
  2019-11-21 16:40   ` Himanshu Madhani
  1 sibling, 0 replies; 36+ messages in thread
From: Quinn Tran @ 2019-11-20 23:53 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel
  Cc: linux, Himanshu Madhani, Krishna Kant, Alexei Potashnik

Acked-by: Quinn Tran <qutran@marvell.com>



-----Original Message-----
From: Roman Bolshakov <r.bolshakov@yadro.com> 
Sent: Wednesday, November 20, 2019 2:27 PM
To: linux-scsi@vger.kernel.org; target-devel@vger.kernel.org
Cc: linux@yadro.com; Roman Bolshakov <r.bolshakov@yadro.com>; Quinn Tran <qutran@marvell.com>; Himanshu Madhani <hmadhani@marvell.com>; Krishna Kant <krishna.kant@purestorage.com>; Alexei Potashnik <alexei@purestorage.com>
Subject: [EXT] [PATCH v2 09/15] scsi: qla2xxx: Send Notify ACK after N2N PLOGI

External Email

----------------------------------------------------------------------
qlt_handle_login schedules session for deletion even if a login is in progress. That causes login bouncing, i.e. a few logins are made before it settles down.

Complete the first login by sending Notify Acknowledge IOCB via qlt_plogi_ack_unref if the session is pending login completion.

Fixes: 9cd883f07a54 ("scsi: qla2xxx: Fix session cleanup for N2N")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Cc: Krishna Kant <krishna.kant@purestorage.com>
Cc: Alexei Potashnik <alexei@purestorage.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index ab62fcba8ab3..853fa187d827 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -4802,6 +4802,7 @@ static int qlt_handle_login(struct scsi_qla_host *vha,
 
 	switch (sess->disc_state) {
 	case DSC_DELETED:
+	case DSC_LOGIN_PEND:
 		qlt_plogi_ack_unref(vha, pla);
 		break;
 
--
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* RE: [EXT] [PATCH v2 10/15] scsi: qla2xxx: Don't defer relogin unconditonally
  2019-11-20 22:27 ` [PATCH v2 10/15] scsi: qla2xxx: Don't defer relogin unconditonally Roman Bolshakov
@ 2019-11-21  0:03   ` Quinn Tran
  0 siblings, 0 replies; 36+ messages in thread
From: Quinn Tran @ 2019-11-21  0:03 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux, Himanshu Madhani

Looks good.

Acked-by: Quinn Tran <qutran@marvell.com>


----------------------------------------------------------------------
qla2x00_configure_local_loop sets RELOGIN_NEEDED bit and calls qla24xx_fcport_handle_login to perform the login. This bit triggers a wake up of DPC later after a successful login.

The deferred call is not needed if login succeeds, and it's set in qla24xx_fcport_handle_login in case of errors, hence it should be safe to drop.

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 096f41fe17d2..e984746e7428 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -5050,7 +5050,6 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha)
 				memcpy(&ha->plogi_els_payld.data,
 				    (void *)ha->init_cb,
 				    sizeof(ha->plogi_els_payld.data));
-				set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
 			} else {
 				ql_dbg(ql_dbg_init, vha, 0x00d1,
 				    "PLOGI ELS param read fail.\n");
--
2.24.0


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 06/15] scsi: qla2xxx: Don't call qlt_async_event twice
  2019-11-20 22:27 ` [PATCH v2 06/15] scsi: qla2xxx: Don't call qlt_async_event twice Roman Bolshakov
@ 2019-11-21 15:43   ` Himanshu Madhani
  0 siblings, 0 replies; 36+ messages in thread
From: Himanshu Madhani @ 2019-11-21 15:43 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux, Quinn Tran



On 11/20/19, 4:27 PM, "target-devel-owner@vger.kernel.org on behalf of Roman Bolshakov" <target-devel-owner@vger.kernel.org on behalf of r.bolshakov@yadro.com> wrote:

    MBA_PORT_UPDATE generates duplicate log lines in target mode because
    qlt_async_event is called twice. Drop the calls within the case as
    the function will be called right after the switch statement.
    
    Cc: Quinn Tran <qutran@marvell.com>
    Cc: Himanshu Madhani <hmadhani@marvell.com>
    Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
    ---
     drivers/scsi/qla2xxx/qla_isr.c | 4 ----
     1 file changed, 4 deletions(-)
    
    diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
    index 1b8f297449cf..c6274178b0b0 100644
    --- a/drivers/scsi/qla2xxx/qla_isr.c
    +++ b/drivers/scsi/qla2xxx/qla_isr.c
    @@ -1061,8 +1061,6 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
     			ql_dbg(ql_dbg_async, vha, 0x5011,
     			    "Asynchronous PORT UPDATE ignored %04x/%04x/%04x.\n",
     			    mb[1], mb[2], mb[3]);
    -
    -			qlt_async_event(mb[0], vha, mb);
     			break;
     		}
     
    @@ -1079,8 +1077,6 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
     		set_bit(LOOP_RESYNC_NEEDED, &vha->dpc_flags);
     		set_bit(LOCAL_LOOP_UPDATE, &vha->dpc_flags);
     		set_bit(VP_CONFIG_OK, &vha->vp_flags);
    -
    -		qlt_async_event(mb[0], vha, mb);
     		break;
     
     	case MBA_RSCN_UPDATE:		/* State Change Registration */
    -- 
    2.24.0
    
    

Looks Good. 

Acked-by: Himanshu Madhani <hmadhani@marvel.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 14/15] scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB
  2019-11-20 22:27 ` [PATCH v2 14/15] scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB Roman Bolshakov
@ 2019-11-21 16:37   ` Himanshu Madhani
  2019-11-21 22:52   ` [EXT] " Quinn Tran
  1 sibling, 0 replies; 36+ messages in thread
From: Himanshu Madhani @ 2019-11-21 16:37 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux, Quinn Tran



On 11/20/19, 4:28 PM, "target-devel-owner@vger.kernel.org on behalf of Roman Bolshakov" <target-devel-owner@vger.kernel.org on behalf of r.bolshakov@yadro.com> wrote:

    The change adds a way to debug LOGO ELS, likewise PLOGI.
    
    Cc: Quinn Tran <qutran@marvell.com>
    Cc: Himanshu Madhani <hmadhani@marvell.com>
    Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
    ---
     drivers/scsi/qla2xxx/qla_iocb.c | 9 +++++++++
     1 file changed, 9 insertions(+)
    
    diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
    index 12b37b711ae8..cd2e0f89e9b5 100644
    --- a/drivers/scsi/qla2xxx/qla_iocb.c
    +++ b/drivers/scsi/qla2xxx/qla_iocb.c
    @@ -2627,6 +2627,10 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
     
     	memcpy(elsio->u.els_logo.els_logo_pyld, &logo_pyld,
     	    sizeof(struct els_logo_payload));
    +	ql_dbg(ql_dbg_disc + ql_dbg_buffer, vha, 0x3075, "LOGO buffer:");
    +	ql_dump_buffer(ql_dbg_disc + ql_dbg_buffer, vha, 0x010a,
    +		       elsio->u.els_logo.els_logo_pyld,
    +		       sizeof(*elsio->u.els_logo.els_logo_pyld));
     
     	rval = qla2x00_start_sp(sp);
     	if (rval != QLA_SUCCESS) {
    @@ -2704,6 +2708,11 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
     		els_iocb->rx_byte_count = 0;
     		els_iocb->rx_address = 0;
     		els_iocb->rx_len = 0;
    +		ql_dbg(ql_dbg_io + ql_dbg_buffer, vha, 0x3076,
    +		       "LOGO ELS IOCB:");
    +		ql_dump_buffer(ql_log_info, vha, 0x010b,
    +			       els_iocb,
    +			       sizeof(*els_iocb));
     	}
     
     	sp->vha->qla_stats.control_requests++;
    -- 
    2.24.0
    
Looks Good.

Acked-by: Himanshu Madhani <hmadhani@marvell.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [EXT] [PATCH v2 05/15] scsi: qla2xxx: Allow PLOGI in target mode
  2019-11-20 22:52   ` [EXT] " Quinn Tran
@ 2019-11-21 16:38     ` Himanshu Madhani
  0 siblings, 0 replies; 36+ messages in thread
From: Himanshu Madhani @ 2019-11-21 16:38 UTC (permalink / raw)
  To: Quinn Tran, Roman Bolshakov, linux-scsi, target-devel; +Cc: linux



On 11/20/19, 4:52 PM, "Quinn Tran" <qutran@marvell.com> wrote:

    According to FC-LS-3 (Fibre Channel Link Services) 6.3.2.4 "N_Port Login - No Fabric present", if both parties in the point-to-point connection know N_Port_Names of each other, Nx_Port with the highest N_Port_name shall transmit PLOGI. The specification sets no restrictions on the port role that should send PLOGI.
    
    However, FCP-4 (Fibre Channel Protocol for SCSI, Fourth Version) 6.2 "Overview of Process Login and Process Logout", instructs that in point-to-point topology, initiator shall send explicit PRLI ELS.
    
    The change fixes stuck P2P login, when target WWPN is higher than initiator WWPN.
    
    Cc: Quinn Tran <qutran@marvell.com>
    Cc: Himanshu Madhani <hmadhani@marvell.com>
    Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
    ---
     drivers/scsi/qla2xxx/qla_init.c | 8 ++++----
     1 file changed, 4 insertions(+), 4 deletions(-)
    
    diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index fcb309be50d9..12391815be06 100644
    --- a/drivers/scsi/qla2xxx/qla_init.c
    +++ b/drivers/scsi/qla2xxx/qla_init.c
    @@ -1527,10 +1527,6 @@ int qla24xx_fcport_handle_login(struct scsi_qla_host *vha, fc_port_t *fcport)
     		}
     	}
     
    -	/* for pure Target Mode. Login will not be initiated */
    -	if (vha->host->active_mode == MODE_TARGET)
    -		return 0;
    -
    QT:  Nack.  Leave this hunk.  Instead do this

    
    if (vha->host->active_mode == MODE_TARGET) && !N2N_TOPO(vha->hw)
    
    --

Agree. Please resend with update.
    
    
     	if (fcport->flags & FCF_ASYNC_SENT) {
     		set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
     		return 0;
    @@ -1720,6 +1716,10 @@ void qla24xx_handle_relogin_event(scsi_qla_host_t *vha,  void qla_handle_els_plogi_done(scsi_qla_host_t *vha,
     				      struct event_arg *ea)
     {
    +	/* for pure Target Mode, PRLI will not be initiated */
    +	if (vha->host->active_mode == MODE_TARGET)
    +		return;
    +
    QT:  Ack.
    
     	ql_dbg(ql_dbg_disc, vha, 0x2118,
     	    "%s %d %8phC post PRLI\n",
     	    __func__, __LINE__, ea->fcport->port_name);
    --
    2.24.0
    
    


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [EXT] [PATCH v2 08/15] scsi: qla2xxx: Configure local loop for N2N target
  2019-11-20 22:27 ` [PATCH v2 08/15] scsi: qla2xxx: Configure local loop for N2N target Roman Bolshakov
  2019-11-20 23:06   ` [EXT] " Quinn Tran
@ 2019-11-21 16:39   ` Himanshu Madhani
  1 sibling, 0 replies; 36+ messages in thread
From: Himanshu Madhani @ 2019-11-21 16:39 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux, Quinn Tran



On 11/20/19, 4:27 PM, "Roman Bolshakov" <r.bolshakov@yadro.com> wrote:

    External Email
    
    ----------------------------------------------------------------------
    qla2x00_configure_local_loop initializes PLOGI payload for PLOGI ELS
    using Get Parameters mailbox command.
    
    In the case when the driver is running in target mode, the topology is
    N2N and the target port has higher WWPN, LOCAL_LOOP_UPDATE bit is
    cleared too early and PLOGI payload is not initialized by the Get
    Parameters command. That causes a failure of ELS IOCB carrying the
    PLOGI with 0x15 aka Data Underrun error.
    
    LOCAL_LOOP_UPDATE has to be set to initialize PLOGI payload.
    
    Fixes: 48acad099074 ("scsi: qla2xxx: Fix N2N link re-connect")
    Cc: Quinn Tran <qutran@marvell.com>
    Cc: Himanshu Madhani <hmadhani@marvell.com>
    Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
    ---
     drivers/scsi/qla2xxx/qla_init.c | 10 ++--------
     1 file changed, 2 insertions(+), 8 deletions(-)
    
    diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
    index 12391815be06..096f41fe17d2 100644
    --- a/drivers/scsi/qla2xxx/qla_init.c
    +++ b/drivers/scsi/qla2xxx/qla_init.c
    @@ -4932,14 +4932,8 @@ qla2x00_configure_loop(scsi_qla_host_t *vha)
     		set_bit(RSCN_UPDATE, &flags);
     		clear_bit(LOCAL_LOOP_UPDATE, &flags);
     
    -	} else if (ha->current_topology == ISP_CFG_N) {
    -		clear_bit(RSCN_UPDATE, &flags);
    -		if (qla_tgt_mode_enabled(vha)) {
    -			/* allow the other side to start the login */
    -			clear_bit(LOCAL_LOOP_UPDATE, &flags);
    -			set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
    -		}
    -	} else if (ha->current_topology == ISP_CFG_NL) {
    +	} else if (ha->current_topology == ISP_CFG_NL ||
    +		   ha->current_topology == ISP_CFG_N) {
     		clear_bit(RSCN_UPDATE, &flags);
     		set_bit(LOCAL_LOOP_UPDATE, &flags);
     	} else if (!vha->flags.online ||
    -- 
    2.24.0
    
    
Looks Good.

Acked-by: Himanshu Madhani <hmadhani@marvell.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [EXT] [PATCH v2 07/15] scsi: qla2xxx: Fix PLOGI payload and ELS IOCB dump length
  2019-11-20 22:27 ` [PATCH v2 07/15] scsi: qla2xxx: Fix PLOGI payload and ELS IOCB dump length Roman Bolshakov
@ 2019-11-21 16:39   ` Himanshu Madhani
  0 siblings, 0 replies; 36+ messages in thread
From: Himanshu Madhani @ 2019-11-21 16:39 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux, Quinn Tran



On 11/20/19, 4:28 PM, "Roman Bolshakov" <r.bolshakov@yadro.com> wrote:

    External Email
    
    ----------------------------------------------------------------------
    The size of the buffer is hardcoded as 0x70 or 112 bytes, while the
    size of ELS IOCB is 0x40 and the size of PLOGI payload returned by
    Get Parameters command is 0x74.
    
    Cc: Quinn Tran <qutran@marvell.com>
    Cc: Himanshu Madhani <hmadhani@marvell.com>
    Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
    ---
     drivers/scsi/qla2xxx/qla_iocb.c | 6 ++++--
     1 file changed, 4 insertions(+), 2 deletions(-)
    
    diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
    index b25f87ff8cde..bd8160fddcd3 100644
    --- a/drivers/scsi/qla2xxx/qla_iocb.c
    +++ b/drivers/scsi/qla2xxx/qla_iocb.c
    @@ -2676,7 +2676,8 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
     		ql_dbg(ql_dbg_io + ql_dbg_buffer, vha, 0x3073,
     		    "PLOGI ELS IOCB:\n");
     		ql_dump_buffer(ql_log_info, vha, 0x0109,
    -		    (uint8_t *)els_iocb, 0x70);
    +		    (uint8_t *)els_iocb,
    +		    sizeof(*els_iocb));
     	} else {
     		els_iocb->control_flags = 1 << 13;
     		els_iocb->tx_byte_count =
    @@ -2934,7 +2935,8 @@ qla24xx_els_dcmd2_iocb(scsi_qla_host_t *vha, int els_opcode,
     
     	ql_dbg(ql_dbg_disc + ql_dbg_buffer, vha, 0x3073, "PLOGI buffer:\n");
     	ql_dump_buffer(ql_dbg_disc + ql_dbg_buffer, vha, 0x0109,
    -	    (uint8_t *)elsio->u.els_plogi.els_plogi_pyld, 0x70);
    +	    (uint8_t *)elsio->u.els_plogi.els_plogi_pyld,
    +	    sizeof(*elsio->u.els_plogi.els_plogi_pyld));
     
     	rval = qla2x00_start_sp(sp);
     	if (rval != QLA_SUCCESS) {
    -- 
    2.24.0
    
    
Looks Good.

Acked-by: Himanshu Madhani <hmadhani@marvell.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [EXT] [PATCH v2 09/15] scsi: qla2xxx: Send Notify ACK after N2N PLOGI
  2019-11-20 22:27 ` [PATCH v2 09/15] scsi: qla2xxx: Send Notify ACK after N2N PLOGI Roman Bolshakov
  2019-11-20 23:53   ` [EXT] " Quinn Tran
@ 2019-11-21 16:40   ` Himanshu Madhani
  1 sibling, 0 replies; 36+ messages in thread
From: Himanshu Madhani @ 2019-11-21 16:40 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel
  Cc: linux, Quinn Tran, Krishna Kant, Alexei Potashnik



On 11/20/19, 4:27 PM, "Roman Bolshakov" <r.bolshakov@yadro.com> wrote:

    External Email
    
    ----------------------------------------------------------------------
    qlt_handle_login schedules session for deletion even if a login is in
    progress. That causes login bouncing, i.e. a few logins are made before
    it settles down.
    
    Complete the first login by sending Notify Acknowledge IOCB via
    qlt_plogi_ack_unref if the session is pending login completion.
    
    Fixes: 9cd883f07a54 ("scsi: qla2xxx: Fix session cleanup for N2N")
    Cc: Quinn Tran <qutran@marvell.com>
    Cc: Himanshu Madhani <hmadhani@marvell.com>
    Cc: Krishna Kant <krishna.kant@purestorage.com>
    Cc: Alexei Potashnik <alexei@purestorage.com>
    Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
    ---
     drivers/scsi/qla2xxx/qla_target.c | 1 +
     1 file changed, 1 insertion(+)
    
    diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
    index ab62fcba8ab3..853fa187d827 100644
    --- a/drivers/scsi/qla2xxx/qla_target.c
    +++ b/drivers/scsi/qla2xxx/qla_target.c
    @@ -4802,6 +4802,7 @@ static int qlt_handle_login(struct scsi_qla_host *vha,
     
     	switch (sess->disc_state) {
     	case DSC_DELETED:
    +	case DSC_LOGIN_PEND:
     		qlt_plogi_ack_unref(vha, pla);
     		break;
     
    -- 
    2.24.0
    
    
Looks Good. 

Acked-by: Himanshu Madhani <hmadhani@marvell.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [EXT] [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
  2019-11-20 22:27 ` [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb Roman Bolshakov
@ 2019-11-21 22:50   ` Quinn Tran
  2019-11-22  5:04     ` Mark Harvey
  0 siblings, 1 reply; 36+ messages in thread
From: Quinn Tran @ 2019-11-21 22:50 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux

Looks good.

Acked-by: Quinn Tran <qutran@marvell.com>


Regards,
Quinn Tran

-----Original Message-----
From: Roman Bolshakov <r.bolshakov@yadro.com> 
Sent: Wednesday, November 20, 2019 2:27 PM
To: linux-scsi@vger.kernel.org; target-devel@vger.kernel.org
Cc: linux@yadro.com; Roman Bolshakov <r.bolshakov@yadro.com>; Quinn Tran <qutran@marvell.com>
Subject: [EXT] [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb

External Email

----------------------------------------------------------------------
qla24xx_els_dcmd_iocb() performs LOGO and might be invoked in contexts where it is prohibited to sleep. The new wait parameter provides a way to use the function in such context, similarly to qla24xx_els_dcmd2_iocb().

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_attr.c   |  2 +-
 drivers/scsi/qla2xxx/qla_gbl.h    |  2 +-
 drivers/scsi/qla2xxx/qla_iocb.c   | 11 +++++++++--
 drivers/scsi/qla2xxx/qla_target.c |  2 +-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c index 481c05dbea06..35c703ec59ad 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -828,7 +828,7 @@ qla2x00_issue_logo(struct file *filp, struct kobject *kobj,
 
 	ql_log(ql_log_info, vha, 0x70e4, "%s: %d\n", __func__, type);
 
-	qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did);
+	qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did, true);
 	return count;
 }
 
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index 5b163ad85c34..df0f3421e3bb 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -43,7 +43,7 @@ extern void qla2x00_clear_loop_id(fc_port_t *fcport);  extern int qla2x00_fabric_login(scsi_qla_host_t *, fc_port_t *, uint16_t *);  extern int qla2x00_local_device_login(scsi_qla_host_t *, fc_port_t *);
 
-extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t);
+extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t, 
+bool);
 extern int qla24xx_els_dcmd2_iocb(scsi_qla_host_t *, int, fc_port_t *, bool);  extern void qla2x00_els_dcmd2_free(scsi_qla_host_t *vha,
 				   struct els_plogi *els_plogi);
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c index 53ccbd6b71ed..12b37b711ae8 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2562,7 +2562,7 @@ static void qla2x00_els_dcmd_sp_done(srb_t *sp, int res)
 
 int
 qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
-    port_id_t remote_did)
+    port_id_t remote_did, bool wait)
 {
 	srb_t *sp;
 	fc_port_t *fcport = NULL;
@@ -2601,6 +2601,8 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
 	elsio->timeout = qla2x00_els_dcmd_iocb_timeout;
 	qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT);
 	init_completion(&sp->u.iocb_cmd.u.els_logo.comp);
+	if (wait)
+		sp->flags = SRB_WAKEUP_ON_COMP;
 	sp->done = qla2x00_els_dcmd_sp_done;
 	sp->free = qla2x00_els_dcmd_sp_free;
 
@@ -2637,9 +2639,14 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
 	    sp->name, sp->handle, fcport->loop_id, fcport->d_id.b.domain,
 	    fcport->d_id.b.area, fcport->d_id.b.al_pa);
 
-	wait_for_completion(&elsio->u.els_logo.comp);
+	if (wait) {
+		wait_for_completion(&elsio->u.els_logo.comp);
+	} else {
+		goto done;
+	}
 
 	sp->free(sp);
+done:
 	return rval;
 }
 
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 68c14143e50e..0f2bc4cd562f 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -932,7 +932,7 @@ qlt_send_first_logo(struct scsi_qla_host *vha, qlt_port_logo_t *logo)
 
 	mutex_unlock(&vha->vha_tgt.tgt_mutex);
 
-	res = qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, logo->id);
+	res = qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, logo->id, true);
 
 	mutex_lock(&vha->vha_tgt.tgt_mutex);
 	list_del(&logo->list);
--
2.24.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* RE: [EXT] [PATCH v2 14/15] scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB
  2019-11-20 22:27 ` [PATCH v2 14/15] scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB Roman Bolshakov
  2019-11-21 16:37   ` Himanshu Madhani
@ 2019-11-21 22:52   ` Quinn Tran
  1 sibling, 0 replies; 36+ messages in thread
From: Quinn Tran @ 2019-11-21 22:52 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux, Himanshu Madhani

Looks good.

Acked-by: Quinn Tran <qutran@marvell.com>



Regards,
Quinn Tran

-----Original Message-----
From: Roman Bolshakov <r.bolshakov@yadro.com> 
Sent: Wednesday, November 20, 2019 2:27 PM
To: linux-scsi@vger.kernel.org; target-devel@vger.kernel.org
Cc: linux@yadro.com; Roman Bolshakov <r.bolshakov@yadro.com>; Quinn Tran <qutran@marvell.com>; Himanshu Madhani <hmadhani@marvell.com>
Subject: [EXT] [PATCH v2 14/15] scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB

External Email

----------------------------------------------------------------------
The change adds a way to debug LOGO ELS, likewise PLOGI.

Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_iocb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c index 12b37b711ae8..cd2e0f89e9b5 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2627,6 +2627,10 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
 
 	memcpy(elsio->u.els_logo.els_logo_pyld, &logo_pyld,
 	    sizeof(struct els_logo_payload));
+	ql_dbg(ql_dbg_disc + ql_dbg_buffer, vha, 0x3075, "LOGO buffer:");
+	ql_dump_buffer(ql_dbg_disc + ql_dbg_buffer, vha, 0x010a,
+		       elsio->u.els_logo.els_logo_pyld,
+		       sizeof(*elsio->u.els_logo.els_logo_pyld));
 
 	rval = qla2x00_start_sp(sp);
 	if (rval != QLA_SUCCESS) {
@@ -2704,6 +2708,11 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
 		els_iocb->rx_byte_count = 0;
 		els_iocb->rx_address = 0;
 		els_iocb->rx_len = 0;
+		ql_dbg(ql_dbg_io + ql_dbg_buffer, vha, 0x3076,
+		       "LOGO ELS IOCB:");
+		ql_dump_buffer(ql_log_info, vha, 0x010b,
+			       els_iocb,
+			       sizeof(*els_iocb));
 	}
 
 	sp->vha->qla_stats.control_requests++;
--
2.24.0


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [EXT] [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
  2019-11-21 22:50   ` [EXT] " Quinn Tran
@ 2019-11-22  5:04     ` Mark Harvey
  2019-11-22 17:00       ` Quinn Tran
  2019-11-24 19:15       ` Roman Bolshakov
  0 siblings, 2 replies; 36+ messages in thread
From: Mark Harvey @ 2019-11-22  5:04 UTC (permalink / raw)
  To: Quinn Tran, Roman Bolshakov, linux-scsi, target-devel; +Cc: linux

Would this not result in a memory leak in the 'else' path - skiping sp->free(sp)?
  
  -	wait_for_completion(&elsio->u.els_logo.comp);
    +	if (wait) {
    +		wait_for_completion(&elsio->u.els_logo.comp);
    +	} else {
    +		goto done;
    +	}
     
     	sp->free(sp);
    +done:
     	return rval;
     }

On 22/11/19, 9:51 am, "target-devel-owner@vger.kernel.org on behalf of Quinn Tran" <target-devel-owner@vger.kernel.org on behalf of qutran@marvell.com> wrote:

    Looks good.
    
    Acked-by: Quinn Tran <qutran@marvell.com>
    
    
    Regards,
    Quinn Tran
    
    -----Original Message-----
    From: Roman Bolshakov <r.bolshakov@yadro.com> 
    Sent: Wednesday, November 20, 2019 2:27 PM
    To: linux-scsi@vger.kernel.org; target-devel@vger.kernel.org
    Cc: linux@yadro.com; Roman Bolshakov <r.bolshakov@yadro.com>; Quinn Tran <qutran@marvell.com>
    Subject: [EXT] [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
    
    External Email
    
    ----------------------------------------------------------------------
    qla24xx_els_dcmd_iocb() performs LOGO and might be invoked in contexts where it is prohibited to sleep. The new wait parameter provides a way to use the function in such context, similarly to qla24xx_els_dcmd2_iocb().
    
    Cc: Quinn Tran <qutran@marvell.com>
    Cc: Himanshu Madhani <hmadhani@marvell.com
    Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
    ---
     drivers/scsi/qla2xxx/qla_attr.c   |  2 +-
     drivers/scsi/qla2xxx/qla_gbl.h    |  2 +-
     drivers/scsi/qla2xxx/qla_iocb.c   | 11 +++++++++--
     drivers/scsi/qla2xxx/qla_target.c |  2 +-
     4 files changed, 12 insertions(+), 5 deletions(-)
    
    diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c index 481c05dbea06..35c703ec59ad 100644
    --- a/drivers/scsi/qla2xxx/qla_attr.c
    +++ b/drivers/scsi/qla2xxx/qla_attr.c
    @@ -828,7 +828,7 @@ qla2x00_issue_logo(struct file *filp, struct kobject *kobj,
     
     	ql_log(ql_log_info, vha, 0x70e4, "%s: %d\n", __func__, type);
     
    -	qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did);
    +	qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did, true);
     	return count;
     }
     
    diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index 5b163ad85c34..df0f3421e3bb 100644
    --- a/drivers/scsi/qla2xxx/qla_gbl.h
    +++ b/drivers/scsi/qla2xxx/qla_gbl.h
    @@ -43,7 +43,7 @@ extern void qla2x00_clear_loop_id(fc_port_t *fcport);  extern int qla2x00_fabric_login(scsi_qla_host_t *, fc_port_t *, uint16_t *);  extern int qla2x00_local_device_login(scsi_qla_host_t *, fc_port_t *);
     
    -extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t);
    +extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t, 
    +bool);
     extern int qla24xx_els_dcmd2_iocb(scsi_qla_host_t *, int, fc_port_t *, bool);  extern void qla2x00_els_dcmd2_free(scsi_qla_host_t *vha,
     				   struct els_plogi *els_plogi);
    diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c index 53ccbd6b71ed..12b37b711ae8 100644
    --- a/drivers/scsi/qla2xxx/qla_iocb.c
    +++ b/drivers/scsi/qla2xxx/qla_iocb.c
    @@ -2562,7 +2562,7 @@ static void qla2x00_els_dcmd_sp_done(srb_t *sp, int res)
     
     int
     qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
    -    port_id_t remote_did)
    +    port_id_t remote_did, bool wait)
     {
     	srb_t *sp;
     	fc_port_t *fcport = NULL;
    @@ -2601,6 +2601,8 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
     	elsio->timeout = qla2x00_els_dcmd_iocb_timeout;
     	qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT);
     	init_completion(&sp->u.iocb_cmd.u.els_logo.comp);
    +	if (wait)
    +		sp->flags = SRB_WAKEUP_ON_COMP;
     	sp->done = qla2x00_els_dcmd_sp_done;
     	sp->free = qla2x00_els_dcmd_sp_free;
     
    @@ -2637,9 +2639,14 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
     	    sp->name, sp->handle, fcport->loop_id, fcport->d_id.b.domain,
     	    fcport->d_id.b.area, fcport->d_id.b.al_pa);
     
    -	wait_for_completion(&elsio->u.els_logo.comp);
    +	if (wait) {
    +		wait_for_completion(&elsio->u.els_logo.comp);
    +	} else {
    +		goto done;
    +	}
     
     	sp->free(sp);
    +done:
     	return rval;
     }
     
    diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
    index 68c14143e50e..0f2bc4cd562f 100644
    --- a/drivers/scsi/qla2xxx/qla_target.c
    +++ b/drivers/scsi/qla2xxx/qla_target.c
    @@ -932,7 +932,7 @@ qlt_send_first_logo(struct scsi_qla_host *vha, qlt_port_logo_t *logo)
     
     	mutex_unlock(&vha->vha_tgt.tgt_mutex);
     
    -	res = qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, logo->id);
    +	res = qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, logo->id, true);
     
     	mutex_lock(&vha->vha_tgt.tgt_mutex);
     	list_del(&logo->list);
    --
    2.24.0
    
    


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 00/15] scsi: qla2xxx: Bug fixes
  2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
                   ` (14 preceding siblings ...)
  2019-11-20 22:27 ` [PATCH v2 15/15] scsi: qla2xxx: Handle ABTS according to FCP spec for logged out ports Roman Bolshakov
@ 2019-11-22  9:14 ` Hannes Reinecke
  2019-11-22 21:36   ` Martin Wilck
  15 siblings, 1 reply; 36+ messages in thread
From: Hannes Reinecke @ 2019-11-22  9:14 UTC (permalink / raw)
  To: Roman Bolshakov, linux-scsi, target-devel; +Cc: linux

On 11/20/19 11:27 PM, Roman Bolshakov wrote:
> 
> Hi Martin,
> 
> The patch series contains fixes for qla2xxx and solves two visible
> issues:
>   - Target port in N2N topology doesn't perform login if it has higher
>     WWPN than initiator
>   - ABORT TASK TMF leads to crash if it's received shortly after ACL of
>     an initiator is deleted and there's active I/O from the initiator
> 
> It also contains reliability improvements and cleanups.
> 
> Unfortunately, there's still an issue the latest patch. The discard
> works but ELS IOCB for LOGO is likely built incorrectly by
> qla24xx_els_dcmd_iocb(). The issue can also be exposed when "1" is
> written to fc_host/hostN/device/issue_logo file with logging turned on.
> 
> Changes since v1 (https://patchwork.kernel.org/cover/11141979/):
> - Fixes target port in N2N mode were added (patches 5-11);
> - Target port makes explicit LOGO on session teardown in the patch made
>   by Quinn. Together with patch 1, it helps to immediately turn
>   fc_remote_port to the Blocked stated on client side and avoids visibly
>   stuck session;
> - The last three patches address violation of FCP specification with
>   regards to handling of ABTS-LS from ports that are not currently
>   logged in.
> 
> Thank you,
> Roman
> 
> Quinn Tran (1):
>   scsi: qla2xxx: Use explicit LOGO in target mode
> 
> Roman Bolshakov (14):
>   scsi: qla2xxx: Ignore NULL pointer in tcm_qla2xxx_free_mcmd
>   scsi: qla2xxx: Initialize free_work before flushing it
>   scsi: qla2xxx: Drop superfluous INIT_WORK of del_work
>   scsi: qla2xxx: Change discovery state before PLOGI
>   scsi: qla2xxx: Allow PLOGI in target mode
>   scsi: qla2xxx: Don't call qlt_async_event twice
>   scsi: qla2xxx: Fix PLOGI payload and ELS IOCB dump length
>   scsi: qla2xxx: Configure local loop for N2N target
>   scsi: qla2xxx: Send Notify ACK after N2N PLOGI
>   scsi: qla2xxx: Don't defer relogin unconditonally
>   scsi: qla2xxx: Ignore PORT UPDATE after N2N PLOGI
>   scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
>   scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB
>   scsi: qla2xxx: Handle ABTS according to FCP spec for logged out ports
> 
>  drivers/scsi/qla2xxx/qla_attr.c    |  2 +-
>  drivers/scsi/qla2xxx/qla_def.h     |  1 +
>  drivers/scsi/qla2xxx/qla_gbl.h     |  2 +-
>  drivers/scsi/qla2xxx/qla_init.c    | 21 ++++++---------
>  drivers/scsi/qla2xxx/qla_iocb.c    | 42 ++++++++++++++++++++++++------
>  drivers/scsi/qla2xxx/qla_isr.c     |  4 ---
>  drivers/scsi/qla2xxx/qla_mbx.c     |  3 ++-
>  drivers/scsi/qla2xxx/qla_target.c  | 34 ++++++++++++++++--------
>  drivers/scsi/qla2xxx/tcm_qla2xxx.c |  3 +++
>  9 files changed, 73 insertions(+), 39 deletions(-)
> 
This patchset has the nice benefit that it has fixed the crashes on
rmmod we had been seeing.

So, for the entire patchset:

Reviewed-by: Hannes Reinecke <hare@suse.de>
Tested-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      Teamlead Storage & Networking
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [EXT] [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
  2019-11-22  5:04     ` Mark Harvey
@ 2019-11-22 17:00       ` Quinn Tran
  2019-11-24 19:15       ` Roman Bolshakov
  1 sibling, 0 replies; 36+ messages in thread
From: Quinn Tran @ 2019-11-22 17:00 UTC (permalink / raw)
  To: Mark Harvey, Roman Bolshakov, linux-scsi, target-devel; +Cc: linux


From: Mark Harvey <mark.harvey@nutanix.com> 
Sent: Thursday, November 21, 2019 9:05 PM
To: Quinn Tran <qutran@marvell.com>; Roman Bolshakov <r.bolshakov@yadro.com>; linux-scsi@vger.kernel.org; target-devel@vger.kernel.org
Cc: linux@yadro.com
Subject: Re: [EXT] [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb

Would this not result in a memory leak in the 'else' path - skipping sp->free(sp)?

QT: Good point.  This mean the patch is missing qla2x00_els_dcmd_sp_done checking for SRB_WAKEUP_ON_COMP to wake up or free the sp.
  
  -	wait_for_completion(&elsio->u.els_logo.comp);
    +	if (wait) {
    +		wait_for_completion(&elsio->u.els_logo.comp);
    +	} else {
    +		goto done;
    +	}
     
     	sp->free(sp);
    +done:
     	return rval;
     }

On 22/11/19, 9:51 am, "target-devel-owner@vger.kernel.org on behalf of Quinn Tran" <target-devel-owner@vger.kernel.org on behalf of qutran@marvell.com> wrote:

    Looks good.
    
    Acked-by: Quinn Tran <qutran@marvell.com>
    
    
    Regards,
    Quinn Tran
    
    -----Original Message-----
    From: Roman Bolshakov <r.bolshakov@yadro.com> 
    Sent: Wednesday, November 20, 2019 2:27 PM
    To: linux-scsi@vger.kernel.org; target-devel@vger.kernel.org
    Cc: linux@yadro.com; Roman Bolshakov <r.bolshakov@yadro.com>; Quinn Tran <qutran@marvell.com>
    Subject: [EXT] [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
    
    External Email
    
    ----------------------------------------------------------------------
    qla24xx_els_dcmd_iocb() performs LOGO and might be invoked in contexts where it is prohibited to sleep. The new wait parameter provides a way to use the function in such context, similarly to qla24xx_els_dcmd2_iocb().
    
    Cc: Quinn Tran <qutran@marvell.com>
    Cc: Himanshu Madhani <hmadhani@marvell.com
    Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
    ---
     drivers/scsi/qla2xxx/qla_attr.c   |  2 +-
     drivers/scsi/qla2xxx/qla_gbl.h    |  2 +-
     drivers/scsi/qla2xxx/qla_iocb.c   | 11 +++++++++--
     drivers/scsi/qla2xxx/qla_target.c |  2 +-
     4 files changed, 12 insertions(+), 5 deletions(-)
    
    diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c index 481c05dbea06..35c703ec59ad 100644
    --- a/drivers/scsi/qla2xxx/qla_attr.c
    +++ b/drivers/scsi/qla2xxx/qla_attr.c
    @@ -828,7 +828,7 @@ qla2x00_issue_logo(struct file *filp, struct kobject *kobj,
     
     	ql_log(ql_log_info, vha, 0x70e4, "%s: %d\n", __func__, type);
     
    -	qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did);
    +	qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, did, true);
     	return count;
     }
     
    diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index 5b163ad85c34..df0f3421e3bb 100644
    --- a/drivers/scsi/qla2xxx/qla_gbl.h
    +++ b/drivers/scsi/qla2xxx/qla_gbl.h
    @@ -43,7 +43,7 @@ extern void qla2x00_clear_loop_id(fc_port_t *fcport);  extern int qla2x00_fabric_login(scsi_qla_host_t *, fc_port_t *, uint16_t *);  extern int qla2x00_local_device_login(scsi_qla_host_t *, fc_port_t *);
     
    -extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t);
    +extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t, 
    +bool);
     extern int qla24xx_els_dcmd2_iocb(scsi_qla_host_t *, int, fc_port_t *, bool);  extern void qla2x00_els_dcmd2_free(scsi_qla_host_t *vha,
     				   struct els_plogi *els_plogi);
    diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c index 53ccbd6b71ed..12b37b711ae8 100644
    --- a/drivers/scsi/qla2xxx/qla_iocb.c
    +++ b/drivers/scsi/qla2xxx/qla_iocb.c
    @@ -2562,7 +2562,7 @@ static void qla2x00_els_dcmd_sp_done(srb_t *sp, int res)
     
     int
     qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
    -    port_id_t remote_did)
    +    port_id_t remote_did, bool wait)
     {
     	srb_t *sp;
     	fc_port_t *fcport = NULL;
    @@ -2601,6 +2601,8 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
     	elsio->timeout = qla2x00_els_dcmd_iocb_timeout;
     	qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT);
     	init_completion(&sp->u.iocb_cmd.u.els_logo.comp);
    +	if (wait)
    +		sp->flags = SRB_WAKEUP_ON_COMP;
     	sp->done = qla2x00_els_dcmd_sp_done;
     	sp->free = qla2x00_els_dcmd_sp_free;
     
    @@ -2637,9 +2639,14 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode,
     	    sp->name, sp->handle, fcport->loop_id, fcport->d_id.b.domain,
     	    fcport->d_id.b.area, fcport->d_id.b.al_pa);
     
    -	wait_for_completion(&elsio->u.els_logo.comp);
    +	if (wait) {
    +		wait_for_completion(&elsio->u.els_logo.comp);
    +	} else {
    +		goto done;
    +	}
     
     	sp->free(sp);
    +done:
     	return rval;
     }
     
    diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
    index 68c14143e50e..0f2bc4cd562f 100644
    --- a/drivers/scsi/qla2xxx/qla_target.c
    +++ b/drivers/scsi/qla2xxx/qla_target.c
    @@ -932,7 +932,7 @@ qlt_send_first_logo(struct scsi_qla_host *vha, qlt_port_logo_t *logo)
     
     	mutex_unlock(&vha->vha_tgt.tgt_mutex);
     
    -	res = qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, logo->id);
    +	res = qla24xx_els_dcmd_iocb(vha, ELS_DCMD_LOGO, logo->id, true);
     
     	mutex_lock(&vha->vha_tgt.tgt_mutex);
     	list_del(&logo->list);
    --
    2.24.0
    
    


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 00/15] scsi: qla2xxx: Bug fixes
  2019-11-22  9:14 ` [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Hannes Reinecke
@ 2019-11-22 21:36   ` Martin Wilck
  2019-11-24 18:31     ` Roman Bolshakov
  0 siblings, 1 reply; 36+ messages in thread
From: Martin Wilck @ 2019-11-22 21:36 UTC (permalink / raw)
  To: Hannes Reinecke, Roman Bolshakov, linux-scsi, target-devel; +Cc: linux

On Fri, 2019-11-22 at 10:14 +0100, Hannes Reinecke wrote:
> On 11/20/19 11:27 PM, Roman Bolshakov wrote:
> > Hi Martin,
> > 
> > The patch series contains fixes for qla2xxx and solves two visible
> > issues:
> >   - Target port in N2N topology doesn't perform login if it has
> > higher
> >     WWPN than initiator
> >   - ABORT TASK TMF leads to crash if it's received shortly after
> > ACL of
> >     an initiator is deleted and there's active I/O from the
> > initiator
> > 
> > It also contains reliability improvements and cleanups.
> > 
> > Unfortunately, there's still an issue the latest patch. The discard
> > works but ELS IOCB for LOGO is likely built incorrectly by
> > qla24xx_els_dcmd_iocb(). The issue can also be exposed when "1" is
> > written to fc_host/hostN/device/issue_logo file with logging turned
> > on.
> > 
> > Changes since v1 (https://patchwork.kernel.org/cover/11141979/):
> > - Fixes target port in N2N mode were added (patches 5-11);
> > - Target port makes explicit LOGO on session teardown in the patch
> > made
> >   by Quinn. Together with patch 1, it helps to immediately turn
> >   fc_remote_port to the Blocked stated on client side and avoids
> > visibly
> >   stuck session;
> > - The last three patches address violation of FCP specification
> > with
> >   regards to handling of ABTS-LS from ports that are not currently
> >   logged in.
> > 
> > Thank you,
> > Roman
> > 
> > Quinn Tran (1):
> >   scsi: qla2xxx: Use explicit LOGO in target mode
> > 
> > Roman Bolshakov (14):
> >   scsi: qla2xxx: Ignore NULL pointer in tcm_qla2xxx_free_mcmd
> >   scsi: qla2xxx: Initialize free_work before flushing it
> >   scsi: qla2xxx: Drop superfluous INIT_WORK of del_work
> >   scsi: qla2xxx: Change discovery state before PLOGI
> >   scsi: qla2xxx: Allow PLOGI in target mode
> >   scsi: qla2xxx: Don't call qlt_async_event twice
> >   scsi: qla2xxx: Fix PLOGI payload and ELS IOCB dump length
> >   scsi: qla2xxx: Configure local loop for N2N target
> >   scsi: qla2xxx: Send Notify ACK after N2N PLOGI
> >   scsi: qla2xxx: Don't defer relogin unconditonally
> >   scsi: qla2xxx: Ignore PORT UPDATE after N2N PLOGI
> >   scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
> >   scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB
> >   scsi: qla2xxx: Handle ABTS according to FCP spec for logged out
> > ports
> > 
> >  drivers/scsi/qla2xxx/qla_attr.c    |  2 +-
> >  drivers/scsi/qla2xxx/qla_def.h     |  1 +
> >  drivers/scsi/qla2xxx/qla_gbl.h     |  2 +-
> >  drivers/scsi/qla2xxx/qla_init.c    | 21 ++++++---------
> >  drivers/scsi/qla2xxx/qla_iocb.c    | 42 ++++++++++++++++++++++++
> > ------
> >  drivers/scsi/qla2xxx/qla_isr.c     |  4 ---
> >  drivers/scsi/qla2xxx/qla_mbx.c     |  3 ++-
> >  drivers/scsi/qla2xxx/qla_target.c  | 34 ++++++++++++++++--------
> >  drivers/scsi/qla2xxx/tcm_qla2xxx.c |  3 +++
> >  9 files changed, 73 insertions(+), 39 deletions(-)
> > 
> This patchset has the nice benefit that it has fixed the crashes on
> rmmod we had been seeing.

Well, I investigated two distinct crash-at-rmmod cases, and one was
already fixed by the earlier commit f45bca8c5052 ("scsi: qla2xxx: Fix
double scsi_done for abort path"), whereas the other is still present,
even after applying this series.

Not to say the series is bad - we just shouldn't raise expectations
too high.

Martin

> 
> So, for the entire patchset:
> 
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Tested-by: Hannes Reinecke <hare@suse.de>
> 


> Cheers,
> 
> Hannes



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 12/15] scsi: qla2xxx: Use explicit LOGO in target mode
  2019-11-20 22:27 ` [PATCH v2 12/15] scsi: qla2xxx: Use explicit LOGO in target mode Roman Bolshakov
@ 2019-11-23  4:58   ` kbuild test robot
  0 siblings, 0 replies; 36+ messages in thread
From: kbuild test robot @ 2019-11-23  4:58 UTC (permalink / raw)
  To: Roman Bolshakov
  Cc: kbuild-all, linux-scsi, target-devel, linux, Quinn Tran,
	Himanshu Madhani, Roman Bolshakov

Hi Roman,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on scsi/for-next]
[also build test WARNING on next-20191122]
[cannot apply to v5.4-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Roman-Bolshakov/scsi-qla2xxx-Bug-fixes/20191122-011503
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-36-g9305d48-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   drivers/scsi/qla2xxx/qla_iocb.c:2374:29: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2389:9: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] extended @@    got resunsigned short [usertype] extended @@
   drivers/scsi/qla2xxx/qla_iocb.c:2389:9: sparse:    expected unsigned short [usertype] extended
   drivers/scsi/qla2xxx/qla_iocb.c:2389:9: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2390:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb0 @@    got resunsigned short [usertype] mb0 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2390:18: sparse:    expected unsigned short [usertype] mb0
   drivers/scsi/qla2xxx/qla_iocb.c:2390:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2394:26: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb1 @@    got resunsigned short [usertype] mb1 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2394:26: sparse:    expected unsigned short [usertype] mb1
   drivers/scsi/qla2xxx/qla_iocb.c:2394:26: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2395:27: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb10 @@    got resunsigned short [usertype] mb10 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2395:27: sparse:    expected unsigned short [usertype] mb10
   drivers/scsi/qla2xxx/qla_iocb.c:2395:27: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2397:26: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb1 @@    got resunsigned short [usertype] mb1 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2397:26: sparse:    expected unsigned short [usertype] mb1
   drivers/scsi/qla2xxx/qla_iocb.c:2397:26: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2399:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb2 @@    got resunsigned short [usertype] mb2 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2399:18: sparse:    expected unsigned short [usertype] mb2
   drivers/scsi/qla2xxx/qla_iocb.c:2399:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2400:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb3 @@    got resunsigned short [usertype] mb3 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2400:18: sparse:    expected unsigned short [usertype] mb3
   drivers/scsi/qla2xxx/qla_iocb.c:2400:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2402:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb9 @@    got resunsigned short [usertype] mb9 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2402:18: sparse:    expected unsigned short [usertype] mb9
   drivers/scsi/qla2xxx/qla_iocb.c:2402:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2420:30: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] control_flags @@    got resunsigned short [usertype] control_flags @@
   drivers/scsi/qla2xxx/qla_iocb.c:2420:30: sparse:    expected unsigned short [usertype] control_flags
   drivers/scsi/qla2xxx/qla_iocb.c:2420:30: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2421:29: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] nport_handle @@    got resunsigned short [usertype] nport_handle @@
   drivers/scsi/qla2xxx/qla_iocb.c:2421:29: sparse:    expected unsigned short [usertype] nport_handle
   drivers/scsi/qla2xxx/qla_iocb.c:2421:29: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2434:9: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] extended @@    got resunsigned short [usertype] extended @@
   drivers/scsi/qla2xxx/qla_iocb.c:2434:9: sparse:    expected unsigned short [usertype] extended
   drivers/scsi/qla2xxx/qla_iocb.c:2434:9: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2435:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb0 @@    got resunsigned short [usertype] mb0 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2435:18: sparse:    expected unsigned short [usertype] mb0
   drivers/scsi/qla2xxx/qla_iocb.c:2435:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2436:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb1 @@    got short [usertype] mb1 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2436:18: sparse:    expected unsigned short [usertype] mb1
   drivers/scsi/qla2xxx/qla_iocb.c:2436:18: sparse:    got restricted __le16
   drivers/scsi/qla2xxx/qla_iocb.c:2439:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb2 @@    got resunsigned short [usertype] mb2 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2439:18: sparse:    expected unsigned short [usertype] mb2
   drivers/scsi/qla2xxx/qla_iocb.c:2439:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2440:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb3 @@    got resunsigned short [usertype] mb3 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2440:18: sparse:    expected unsigned short [usertype] mb3
   drivers/scsi/qla2xxx/qla_iocb.c:2440:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2442:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb9 @@    got resunsigned short [usertype] mb9 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2442:18: sparse:    expected unsigned short [usertype] mb9
   drivers/scsi/qla2xxx/qla_iocb.c:2442:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2450:30: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] control_flags @@    got resunsigned short [usertype] control_flags @@
   drivers/scsi/qla2xxx/qla_iocb.c:2450:30: sparse:    expected unsigned short [usertype] control_flags
   drivers/scsi/qla2xxx/qla_iocb.c:2450:30: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2451:29: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] nport_handle @@    got resunsigned short [usertype] nport_handle @@
   drivers/scsi/qla2xxx/qla_iocb.c:2451:29: sparse:    expected unsigned short [usertype] nport_handle
   drivers/scsi/qla2xxx/qla_iocb.c:2451:29: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2461:9: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] extended @@    got resunsigned short [usertype] extended @@
   drivers/scsi/qla2xxx/qla_iocb.c:2461:9: sparse:    expected unsigned short [usertype] extended
   drivers/scsi/qla2xxx/qla_iocb.c:2461:9: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2462:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb0 @@    got resunsigned short [usertype] mb0 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2462:18: sparse:    expected unsigned short [usertype] mb0
   drivers/scsi/qla2xxx/qla_iocb.c:2462:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2464:26: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb1 @@    got resunsigned short [usertype] mb1 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2464:26: sparse:    expected unsigned short [usertype] mb1
   drivers/scsi/qla2xxx/qla_iocb.c:2464:26: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2465:27: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb10 @@    got resunsigned short [usertype] mb10 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2465:27: sparse:    expected unsigned short [usertype] mb10
   drivers/scsi/qla2xxx/qla_iocb.c:2465:27: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2467:26: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb1 @@    got resunsigned short [usertype] mb1 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2467:26: sparse:    expected unsigned short [usertype] mb1
   drivers/scsi/qla2xxx/qla_iocb.c:2467:26: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2469:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb2 @@    got resunsigned short [usertype] mb2 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2469:18: sparse:    expected unsigned short [usertype] mb2
   drivers/scsi/qla2xxx/qla_iocb.c:2469:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2470:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb3 @@    got resunsigned short [usertype] mb3 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2470:18: sparse:    expected unsigned short [usertype] mb3
   drivers/scsi/qla2xxx/qla_iocb.c:2470:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2471:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb6 @@    got resunsigned short [usertype] mb6 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2471:18: sparse:    expected unsigned short [usertype] mb6
   drivers/scsi/qla2xxx/qla_iocb.c:2471:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2472:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb7 @@    got resunsigned short [usertype] mb7 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2472:18: sparse:    expected unsigned short [usertype] mb7
   drivers/scsi/qla2xxx/qla_iocb.c:2472:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2473:18: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] mb9 @@    got resunsigned short [usertype] mb9 @@
   drivers/scsi/qla2xxx/qla_iocb.c:2473:18: sparse:    expected unsigned short [usertype] mb9
   drivers/scsi/qla2xxx/qla_iocb.c:2473:18: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2493:27: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] nport_handle @@    got resunsigned short [usertype] nport_handle @@
   drivers/scsi/qla2xxx/qla_iocb.c:2493:27: sparse:    expected unsigned short [usertype] nport_handle
   drivers/scsi/qla2xxx/qla_iocb.c:2493:27: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2494:22: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] timeout @@    got resunsigned short [usertype] timeout @@
   drivers/scsi/qla2xxx/qla_iocb.c:2494:22: sparse:    expected unsigned short [usertype] timeout
   drivers/scsi/qla2xxx/qla_iocb.c:2494:22: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2495:28: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] control_flags @@    got restrunsigned int [usertype] control_flags @@
   drivers/scsi/qla2xxx/qla_iocb.c:2495:28: sparse:    expected unsigned int [usertype] control_flags
   drivers/scsi/qla2xxx/qla_iocb.c:2495:28: sparse:    got restricted __le32 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2657:32: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] nport_handle @@    got resunsigned short [usertype] nport_handle @@
   drivers/scsi/qla2xxx/qla_iocb.c:2657:32: sparse:    expected unsigned short [usertype] nport_handle
   drivers/scsi/qla2xxx/qla_iocb.c:2657:32: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2987:32: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] nport_handle @@    got resunsigned short [usertype] nport_handle @@
   drivers/scsi/qla2xxx/qla_iocb.c:2987:32: sparse:    expected unsigned short [usertype] nport_handle
   drivers/scsi/qla2xxx/qla_iocb.c:2987:32: sparse:    got restricted __le16 [usertype]
>> drivers/scsi/qla2xxx/qla_iocb.c:2988:32: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] tx_dsd_count @@    got resunsigned short [usertype] tx_dsd_count @@
>> drivers/scsi/qla2xxx/qla_iocb.c:2988:32: sparse:    expected unsigned short [usertype] tx_dsd_count
   drivers/scsi/qla2xxx/qla_iocb.c:2988:32: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_iocb.c:2991:32: sparse: sparse: too many warnings

vim +2988 drivers/scsi/qla2xxx/qla_iocb.c

edd05de1975927 Duane Grigsby       2017-10-13  2975  
9a069e196767d7 Giridhar Malavali   2010-01-12  2976  static void
9a069e196767d7 Giridhar Malavali   2010-01-12  2977  qla24xx_els_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
9a069e196767d7 Giridhar Malavali   2010-01-12  2978  {
75cc8cfc6e13d4 Johannes Thumshirn  2016-11-17  2979  	struct bsg_job *bsg_job = sp->u.bsg_job;
01e0e15c8b3b32 Johannes Thumshirn  2016-11-17  2980  	struct fc_bsg_request *bsg_request = bsg_job->request;
9a069e196767d7 Giridhar Malavali   2010-01-12  2981  
9a069e196767d7 Giridhar Malavali   2010-01-12  2982          els_iocb->entry_type = ELS_IOCB_TYPE;
9a069e196767d7 Giridhar Malavali   2010-01-12  2983          els_iocb->entry_count = 1;
9a069e196767d7 Giridhar Malavali   2010-01-12  2984          els_iocb->sys_define = 0;
9a069e196767d7 Giridhar Malavali   2010-01-12  2985          els_iocb->entry_status = 0;
9a069e196767d7 Giridhar Malavali   2010-01-12  2986          els_iocb->handle = sp->handle;
9a069e196767d7 Giridhar Malavali   2010-01-12  2987          els_iocb->nport_handle = cpu_to_le16(sp->fcport->loop_id);
ad950360eebb5f Bart Van Assche     2015-07-09 @2988  	els_iocb->tx_dsd_count = cpu_to_le16(bsg_job->request_payload.sg_cnt);
25ff6af10562cf Joe Carnuccio       2017-01-19  2989  	els_iocb->vp_index = sp->vha->vp_idx;
9a069e196767d7 Giridhar Malavali   2010-01-12  2990          els_iocb->sof_type = EST_SOFI3;
ad950360eebb5f Bart Van Assche     2015-07-09  2991  	els_iocb->rx_dsd_count = cpu_to_le16(bsg_job->reply_payload.sg_cnt);
9a069e196767d7 Giridhar Malavali   2010-01-12  2992  
4916392b56921b Madhuranath Iyengar 2010-05-04  2993  	els_iocb->opcode =
9ba56b95a58890 Giridhar Malavali   2012-02-09  2994  	    sp->type == SRB_ELS_CMD_RPT ?
01e0e15c8b3b32 Johannes Thumshirn  2016-11-17  2995  	    bsg_request->rqst_data.r_els.els_code :
01e0e15c8b3b32 Johannes Thumshirn  2016-11-17  2996  	    bsg_request->rqst_data.h_els.command_code;
9a069e196767d7 Giridhar Malavali   2010-01-12  2997          els_iocb->port_id[0] = sp->fcport->d_id.b.al_pa;
9a069e196767d7 Giridhar Malavali   2010-01-12  2998          els_iocb->port_id[1] = sp->fcport->d_id.b.area;
9a069e196767d7 Giridhar Malavali   2010-01-12  2999          els_iocb->port_id[2] = sp->fcport->d_id.b.domain;
9a069e196767d7 Giridhar Malavali   2010-01-12  3000          els_iocb->control_flags = 0;
9a069e196767d7 Giridhar Malavali   2010-01-12  3001          els_iocb->rx_byte_count =
9a069e196767d7 Giridhar Malavali   2010-01-12  3002              cpu_to_le32(bsg_job->reply_payload.payload_len);
9a069e196767d7 Giridhar Malavali   2010-01-12  3003          els_iocb->tx_byte_count =
9a069e196767d7 Giridhar Malavali   2010-01-12  3004              cpu_to_le32(bsg_job->request_payload.payload_len);
9a069e196767d7 Giridhar Malavali   2010-01-12  3005  
d4556a4932a546 Bart Van Assche     2019-04-17  3006  	put_unaligned_le64(sg_dma_address(bsg_job->request_payload.sg_list),
d4556a4932a546 Bart Van Assche     2019-04-17  3007  			   &els_iocb->tx_address);
9a069e196767d7 Giridhar Malavali   2010-01-12  3008          els_iocb->tx_len = cpu_to_le32(sg_dma_len
9a069e196767d7 Giridhar Malavali   2010-01-12  3009              (bsg_job->request_payload.sg_list));
9a069e196767d7 Giridhar Malavali   2010-01-12  3010  
d4556a4932a546 Bart Van Assche     2019-04-17  3011  	put_unaligned_le64(sg_dma_address(bsg_job->reply_payload.sg_list),
d4556a4932a546 Bart Van Assche     2019-04-17  3012  			   &els_iocb->rx_address);
9a069e196767d7 Giridhar Malavali   2010-01-12  3013          els_iocb->rx_len = cpu_to_le32(sg_dma_len
9a069e196767d7 Giridhar Malavali   2010-01-12  3014              (bsg_job->reply_payload.sg_list));
fabbb8df8eba6f Joe Carnuccio       2013-08-27  3015  
25ff6af10562cf Joe Carnuccio       2017-01-19  3016  	sp->vha->qla_stats.control_requests++;
9a069e196767d7 Giridhar Malavali   2010-01-12  3017  }
9a069e196767d7 Giridhar Malavali   2010-01-12  3018  

:::::: The code at line 2988 was first introduced by commit
:::::: ad950360eebb5f5f7610b13cfd08c0185ca3f146 qla2xxx: Remove __constant_ prefix

:::::: TO: Bart Van Assche <bart.vanassche@sandisk.com>
:::::: CC: James Bottomley <JBottomley@Odin.com>

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 15/15] scsi: qla2xxx: Handle ABTS according to FCP spec for logged out ports
  2019-11-20 22:27 ` [PATCH v2 15/15] scsi: qla2xxx: Handle ABTS according to FCP spec for logged out ports Roman Bolshakov
@ 2019-11-23  5:57   ` kbuild test robot
  0 siblings, 0 replies; 36+ messages in thread
From: kbuild test robot @ 2019-11-23  5:57 UTC (permalink / raw)
  To: Roman Bolshakov
  Cc: kbuild-all, linux-scsi, target-devel, linux, Roman Bolshakov,
	Quinn Tran, Himanshu Madhani

Hi Roman,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on scsi/for-next]
[also build test WARNING on next-20191122]
[cannot apply to v5.4-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Roman-Bolshakov/scsi-qla2xxx-Bug-fixes/20191122-011503
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-36-g9305d48-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   drivers/scsi/qla2xxx/qla_target.c:5780:21: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:5791:21: sparse: sparse: restricted __le16 degrades to integer
   drivers/scsi/qla2xxx/qla_target.c:5795:29: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:5840:21: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:5850:21: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:5866:29: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:5867:29: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:5870:29: sparse: sparse: restricted __le16 degrades to integer
   drivers/scsi/qla2xxx/qla_target.c:5874:37: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:1689:13: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:1691:25: sparse: sparse: restricted __le32 degrades to integer
   drivers/scsi/qla2xxx/qla_target.c:1700:33: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] srr_flags @@    got resunsigned short [usertype] srr_flags @@
   drivers/scsi/qla2xxx/qla_target.c:1700:33: sparse:    expected unsigned short [usertype] srr_flags
   drivers/scsi/qla2xxx/qla_target.c:1700:33: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2136:13: sparse: sparse: cast to restricted __le32
   drivers/scsi/qla2xxx/qla_target.c:2158:13: sparse: sparse: cast to restricted __le32
   drivers/scsi/qla2xxx/qla_target.c:837:13: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:845:19: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:1177:19: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:1318:36: sparse: sparse: cast to restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:1760:15: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] f_ctl @@    got restrunsigned int [usertype] f_ctl @@
   drivers/scsi/qla2xxx/qla_target.c:1760:15: sparse:    expected unsigned int [usertype] f_ctl
   drivers/scsi/qla2xxx/qla_target.c:1760:15: sparse:    got restricted __le32 [usertype]
>> drivers/scsi/qla2xxx/qla_target.c:1829:37: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] control_flags @@    got resunsigned short [usertype] control_flags @@
>> drivers/scsi/qla2xxx/qla_target.c:1829:37: sparse:    expected unsigned short [usertype] control_flags
   drivers/scsi/qla2xxx/qla_target.c:1829:37: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:1836:15: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] f_ctl @@    got restrunsigned int [usertype] f_ctl @@
   drivers/scsi/qla2xxx/qla_target.c:1836:15: sparse:    expected unsigned int [usertype] f_ctl
   drivers/scsi/qla2xxx/qla_target.c:1836:15: sparse:    got restricted __le32 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:1909:23: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] timeout @@    got resunsigned short [usertype] timeout @@
   drivers/scsi/qla2xxx/qla_target.c:1909:23: sparse:    expected unsigned short [usertype] timeout
   drivers/scsi/qla2xxx/qla_target.c:1909:23: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:1928:31: sparse: sparse: incorrect type in assignment (different base types) @@    expected restricted __le16 [usertype] ox_id @@    got tricted __le16 [usertype] ox_id @@
   drivers/scsi/qla2xxx/qla_target.c:1928:31: sparse:    expected restricted __le16 [usertype] ox_id
   drivers/scsi/qla2xxx/qla_target.c:1928:31: sparse:    got unsigned short [usertype] ox_id
   drivers/scsi/qla2xxx/qla_target.c:2230:23: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] timeout @@    got resunsigned short [usertype] timeout @@
   drivers/scsi/qla2xxx/qla_target.c:2230:23: sparse:    expected unsigned short [usertype] timeout
   drivers/scsi/qla2xxx/qla_target.c:2230:23: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2239:37: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] scsi_status @@    got resunsigned short [usertype] scsi_status @@
   drivers/scsi/qla2xxx/qla_target.c:2239:37: sparse:    expected unsigned short [usertype] scsi_status
   drivers/scsi/qla2xxx/qla_target.c:2239:37: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2241:38: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] response_len @@    got resunsigned short [usertype] response_len @@
   drivers/scsi/qla2xxx/qla_target.c:2241:38: sparse:    expected unsigned short [usertype] response_len
   drivers/scsi/qla2xxx/qla_target.c:2241:38: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2287:23: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] timeout @@    got resunsigned short [usertype] timeout @@
   drivers/scsi/qla2xxx/qla_target.c:2287:23: sparse:    expected unsigned short [usertype] timeout
   drivers/scsi/qla2xxx/qla_target.c:2287:23: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2296:37: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] scsi_status @@    got resunsigned short [usertype] scsi_status @@
   drivers/scsi/qla2xxx/qla_target.c:2296:37: sparse:    expected unsigned short [usertype] scsi_status
   drivers/scsi/qla2xxx/qla_target.c:2296:37: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2298:38: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] response_len @@    got resunsigned short [usertype] response_len @@
   drivers/scsi/qla2xxx/qla_target.c:2298:38: sparse:    expected unsigned short [usertype] response_len
   drivers/scsi/qla2xxx/qla_target.c:2298:38: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.h:382:17: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.h:382:17: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.h:382:17: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.h:382:17: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.h:382:17: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.h:382:17: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.c:2299:34: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] residual @@    got restrunsigned int [usertype] residual @@
   drivers/scsi/qla2xxx/qla_target.c:2299:34: sparse:    expected unsigned int [usertype] residual
   drivers/scsi/qla2xxx/qla_target.c:2299:34: sparse:    got restricted __le32 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2302:45: sparse: sparse: invalid assignment: |=
   drivers/scsi/qla2xxx/qla_target.c:2302:45: sparse:    left side has type unsigned short
   drivers/scsi/qla2xxx/qla_target.c:2302:45: sparse:    right side has type restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:2588:27: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] nport_handle @@    got resunsigned short [usertype] nport_handle @@
   drivers/scsi/qla2xxx/qla_target.c:2588:27: sparse:    expected unsigned short [usertype] nport_handle
   drivers/scsi/qla2xxx/qla_target.c:2588:27: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2589:22: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] timeout @@    got resunsigned short [usertype] timeout @@
   drivers/scsi/qla2xxx/qla_target.c:2589:22: sparse:    expected unsigned short [usertype] timeout
   drivers/scsi/qla2xxx/qla_target.c:2589:22: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2596:40: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] relative_offset @@    got restrunsigned int [usertype] relative_offset @@
   drivers/scsi/qla2xxx/qla_target.c:2596:40: sparse:    expected unsigned int [usertype] relative_offset
   drivers/scsi/qla2xxx/qla_target.c:2596:40: sparse:    got restricted __le32 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2651:42: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] transfer_length @@    got restrunsigned int [usertype] transfer_length @@
   drivers/scsi/qla2xxx/qla_target.c:2651:42: sparse:    expected unsigned int [usertype] transfer_length
   drivers/scsi/qla2xxx/qla_target.c:2651:42: sparse:    got restricted __le32 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2658:35: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] dseg_count @@    got resunsigned short [usertype] dseg_count @@
   drivers/scsi/qla2xxx/qla_target.c:2658:35: sparse:    expected unsigned short [usertype] dseg_count
   drivers/scsi/qla2xxx/qla_target.c:2658:35: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2820:34: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] residual @@    got restrunsigned int [usertype] residual @@
   drivers/scsi/qla2xxx/qla_target.c:2820:34: sparse:    expected unsigned int [usertype] residual
   drivers/scsi/qla2xxx/qla_target.c:2820:34: sparse:    got restricted __le32 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2821:37: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] scsi_status @@    got resunsigned short [usertype] scsi_status @@
   drivers/scsi/qla2xxx/qla_target.c:2821:37: sparse:    expected unsigned short [usertype] scsi_status
   drivers/scsi/qla2xxx/qla_target.c:2821:37: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2842:45: sparse: sparse: invalid assignment: |=
   drivers/scsi/qla2xxx/qla_target.c:2842:45: sparse:    left side has type unsigned short
   drivers/scsi/qla2xxx/qla_target.c:2842:45: sparse:    right side has type restricted __le16
   drivers/scsi/qla2xxx/qla_target.c:2844:46: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] sense_length @@    got resunsigned short [usertype] sense_length @@
   drivers/scsi/qla2xxx/qla_target.c:2844:46: sparse:    expected unsigned short [usertype] sense_length
   drivers/scsi/qla2xxx/qla_target.c:2844:46: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2847:69: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] @@    got restrunsigned int [usertype] @@
   drivers/scsi/qla2xxx/qla_target.c:2847:69: sparse:    expected unsigned int [usertype]
   drivers/scsi/qla2xxx/qla_target.c:2847:69: sparse:    got restricted __be32 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:3101:27: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] nport_handle @@    got resunsigned short [usertype] nport_handle @@
   drivers/scsi/qla2xxx/qla_target.c:3101:27: sparse:    expected unsigned short [usertype] nport_handle
   drivers/scsi/qla2xxx/qla_target.c:3101:27: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:3283:60: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] scsi_status @@    got resunsigned short [usertype] scsi_status @@
   drivers/scsi/qla2xxx/qla_target.c:3283:60: sparse:    expected unsigned short [usertype] scsi_status
   drivers/scsi/qla2xxx/qla_target.c:3283:60: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:3285:57: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] residual @@    got restrunsigned int [usertype] residual @@
   drivers/scsi/qla2xxx/qla_target.c:3285:57: sparse:    expected unsigned int [usertype] residual
   drivers/scsi/qla2xxx/qla_target.c:3285:57: sparse:    got restricted __le32 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:3101:27: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] nport_handle @@    got resunsigned short [usertype] nport_handle @@
   drivers/scsi/qla2xxx/qla_target.c:3101:27: sparse:    expected unsigned short [usertype] nport_handle
   drivers/scsi/qla2xxx/qla_target.c:3101:27: sparse:    got restricted __le16 [usertype]
   drivers/scsi/qla2xxx/qla_target.c:3461:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3461:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3461:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3461:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3462:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3462:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3462:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3462:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3463:26: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.c:3463:26: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.c:3463:26: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.c:3463:26: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.c:3463:26: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.c:3463:26: sparse: sparse: cast to restricted __be32
   drivers/scsi/qla2xxx/qla_target.c:3465:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3465:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3465:26: sparse: sparse: cast to restricted __be16
   drivers/scsi/qla2xxx/qla_target.c:3465:26: sparse: sparse: cast to restricted __be16

vim +1829 drivers/scsi/qla2xxx/qla_target.c

  1797	
  1798	/*
  1799	 * ha->hardware_lock supposed to be held on entry. Might drop it, then reaquire
  1800	 */
  1801	static void qlt_24xx_send_abts_resp(struct qla_qpair *qpair,
  1802		struct abts_recv_from_24xx *abts, uint32_t status,
  1803		bool ids_reversed, bool term_exchange)
  1804	{
  1805		struct scsi_qla_host *vha = qpair->vha;
  1806		struct qla_hw_data *ha = vha->hw;
  1807		struct abts_resp_to_24xx *resp;
  1808		uint32_t f_ctl;
  1809		uint8_t *p;
  1810	
  1811		ql_dbg(ql_dbg_tgt, vha, 0xe006,
  1812		    "Sending task mgmt ABTS response (ha=%p, atio=%p, status=%x\n",
  1813		    ha, abts, status);
  1814	
  1815		resp = (struct abts_resp_to_24xx *)qla2x00_alloc_iocbs_ready(qpair,
  1816		    NULL);
  1817		if (!resp) {
  1818			ql_dbg(ql_dbg_tgt, vha, 0xe04a,
  1819			    "qla_target(%d): %s failed: unable to allocate "
  1820			    "request packet", vha->vp_idx, __func__);
  1821			return;
  1822		}
  1823	
  1824		resp->entry_type = ABTS_RESP_24XX;
  1825		resp->handle = QLA_TGT_SKIP_HANDLE;
  1826		resp->entry_count = 1;
  1827		resp->nport_handle = abts->nport_handle;
  1828		if (term_exchange)
> 1829			resp->control_flags = cpu_to_le16(ABTS_CONTR_FLG_TERM_EXCHG);
  1830		else
  1831			resp->control_flags = 0;
  1832		resp->vp_index = vha->vp_idx;
  1833		resp->sof_type = abts->sof_type;
  1834		resp->exchange_address = abts->exchange_address;
  1835		resp->fcp_hdr_le = abts->fcp_hdr_le;
  1836		f_ctl = cpu_to_le32(F_CTL_EXCH_CONTEXT_RESP |
  1837		    F_CTL_LAST_SEQ | F_CTL_END_SEQ |
  1838		    F_CTL_SEQ_INITIATIVE);
  1839		p = (uint8_t *)&f_ctl;
  1840		resp->fcp_hdr_le.f_ctl[0] = *p++;
  1841		resp->fcp_hdr_le.f_ctl[1] = *p++;
  1842		resp->fcp_hdr_le.f_ctl[2] = *p;
  1843		if (ids_reversed) {
  1844			resp->fcp_hdr_le.d_id = abts->fcp_hdr_le.d_id;
  1845			resp->fcp_hdr_le.s_id = abts->fcp_hdr_le.s_id;
  1846		} else {
  1847			resp->fcp_hdr_le.d_id = abts->fcp_hdr_le.s_id;
  1848			resp->fcp_hdr_le.s_id = abts->fcp_hdr_le.d_id;
  1849		}
  1850		resp->exchange_addr_to_abort = abts->exchange_addr_to_abort;
  1851		if (status == FCP_TMF_CMPL) {
  1852			resp->fcp_hdr_le.r_ctl = R_CTL_BASIC_LINK_SERV | R_CTL_B_ACC;
  1853			resp->payload.ba_acct.seq_id_valid = SEQ_ID_INVALID;
  1854			resp->payload.ba_acct.low_seq_cnt = 0x0000;
  1855			resp->payload.ba_acct.high_seq_cnt = 0xFFFF;
  1856			resp->payload.ba_acct.ox_id = abts->fcp_hdr_le.ox_id;
  1857			resp->payload.ba_acct.rx_id = abts->fcp_hdr_le.rx_id;
  1858		} else {
  1859			resp->fcp_hdr_le.r_ctl = R_CTL_BASIC_LINK_SERV | R_CTL_B_RJT;
  1860			resp->payload.ba_rjt.reason_code =
  1861				BA_RJT_REASON_CODE_UNABLE_TO_PERFORM;
  1862			/* Other bytes are zero */
  1863		}
  1864	
  1865		vha->vha_tgt.qla_tgt->abts_resp_expected++;
  1866	
  1867		/* Memory Barrier */
  1868		wmb();
  1869		if (qpair->reqq_start_iocbs)
  1870			qpair->reqq_start_iocbs(qpair);
  1871		else
  1872			qla2x00_start_iocbs(vha, qpair->req);
  1873	}
  1874	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 00/15] scsi: qla2xxx: Bug fixes
  2019-11-22 21:36   ` Martin Wilck
@ 2019-11-24 18:31     ` Roman Bolshakov
  0 siblings, 0 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-24 18:31 UTC (permalink / raw)
  To: Martin Wilck; +Cc: Hannes Reinecke, linux-scsi, target-devel, linux

On Fri, Nov 22, 2019 at 10:36:06PM +0100, Martin Wilck wrote:
> On Fri, 2019-11-22 at 10:14 +0100, Hannes Reinecke wrote:
> > This patchset has the nice benefit that it has fixed the crashes on
> > rmmod we had been seeing.
> 
> Well, I investigated two distinct crash-at-rmmod cases, and one was
> already fixed by the earlier commit f45bca8c5052 ("scsi: qla2xxx: Fix
> double scsi_done for abort path"), whereas the other is still present,
> even after applying this series.
> 
> Not to say the series is bad - we just shouldn't raise expectations
> too high.
> 

Hi Martin,

This patch series only fixes a crash when there's active I/O and ACL of
the initiator is getting deleted. The issue can be reproduced quite
easily:

  1. Configure a target with 1 LUN and 1 ACL (and 1 Mapped LUN inside)
  2. Run I/O from initiator
  3. Delete ACL while running the I/O

The crash happens ~30s after the ACL is deleted when the initiator
starts sending ABORT TASK TMF to abort timed out I/O. It might happen at
rmmod time but that's just coincidence of ABORT TASK being processed. It
might not happen if a rig shuts off in less than 30 seconds.

Thanks,
Roman

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [EXT] [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb
  2019-11-22  5:04     ` Mark Harvey
  2019-11-22 17:00       ` Quinn Tran
@ 2019-11-24 19:15       ` Roman Bolshakov
  1 sibling, 0 replies; 36+ messages in thread
From: Roman Bolshakov @ 2019-11-24 19:15 UTC (permalink / raw)
  To: Mark Harvey; +Cc: Quinn Tran, linux-scsi, target-devel, linux

On Fri, Nov 22, 2019 at 05:04:52AM +0000, Mark Harvey wrote:
> Would this not result in a memory leak in the 'else' path - skiping sp->free(sp)?
>   
>   -	wait_for_completion(&elsio->u.els_logo.comp);
>     +	if (wait) {
>     +		wait_for_completion(&elsio->u.els_logo.comp);
>     +	} else {
>     +		goto done;
>     +	}
>      
>      	sp->free(sp);
>     +done:
>      	return rval;
>      }
> 

Hi Mark,

Good catch, it definetely will be a leak. I had this on mind but forgot
while rushing to post v2. I'll add proper cleanup in v3.

Thank you,
Roman

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2019-11-24 19:15 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20 22:27 [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Roman Bolshakov
2019-11-20 22:27 ` [PATCH v2 01/15] scsi: qla2xxx: Ignore NULL pointer in tcm_qla2xxx_free_mcmd Roman Bolshakov
2019-11-20 22:27 ` [PATCH v2 02/15] scsi: qla2xxx: Initialize free_work before flushing it Roman Bolshakov
2019-11-20 22:27 ` [PATCH v2 03/15] scsi: qla2xxx: Drop superfluous INIT_WORK of del_work Roman Bolshakov
2019-11-20 22:27 ` [PATCH v2 04/15] scsi: qla2xxx: Change discovery state before PLOGI Roman Bolshakov
2019-11-20 22:27 ` [PATCH v2 05/15] scsi: qla2xxx: Allow PLOGI in target mode Roman Bolshakov
2019-11-20 22:52   ` [EXT] " Quinn Tran
2019-11-21 16:38     ` Himanshu Madhani
2019-11-20 22:27 ` [PATCH v2 06/15] scsi: qla2xxx: Don't call qlt_async_event twice Roman Bolshakov
2019-11-21 15:43   ` Himanshu Madhani
2019-11-20 22:27 ` [PATCH v2 07/15] scsi: qla2xxx: Fix PLOGI payload and ELS IOCB dump length Roman Bolshakov
2019-11-21 16:39   ` [EXT] " Himanshu Madhani
2019-11-20 22:27 ` [PATCH v2 08/15] scsi: qla2xxx: Configure local loop for N2N target Roman Bolshakov
2019-11-20 23:06   ` [EXT] " Quinn Tran
2019-11-21 16:39   ` Himanshu Madhani
2019-11-20 22:27 ` [PATCH v2 09/15] scsi: qla2xxx: Send Notify ACK after N2N PLOGI Roman Bolshakov
2019-11-20 23:53   ` [EXT] " Quinn Tran
2019-11-21 16:40   ` Himanshu Madhani
2019-11-20 22:27 ` [PATCH v2 10/15] scsi: qla2xxx: Don't defer relogin unconditonally Roman Bolshakov
2019-11-21  0:03   ` [EXT] " Quinn Tran
2019-11-20 22:27 ` [PATCH v2 11/15] scsi: qla2xxx: Ignore PORT UPDATE after N2N PLOGI Roman Bolshakov
2019-11-20 22:27 ` [PATCH v2 12/15] scsi: qla2xxx: Use explicit LOGO in target mode Roman Bolshakov
2019-11-23  4:58   ` kbuild test robot
2019-11-20 22:27 ` [PATCH v2 13/15] scsi: qla2xxx: Add async mode for qla24xx_els_dcmd_iocb Roman Bolshakov
2019-11-21 22:50   ` [EXT] " Quinn Tran
2019-11-22  5:04     ` Mark Harvey
2019-11-22 17:00       ` Quinn Tran
2019-11-24 19:15       ` Roman Bolshakov
2019-11-20 22:27 ` [PATCH v2 14/15] scsi: qla2xxx: Add debug dump of LOGO payload and ELS IOCB Roman Bolshakov
2019-11-21 16:37   ` Himanshu Madhani
2019-11-21 22:52   ` [EXT] " Quinn Tran
2019-11-20 22:27 ` [PATCH v2 15/15] scsi: qla2xxx: Handle ABTS according to FCP spec for logged out ports Roman Bolshakov
2019-11-23  5:57   ` kbuild test robot
2019-11-22  9:14 ` [PATCH v2 00/15] scsi: qla2xxx: Bug fixes Hannes Reinecke
2019-11-22 21:36   ` Martin Wilck
2019-11-24 18:31     ` Roman Bolshakov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).