From 4d2095cc42a2d8062590891f929d9d694cbd927f Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Fri, 30 Sep 2016 11:01:14 +0200 Subject: scsi: libfc: Revisit kref handling The kref handling in fc_rport is a mess. This patch updates the kref handling according to the following rules: - Take a reference whenever scheduling a workqueue - Take a reference whenever an ELS command is send - Drop the reference at the end of the workqueue function - Drop the reference at the end of handling ELS replies - Take a reference when allocating an rport - Drop the reference when removing an rport Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_rport.c | 131 ++++++++++++++++++++++++++++++++---------- 1 file changed, 101 insertions(+), 30 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index 97aeaddd600d..4ec896e42120 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -44,6 +44,19 @@ * path this potential over-use of the mutex is acceptable. */ +/* + * RPORT REFERENCE COUNTING + * + * A rport reference should be taken when: + * - an rport is allocated + * - a workqueue item is scheduled + * - an ELS request is send + * The reference should be dropped when: + * - the workqueue function has finished + * - the ELS response is handled + * - an rport is removed + */ + #include #include #include @@ -242,6 +255,8 @@ static void fc_rport_state_enter(struct fc_rport_priv *rdata, /** * fc_rport_work() - Handler for remote port events in the rport_event_queue * @work: Handle to the remote port being dequeued + * + * Reference counting: drops kref on return */ static void fc_rport_work(struct work_struct *work) { @@ -329,7 +344,8 @@ static void fc_rport_work(struct work_struct *work) FC_RPORT_DBG(rdata, "lld callback ev %d\n", event); rdata->lld_event_callback(lport, rdata, event); } - cancel_delayed_work_sync(&rdata->retry_work); + if (cancel_delayed_work_sync(&rdata->retry_work)) + kref_put(&rdata->kref, lport->tt.rport_destroy); /* * Reset any outstanding exchanges before freeing rport. @@ -381,6 +397,7 @@ static void fc_rport_work(struct work_struct *work) mutex_unlock(&rdata->rp_mutex); break; } + kref_put(&rdata->kref, lport->tt.rport_destroy); } /** @@ -431,10 +448,14 @@ static int fc_rport_login(struct fc_rport_priv *rdata) * Set the new event so that the old pending event will not occur. * Since we have the mutex, even if fc_rport_work() is already started, * it'll see the new event. + * + * Reference counting: does not modify kref */ static void fc_rport_enter_delete(struct fc_rport_priv *rdata, enum fc_rport_event event) { + struct fc_lport *lport = rdata->local_port; + if (rdata->rp_state == RPORT_ST_DELETE) return; @@ -442,8 +463,11 @@ static void fc_rport_enter_delete(struct fc_rport_priv *rdata, fc_rport_state_enter(rdata, RPORT_ST_DELETE); - if (rdata->event == RPORT_EV_NONE) - queue_work(rport_event_queue, &rdata->event_work); + kref_get(&rdata->kref); + if (rdata->event == RPORT_EV_NONE && + !queue_work(rport_event_queue, &rdata->event_work)) + kref_put(&rdata->kref, lport->tt.rport_destroy); + rdata->event = event; } @@ -496,15 +520,22 @@ out: * * Locking Note: The rport lock is expected to be held before calling * this routine. + * + * Reference counting: schedules workqueue, does not modify kref */ static void fc_rport_enter_ready(struct fc_rport_priv *rdata) { + struct fc_lport *lport = rdata->local_port; + fc_rport_state_enter(rdata, RPORT_ST_READY); FC_RPORT_DBG(rdata, "Port is Ready\n"); - if (rdata->event == RPORT_EV_NONE) - queue_work(rport_event_queue, &rdata->event_work); + kref_get(&rdata->kref); + if (rdata->event == RPORT_EV_NONE && + !queue_work(rport_event_queue, &rdata->event_work)) + kref_put(&rdata->kref, lport->tt.rport_destroy); + rdata->event = RPORT_EV_READY; } @@ -515,11 +546,14 @@ static void fc_rport_enter_ready(struct fc_rport_priv *rdata) * Locking Note: Called without the rport lock held. This * function will hold the rport lock, call an _enter_* * function and then unlock the rport. + * + * Reference counting: Drops kref on return. */ static void fc_rport_timeout(struct work_struct *work) { struct fc_rport_priv *rdata = container_of(work, struct fc_rport_priv, retry_work.work); + struct fc_lport *lport = rdata->local_port; mutex_lock(&rdata->rp_mutex); @@ -547,6 +581,7 @@ static void fc_rport_timeout(struct work_struct *work) } mutex_unlock(&rdata->rp_mutex); + kref_put(&rdata->kref, lport->tt.rport_destroy); } /** @@ -556,6 +591,8 @@ static void fc_rport_timeout(struct work_struct *work) * * Locking Note: The rport lock is expected to be held before * calling this routine + * + * Reference counting: does not modify kref */ static void fc_rport_error(struct fc_rport_priv *rdata, struct fc_frame *fp) { @@ -602,11 +639,14 @@ static void fc_rport_error(struct fc_rport_priv *rdata, struct fc_frame *fp) * * Locking Note: The rport lock is expected to be held before * calling this routine + * + * Reference counting: increments kref when scheduling retry_work */ static void fc_rport_error_retry(struct fc_rport_priv *rdata, struct fc_frame *fp) { unsigned long delay = msecs_to_jiffies(FC_DEF_E_D_TOV); + struct fc_lport *lport = rdata->local_port; /* make sure this isn't an FC_EX_CLOSED error, never retry those */ if (PTR_ERR(fp) == -FC_EX_CLOSED) @@ -619,7 +659,9 @@ static void fc_rport_error_retry(struct fc_rport_priv *rdata, /* no additional delay on exchange timeouts */ if (PTR_ERR(fp) == -FC_EX_TIMEOUT) delay = 0; - schedule_delayed_work(&rdata->retry_work, delay); + kref_get(&rdata->kref); + if (!schedule_delayed_work(&rdata->retry_work, delay)) + kref_put(&rdata->kref, lport->tt.rport_destroy); return; } @@ -740,6 +782,8 @@ bad: * * Locking Note: The rport lock is expected to be held before calling * this routine. + * + * Reference counting: increments kref when sending ELS */ static void fc_rport_enter_flogi(struct fc_rport_priv *rdata) { @@ -758,18 +802,21 @@ static void fc_rport_enter_flogi(struct fc_rport_priv *rdata) if (!fp) return fc_rport_error_retry(rdata, fp); + kref_get(&rdata->kref); if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_FLOGI, fc_rport_flogi_resp, rdata, - 2 * lport->r_a_tov)) + 2 * lport->r_a_tov)) { fc_rport_error_retry(rdata, NULL); - else - kref_get(&rdata->kref); + kref_put(&rdata->kref, lport->tt.rport_destroy); + } } /** * fc_rport_recv_flogi_req() - Handle Fabric Login (FLOGI) request in p-mp mode * @lport: The local port that received the PLOGI request * @rx_fp: The PLOGI request frame + * + * Reference counting: drops kref on return */ static void fc_rport_recv_flogi_req(struct fc_lport *lport, struct fc_frame *rx_fp) @@ -824,8 +871,7 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, * RPORT wouldn;t have created and 'rport_lookup' would have * failed anyway in that case. */ - if (lport->point_to_multipoint) - break; + break; case RPORT_ST_DELETE: mutex_unlock(&rdata->rp_mutex); rjt_data.reason = ELS_RJT_FIP; @@ -969,6 +1015,8 @@ fc_rport_compatible_roles(struct fc_lport *lport, struct fc_rport_priv *rdata) * * Locking Note: The rport lock is expected to be held before calling * this routine. + * + * Reference counting: increments kref when sending ELS */ static void fc_rport_enter_plogi(struct fc_rport_priv *rdata) { @@ -995,12 +1043,13 @@ static void fc_rport_enter_plogi(struct fc_rport_priv *rdata) } rdata->e_d_tov = lport->e_d_tov; + kref_get(&rdata->kref); if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_PLOGI, fc_rport_plogi_resp, rdata, - 2 * lport->r_a_tov)) + 2 * lport->r_a_tov)) { fc_rport_error_retry(rdata, NULL); - else - kref_get(&rdata->kref); + kref_put(&rdata->kref, lport->tt.rport_destroy); + } } /** @@ -1108,6 +1157,8 @@ err: * * Locking Note: The rport lock is expected to be held before calling * this routine. + * + * Reference counting: increments kref when sending ELS */ static void fc_rport_enter_prli(struct fc_rport_priv *rdata) { @@ -1151,11 +1202,12 @@ static void fc_rport_enter_prli(struct fc_rport_priv *rdata) fc_host_port_id(lport->host), FC_TYPE_ELS, FC_FC_FIRST_SEQ | FC_FC_END_SEQ | FC_FC_SEQ_INIT, 0); + kref_get(&rdata->kref); if (!lport->tt.exch_seq_send(lport, fp, fc_rport_prli_resp, - NULL, rdata, 2 * lport->r_a_tov)) + NULL, rdata, 2 * lport->r_a_tov)) { fc_rport_error_retry(rdata, NULL); - else - kref_get(&rdata->kref); + kref_put(&rdata->kref, lport->tt.rport_destroy); + } } /** @@ -1230,6 +1282,8 @@ err: * * Locking Note: The rport lock is expected to be held before calling * this routine. + * + * Reference counting: increments kref when sending ELS */ static void fc_rport_enter_rtv(struct fc_rport_priv *rdata) { @@ -1247,12 +1301,13 @@ static void fc_rport_enter_rtv(struct fc_rport_priv *rdata) return; } + kref_get(&rdata->kref); if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_RTV, fc_rport_rtv_resp, rdata, - 2 * lport->r_a_tov)) + 2 * lport->r_a_tov)) { fc_rport_error_retry(rdata, NULL); - else - kref_get(&rdata->kref); + kref_put(&rdata->kref, lport->tt.rport_destroy); + } } /** @@ -1262,15 +1317,16 @@ static void fc_rport_enter_rtv(struct fc_rport_priv *rdata) * @lport_arg: The local port */ static void fc_rport_logo_resp(struct fc_seq *sp, struct fc_frame *fp, - void *lport_arg) + void *rdata_arg) { - struct fc_lport *lport = lport_arg; + struct fc_rport_priv *rdata = rdata_arg; + struct fc_lport *lport = rdata->local_port; FC_RPORT_ID_DBG(lport, fc_seq_exch(sp)->did, "Received a LOGO %s\n", fc_els_resp_type(fp)); - if (IS_ERR(fp)) - return; - fc_frame_free(fp); + if (!IS_ERR(fp)) + fc_frame_free(fp); + kref_put(&rdata->kref, lport->tt.rport_destroy); } /** @@ -1279,6 +1335,8 @@ static void fc_rport_logo_resp(struct fc_seq *sp, struct fc_frame *fp, * * Locking Note: The rport lock is expected to be held before calling * this routine. + * + * Reference counting: increments kref when sending ELS */ static void fc_rport_enter_logo(struct fc_rport_priv *rdata) { @@ -1291,8 +1349,10 @@ static void fc_rport_enter_logo(struct fc_rport_priv *rdata) fp = fc_frame_alloc(lport, sizeof(struct fc_els_logo)); if (!fp) return; - (void)lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_LOGO, - fc_rport_logo_resp, lport, 0); + kref_get(&rdata->kref); + if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_LOGO, + fc_rport_logo_resp, rdata, 0)) + kref_put(&rdata->kref, lport->tt.rport_destroy); } /** @@ -1359,6 +1419,8 @@ err: * * Locking Note: The rport lock is expected to be held before calling * this routine. + * + * Reference counting: increments kref when sending ELS */ static void fc_rport_enter_adisc(struct fc_rport_priv *rdata) { @@ -1375,12 +1437,13 @@ static void fc_rport_enter_adisc(struct fc_rport_priv *rdata) fc_rport_error_retry(rdata, fp); return; } + kref_get(&rdata->kref); if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_ADISC, fc_rport_adisc_resp, rdata, - 2 * lport->r_a_tov)) + 2 * lport->r_a_tov)) { fc_rport_error_retry(rdata, NULL); - else - kref_get(&rdata->kref); + kref_put(&rdata->kref, lport->tt.rport_destroy); + } } /** @@ -1494,6 +1557,8 @@ out: * The ELS opcode has already been validated by the caller. * * Locking Note: Called with the lport lock held. + * + * Reference counting: does not modify kref */ static void fc_rport_recv_els_req(struct fc_lport *lport, struct fc_frame *fp) { @@ -1561,6 +1626,8 @@ reject: * @fp: The request frame * * Locking Note: Called with the lport lock held. + * + * Reference counting: does not modify kref */ static void fc_rport_recv_req(struct fc_lport *lport, struct fc_frame *fp) { @@ -1605,6 +1672,8 @@ static void fc_rport_recv_req(struct fc_lport *lport, struct fc_frame *fp) * @rx_fp: The PLOGI request frame * * Locking Note: The rport lock is held before calling this function. + * + * Reference counting: increments kref on return */ static void fc_rport_recv_plogi_req(struct fc_lport *lport, struct fc_frame *rx_fp) @@ -1919,6 +1988,8 @@ drop: * * Locking Note: The rport lock is expected to be held before calling * this function. + * + * Reference counting: drops kref on return */ static void fc_rport_recv_logo_req(struct fc_lport *lport, struct fc_frame *fp) { -- cgit v1.2.3 From a407c593398c886db4fa1fc5c6fec55e61187a09 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Fri, 30 Sep 2016 11:01:15 +0200 Subject: scsi: libfc: Fixup disc_mutex handling The list of attached 'rdata' remote port structures is RCU protected, so there is no need to take the 'disc_mutex' when traversing it. Rather we should be using rcu_read_lock() and kref_get_unless_zero() to validate the entries. We need, however, take the disc_mutex when deleting an entry; otherwise we risk clashes with list_add. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_disc.c | 38 +++++++++++++++++++++++++------------- drivers/scsi/libfc/fc_lport.c | 9 +++++++-- drivers/scsi/libfc/fc_rport.c | 2 ++ 3 files changed, 34 insertions(+), 15 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_disc.c b/drivers/scsi/libfc/fc_disc.c index 880a9068ca12..ad3965f9d03d 100644 --- a/drivers/scsi/libfc/fc_disc.c +++ b/drivers/scsi/libfc/fc_disc.c @@ -68,10 +68,14 @@ static void fc_disc_stop_rports(struct fc_disc *disc) lport = fc_disc_lport(disc); - mutex_lock(&disc->disc_mutex); - list_for_each_entry_rcu(rdata, &disc->rports, peers) - lport->tt.rport_logoff(rdata); - mutex_unlock(&disc->disc_mutex); + rcu_read_lock(); + list_for_each_entry_rcu(rdata, &disc->rports, peers) { + if (kref_get_unless_zero(&rdata->kref)) { + lport->tt.rport_logoff(rdata); + kref_put(&rdata->kref, lport->tt.rport_destroy); + } + } + rcu_read_unlock(); } /** @@ -289,15 +293,19 @@ static void fc_disc_done(struct fc_disc *disc, enum fc_disc_event event) * Skip ports which were never discovered. These are the dNS port * and ports which were created by PLOGI. */ + rcu_read_lock(); list_for_each_entry_rcu(rdata, &disc->rports, peers) { - if (!rdata->disc_id) + if (!kref_get_unless_zero(&rdata->kref)) continue; - if (rdata->disc_id == disc->disc_id) - lport->tt.rport_login(rdata); - else - lport->tt.rport_logoff(rdata); + if (rdata->disc_id) { + if (rdata->disc_id == disc->disc_id) + lport->tt.rport_login(rdata); + else + lport->tt.rport_logoff(rdata); + } + kref_put(&rdata->kref, lport->tt.rport_destroy); } - + rcu_read_unlock(); mutex_unlock(&disc->disc_mutex); disc->disc_callback(lport, event); mutex_lock(&disc->disc_mutex); @@ -592,7 +600,6 @@ static void fc_disc_gpn_id_resp(struct fc_seq *sp, struct fc_frame *fp, lport = rdata->local_port; disc = &lport->disc; - mutex_lock(&disc->disc_mutex); if (PTR_ERR(fp) == -FC_EX_CLOSED) goto out; if (IS_ERR(fp)) @@ -607,16 +614,19 @@ static void fc_disc_gpn_id_resp(struct fc_seq *sp, struct fc_frame *fp, goto redisc; pn = (struct fc_ns_gid_pn *)(cp + 1); port_name = get_unaligned_be64(&pn->fn_wwpn); + mutex_lock(&rdata->rp_mutex); if (rdata->ids.port_name == -1) rdata->ids.port_name = port_name; else if (rdata->ids.port_name != port_name) { FC_DISC_DBG(disc, "GPN_ID accepted. WWPN changed. " "Port-id %6.6x wwpn %16.16llx\n", rdata->ids.port_id, port_name); + mutex_unlock(&rdata->rp_mutex); lport->tt.rport_logoff(rdata); - + mutex_lock(&lport->disc.disc_mutex); new_rdata = lport->tt.rport_create(lport, rdata->ids.port_id); + mutex_unlock(&lport->disc.disc_mutex); if (new_rdata) { new_rdata->disc_id = disc->disc_id; lport->tt.rport_login(new_rdata); @@ -624,6 +634,7 @@ static void fc_disc_gpn_id_resp(struct fc_seq *sp, struct fc_frame *fp, goto out; } rdata->disc_id = disc->disc_id; + mutex_unlock(&rdata->rp_mutex); lport->tt.rport_login(rdata); } else if (ntohs(cp->ct_cmd) == FC_FS_RJT) { FC_DISC_DBG(disc, "GPN_ID rejected reason %x exp %x\n", @@ -633,10 +644,11 @@ static void fc_disc_gpn_id_resp(struct fc_seq *sp, struct fc_frame *fp, FC_DISC_DBG(disc, "GPN_ID unexpected response code %x\n", ntohs(cp->ct_cmd)); redisc: + mutex_lock(&disc->disc_mutex); fc_disc_restart(disc); + mutex_unlock(&disc->disc_mutex); } out: - mutex_unlock(&disc->disc_mutex); kref_put(&rdata->kref, lport->tt.rport_destroy); } diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c index 04ce7cfb6d1b..4e11c90c783c 100644 --- a/drivers/scsi/libfc/fc_lport.c +++ b/drivers/scsi/libfc/fc_lport.c @@ -237,16 +237,19 @@ static const char *fc_lport_state(struct fc_lport *lport) * @remote_fid: The FID of the ptp rport * @remote_wwpn: The WWPN of the ptp rport * @remote_wwnn: The WWNN of the ptp rport + * + * Locking Note: The lport lock is expected to be held before calling + * this routine. */ static void fc_lport_ptp_setup(struct fc_lport *lport, u32 remote_fid, u64 remote_wwpn, u64 remote_wwnn) { - mutex_lock(&lport->disc.disc_mutex); if (lport->ptp_rdata) { lport->tt.rport_logoff(lport->ptp_rdata); kref_put(&lport->ptp_rdata->kref, lport->tt.rport_destroy); } + mutex_lock(&lport->disc.disc_mutex); lport->ptp_rdata = lport->tt.rport_create(lport, remote_fid); kref_get(&lport->ptp_rdata->kref); lport->ptp_rdata->ids.port_name = remote_wwpn; @@ -1007,8 +1010,10 @@ EXPORT_SYMBOL(fc_lport_reset); */ static void fc_lport_reset_locked(struct fc_lport *lport) { - if (lport->dns_rdata) + if (lport->dns_rdata) { lport->tt.rport_logoff(lport->dns_rdata); + lport->dns_rdata = NULL; + } if (lport->ptp_rdata) { lport->tt.rport_logoff(lport->ptp_rdata); diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index 4ec896e42120..a0ceba10c679 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -378,7 +378,9 @@ static void fc_rport_work(struct work_struct *work) mutex_unlock(&rdata->rp_mutex); } else { FC_RPORT_DBG(rdata, "work delete\n"); + mutex_lock(&lport->disc.disc_mutex); list_del_rcu(&rdata->peers); + mutex_unlock(&lport->disc.disc_mutex); mutex_unlock(&rdata->rp_mutex); kref_put(&rdata->kref, lport->tt.rport_destroy); } -- cgit v1.2.3 From 785141c62a26f055b27355ee9234e145955a51c6 Mon Sep 17 00:00:00 2001 From: Chad Dupuis Date: Fri, 30 Sep 2016 11:01:16 +0200 Subject: scsi: libfc: Do not take rdata->rp_mutex when processing a -FC_EX_CLOSED ELS response. When an ELS response handler receives a -FC_EX_CLOSED, the rdata->rp_mutex is already held which can lead to a deadlock condition like the following stack trace: [] fc_rport_plogi_resp+0x28/0x200 [libfc] [] fc_invoke_resp+0x6a/0xe0 [libfc] [] fc_exch_mgr_reset+0x1b8/0x280 [libfc] [] fc_rport_logoff+0x43/0xd0 [libfc] [] fc_disc_stop+0x6d/0xf0 [libfc] [] fc_disc_stop_final+0xe/0x20 [libfc] [] fc_fabric_logoff+0x17/0x70 [libfc] The other ELS handlers need to follow the FLOGI response handler and simply do a kref_put against the fc_rport_priv struct and exit when receving a -FC_EX_CLOSED response. Signed-off-by: Chad Dupuis Reviewed-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_rport.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index a0ceba10c679..ff33ae6bdf7c 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -952,10 +952,13 @@ static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp, u16 cssp_seq; u8 op; - mutex_lock(&rdata->rp_mutex); - FC_RPORT_DBG(rdata, "Received a PLOGI %s\n", fc_els_resp_type(fp)); + if (fp == ERR_PTR(-FC_EX_CLOSED)) + goto put; + + mutex_lock(&rdata->rp_mutex); + if (rdata->rp_state != RPORT_ST_PLOGI) { FC_RPORT_DBG(rdata, "Received a PLOGI response, but in state " "%s\n", fc_rport_state(rdata)); @@ -994,6 +997,7 @@ out: fc_frame_free(fp); err: mutex_unlock(&rdata->rp_mutex); +put: kref_put(&rdata->kref, lport->tt.rport_destroy); } @@ -1079,10 +1083,13 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, u8 op; u8 resp_code = 0; - mutex_lock(&rdata->rp_mutex); - FC_RPORT_DBG(rdata, "Received a PRLI %s\n", fc_els_resp_type(fp)); + if (fp == ERR_PTR(-FC_EX_CLOSED)) + goto put; + + mutex_lock(&rdata->rp_mutex); + if (rdata->rp_state != RPORT_ST_PRLI) { FC_RPORT_DBG(rdata, "Received a PRLI response, but in state " "%s\n", fc_rport_state(rdata)); @@ -1150,6 +1157,7 @@ out: fc_frame_free(fp); err: mutex_unlock(&rdata->rp_mutex); +put: kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); } @@ -1230,10 +1238,13 @@ static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp, struct fc_rport_priv *rdata = rdata_arg; u8 op; - mutex_lock(&rdata->rp_mutex); - FC_RPORT_DBG(rdata, "Received a RTV %s\n", fc_els_resp_type(fp)); + if (fp == ERR_PTR(-FC_EX_CLOSED)) + goto put; + + mutex_lock(&rdata->rp_mutex); + if (rdata->rp_state != RPORT_ST_RTV) { FC_RPORT_DBG(rdata, "Received a RTV response, but in state " "%s\n", fc_rport_state(rdata)); @@ -1275,6 +1286,7 @@ out: fc_frame_free(fp); err: mutex_unlock(&rdata->rp_mutex); +put: kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); } @@ -1374,10 +1386,13 @@ static void fc_rport_adisc_resp(struct fc_seq *sp, struct fc_frame *fp, struct fc_els_adisc *adisc; u8 op; - mutex_lock(&rdata->rp_mutex); - FC_RPORT_DBG(rdata, "Received a ADISC response\n"); + if (fp == ERR_PTR(-FC_EX_CLOSED)) + goto put; + + mutex_lock(&rdata->rp_mutex); + if (rdata->rp_state != RPORT_ST_ADISC) { FC_RPORT_DBG(rdata, "Received a ADISC resp but in state %s\n", fc_rport_state(rdata)); @@ -1412,6 +1427,7 @@ out: fc_frame_free(fp); err: mutex_unlock(&rdata->rp_mutex); +put: kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); } -- cgit v1.2.3 From e5a20009dae054344d71a79e9bfbea84152f3eb8 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Fri, 30 Sep 2016 11:01:17 +0200 Subject: scsi: libfc: Do not drop down to FLOGI for fc_rport_login() When fc_rport_login() is called while the rport is not in RPORT_ST_INIT, RPORT_ST_READY, or RPORT_ST_DELETE login is already in progress and there's no need to drop down to FLOGI; doing so will only confuse the other side. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_rport.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index ff33ae6bdf7c..4e4087a00836 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -427,10 +427,14 @@ static int fc_rport_login(struct fc_rport_priv *rdata) case RPORT_ST_DELETE: FC_RPORT_DBG(rdata, "Restart deleted port\n"); break; - default: + case RPORT_ST_INIT: FC_RPORT_DBG(rdata, "Login to port\n"); fc_rport_enter_flogi(rdata); break; + default: + FC_RPORT_DBG(rdata, "Login in progress, state %s\n", + fc_rport_state(rdata)); + break; } mutex_unlock(&rdata->rp_mutex); -- cgit v1.2.3 From 06ee2571a438653bd14c33c70379a5f008a91901 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Fri, 30 Sep 2016 11:01:18 +0200 Subject: scsi: libfc: Do not login if the port is already started When the port is already started we don't need to login; that will only confuse the state machine. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_rport.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index 4e4087a00836..72a7183fdd06 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -418,6 +418,12 @@ static int fc_rport_login(struct fc_rport_priv *rdata) { mutex_lock(&rdata->rp_mutex); + if (rdata->flags & FC_RP_STARTED) { + FC_RPORT_DBG(rdata, "port already started\n"); + mutex_unlock(&rdata->rp_mutex); + return 0; + } + rdata->flags |= FC_RP_STARTED; switch (rdata->rp_state) { case RPORT_ST_READY: -- cgit v1.2.3 From f89b8d67db792a8a303e14e3d02785035e6f1a05 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Fri, 30 Sep 2016 11:01:19 +0200 Subject: scsi: libfc: don't advance state machine for incoming FLOGI When we receive an FLOGI but have already sent our own we should not advance the state machine but rather wait for our FLOGI to return before continuing with PLOGI. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_rport.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index 72a7183fdd06..4b9bb6dd4004 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -765,8 +765,10 @@ static void fc_rport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, goto bad; flogi = fc_frame_payload_get(fp, sizeof(*flogi)); - if (!flogi) + if (!flogi) { + FC_RPORT_DBG(rdata, "Bad FLOGI response\n"); goto bad; + } r_a_tov = ntohl(flogi->fl_csp.sp_r_a_tov); if (r_a_tov > rdata->r_a_tov) rdata->r_a_tov = r_a_tov; @@ -783,7 +785,6 @@ put: kref_put(&rdata->kref, lport->tt.rport_destroy); return; bad: - FC_RPORT_DBG(rdata, "Bad FLOGI response\n"); fc_rport_error_retry(rdata, fp); goto out; } @@ -925,10 +926,17 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, fc_fill_reply_hdr(fp, rx_fp, FC_RCTL_ELS_REP, 0); lport->tt.frame_send(lport, fp); - if (rdata->ids.port_name < lport->wwpn) - fc_rport_enter_plogi(rdata); - else - fc_rport_state_enter(rdata, RPORT_ST_PLOGI_WAIT); + /* + * Do not proceed with the state machine if our + * FLOGI has crossed with an FLOGI from the + * remote port; wait for the FLOGI response instead. + */ + if (rdata->rp_state != RPORT_ST_FLOGI) { + if (rdata->ids.port_name < lport->wwpn) + fc_rport_enter_plogi(rdata); + else + fc_rport_state_enter(rdata, RPORT_ST_PLOGI_WAIT); + } out: mutex_unlock(&rdata->rp_mutex); kref_put(&rdata->kref, lport->tt.rport_destroy); -- cgit v1.2.3 From f8f91f3f3120b2168189100c588aeaf2ff5e9ac4 Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Fri, 14 Oct 2016 16:37:29 -0400 Subject: scsi: libfc: Revert "[SCSI] libfc: use offload EM instance again instead jumping to next EM" This reverts commit 3e22760d4db6fd89e0be46c3d132390a251da9c6. This revert came about because of efforts by Ewan Milne, Curtis Taylor and I. In researching this issue, significant performance issues were seen on large CPU count systems using the software FCOE stack. Hannes also weighed in. The same was not apparent on much smaller low count CPU systems. The behavior introduced by commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 lands sup with large count CPU systems seeing continual blk_requeue_request() calls due to ML_QUEUE_HOST_BUSY. fc_exch_alloc() used to try all the available exchange managers in the list for an available exchange id, but this was changed in 2010 so that if the first matched exchange manager couldn't allocate one, it fails and we end up returning host busy. This was due to commit: Setting the ddp_min module parameter to fcoe to 128MB prevents the ->match function from permitting the use of the offload exchange manager for the frame, and we no longer see the problem with host busy status, since it uses the larger non-offloaded pool. Reverting commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 was tested to also prevent the host busy issue due to failing allocations. Suggested-by: Ewan Milne Suggested-by: Curtis Taylor Tested-by: Laurence Oberman Signed-off-by: Laurence Oberman --- drivers/scsi/libfc/fc_exch.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c index 16ca31ad5ec0..dd95e2aece66 100644 --- a/drivers/scsi/libfc/fc_exch.c +++ b/drivers/scsi/libfc/fc_exch.c @@ -888,14 +888,19 @@ err: * EM is selected when a NULL match function pointer is encountered * or when a call to a match function returns true. */ -static inline struct fc_exch *fc_exch_alloc(struct fc_lport *lport, - struct fc_frame *fp) +static struct fc_exch *fc_exch_alloc(struct fc_lport *lport, + struct fc_frame *fp) { struct fc_exch_mgr_anchor *ema; + struct fc_exch *ep; - list_for_each_entry(ema, &lport->ema_list, ema_list) - if (!ema->match || ema->match(fp)) - return fc_exch_em_alloc(lport, ema->mp); + list_for_each_entry(ema, &lport->ema_list, ema_list) { + if (!ema->match || ema->match(fp)) { + ep = fc_exch_em_alloc(lport, ema->mp); + if (ep) + return ep; + } + } return NULL; } -- cgit v1.2.3 From 57d3ec7e468bb6659d9a461294d8747906fb7231 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 13 Oct 2016 15:10:37 +0200 Subject: scsi: libfc: additional debugging messages Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_exch.c | 59 +++++++++++++++++++++++++++++++++++-------- drivers/scsi/libfc/fc_fcp.c | 41 ++++++++++++++++++++++++------ drivers/scsi/libfc/fc_rport.c | 25 ++++++++++++++---- 3 files changed, 101 insertions(+), 24 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c index dd95e2aece66..91800cb776ec 100644 --- a/drivers/scsi/libfc/fc_exch.c +++ b/drivers/scsi/libfc/fc_exch.c @@ -362,8 +362,10 @@ static inline void fc_exch_timer_set_locked(struct fc_exch *ep, fc_exch_hold(ep); /* hold for timer */ if (!queue_delayed_work(fc_exch_workqueue, &ep->timeout_work, - msecs_to_jiffies(timer_msec))) + msecs_to_jiffies(timer_msec))) { + FC_EXCH_DBG(ep, "Exchange already queued\n"); fc_exch_release(ep); + } } /** @@ -632,9 +634,13 @@ static int fc_exch_abort_locked(struct fc_exch *ep, struct fc_frame *fp; int error; + FC_EXCH_DBG(ep, "exch: abort, time %d msecs\n", timer_msec); if (ep->esb_stat & (ESB_ST_COMPLETE | ESB_ST_ABNORMAL) || - ep->state & (FC_EX_DONE | FC_EX_RST_CLEANUP)) + ep->state & (FC_EX_DONE | FC_EX_RST_CLEANUP)) { + FC_EXCH_DBG(ep, "exch: already completed esb %x state %x\n", + ep->esb_stat, ep->state); return -ENXIO; + } /* * Send the abort on a new sequence if possible. @@ -758,7 +764,7 @@ static void fc_exch_timeout(struct work_struct *work) u32 e_stat; int rc = 1; - FC_EXCH_DBG(ep, "Exchange timed out\n"); + FC_EXCH_DBG(ep, "Exchange timed out state %x\n", ep->state); spin_lock_bh(&ep->ex_lock); if (ep->state & (FC_EX_RST_CLEANUP | FC_EX_DONE)) @@ -1263,8 +1269,10 @@ static void fc_seq_send_ack(struct fc_seq *sp, const struct fc_frame *rx_fp) */ if (fc_sof_needs_ack(fr_sof(rx_fp))) { fp = fc_frame_alloc(lport, 0); - if (!fp) + if (!fp) { + FC_EXCH_DBG(ep, "Drop ACK request, out of memory\n"); return; + } fh = fc_frame_header_get(fp); fh->fh_r_ctl = FC_RCTL_ACK_1; @@ -1317,13 +1325,18 @@ static void fc_exch_send_ba_rjt(struct fc_frame *rx_fp, struct fc_frame_header *rx_fh; struct fc_frame_header *fh; struct fc_ba_rjt *rp; + struct fc_seq *sp; struct fc_lport *lport; unsigned int f_ctl; lport = fr_dev(rx_fp); + sp = fr_seq(rx_fp); fp = fc_frame_alloc(lport, sizeof(*rp)); - if (!fp) + if (!fp) { + FC_EXCH_DBG(fc_seq_exch(sp), + "Drop BA_RJT request, out of memory\n"); return; + } fh = fc_frame_header_get(fp); rx_fh = fc_frame_header_get(rx_fp); @@ -1388,14 +1401,17 @@ static void fc_exch_recv_abts(struct fc_exch *ep, struct fc_frame *rx_fp) if (!ep) goto reject; + FC_EXCH_DBG(ep, "exch: ABTS received\n"); fp = fc_frame_alloc(ep->lp, sizeof(*ap)); - if (!fp) + if (!fp) { + FC_EXCH_DBG(ep, "Drop ABTS request, out of memory\n"); goto free; + } spin_lock_bh(&ep->ex_lock); if (ep->esb_stat & ESB_ST_COMPLETE) { spin_unlock_bh(&ep->ex_lock); - + FC_EXCH_DBG(ep, "exch: ABTS rejected, exchange complete\n"); fc_frame_free(fp); goto reject; } @@ -1789,11 +1805,16 @@ static void fc_seq_ls_acc(struct fc_frame *rx_fp) struct fc_lport *lport; struct fc_els_ls_acc *acc; struct fc_frame *fp; + struct fc_seq *sp; lport = fr_dev(rx_fp); + sp = fr_seq(rx_fp); fp = fc_frame_alloc(lport, sizeof(*acc)); - if (!fp) + if (!fp) { + FC_EXCH_DBG(fc_seq_exch(sp), + "exch: drop LS_ACC, out of memory\n"); return; + } acc = fc_frame_payload_get(fp, sizeof(*acc)); memset(acc, 0, sizeof(*acc)); acc->la_cmd = ELS_LS_ACC; @@ -1816,11 +1837,16 @@ static void fc_seq_ls_rjt(struct fc_frame *rx_fp, enum fc_els_rjt_reason reason, struct fc_lport *lport; struct fc_els_ls_rjt *rjt; struct fc_frame *fp; + struct fc_seq *sp; lport = fr_dev(rx_fp); + sp = fr_seq(rx_fp); fp = fc_frame_alloc(lport, sizeof(*rjt)); - if (!fp) + if (!fp) { + FC_EXCH_DBG(fc_seq_exch(sp), + "exch: drop LS_ACC, out of memory\n"); return; + } rjt = fc_frame_payload_get(fp, sizeof(*rjt)); memset(rjt, 0, sizeof(*rjt)); rjt->er_cmd = ELS_LS_RJT; @@ -1980,15 +2006,23 @@ static void fc_exch_els_rec(struct fc_frame *rfp) ep = fc_exch_lookup(lport, sid == fc_host_port_id(lport->host) ? oxid : rxid); explan = ELS_EXPL_OXID_RXID; - if (!ep) + if (!ep) { + FC_LPORT_DBG(lport, + "REC request from %x: rxid %x oxid %x not found\n", + sid, rxid, oxid); goto reject; + } + FC_EXCH_DBG(ep, "REC request from %x: rxid %x oxid %x\n", + sid, rxid, oxid); if (ep->oid != sid || oxid != ep->oxid) goto rel; if (rxid != FC_XID_UNKNOWN && rxid != ep->rxid) goto rel; fp = fc_frame_alloc(lport, sizeof(*acc)); - if (!fp) + if (!fp) { + FC_EXCH_DBG(ep, "Drop REC request, out of memory\n"); goto out; + } acc = fc_frame_payload_get(fp, sizeof(*acc)); memset(acc, 0, sizeof(*acc)); @@ -2181,6 +2215,7 @@ static void fc_exch_rrq(struct fc_exch *ep) return; retry: + FC_EXCH_DBG(ep, "exch: RRQ send failed\n"); spin_lock_bh(&ep->ex_lock); if (ep->state & (FC_EX_RST_CLEANUP | FC_EX_DONE)) { spin_unlock_bh(&ep->ex_lock); @@ -2223,6 +2258,8 @@ static void fc_exch_els_rrq(struct fc_frame *fp) if (!ep) goto reject; spin_lock_bh(&ep->ex_lock); + FC_EXCH_DBG(ep, "RRQ request from %x: xid %x rxid %x oxid %x\n", + sid, xid, ntohs(rp->rrq_rx_id), ntohs(rp->rrq_ox_id)); if (ep->oxid != ntohs(rp->rrq_ox_id)) goto unlock_reject; if (ep->rxid != ntohs(rp->rrq_rx_id) && diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 5121272f28fd..5c6c73a80379 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -764,8 +764,11 @@ static void fc_fcp_recv(struct fc_seq *seq, struct fc_frame *fp, void *arg) fh = fc_frame_header_get(fp); r_ctl = fh->fh_r_ctl; - if (lport->state != LPORT_ST_READY) + if (lport->state != LPORT_ST_READY) { + FC_FCP_DBG(fsp, "lport state %d, ignoring r_ctl %x\n", + lport->state, r_ctl); goto out; + } if (fc_fcp_lock_pkt(fsp)) goto out; @@ -774,8 +777,10 @@ static void fc_fcp_recv(struct fc_seq *seq, struct fc_frame *fp, void *arg) goto unlock; } - if (fsp->state & (FC_SRB_ABORTED | FC_SRB_ABORT_PENDING)) + if (fsp->state & (FC_SRB_ABORTED | FC_SRB_ABORT_PENDING)) { + FC_FCP_DBG(fsp, "command aborted, ignoring r_ctl %x\n", r_ctl); goto unlock; + } if (r_ctl == FC_RCTL_DD_DATA_DESC) { /* @@ -910,6 +915,10 @@ static void fc_fcp_resp(struct fc_fcp_pkt *fsp, struct fc_frame *fp) * Wait a at least one jiffy to see if it is delivered. * If this expires without data, we may do SRR. */ + FC_FCP_DBG(fsp, "tgt %6.6x xfer len %zx data underrun " + "len %x, data len %x\n", + fsp->rport->port_id, + fsp->xfer_len, expected_len, fsp->data_len); fc_fcp_timer_set(fsp, 2); return; } @@ -959,8 +968,11 @@ static void fc_fcp_complete_locked(struct fc_fcp_pkt *fsp) if (fsp->cdb_status == SAM_STAT_GOOD && fsp->xfer_len < fsp->data_len && !fsp->io_status && (!(fsp->scsi_comp_flags & FCP_RESID_UNDER) || - fsp->xfer_len < fsp->data_len - fsp->scsi_resid)) + fsp->xfer_len < fsp->data_len - fsp->scsi_resid)) { + FC_FCP_DBG(fsp, "data underrun, xfer %zx data %x\n", + fsp->xfer_len, fsp->data_len); fsp->status_code = FC_DATA_UNDRUN; + } } seq = fsp->seq_ptr; @@ -1222,8 +1234,11 @@ static int fc_fcp_pkt_abort(struct fc_fcp_pkt *fsp) int rc = FAILED; unsigned long ticks_left; - if (fc_fcp_send_abort(fsp)) + FC_FCP_DBG(fsp, "pkt abort state %x\n", fsp->state); + if (fc_fcp_send_abort(fsp)) { + FC_FCP_DBG(fsp, "failed to send abort\n"); return FAILED; + } init_completion(&fsp->tm_done); fsp->wait_for_comp = 1; @@ -1394,6 +1409,8 @@ static void fc_fcp_timeout(unsigned long data) if (fsp->cdb_cmd.fc_tm_flags) goto unlock; + FC_FCP_DBG(fsp, "fcp timeout, flags %x state %x\n", + rpriv->flags, fsp->state); fsp->state |= FC_SRB_FCP_PROCESSING_TMO; if (rpriv->flags & FC_RP_FLAGS_REC_SUPPORTED) @@ -1486,8 +1503,8 @@ static void fc_fcp_rec_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg) rjt = fc_frame_payload_get(fp, sizeof(*rjt)); switch (rjt->er_reason) { default: - FC_FCP_DBG(fsp, "device %x unexpected REC reject " - "reason %d expl %d\n", + FC_FCP_DBG(fsp, + "device %x invalid REC reject %d/%d\n", fsp->rport->port_id, rjt->er_reason, rjt->er_explan); /* fall through */ @@ -1503,6 +1520,9 @@ static void fc_fcp_rec_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg) break; case ELS_RJT_LOGIC: case ELS_RJT_UNAB: + FC_FCP_DBG(fsp, "device %x REC reject %d/%d\n", + fsp->rport->port_id, rjt->er_reason, + rjt->er_explan); /* * If no data transfer, the command frame got dropped * so we just retry. If data was transferred, we @@ -1608,6 +1628,8 @@ static void fc_fcp_rec_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp) switch (error) { case -FC_EX_CLOSED: + FC_FCP_DBG(fsp, "REC %p fid %6.6x exchange closed\n", + fsp, fsp->rport->port_id); fc_fcp_retry_cmd(fsp); break; @@ -1622,8 +1644,8 @@ static void fc_fcp_rec_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp) * Assume REC or LS_ACC was lost. * The exchange manager will have aborted REC, so retry. */ - FC_FCP_DBG(fsp, "REC fid %6.6x error error %d retry %d/%d\n", - fsp->rport->port_id, error, fsp->recov_retry, + FC_FCP_DBG(fsp, "REC %p fid %6.6x exchange timeout retry %d/%d\n", + fsp, fsp->rport->port_id, fsp->recov_retry, FC_MAX_RECOV_RETRY); if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY) fc_fcp_rec(fsp); @@ -1642,6 +1664,7 @@ out: */ static void fc_fcp_recovery(struct fc_fcp_pkt *fsp, u8 code) { + FC_FCP_DBG(fsp, "start recovery code %x\n", code); fsp->status_code = code; fsp->cdb_status = 0; fsp->io_status = 0; @@ -1768,12 +1791,14 @@ static void fc_fcp_srr_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp) goto out; switch (PTR_ERR(fp)) { case -FC_EX_TIMEOUT: + FC_FCP_DBG(fsp, "SRR timeout, retries %d\n", fsp->recov_retry); if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY) fc_fcp_rec(fsp); else fc_fcp_recovery(fsp, FC_TIMED_OUT); break; case -FC_EX_CLOSED: /* e.g., link failure */ + FC_FCP_DBG(fsp, "SRR error, exchange closed\n"); /* fall through */ default: fc_fcp_retry_cmd(fsp); diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index 4b9bb6dd4004..bcd1cd3c5285 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -287,8 +287,10 @@ static void fc_rport_work(struct work_struct *work) kref_get(&rdata->kref); mutex_unlock(&rdata->rp_mutex); - if (!rport) + if (!rport) { + FC_RPORT_DBG(rdata, "No rport!\n"); rport = fc_remote_port_add(lport->host, 0, &ids); + } if (!rport) { FC_RPORT_DBG(rdata, "Failed to add the rport\n"); lport->tt.rport_logoff(rdata); @@ -389,8 +391,10 @@ static void fc_rport_work(struct work_struct *work) * Re-open for events. Reissue READY event if ready. */ rdata->event = RPORT_EV_NONE; - if (rdata->rp_state == RPORT_ST_READY) + if (rdata->rp_state == RPORT_ST_READY) { + FC_RPORT_DBG(rdata, "work reopen\n"); fc_rport_enter_ready(rdata); + } mutex_unlock(&rdata->rp_mutex); } break; @@ -568,6 +572,7 @@ static void fc_rport_timeout(struct work_struct *work) struct fc_lport *lport = rdata->local_port; mutex_lock(&rdata->rp_mutex); + FC_RPORT_DBG(rdata, "Port timeout, state %s\n", fc_rport_state(rdata)); switch (rdata->rp_state) { case RPORT_ST_FLOGI: @@ -1095,6 +1100,7 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, struct fc_els_spp spp; } *pp; struct fc_els_spp temp_spp; + struct fc_els_ls_rjt *rjt; struct fc4_prov *prov; u32 roles = FC_RPORT_ROLE_UNKNOWN; u32 fcp_parm = 0; @@ -1167,8 +1173,10 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, fc_rport_enter_rtv(rdata); } else { - FC_RPORT_DBG(rdata, "Bad ELS response for PRLI command\n"); - fc_rport_error_retry(rdata, fp); + rjt = fc_frame_payload_get(fp, sizeof(*rjt)); + FC_RPORT_DBG(rdata, "PRLI ELS rejected, reason %x expl %x\n", + rjt->er_reason, rjt->er_explan); + fc_rport_error_retry(rdata, NULL); } out: @@ -1602,8 +1610,12 @@ static void fc_rport_recv_els_req(struct fc_lport *lport, struct fc_frame *fp) struct fc_seq_els_data els_data; rdata = lport->tt.rport_lookup(lport, fc_frame_sid(fp)); - if (!rdata) + if (!rdata) { + FC_RPORT_ID_DBG(lport, fc_frame_sid(fp), + "Received ELS 0x%02x from non-logged-in port\n", + fc_frame_payload_op(fp)); goto reject; + } mutex_lock(&rdata->rp_mutex); @@ -1614,6 +1626,9 @@ static void fc_rport_recv_els_req(struct fc_lport *lport, struct fc_frame *fp) case RPORT_ST_ADISC: break; default: + FC_RPORT_DBG(rdata, + "Reject ELS 0x%02x while in state %s\n", + fc_frame_payload_op(fp), fc_rport_state(rdata)); mutex_unlock(&rdata->rp_mutex); kref_put(&rdata->kref, lport->tt.rport_destroy); goto reject; -- cgit v1.2.3 From a0452bb45c6bcf523acff635325fee7dd961c6bd Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 13 Oct 2016 15:10:38 +0200 Subject: scsi: libfc: spurious I/O error under high load If a command times out libfc is sending an REC, which also might fail (due to frames being lost or something). If no data has been transferred we can simply retry the command, but the current code sets a state of FC_ERROR, which then is being translated into DID_ERROR, resulting in an I/O error. So to handle this properly we need to set a separate state FC_TRANS_RESET and mapping it onto DID_SOFT_RETRY. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_fcp.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 5c6c73a80379..9f9eb7b0ba2d 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -122,6 +122,7 @@ static void fc_fcp_srr_error(struct fc_fcp_pkt *, struct fc_frame *); #define FC_HRD_ERROR 9 #define FC_CRC_ERROR 10 #define FC_TIMED_OUT 11 +#define FC_TRANS_RESET 12 /* * Error recovery timeout values. @@ -283,7 +284,7 @@ static int fc_fcp_send_abort(struct fc_fcp_pkt *fsp) * fc_io_compl() will notify the SCSI-ml that the I/O is done. * The SCSI-ml will retry the command. */ -static void fc_fcp_retry_cmd(struct fc_fcp_pkt *fsp) +static void fc_fcp_retry_cmd(struct fc_fcp_pkt *fsp, int status_code) { if (fsp->seq_ptr) { fsp->lp->tt.exch_done(fsp->seq_ptr); @@ -292,7 +293,7 @@ static void fc_fcp_retry_cmd(struct fc_fcp_pkt *fsp) fsp->state &= ~FC_SRB_ABORT_PENDING; fsp->io_status = 0; - fsp->status_code = FC_ERROR; + fsp->status_code = status_code; fc_fcp_complete_locked(fsp); } @@ -1208,7 +1209,7 @@ static void fc_fcp_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp) return; if (error == -FC_EX_CLOSED) { - fc_fcp_retry_cmd(fsp); + fc_fcp_retry_cmd(fsp, FC_ERROR); goto unlock; } @@ -1531,10 +1532,11 @@ static void fc_fcp_rec_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg) */ if (rjt->er_explan == ELS_EXPL_OXID_RXID && fsp->xfer_len == 0) { - fc_fcp_retry_cmd(fsp); + fsp->state |= FC_SRB_ABORTED; + fc_fcp_retry_cmd(fsp, FC_TRANS_RESET); break; } - fc_fcp_recovery(fsp, FC_ERROR); + fc_fcp_recovery(fsp, FC_TRANS_RESET); break; } } else if (opcode == ELS_LS_ACC) { @@ -1630,7 +1632,7 @@ static void fc_fcp_rec_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp) case -FC_EX_CLOSED: FC_FCP_DBG(fsp, "REC %p fid %6.6x exchange closed\n", fsp, fsp->rport->port_id); - fc_fcp_retry_cmd(fsp); + fc_fcp_retry_cmd(fsp, FC_ERROR); break; default: @@ -1729,7 +1731,7 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset) fc_fcp_pkt_hold(fsp); /* hold for outstanding SRR */ return; retry: - fc_fcp_retry_cmd(fsp); + fc_fcp_retry_cmd(fsp, FC_TRANS_RESET); } /** @@ -1801,7 +1803,7 @@ static void fc_fcp_srr_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp) FC_FCP_DBG(fsp, "SRR error, exchange closed\n"); /* fall through */ default: - fc_fcp_retry_cmd(fsp); + fc_fcp_retry_cmd(fsp, FC_ERROR); break; } fc_fcp_unlock_pkt(fsp); @@ -2014,6 +2016,11 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp) "due to FC_CMD_RESET\n"); sc_cmd->result = (DID_RESET << 16); break; + case FC_TRANS_RESET: + FC_FCP_DBG(fsp, "Returning DID_SOFT_ERROR to scsi-ml " + "due to FC_TRANS_RESET\n"); + sc_cmd->result = (DID_SOFT_ERROR << 16); + break; case FC_HRD_ERROR: FC_FCP_DBG(fsp, "Returning DID_NO_CONNECT to scsi-ml " "due to FC_HRD_ERROR\n"); -- cgit v1.2.3 From f7ce413ceac01d502ea4a27c0ba542b57b728e5c Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 13 Oct 2016 15:10:39 +0200 Subject: scsi: libfc: use configured lport R_A_TOV We should be using the configured R_A_TOV value when sending the exchange. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_exch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c index 91800cb776ec..99cc5a941997 100644 --- a/drivers/scsi/libfc/fc_exch.c +++ b/drivers/scsi/libfc/fc_exch.c @@ -2140,7 +2140,7 @@ static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport, ep->resp = resp; ep->destructor = destructor; ep->arg = arg; - ep->r_a_tov = FC_DEF_R_A_TOV; + ep->r_a_tov = lport->r_a_tov; ep->lp = lport; sp = &ep->seq; -- cgit v1.2.3 From a50cc9eccce6ae9708e8a713c4070dc2efd1b3d5 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 13 Oct 2016 15:10:40 +0200 Subject: scsi: libfc: use configured rport E_D_TOV If fc_rport_error_retry() is attempting to retry the remote port state we should be waiting for the configured e_d_tov value rather than the default. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_rport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index bcd1cd3c5285..bb7e9d9a3179 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -662,7 +662,7 @@ static void fc_rport_error(struct fc_rport_priv *rdata, struct fc_frame *fp) static void fc_rport_error_retry(struct fc_rport_priv *rdata, struct fc_frame *fp) { - unsigned long delay = msecs_to_jiffies(FC_DEF_E_D_TOV); + unsigned long delay = msecs_to_jiffies(rdata->e_d_tov); struct fc_lport *lport = rdata->local_port; /* make sure this isn't an FC_EX_CLOSED error, never retry those */ -- cgit v1.2.3 From 76e72ad117812bb79abf647ac40ca6df1740b729 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 13 Oct 2016 15:10:41 +0200 Subject: scsi: libfc: sanitize E_D_TOV and R_A_TOV setting When setting the FCP timeout we need to ensure a lower boundary for E_D_TOV and R_A_TOV, otherwise we'd be getting spurious I/O issues due to the fcp timer firing too early. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_fcp.c | 9 +++++---- drivers/scsi/libfc/fc_lport.c | 8 +++++--- drivers/scsi/libfc/fc_rport.c | 6 ++++-- 3 files changed, 14 insertions(+), 9 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 9f9eb7b0ba2d..10faca2686ba 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -1137,8 +1137,11 @@ static int fc_fcp_pkt_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp) static inline unsigned int get_fsp_rec_tov(struct fc_fcp_pkt *fsp) { struct fc_rport_libfc_priv *rpriv = fsp->rport->dd_data; + unsigned int e_d_tov = FC_DEF_E_D_TOV; - return msecs_to_jiffies(rpriv->e_d_tov) + HZ; + if (rpriv && rpriv->e_d_tov > e_d_tov) + e_d_tov = rpriv->e_d_tov; + return msecs_to_jiffies(e_d_tov) + HZ; } /** @@ -1693,7 +1696,6 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset) struct fc_seq *seq; struct fcp_srr *srr; struct fc_frame *fp; - unsigned int rec_tov; rport = fsp->rport; rpriv = rport->dd_data; @@ -1717,10 +1719,9 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset) rpriv->local_port->port_id, FC_TYPE_FCP, FC_FCTL_REQ, 0); - rec_tov = get_fsp_rec_tov(fsp); seq = lport->tt.exch_seq_send(lport, fp, fc_fcp_srr_resp, fc_fcp_pkt_destroy, - fsp, jiffies_to_msecs(rec_tov)); + fsp, get_fsp_rec_tov(fsp)); if (!seq) goto retry; diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c index 4e11c90c783c..81ad8ac5a33d 100644 --- a/drivers/scsi/libfc/fc_lport.c +++ b/drivers/scsi/libfc/fc_lport.c @@ -1777,7 +1777,7 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, if ((csp_flags & FC_SP_FT_FPORT) == 0) { if (e_d_tov > lport->e_d_tov) lport->e_d_tov = e_d_tov; - lport->r_a_tov = 2 * e_d_tov; + lport->r_a_tov = 2 * lport->e_d_tov; fc_lport_set_port_id(lport, did, fp); printk(KERN_INFO "host%d: libfc: " "Port (%6.6x) entered " @@ -1789,8 +1789,10 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, get_unaligned_be64( &flp->fl_wwnn)); } else { - lport->e_d_tov = e_d_tov; - lport->r_a_tov = r_a_tov; + if (e_d_tov > lport->e_d_tov) + lport->e_d_tov = e_d_tov; + if (r_a_tov > lport->r_a_tov) + lport->r_a_tov = r_a_tov; fc_host_fabric_name(lport->host) = get_unaligned_be64(&flp->fl_wwnn); fc_lport_set_port_id(lport, did, fp); diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index bb7e9d9a3179..3d2baba4103e 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -1296,13 +1296,15 @@ static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp, tov = ntohl(rtv->rtv_r_a_tov); if (tov == 0) tov = 1; - rdata->r_a_tov = tov; + if (tov > rdata->r_a_tov) + rdata->r_a_tov = tov; tov = ntohl(rtv->rtv_e_d_tov); if (toq & FC_ELS_RTV_EDRES) tov /= 1000000; if (tov == 0) tov = 1; - rdata->e_d_tov = tov; + if (tov > rdata->e_d_tov) + rdata->e_d_tov = tov; } } -- cgit v1.2.3 From 0f4c16a2f41400dba12e5039429e780aa938aa0c Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 13 Oct 2016 15:10:43 +0200 Subject: scsi: libfc: do not overwrite DID_TIME_OUT status When a command is aborted it might already have the DID_TIME_OUT status set, so we shouldn't be overwriting that. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_fcp.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 10faca2686ba..0e2a2016af71 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -2008,9 +2008,15 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp) sc_cmd->result = (DID_ERROR << 16) | fsp->cdb_status; break; case FC_CMD_ABORTED: - FC_FCP_DBG(fsp, "Returning DID_ERROR to scsi-ml " - "due to FC_CMD_ABORTED\n"); - sc_cmd->result = (DID_ERROR << 16) | fsp->io_status; + if (host_byte(sc_cmd->result) == DID_TIME_OUT) + FC_FCP_DBG(fsp, "Returning DID_TIME_OUT to scsi-ml " + "due to FC_CMD_ABORTED\n"); + else { + FC_FCP_DBG(fsp, "Returning DID_ERROR to scsi-ml " + "due to FC_CMD_ABORTED\n"); + set_host_byte(sc_cmd, DID_ERROR); + } + sc_cmd->result |= fsp->io_status; break; case FC_CMD_RESET: FC_FCP_DBG(fsp, "Returning DID_RESET to scsi-ml " -- cgit v1.2.3 From 9f9504a7cdee39e167f0421346ff17568a5f29a0 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 13 Oct 2016 15:10:44 +0200 Subject: scsi: libfc: use error code for fc_rport_error() We only ever use the 'fp' argument for fc_rport_error() to encapsulate the error code, so we can as well do away with that and pass the error directly. Signed-off-by: Hannes Reinecke Acked-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/libfc/fc_rport.c | 101 +++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 40 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index 3d2baba4103e..d2d8607ff6a0 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -87,8 +87,8 @@ static void fc_rport_recv_prli_req(struct fc_rport_priv *, struct fc_frame *); static void fc_rport_recv_prlo_req(struct fc_rport_priv *, struct fc_frame *); static void fc_rport_recv_logo_req(struct fc_lport *, struct fc_frame *); static void fc_rport_timeout(struct work_struct *); -static void fc_rport_error(struct fc_rport_priv *, struct fc_frame *); -static void fc_rport_error_retry(struct fc_rport_priv *, struct fc_frame *); +static void fc_rport_error(struct fc_rport_priv *, int); +static void fc_rport_error_retry(struct fc_rport_priv *, int); static void fc_rport_work(struct work_struct *); static const char *fc_rport_state_names[] = { @@ -604,20 +604,19 @@ static void fc_rport_timeout(struct work_struct *work) /** * fc_rport_error() - Error handler, called once retries have been exhausted * @rdata: The remote port the error is happened on - * @fp: The error code encapsulated in a frame pointer + * @err: The error code * * Locking Note: The rport lock is expected to be held before * calling this routine * * Reference counting: does not modify kref */ -static void fc_rport_error(struct fc_rport_priv *rdata, struct fc_frame *fp) +static void fc_rport_error(struct fc_rport_priv *rdata, int err) { struct fc_lport *lport = rdata->local_port; - FC_RPORT_DBG(rdata, "Error %ld in state %s, retries %d\n", - IS_ERR(fp) ? -PTR_ERR(fp) : 0, - fc_rport_state(rdata), rdata->retries); + FC_RPORT_DBG(rdata, "Error %d in state %s, retries %d\n", + -err, fc_rport_state(rdata), rdata->retries); switch (rdata->rp_state) { case RPORT_ST_FLOGI: @@ -649,7 +648,7 @@ static void fc_rport_error(struct fc_rport_priv *rdata, struct fc_frame *fp) /** * fc_rport_error_retry() - Handler for remote port state retries * @rdata: The remote port whose state is to be retried - * @fp: The error code encapsulated in a frame pointer + * @err: The error code * * If the error was an exchange timeout retry immediately, * otherwise wait for E_D_TOV. @@ -659,22 +658,21 @@ static void fc_rport_error(struct fc_rport_priv *rdata, struct fc_frame *fp) * * Reference counting: increments kref when scheduling retry_work */ -static void fc_rport_error_retry(struct fc_rport_priv *rdata, - struct fc_frame *fp) +static void fc_rport_error_retry(struct fc_rport_priv *rdata, int err) { unsigned long delay = msecs_to_jiffies(rdata->e_d_tov); struct fc_lport *lport = rdata->local_port; /* make sure this isn't an FC_EX_CLOSED error, never retry those */ - if (PTR_ERR(fp) == -FC_EX_CLOSED) + if (err == -FC_EX_CLOSED) goto out; if (rdata->retries < rdata->local_port->max_rport_retry_count) { - FC_RPORT_DBG(rdata, "Error %ld in state %s, retrying\n", - PTR_ERR(fp), fc_rport_state(rdata)); + FC_RPORT_DBG(rdata, "Error %d in state %s, retrying\n", + err, fc_rport_state(rdata)); rdata->retries++; /* no additional delay on exchange timeouts */ - if (PTR_ERR(fp) == -FC_EX_TIMEOUT) + if (err == -FC_EX_TIMEOUT) delay = 0; kref_get(&rdata->kref); if (!schedule_delayed_work(&rdata->retry_work, delay)) @@ -683,7 +681,7 @@ static void fc_rport_error_retry(struct fc_rport_priv *rdata, } out: - fc_rport_error(rdata, fp); + fc_rport_error(rdata, err); } /** @@ -743,8 +741,11 @@ static void fc_rport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, struct fc_lport *lport = rdata->local_port; struct fc_els_flogi *flogi; unsigned int r_a_tov; + u8 opcode; + int err = 0; - FC_RPORT_DBG(rdata, "Received a FLOGI %s\n", fc_els_resp_type(fp)); + FC_RPORT_DBG(rdata, "Received a FLOGI %s\n", + IS_ERR(fp) ? "error" : fc_els_resp_type(fp)); if (fp == ERR_PTR(-FC_EX_CLOSED)) goto put; @@ -760,18 +761,32 @@ static void fc_rport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, } if (IS_ERR(fp)) { - fc_rport_error(rdata, fp); + fc_rport_error(rdata, PTR_ERR(fp)); goto err; } + opcode = fc_frame_payload_op(fp); + if (opcode == ELS_LS_RJT) { + struct fc_els_ls_rjt *rjt; - if (fc_frame_payload_op(fp) != ELS_LS_ACC) + rjt = fc_frame_payload_get(fp, sizeof(*rjt)); + FC_RPORT_DBG(rdata, "FLOGI ELS rejected, reason %x expl %x\n", + rjt->er_reason, rjt->er_explan); + err = -FC_EX_ELS_RJT; + goto bad; + } else if (opcode != ELS_LS_ACC) { + FC_RPORT_DBG(rdata, "FLOGI ELS invalid opcode %x\n", opcode); + err = -FC_EX_ELS_RJT; goto bad; - if (fc_rport_login_complete(rdata, fp)) + } + if (fc_rport_login_complete(rdata, fp)) { + FC_RPORT_DBG(rdata, "FLOGI failed, no login\n"); + err = -FC_EX_INV_LOGIN; goto bad; + } flogi = fc_frame_payload_get(fp, sizeof(*flogi)); if (!flogi) { - FC_RPORT_DBG(rdata, "Bad FLOGI response\n"); + err = -FC_EX_ALLOC_ERR; goto bad; } r_a_tov = ntohl(flogi->fl_csp.sp_r_a_tov); @@ -790,7 +805,8 @@ put: kref_put(&rdata->kref, lport->tt.rport_destroy); return; bad: - fc_rport_error_retry(rdata, fp); + FC_RPORT_DBG(rdata, "Bad FLOGI response\n"); + fc_rport_error_retry(rdata, err); goto out; } @@ -818,13 +834,13 @@ static void fc_rport_enter_flogi(struct fc_rport_priv *rdata) fp = fc_frame_alloc(lport, sizeof(struct fc_els_flogi)); if (!fp) - return fc_rport_error_retry(rdata, fp); + return fc_rport_error_retry(rdata, -FC_EX_ALLOC_ERR); kref_get(&rdata->kref); if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_FLOGI, fc_rport_flogi_resp, rdata, 2 * lport->r_a_tov)) { - fc_rport_error_retry(rdata, NULL); + fc_rport_error_retry(rdata, -FC_EX_XMIT_ERR); kref_put(&rdata->kref, lport->tt.rport_destroy); } } @@ -991,7 +1007,7 @@ static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp, } if (IS_ERR(fp)) { - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, PTR_ERR(fp)); goto err; } @@ -1013,9 +1029,14 @@ static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp, rdata->max_seq = csp_seq; rdata->maxframe_size = fc_plogi_get_maxframe(plp, lport->mfs); fc_rport_enter_prli(rdata); - } else - fc_rport_error_retry(rdata, fp); + } else { + struct fc_els_ls_rjt *rjt; + rjt = fc_frame_payload_get(fp, sizeof(*rjt)); + FC_RPORT_DBG(rdata, "PLOGI ELS rejected, reason %x expl %x\n", + rjt->er_reason, rjt->er_explan); + fc_rport_error_retry(rdata, -FC_EX_ELS_RJT); + } out: fc_frame_free(fp); err: @@ -1067,7 +1088,7 @@ static void fc_rport_enter_plogi(struct fc_rport_priv *rdata) fp = fc_frame_alloc(lport, sizeof(struct fc_els_flogi)); if (!fp) { FC_RPORT_DBG(rdata, "%s frame alloc failed\n", __func__); - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, -FC_EX_ALLOC_ERR); return; } rdata->e_d_tov = lport->e_d_tov; @@ -1076,7 +1097,7 @@ static void fc_rport_enter_plogi(struct fc_rport_priv *rdata) if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_PLOGI, fc_rport_plogi_resp, rdata, 2 * lport->r_a_tov)) { - fc_rport_error_retry(rdata, NULL); + fc_rport_error_retry(rdata, -FC_EX_XMIT_ERR); kref_put(&rdata->kref, lport->tt.rport_destroy); } } @@ -1123,7 +1144,7 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, } if (IS_ERR(fp)) { - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, PTR_ERR(fp)); goto err; } @@ -1142,9 +1163,9 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, rdata->spp_type = pp->spp.spp_type; if (resp_code != FC_SPP_RESP_ACK) { if (resp_code == FC_SPP_RESP_CONF) - fc_rport_error(rdata, fp); + fc_rport_error(rdata, -FC_EX_SEQ_ERR); else - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, -FC_EX_SEQ_ERR); goto out; } if (pp->prli.prli_spp_len < sizeof(pp->spp)) @@ -1176,7 +1197,7 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, rjt = fc_frame_payload_get(fp, sizeof(*rjt)); FC_RPORT_DBG(rdata, "PRLI ELS rejected, reason %x expl %x\n", rjt->er_reason, rjt->er_explan); - fc_rport_error_retry(rdata, NULL); + fc_rport_error_retry(rdata, FC_EX_ELS_RJT); } out: @@ -1222,7 +1243,7 @@ static void fc_rport_enter_prli(struct fc_rport_priv *rdata) fp = fc_frame_alloc(lport, sizeof(*pp)); if (!fp) { - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, -FC_EX_ALLOC_ERR); return; } @@ -1241,7 +1262,7 @@ static void fc_rport_enter_prli(struct fc_rport_priv *rdata) kref_get(&rdata->kref); if (!lport->tt.exch_seq_send(lport, fp, fc_rport_prli_resp, NULL, rdata, 2 * lport->r_a_tov)) { - fc_rport_error_retry(rdata, NULL); + fc_rport_error_retry(rdata, -FC_EX_XMIT_ERR); kref_put(&rdata->kref, lport->tt.rport_destroy); } } @@ -1280,7 +1301,7 @@ static vo