[RFC PATCH 0/4] libfc: Do not send ABTS when resetting exchanges

[RFC PATCH 0/4] libfc: Do not send ABTS when resetting exchanges

[Hannes Reinecke]

Hi all,

here's a patchset to fixup an issue reported by QLogic: when running
against a certain target the libfc stack would get in an infinite loop
sending ABTS when trying to reset a connection.
The problem is that the libfc stack is sending ABTS for any open exchanges
when removing an rport, totally ignorant of the fact that in most cases
we are already logged out of the said remote port.
This patchset alters the handling slightly so that we will only ever reset
open exchanges after we have either logged out of the port or an implicit
logout can be assumed.
With that we don't need to send ABTS any more during rport reset.

As usual, comments and reviews are welcome.

Hannes Reinecke (4):
  libfc: Issue PRLI after a PRLO has been received
  libfc: send LOGO for PLOGI failure
  libfc: reset exchange manager during LOGO handling
  libfc: do not send ABTS when resetting exchanges

 drivers/scsi/libfc/fc_exch.c  |  1 -
 drivers/scsi/libfc/fc_rport.c | 26 +++++++++++++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

-- 
1.8.5.6

[PATCH 1/4] libfc: Issue PRLI after a PRLO has been received

[Hannes Reinecke]

When receiving a PRLO it just means that the operating parameters
have changed, it does _not_ mean that the port doesn't want to
communicate with us.
So instead of implicitly logging out we should be issueing a PRLI
to figure out the new operating parameters.
We can always recover once PRLI fails.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libfc/fc_rport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index 7fb6685..0ea0d4f 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -2034,7 +2034,7 @@ static void fc_rport_recv_prlo_req(struct fc_rport_priv *rdata,
 	spp->spp_type_ext = rspp->spp_type_ext;
 	spp->spp_flags = FC_SPP_RESP_ACK;
 
-	fc_rport_enter_delete(rdata, RPORT_EV_LOGO);
+	fc_rport_enter_prli(rdata);
 
 	fc_fill_reply_hdr(fp, rx_fp, FC_RCTL_ELS_REP, 0);
 	lport->tt.frame_send(lport, fp);
-- 
1.8.5.6

[PATCH 2/4] libfc: send LOGO for PLOGI failure

[Hannes Reinecke]

When running in point-to-multipoint mode PLOGI is done after
FLOGI completed. So when the PLOGI fails we should be sending
a LOGO to the remote port.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libfc/fc_rport.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index 0ea0d4f..bfad971 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -593,15 +593,23 @@ static void fc_rport_timeout(struct work_struct *work)
  */
 static void fc_rport_error(struct fc_rport_priv *rdata, int err)
 {
+	struct fc_lport *lport = rdata->local_port;
+
 	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:
-	case RPORT_ST_PLOGI:
 		rdata->flags &= ~FC_RP_STARTED;
 		fc_rport_enter_delete(rdata, RPORT_EV_FAILED);
 		break;
+	case RPORT_ST_PLOGI:
+		if (lport->point_to_multipoint) {
+			rdata->flags &= ~FC_RP_STARTED;
+			fc_rport_enter_delete(rdata, RPORT_EV_FAILED);
+		} else
+			fc_rport_enter_logo(rdata);
+		break;
 	case RPORT_ST_RTV:
 		fc_rport_enter_ready(rdata);
 		break;
-- 
1.8.5.6

[PATCH 3/4] libfc: reset exchange manager during LOGO handling

[Hannes Reinecke]

FC-LS mandates that we should invalidate all sequences before
sending a LOGO. And we should set the event to RPORT_EV_STOP
when a LOGO request has been received to signal that all
exchanges are terminated.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libfc/fc_rport.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index bfad971..05fccd3 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -489,6 +489,9 @@ static void fc_rport_enter_delete(struct fc_rport_priv *rdata,
  */
 static int fc_rport_logoff(struct fc_rport_priv *rdata)
 {
+	struct fc_lport *lport = rdata->local_port;
+	u32 port_id = rdata->ids.port_id;
+
 	mutex_lock(&rdata->rp_mutex);
 
 	FC_RPORT_DBG(rdata, "Remove port\n");
@@ -498,6 +501,15 @@ static int fc_rport_logoff(struct fc_rport_priv *rdata)
 		FC_RPORT_DBG(rdata, "Port in Delete state, not removing\n");
 		goto out;
 	}
+	/*
+	 * FC-LS states:
+	 * To explicitly Logout, the initiating Nx_Port shall terminate
+	 * other open Sequences that it initiated with the destination
+	 * Nx_Port prior to performing Logout.
+	 */
+	lport->tt.exch_mgr_reset(lport, 0, port_id);
+	lport->tt.exch_mgr_reset(lport, port_id, 0);
+
 	fc_rport_enter_logo(rdata);
 
 	/*
@@ -2082,7 +2094,7 @@ static void fc_rport_recv_logo_req(struct fc_lport *lport, struct fc_frame *fp)
 		FC_RPORT_DBG(rdata, "Received LOGO request while in state %s\n",
 			     fc_rport_state(rdata));
 
-		fc_rport_enter_delete(rdata, RPORT_EV_LOGO);
+		fc_rport_enter_delete(rdata, RPORT_EV_STOP);
 		mutex_unlock(&rdata->rp_mutex);
 		kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy);
 	} else
-- 
1.8.5.6

[PATCH 4/4] libfc: do not send ABTS when resetting exchanges

[Hannes Reinecke]

When all exchanges are reset the upper layers have already logged
out of the remote port, so the exchanges can be reset without
sending any ABTS.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libfc/fc_exch.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 3e9275f..823cc31 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1860,7 +1860,6 @@ static void fc_exch_reset(struct fc_exch *ep)
 	int rc = 1;
 
 	spin_lock_bh(&ep->ex_lock);
-	fc_exch_abort_locked(ep, 0);
 	ep->state |= FC_EX_RST_CLEANUP;
 	fc_exch_timer_cancel(ep);
 	if (ep->esb_stat & ESB_ST_REC_QUAL)
-- 
1.8.5.6

Re: [RFC PATCH 0/4] libfc: Do not send ABTS when resetting exchanges

[Chad Dupuis]

On Fri, 5 Aug 2016, 12:54pm -0000, Hannes Reinecke wrote:

> Hi all,
> 
> here's a patchset to fixup an issue reported by QLogic: when running
> against a certain target the libfc stack would get in an infinite loop
> sending ABTS when trying to reset a connection.
> The problem is that the libfc stack is sending ABTS for any open exchanges
> when removing an rport, totally ignorant of the fact that in most cases
> we are already logged out of the said remote port.
> This patchset alters the handling slightly so that we will only ever reset
> open exchanges after we have either logged out of the port or an implicit
> logout can be assumed.
> With that we don't need to send ABTS any more during rport reset.
> 
> As usual, comments and reviews are welcome.
> 
> Hannes Reinecke (4):
>   libfc: Issue PRLI after a PRLO has been received
>   libfc: send LOGO for PLOGI failure
>   libfc: reset exchange manager during LOGO handling
>   libfc: do not send ABTS when resetting exchanges
> 
>  drivers/scsi/libfc/fc_exch.c  |  1 -
>  drivers/scsi/libfc/fc_rport.c | 26 +++++++++++++++++++++++---
>  2 files changed, 23 insertions(+), 4 deletions(-)
> 
> 

Hannes, thanks for posting this.  I've been working witih a customer of 
ours and have successfully tested this patch set.

Reviewed-by: Chad Dupuis <chad.dupuis@qlogic.com>
Tested-by: Chad Dupuis <chad.dupuis@qlogic.com>

Re: [RFC PATCH 0/4] libfc: Do not send ABTS when resetting exchanges

[Martin K. Petersen]

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

Hannes> Hi all, here's a patchset to fixup an issue reported by QLogic:
Hannes> when running against a certain target the libfc stack would get
Hannes> in an infinite loop sending ABTS when trying to reset a
Hannes> connection.  The problem is that the libfc stack is sending ABTS
Hannes> for any open exchanges when removing an rport, totally ignorant
Hannes> of the fact that in most cases we are already logged out of the
Hannes> said remote port.  This patchset alters the handling slightly so
Hannes> that we will only ever reset open exchanges after we have either
Hannes> logged out of the port or an implicit logout can be assumed.
Hannes> With that we don't need to send ABTS any more during rport
Hannes> reset.

Applied to 4.9/scsi-queue. I had to do patch 2 by hand. Please check.

-- 
Martin K. Petersen	Oracle Linux Engineering