[PATCH 00/16] libfc, libfcoe and fcoe fixes for 2.6.34

[PATCH 00/16] libfc, libfcoe and fcoe fixes for 2.6.34

[Robert Love]

The following series implements a few fixes to libfc, libfcoe and fcoe. The majority
of fixes are to the Point-to-point code, but there are a few other fixes as well.

These patches were applied and tested against 2.6.34-rc1 before submission.

---

Joe Eykholt (12):
      libfc, libfcoe, fcoe: use smp_processor_id() only when preempt disabled
      libfc: fix sequence-initiative WARN in fc_seq_start_next
      libfcoe: eliminate unused link and last_link fields
      fcoe: remove an unused variable in fcoe_recv_frame()
      libfc: send point-to-poin FLOGI LS_ACC to assigned D_DID
      libfc: recognize incoming FLOGI for point-to-point mode
      fcoe: save gateway address when receiving FLOGI request
      libfcoe: fix debug message entering non-FIP mode
      fcoe: call fcoe_ctlr_els_send even for ELS responses
      libfc: fix oops in point-to-point mode
      libfc: add definition for task attribute mask
      libfc: recode incoming PRLI handling

Robert Love (1):
      libfc: Add debug statements when fc_fcp returns DID_ERROR to scsi-ml

Vasu Dev (3):
      libfc: fix fcp pkt recovery in fc_fcp_recv_data
      libfc: use offload EM instance again instead jumping to next EM
      libfc: fixes unnecessary seq id jump


 drivers/scsi/fcoe/fcoe.c      |   82 +++++++++++------
 drivers/scsi/fcoe/libfcoe.c   |   65 ++++++--------
 drivers/scsi/libfc/fc_exch.c  |   39 +++-----
 drivers/scsi/libfc/fc_fcp.c   |   74 +++++++++++-----
 drivers/scsi/libfc/fc_lport.c |   13 ++-
 drivers/scsi/libfc/fc_npiv.c  |    3 +
 drivers/scsi/libfc/fc_rport.c |  195 ++++++++++++++++++-----------------------
 include/scsi/fc/fc_fcp.h      |    1 
 include/scsi/libfc.h          |    9 --
 include/scsi/libfcoe.h        |    8 --
 10 files changed, 243 insertions(+), 246 deletions(-)

-- 
//Rob

[PATCH 01/16] libfc: recode incoming PRLI handling

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

Reduce indentation in fc_rport_recv_prli_req() using gotos.
Also add payload length checks.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/libfc/fc_rport.c |  195 ++++++++++++++++++-----------------------
 1 files changed, 87 insertions(+), 108 deletions(-)

diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index 97923bb..09ec635 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -1441,136 +1441,115 @@ static void fc_rport_recv_prli_req(struct fc_rport_priv *rdata,
 	struct fc_els_spp *spp;	/* response spp */
 	unsigned int len;
 	unsigned int plen;
-	enum fc_els_rjt_reason reason = ELS_RJT_UNAB;
-	enum fc_els_rjt_explan explan = ELS_EXPL_NONE;
 	enum fc_els_spp_resp resp;
 	struct fc_seq_els_data rjt_data;
 	u32 f_ctl;
 	u32 fcp_parm;
 	u32 roles = FC_RPORT_ROLE_UNKNOWN;
-	rjt_data.fp = NULL;
 
+	rjt_data.fp = NULL;
 	fh = fc_frame_header_get(rx_fp);
 
 	FC_RPORT_DBG(rdata, "Received PRLI request while in state %s\n",
 		     fc_rport_state(rdata));
 
-	switch (rdata->rp_state) {
-	case RPORT_ST_PRLI:
-	case RPORT_ST_RTV:
-	case RPORT_ST_READY:
-	case RPORT_ST_ADISC:
-		reason = ELS_RJT_NONE;
-		break;
-	default:
-		fc_frame_free(rx_fp);
-		return;
-		break;
-	}
 	len = fr_len(rx_fp) - sizeof(*fh);
 	pp = fc_frame_payload_get(rx_fp, sizeof(*pp));
-	if (pp == NULL) {
-		reason = ELS_RJT_PROT;
-		explan = ELS_EXPL_INV_LEN;
-	} else {
-		plen = ntohs(pp->prli.prli_len);
-		if ((plen % 4) != 0 || plen > len) {
-			reason = ELS_RJT_PROT;
-			explan = ELS_EXPL_INV_LEN;
-		} else if (plen < len) {
-			len = plen;
-		}
-		plen = pp->prli.prli_spp_len;
-		if ((plen % 4) != 0 || plen < sizeof(*spp) ||
-		    plen > len || len < sizeof(*pp)) {
-			reason = ELS_RJT_PROT;
-			explan = ELS_EXPL_INV_LEN;
-		}
-		rspp = &pp->spp;
+	if (!pp)
+		goto reject_len;
+	plen = ntohs(pp->prli.prli_len);
+	if ((plen % 4) != 0 || plen > len || plen < 16)
+		goto reject_len;
+	if (plen < len)
+		len = plen;
+	plen = pp->prli.prli_spp_len;
+	if ((plen % 4) != 0 || plen < sizeof(*spp) ||
+	    plen > len || len < sizeof(*pp) || plen < 12)
+		goto reject_len;
+	rspp = &pp->spp;
+
+	fp = fc_frame_alloc(lport, len);
+	if (!fp) {
+		rjt_data.reason = ELS_RJT_UNAB;
+		rjt_data.explan = ELS_EXPL_INSUF_RES;
+		goto reject;
 	}
-	if (reason != ELS_RJT_NONE ||
-	    (fp = fc_frame_alloc(lport, len)) == NULL) {
-		rjt_data.reason = reason;
-		rjt_data.explan = explan;
-		lport->tt.seq_els_rsp_send(sp, ELS_LS_RJT, &rjt_data);
-	} else {
-		sp = lport->tt.seq_start_next(sp);
-		WARN_ON(!sp);
-		pp = fc_frame_payload_get(fp, len);
-		WARN_ON(!pp);
-		memset(pp, 0, len);
-		pp->prli.prli_cmd = ELS_LS_ACC;
-		pp->prli.prli_spp_len = plen;
-		pp->prli.prli_len = htons(len);
-		len -= sizeof(struct fc_els_prli);
-
-		/* reinitialize remote port roles */
-		rdata->ids.roles = FC_RPORT_ROLE_UNKNOWN;
-
-		/*
-		 * Go through all the service parameter pages and build
-		 * response.  If plen indicates longer SPP than standard,
-		 * use that.  The entire response has been pre-cleared above.
-		 */
-		spp = &pp->spp;
-		while (len >= plen) {
-			spp->spp_type = rspp->spp_type;
-			spp->spp_type_ext = rspp->spp_type_ext;
-			spp->spp_flags = rspp->spp_flags & FC_SPP_EST_IMG_PAIR;
-			resp = FC_SPP_RESP_ACK;
-			if (rspp->spp_flags & FC_SPP_RPA_VAL)
-				resp = FC_SPP_RESP_NO_PA;
-			switch (rspp->spp_type) {
-			case 0:	/* common to all FC-4 types */
-				break;
-			case FC_TYPE_FCP:
-				fcp_parm = ntohl(rspp->spp_params);
-				if (fcp_parm & FCP_SPPF_RETRY)
-					rdata->flags |= FC_RP_FLAGS_RETRY;
-				rdata->supported_classes = FC_COS_CLASS3;
-				if (fcp_parm & FCP_SPPF_INIT_FCN)
-					roles |= FC_RPORT_ROLE_FCP_INITIATOR;
-				if (fcp_parm & FCP_SPPF_TARG_FCN)
-					roles |= FC_RPORT_ROLE_FCP_TARGET;
-				rdata->ids.roles = roles;
-
-				spp->spp_params =
-					htonl(lport->service_params);
-				break;
-			default:
-				resp = FC_SPP_RESP_INVL;
-				break;
-			}
-			spp->spp_flags |= resp;
-			len -= plen;
-			rspp = (struct fc_els_spp *)((char *)rspp + plen);
-			spp = (struct fc_els_spp *)((char *)spp + plen);
-		}
+	sp = lport->tt.seq_start_next(sp);
+	WARN_ON(!sp);
+	pp = fc_frame_payload_get(fp, len);
+	WARN_ON(!pp);
+	memset(pp, 0, len);
+	pp->prli.prli_cmd = ELS_LS_ACC;
+	pp->prli.prli_spp_len = plen;
+	pp->prli.prli_len = htons(len);
+	len -= sizeof(struct fc_els_prli);
 
-		/*
-		 * Send LS_ACC.	 If this fails, the originator should retry.
-		 */
-		f_ctl = FC_FC_EX_CTX | FC_FC_LAST_SEQ;
-		f_ctl |= FC_FC_END_SEQ | FC_FC_SEQ_INIT;
-		ep = fc_seq_exch(sp);
-		fc_fill_fc_hdr(fp, FC_RCTL_ELS_REP, ep->did, ep->sid,
-			       FC_TYPE_ELS, f_ctl, 0);
-		lport->tt.seq_send(lport, sp, fp);
+	/* reinitialize remote port roles */
+	rdata->ids.roles = FC_RPORT_ROLE_UNKNOWN;
 
-		/*
-		 * Get lock and re-check state.
-		 */
-		switch (rdata->rp_state) {
-		case RPORT_ST_PRLI:
-			fc_rport_enter_ready(rdata);
+	/*
+	 * Go through all the service parameter pages and build
+	 * response.  If plen indicates longer SPP than standard,
+	 * use that.  The entire response has been pre-cleared above.
+	 */
+	spp = &pp->spp;
+	while (len >= plen) {
+		spp->spp_type = rspp->spp_type;
+		spp->spp_type_ext = rspp->spp_type_ext;
+		spp->spp_flags = rspp->spp_flags & FC_SPP_EST_IMG_PAIR;
+		resp = FC_SPP_RESP_ACK;
+
+		switch (rspp->spp_type) {
+		case 0:	/* common to all FC-4 types */
 			break;
-		case RPORT_ST_READY:
-		case RPORT_ST_ADISC:
+		case FC_TYPE_FCP:
+			fcp_parm = ntohl(rspp->spp_params);
+			if (fcp_parm & FCP_SPPF_RETRY)
+				rdata->flags |= FC_RP_FLAGS_RETRY;
+			rdata->supported_classes = FC_COS_CLASS3;
+			if (fcp_parm & FCP_SPPF_INIT_FCN)
+				roles |= FC_RPORT_ROLE_FCP_INITIATOR;
+			if (fcp_parm & FCP_SPPF_TARG_FCN)
+				roles |= FC_RPORT_ROLE_FCP_TARGET;
+			rdata->ids.roles = roles;
+
+			spp->spp_params = htonl(lport->service_params);
 			break;
 		default:
+			resp = FC_SPP_RESP_INVL;
 			break;
 		}
+		spp->spp_flags |= resp;
+		len -= plen;
+		rspp = (struct fc_els_spp *)((char *)rspp + plen);
+		spp = (struct fc_els_spp *)((char *)spp + plen);
+	}
+
+	/*
+	 * Send LS_ACC.	 If this fails, the originator should retry.
+	 */
+	f_ctl = FC_FC_EX_CTX | FC_FC_LAST_SEQ;
+	f_ctl |= FC_FC_END_SEQ | FC_FC_SEQ_INIT;
+	ep = fc_seq_exch(sp);
+	fc_fill_fc_hdr(fp, FC_RCTL_ELS_REP, ep->did, ep->sid,
+		       FC_TYPE_ELS, f_ctl, 0);
+	lport->tt.seq_send(lport, sp, fp);
+
+	switch (rdata->rp_state) {
+	case RPORT_ST_PRLI:
+		fc_rport_enter_ready(rdata);
+		break;
+	default:
+		break;
 	}
+	goto drop;
+
+reject_len:
+	rjt_data.reason = ELS_RJT_PROT;
+	rjt_data.explan = ELS_EXPL_INV_LEN;
+reject:
+	lport->tt.seq_els_rsp_send(sp, ELS_LS_RJT, &rjt_data);
+drop:
 	fc_frame_free(rx_fp);
 }
 

[PATCH 02/16] libfc: add definition for task attribute mask

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

The FCP command header definition should define a mask for
the task attribute field.  This adds that #define.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 include/scsi/fc/fc_fcp.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/scsi/fc/fc_fcp.h b/include/scsi/fc/fc_fcp.h
index 747e2c7..8e9b222 100644
--- a/include/scsi/fc/fc_fcp.h
+++ b/include/scsi/fc/fc_fcp.h
@@ -76,6 +76,7 @@ struct fcp_cmnd32 {
 #define	FCP_PTA_HEADQ	    1	/* head of queue task attribute */
 #define	FCP_PTA_ORDERED     2	/* ordered task attribute */
 #define	FCP_PTA_ACA	    4	/* auto. contigent allegiance */
+#define	FCP_PTA_MASK	    7	/* mask for task attribute field */
 #define	FCP_PRI_SHIFT	    3	/* priority field starts in bit 3 */
 #define	FCP_PRI_RESVD_MASK  0x80	/* reserved bits in priority field */
 

[PATCH 03/16] libfc: fix oops in point-to-point mode

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

In point-to-point mode, if the PLOGI to the remote port times
out, it can get deleted by the remote port module.  Since there's
no reference by the local port, lport->ptp_data points to a freed
rport, and when the local port is reset and tries to logout again,
an oops occurs in mutex_lock_nested().

Hold a reference count on the point-to-point rdata.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/libfc/fc_lport.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index 7ec8ce7..af7343a 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -227,9 +227,12 @@ static void fc_lport_ptp_setup(struct fc_lport *lport,
 			       u64 remote_wwnn)
 {
 	mutex_lock(&lport->disc.disc_mutex);
-	if (lport->ptp_rdata)
+	if (lport->ptp_rdata) {
 		lport->tt.rport_logoff(lport->ptp_rdata);
+		kref_put(&lport->ptp_rdata->kref, lport->tt.rport_destroy);
+	}
 	lport->ptp_rdata = lport->tt.rport_create(lport, remote_fid);
+	kref_get(&lport->ptp_rdata->kref);
 	lport->ptp_rdata->ids.port_name = remote_wwpn;
 	lport->ptp_rdata->ids.node_name = remote_wwnn;
 	mutex_unlock(&lport->disc.disc_mutex);
@@ -946,7 +949,11 @@ static void fc_lport_reset_locked(struct fc_lport *lport)
 	if (lport->dns_rdata)
 		lport->tt.rport_logoff(lport->dns_rdata);
 
-	lport->ptp_rdata = NULL;
+	if (lport->ptp_rdata) {
+		lport->tt.rport_logoff(lport->ptp_rdata);
+		kref_put(&lport->ptp_rdata->kref, lport->tt.rport_destroy);
+		lport->ptp_rdata = NULL;
+	}
 
 	lport->tt.disc_stop(lport);
 

[PATCH 04/16] fcoe: call fcoe_ctlr_els_send even for ELS responses

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

In point-to-point mode, the destination MAC address for
the FLOGI response was zero because the LS_ACC for the FLOGI
wasn't getting intercepted by FIP.

Change to call fcoe_ctlr_els_send when sending any ELS,
not just requests.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/fcoe/fcoe.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 2f47ae7..2f3b73c 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1443,7 +1443,7 @@ int fcoe_xmit(struct fc_lport *lport, struct fc_frame *fp)
 		return 0;
 	}
 
-	if (unlikely(fh->fh_r_ctl == FC_RCTL_ELS_REQ) &&
+	if (unlikely(fh->fh_type == FC_TYPE_ELS) &&
 	    fcoe_ctlr_els_send(&fcoe->ctlr, lport, skb))
 		return 0;
 

[PATCH 05/16] libfcoe: fix debug message entering non-FIP mode

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

The debug message that indicated we are using non-FIP mode was
being printed only if we were already in non-FIP mode.
Also changed the message text to make it more clear the mode
is being set, not that the message is indicating how FLOGI
was received.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/fcoe/libfcoe.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c
index 511cb6b..b2af53f 100644
--- a/drivers/scsi/fcoe/libfcoe.c
+++ b/drivers/scsi/fcoe/libfcoe.c
@@ -1333,9 +1333,9 @@ int fcoe_ctlr_recv_flogi(struct fcoe_ctlr *fip, struct fc_lport *lport,
 		if (fip->state == FIP_ST_AUTO || fip->state == FIP_ST_NON_FIP) {
 			memcpy(fip->dest_addr, sa, ETH_ALEN);
 			fip->map_dest = 0;
-			if (fip->state == FIP_ST_NON_FIP)
-				LIBFCOE_FIP_DBG(fip, "received FLOGI REQ, "
-						"using non-FIP mode\n");
+			if (fip->state == FIP_ST_AUTO)
+				LIBFCOE_FIP_DBG(fip, "received non-FIP FLOGI. "
+						"Setting non-FIP mode\n");
 			fip->state = FIP_ST_NON_FIP;
 		}
 		spin_unlock_bh(&fip->lock);

[PATCH 06/16] fcoe: save gateway address when receiving FLOGI request

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

In point-to-point mode, we need to save the source MAC
from received FLOGI requests to use as the destination MAC
for all outgoing frames.  We stopped doing that at some point.

Use the lport_set_port_id method to catch incoming FLOGI frames
and pass them to fcoe_ctlr_recv_flogi() so it can save the source MAC.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/fcoe/fcoe.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 2f3b73c..546e69a 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -145,6 +145,7 @@ static int fcoe_vport_destroy(struct fc_vport *);
 static int fcoe_vport_create(struct fc_vport *, bool disabled);
 static int fcoe_vport_disable(struct fc_vport *, bool disable);
 static void fcoe_set_vport_symbolic_name(struct fc_vport *);
+static void fcoe_set_port_id(struct fc_lport *, u32, struct fc_frame *);
 
 static struct libfc_function_template fcoe_libfc_fcn_templ = {
 	.frame_send = fcoe_xmit,
@@ -152,6 +153,7 @@ static struct libfc_function_template fcoe_libfc_fcn_templ = {
 	.ddp_done = fcoe_ddp_done,
 	.elsct_send = fcoe_elsct_send,
 	.get_lesb = fcoe_get_lesb,
+	.lport_set_port_id = fcoe_set_port_id,
 };
 
 struct fc_function_template fcoe_transport_function = {
@@ -2630,3 +2632,25 @@ static void fcoe_get_lesb(struct fc_lport *lport,
 	lesb->lesb_miss_fka = htonl(mdac);
 	lesb->lesb_fcs_error = htonl(dev_get_stats(netdev)->rx_crc_errors);
 }
+
+/**
+ * fcoe_set_port_id() - Callback from libfc when Port_ID is set.
+ * @lport: the local port
+ * @port_id: the port ID
+ * @fp: the received frame, if any, that caused the port_id to be set.
+ *
+ * This routine handles the case where we received a FLOGI and are
+ * entering point-to-point mode.  We need to call fcoe_ctlr_recv_flogi()
+ * so it can set the non-mapped mode and gateway address.
+ *
+ * The FLOGI LS_ACC is handled by fcoe_flogi_resp().
+ */
+static void fcoe_set_port_id(struct fc_lport *lport,
+			     u32 port_id, struct fc_frame *fp)
+{
+	struct fcoe_port *port = lport_priv(lport);
+	struct fcoe_interface *fcoe = port->fcoe;
+
+	if (fp && fc_frame_payload_op(fp) == ELS_FLOGI)
+		fcoe_ctlr_recv_flogi(&fcoe->ctlr, lport, fp);
+}

[PATCH 07/16] libfc: recognize incoming FLOGI for point-to-point mode

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

When receiving a FLOGI request from a point-to-point peer,
the D_ID of 0xfffffe was not recognized as belonging to one
of the lports, so it was dropped.

Change fc_vport_id_lookup() to treat d_id 0xfffffe as a match.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/libfc/fc_npiv.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libfc/fc_npiv.c b/drivers/scsi/libfc/fc_npiv.c
index c68f6c7..45b6f1e 100644
--- a/drivers/scsi/libfc/fc_npiv.c
+++ b/drivers/scsi/libfc/fc_npiv.c
@@ -72,6 +72,9 @@ struct fc_lport *fc_vport_id_lookup(struct fc_lport *n_port, u32 port_id)
 	if (fc_host_port_id(n_port->host) == port_id)
 		return n_port;
 
+	if (port_id == FC_FID_FLOGI)
+		return n_port;		/* for point-to-point */
+
 	mutex_lock(&n_port->lp_mutex);
 	list_for_each_entry(vn_port, &n_port->vports, list) {
 		if (fc_host_port_id(vn_port->host) == port_id) {

[PATCH 08/16] libfc: send point-to-poin FLOGI LS_ACC to assigned D_DID

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

The method we've been using for point-to-point mode requires
that the LS_ACC for the FLOGI uses the D_ID and S_ID assigned
to the remote port and local port, not those in the exchange.

This is not the correct method, but for now, it's what works
with the old target, as well as with new targets based on libfc.

This patch changes the addresses used accordingly.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/libfc/fc_lport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index af7343a..02e6302 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -834,7 +834,7 @@ static void fc_lport_recv_flogi_req(struct fc_seq *sp_in,
 		 */
 		f_ctl = FC_FC_EX_CTX | FC_FC_LAST_SEQ | FC_FC_END_SEQ;
 		ep = fc_seq_exch(sp);
-		fc_fill_fc_hdr(fp, FC_RCTL_ELS_REP, ep->did, ep->sid,
+		fc_fill_fc_hdr(fp, FC_RCTL_ELS_REP, remote_fid, local_fid,
 			       FC_TYPE_ELS, f_ctl, 0);
 		lport->tt.seq_send(lport, sp, fp);
 

[PATCH 09/16] fcoe: remove an unused variable in fcoe_recv_frame()

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

Remove an unused variable, mac, in fcoe_recv_frame().

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/fcoe/fcoe.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 546e69a..feddb53 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1564,7 +1564,6 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 	struct fc_frame_header *fh;
 	struct fcoe_crc_eof crc_eof;
 	struct fc_frame *fp;
-	u8 *mac = NULL;
 	struct fcoe_port *port;
 	struct fcoe_hdr *hp;
 
@@ -1584,13 +1583,9 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 			skb_end_pointer(skb), skb->csum,
 			skb->dev ? skb->dev->name : "<NULL>");
 
-	/*
-	 * Save source MAC address before discarding header.
-	 */
 	port = lport_priv(lport);
 	if (skb_is_nonlinear(skb))
 		skb_linearize(skb);	/* not ideal */
-	mac = eth_hdr(skb)->h_source;
 
 	/*
 	 * Frame length checks and setting up the header pointers

[PATCH 10/16] libfcoe: eliminate unused link and last_link fields

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

The link and last_link fields in the fcoe_ctlr struct are no
longer useful, since they are always set to the same value,
and FIP always calls libfc to pass link information to the lport.

Eliminate those fields and rename link_work to timer_work, since
it no longer has any link change work to do.

Thanks to Brian Uchino for discovering this issue.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/fcoe/libfcoe.c |   40 +++++++++++-----------------------------
 include/scsi/libfcoe.h      |    8 ++------
 2 files changed, 13 insertions(+), 35 deletions(-)

diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c
index b2af53f..d60f332 100644
--- a/drivers/scsi/fcoe/libfcoe.c
+++ b/drivers/scsi/fcoe/libfcoe.c
@@ -50,7 +50,7 @@ MODULE_LICENSE("GPL v2");
 #define	FCOE_CTLR_DEF_FKA	FIP_DEF_FKA	/* default keep alive (mS) */
 
 static void fcoe_ctlr_timeout(unsigned long);
-static void fcoe_ctlr_link_work(struct work_struct *);
+static void fcoe_ctlr_timer_work(struct work_struct *);
 static void fcoe_ctlr_recv_work(struct work_struct *);
 
 static u8 fcoe_all_fcfs[ETH_ALEN] = FIP_ALL_FCF_MACS;
@@ -115,7 +115,7 @@ void fcoe_ctlr_init(struct fcoe_ctlr *fip)
 	spin_lock_init(&fip->lock);
 	fip->flogi_oxid = FC_XID_UNKNOWN;
 	setup_timer(&fip->timer, fcoe_ctlr_timeout, (unsigned long)fip);
-	INIT_WORK(&fip->link_work, fcoe_ctlr_link_work);
+	INIT_WORK(&fip->timer_work, fcoe_ctlr_timer_work);
 	INIT_WORK(&fip->recv_work, fcoe_ctlr_recv_work);
 	skb_queue_head_init(&fip->fip_recv_list);
 }
@@ -163,7 +163,7 @@ void fcoe_ctlr_destroy(struct fcoe_ctlr *fip)
 	fcoe_ctlr_reset_fcfs(fip);
 	spin_unlock_bh(&fip->lock);
 	del_timer_sync(&fip->timer);
-	cancel_work_sync(&fip->link_work);
+	cancel_work_sync(&fip->timer_work);
 }
 EXPORT_SYMBOL(fcoe_ctlr_destroy);
 
@@ -256,14 +256,10 @@ void fcoe_ctlr_link_up(struct fcoe_ctlr *fip)
 {
 	spin_lock_bh(&fip->lock);
 	if (fip->state == FIP_ST_NON_FIP || fip->state == FIP_ST_AUTO) {
-		fip->last_link = 1;
-		fip->link = 1;
 		spin_unlock_bh(&fip->lock);
 		fc_linkup(fip->lp);
 	} else if (fip->state == FIP_ST_LINK_WAIT) {
 		fip->state = fip->mode;
-		fip->last_link = 1;
-		fip->link = 1;
 		spin_unlock_bh(&fip->lock);
 		if (fip->state == FIP_ST_AUTO)
 			LIBFCOE_FIP_DBG(fip, "%s", "setting AUTO mode.\n");
@@ -305,9 +301,7 @@ int fcoe_ctlr_link_down(struct fcoe_ctlr *fip)
 	LIBFCOE_FIP_DBG(fip, "link down.\n");
 	spin_lock_bh(&fip->lock);
 	fcoe_ctlr_reset(fip);
-	link_dropped = fip->link;
-	fip->link = 0;
-	fip->last_link = 0;
+	link_dropped = fip->state != FIP_ST_LINK_WAIT;
 	fip->state = FIP_ST_LINK_WAIT;
 	spin_unlock_bh(&fip->lock);
 
@@ -1174,7 +1168,7 @@ static void fcoe_ctlr_timeout(unsigned long arg)
 			       "Starting FCF discovery.\n",
 			       fip->lp->host->host_no);
 			fip->reset_req = 1;
-			schedule_work(&fip->link_work);
+			schedule_work(&fip->timer_work);
 		}
 	}
 
@@ -1200,43 +1194,31 @@ static void fcoe_ctlr_timeout(unsigned long arg)
 		mod_timer(&fip->timer, next_timer);
 	}
 	if (fip->send_ctlr_ka || fip->send_port_ka)
-		schedule_work(&fip->link_work);
+		schedule_work(&fip->timer_work);
 	spin_unlock_bh(&fip->lock);
 }
 
 /**
- * fcoe_ctlr_link_work() - Worker thread function for link changes
+ * fcoe_ctlr_timer_work() - Worker thread function for timer work
  * @work: Handle to a FCoE controller
  *
- * See if the link status has changed and if so, report it.
- *
- * This is here because fc_linkup() and fc_linkdown() must not
+ * Sends keep-alives and resets which must not
  * be called from the timer directly, since they use a mutex.
  */
-static void fcoe_ctlr_link_work(struct work_struct *work)
+static void fcoe_ctlr_timer_work(struct work_struct *work)
 {
 	struct fcoe_ctlr *fip;
 	struct fc_lport *vport;
 	u8 *mac;
-	int link;
-	int last_link;
 	int reset;
 
-	fip = container_of(work, struct fcoe_ctlr, link_work);
+	fip = container_of(work, struct fcoe_ctlr, timer_work);
 	spin_lock_bh(&fip->lock);
-	last_link = fip->last_link;
-	link = fip->link;
-	fip->last_link = link;
 	reset = fip->reset_req;
 	fip->reset_req = 0;
 	spin_unlock_bh(&fip->lock);
 
-	if (last_link != link) {
-		if (link)
-			fc_linkup(fip->lp);
-		else
-			fc_linkdown(fip->lp);
-	} else if (reset && link)
+	if (reset)
 		fc_lport_reset(fip->lp);
 
 	if (fip->send_ctlr_ka) {
diff --git a/include/scsi/libfcoe.h b/include/scsi/libfcoe.h
index c603f4a..868ed26 100644
--- a/include/scsi/libfcoe.h
+++ b/include/scsi/libfcoe.h
@@ -65,14 +65,12 @@ enum fip_state {
  * @port_ka_time:  time of next port keep-alive.
  * @ctlr_ka_time:  time of next controller keep-alive.
  * @timer:	   timer struct used for all delayed events.
- * @link_work:	   &work_struct for doing FCF selection.
+ * @timer_work:	   &work_struct for doing keep-alives and resets.
  * @recv_work:	   &work_struct for receiving FIP frames.
  * @fip_recv_list: list of received FIP frames.
  * @user_mfs:	   configured maximum FC frame size, including FC header.
  * @flogi_oxid:    exchange ID of most recent fabric login.
  * @flogi_count:   number of FLOGI attempts in AUTO mode.
- * @link:	   current link status for libfc.
- * @last_link:	   last link state reported to libfc.
  * @map_dest:	   use the FC_MAP mode for destination MAC addresses.
  * @spma:	   supports SPMA server-provided MACs mode
  * @send_ctlr_ka:  need to send controller keep alive
@@ -100,14 +98,12 @@ struct fcoe_ctlr {
 	unsigned long port_ka_time;
 	unsigned long ctlr_ka_time;
 	struct timer_list timer;
-	struct work_struct link_work;
+	struct work_struct timer_work;
 	struct work_struct recv_work;
 	struct sk_buff_head fip_recv_list;
 	u16 user_mfs;
 	u16 flogi_oxid;
 	u8 flogi_count;
-	u8 link;
-	u8 last_link;
 	u8 reset_req;
 	u8 map_dest;
 	u8 spma;

[PATCH 11/16] libfc: fix sequence-initiative WARN in fc_seq_start_next

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

When starting a new response sequence in a multi-sequence
exchange, a warning was issued that sequence initiative
wasn't held.

The bug was that sequence initiative was cleared by the previous
sequence due to the END_SEQ flag being on.  The intent may have
been to check LAST_SEQ.  Change just to check SEQ_INIT.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

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

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 7f43647..1ad1ded 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -488,7 +488,7 @@ static int fc_seq_send(struct fc_lport *lport, struct fc_seq *sp,
 	 */
 	spin_lock_bh(&ep->ex_lock);
 	ep->f_ctl = f_ctl & ~FC_FC_FIRST_SEQ;	/* not first seq */
-	if (f_ctl & (FC_FC_END_SEQ | FC_FC_SEQ_INIT))
+	if (f_ctl & FC_FC_SEQ_INIT)
 		ep->esb_stat &= ~ESB_ST_SEQ_INIT;
 	spin_unlock_bh(&ep->ex_lock);
 	return error;

[PATCH 12/16] libfc: fixes unnecessary seq id jump

[Robert Love]

From: Vasu Dev <vasu.dev@intel.com>

In some cases seq is incremented twice causing unnecessary
seq jump, for instance fc_exch_recv_seq_resp increments
seq id when fc_sof_is_init is true and that is true for
each incoming xfer ready but then fc_fcp_send_data does
another seq increment to send data for xfer ready.

This patch removes all such seq id jumps, at least it
eliminates few calls to fc_seq_start_next using ex_lock.

Also removes seq id update with incoming frame's seq id
as this is not needed since each end (I or T) just need
to send incremented their own seq id on each TSI from
other end & before sending new sequence within a
exchange.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/libfc/fc_exch.c |   19 +++++--------------
 1 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 1ad1ded..dd7e188 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -920,12 +920,7 @@ static enum fc_pf_rjt_reason fc_seq_lookup_recip(struct fc_lport *lport,
 	 * Find or create the sequence.
 	 */
 	if (fc_sof_is_init(fr_sof(fp))) {
-		sp = fc_seq_start_next(&ep->seq);
-		if (!sp) {
-			reject = FC_RJT_SEQ_XS;	/* exchange shortage */
-			goto rel;
-		}
-		sp->id = fh->fh_seq_id;
+		sp = &ep->seq;
 		sp->ssb_stat |= SSB_ST_RESP;
 	} else {
 		sp = &ep->seq;
@@ -1336,17 +1331,14 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
 		goto rel;
 	}
 	sof = fr_sof(fp);
-	if (fc_sof_is_init(sof)) {
-		sp = fc_seq_start_next(&ep->seq);
-		sp->id = fh->fh_seq_id;
+	sp = &ep->seq;
+	if (fc_sof_is_init(sof))
 		sp->ssb_stat |= SSB_ST_RESP;
-	} else {
-		sp = &ep->seq;
-		if (sp->id != fh->fh_seq_id) {
+	else if (sp->id != fh->fh_seq_id) {
 			atomic_inc(&mp->stats.seq_not_found);
 			goto rel;
-		}
 	}
+
 	f_ctl = ntoh24(fh->fh_f_ctl);
 	fr_seq(fp) = sp;
 	if (f_ctl & FC_FC_SEQ_INIT)
@@ -1763,7 +1755,6 @@ static void fc_exch_els_rec(struct fc_seq *sp, struct fc_frame *rfp)
 		fc_exch_done(sp);
 		goto out;
 	}
-	sp = fc_seq_start_next(sp);
 	acc = fc_frame_payload_get(fp, sizeof(*acc));
 	memset(acc, 0, sizeof(*acc));
 	acc->reca_cmd = ELS_LS_ACC;

[PATCH 13/16] libfc: use offload EM instance again instead jumping to next EM

[Robert Love]

From: Vasu Dev <vasu.dev@intel.com>

Since use of offloads is more efficient than switching
to non-offload EM. However kept logic same to call em_match
if it is provided in the list of EMs.

Converted fc_exch_alloc to inline being now tiny a function
and already not an exported libfc API any more.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/libfc/fc_exch.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index dd7e188..fe1ceb3 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -734,19 +734,14 @@ err:
  * EM is selected when a NULL match function pointer is encountered
  * or when a call to a match function returns true.
  */
-static struct fc_exch *fc_exch_alloc(struct fc_lport *lport,
-				     struct fc_frame *fp)
+static inline 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)) {
-			ep = fc_exch_em_alloc(lport, ema->mp);
-			if (ep)
-				return 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);
 	return NULL;
 }
 

[PATCH 14/16] libfc: fix fcp pkt recovery in fc_fcp_recv_data

[Robert Love]

From: Vasu Dev <vasu.dev@intel.com>

Currently fc_fcp_recv_data calls fc_fcp_retry_cmd to
retry failed IO but in this case tgt is still sending
data frames, therefore exchange needs to be aborted
first before initiating retry. So this patch fixes
this by aborting exchange first then have retry.

Renames fc_timeout_error to fc_fcp_recovery since
fc_timeout_error is already called from several other
places beside from fcp timeout handler and then
used fc_fcp_recovery for abort & retry from
fc_fcp_recv_data, this rename also required renaming
FC_CMD_TIME_OUT status to FC_CMD_RECOVERY to be
consistent with new fc_fcp_recovery.

Data frames are not expected for an DDPed exchange and
potentially it could be tampered data frame, so does
recovery in this case by calling fc_fcp_recovery.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/libfc/fc_fcp.c |   47 ++++++++++++++++++++++++++-----------------
 1 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 6fde2fa..df8be9c 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -96,7 +96,7 @@ static void fc_fcp_resp(struct fc_fcp_pkt *, struct fc_frame *);
 static void fc_fcp_complete_locked(struct fc_fcp_pkt *);
 static void fc_tm_done(struct fc_seq *, struct fc_frame *, void *);
 static void fc_fcp_error(struct fc_fcp_pkt *, struct fc_frame *);
-static void fc_timeout_error(struct fc_fcp_pkt *);
+static void fc_fcp_recovery(struct fc_fcp_pkt *);
 static void fc_fcp_timeout(unsigned long);
 static void fc_fcp_rec(struct fc_fcp_pkt *);
 static void fc_fcp_rec_error(struct fc_fcp_pkt *, struct fc_frame *);
@@ -120,7 +120,7 @@ static void fc_fcp_srr_error(struct fc_fcp_pkt *, struct fc_frame *);
 #define FC_DATA_UNDRUN		7
 #define FC_ERROR		8
 #define FC_HRD_ERROR		9
-#define FC_CMD_TIME_OUT		10
+#define FC_CMD_RECOVERY		10
 
 /*
  * Error recovery timeout values.
@@ -445,9 +445,16 @@ static void fc_fcp_recv_data(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 	len = fr_len(fp) - sizeof(*fh);
 	buf = fc_frame_payload_get(fp, 0);
 
-	/* if this I/O is ddped, update xfer len */
-	fc_fcp_ddp_done(fsp);
-
+	/*
+	 * if this I/O is ddped then clear it
+	 * and initiate recovery since data
+	 * frames are expected to be placed
+	 * directly in that case.
+	 */
+	if (fsp->xfer_ddp != FC_XID_UNKNOWN) {
+		fc_fcp_ddp_done(fsp);
+		goto err;
+	}
 	if (offset + len > fsp->data_len) {
 		/* this should never happen */
 		if ((fr_flags(fp) & FCPHF_CRC_UNCHECKED) &&
@@ -455,8 +462,7 @@ static void fc_fcp_recv_data(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 			goto crc_err;
 		FC_FCP_DBG(fsp, "data received past end. len %zx offset %zx "
 			   "data_len %x\n", len, offset, fsp->data_len);
-		fc_fcp_retry_cmd(fsp);
-		return;
+		goto err;
 	}
 	if (offset != fsp->xfer_len)
 		fsp->state |= FC_SRB_DISCONTIG;
@@ -492,7 +498,7 @@ crc_err:
 			 * Otherwise, ignore it.
 			 */
 			if (fsp->state & FC_SRB_DISCONTIG)
-				fc_fcp_retry_cmd(fsp);
+				goto err;
 			return;
 		}
 	}
@@ -508,6 +514,9 @@ crc_err:
 	if (unlikely(fsp->state & FC_SRB_RCV_STATUS) &&
 	    fsp->xfer_len == fsp->data_len - fsp->scsi_resid)
 		fc_fcp_complete_locked(fsp);
+	return;
+err:
+	fc_fcp_recovery(fsp);
 }
 
 /**
@@ -1340,7 +1349,7 @@ static void fc_fcp_timeout(unsigned long data)
 	else if (fsp->state & FC_SRB_RCV_STATUS)
 		fc_fcp_complete_locked(fsp);
 	else
-		fc_timeout_error(fsp);
+		fc_fcp_recovery(fsp);
 	fsp->state &= ~FC_SRB_FCP_PROCESSING_TMO;
 unlock:
 	fc_fcp_unlock_pkt(fsp);
@@ -1384,7 +1393,7 @@ retry:
 	if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY)
 		fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV);
 	else
-		fc_timeout_error(fsp);
+		fc_fcp_recovery(fsp);
 }
 
 /**
@@ -1453,7 +1462,7 @@ static void fc_fcp_rec_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
 				fc_fcp_retry_cmd(fsp);
 				break;
 			}
-			fc_timeout_error(fsp);
+			fc_fcp_recovery(fsp);
 			break;
 		}
 	} else if (opcode == ELS_LS_ACC) {
@@ -1568,7 +1577,7 @@ static void fc_fcp_rec_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 		if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY)
 			fc_fcp_rec(fsp);
 		else
-			fc_timeout_error(fsp);
+			fc_fcp_recovery(fsp);
 		break;
 	}
 	fc_fcp_unlock_pkt(fsp);
@@ -1577,12 +1586,12 @@ out:
 }
 
 /**
- * fc_timeout_error() - Handler for fcp_pkt timeouts
- * @fsp: The FCP packt that has timed out
+ * fc_fcp_recovery() - Handler for fcp_pkt recovery
+ * @fsp: The FCP pkt that needs to be aborted
  */
-static void fc_timeout_error(struct fc_fcp_pkt *fsp)
+static void fc_fcp_recovery(struct fc_fcp_pkt *fsp)
 {
-	fsp->status_code = FC_CMD_TIME_OUT;
+	fsp->status_code = FC_CMD_RECOVERY;
 	fsp->cdb_status = 0;
 	fsp->io_status = 0;
 	/*
@@ -1688,7 +1697,7 @@ static void fc_fcp_srr_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
 		break;
 	case ELS_LS_RJT:
 	default:
-		fc_timeout_error(fsp);
+		fc_fcp_recovery(fsp);
 		break;
 	}
 	fc_fcp_unlock_pkt(fsp);
@@ -1714,7 +1723,7 @@ static void fc_fcp_srr_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 		if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY)
 			fc_fcp_rec(fsp);
 		else
-			fc_timeout_error(fsp);
+			fc_fcp_recovery(fsp);
 		break;
 	case -FC_EX_CLOSED:			/* e.g., link failure */
 		/* fall through */
@@ -1933,7 +1942,7 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp)
 	case FC_CMD_ABORTED:
 		sc_cmd->result = (DID_ERROR << 16) | fsp->io_status;
 		break;
-	case FC_CMD_TIME_OUT:
+	case FC_CMD_RECOVERY:
 		sc_cmd->result = (DID_BUS_BUSY << 16) | fsp->io_status;
 		break;
 	case FC_CMD_RESET:

[PATCH 15/16] libfc: Add debug statements when fc_fcp returns DID_ERROR to scsi-ml

[Robert Love]

DID_ERROR cases can be ambigouos. Debugging FCP error cases
will be much easier if we have debug statements when we hit
these error conditions.

This patch simply adds debug messages using the FC_FCP_DBG
macro when we return DID_ERROR to SCSI. This way if a DID_ERROR
is reproducible turning on debug_logging will give a clue
to developers as to what the problem might be.

Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/libfc/fc_fcp.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index df8be9c..5340d00 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -1915,6 +1915,8 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp)
 		}
 		break;
 	case FC_ERROR:
+		FC_FCP_DBG(fsp, "Returning DID_ERROR to scsi-ml "
+			   "due to FC_ERROR\n");
 		sc_cmd->result = DID_ERROR << 16;
 		break;
 	case FC_DATA_UNDRUN:
@@ -1923,12 +1925,19 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp)
 			 * scsi status is good but transport level
 			 * underrun.
 			 */
-			sc_cmd->result = (fsp->state & FC_SRB_RCV_STATUS ?
-					  DID_OK : DID_ERROR) << 16;
+			if (fsp->state & FC_SRB_RCV_STATUS) {
+				sc_cmd->result = DID_OK << 16;
+			} else {
+				FC_FCP_DBG(fsp, "Returning DID_ERROR to scsi-ml"
+					   " due to FC_DATA_UNDRUN (trans)\n");
+				sc_cmd->result = DID_ERROR << 16;
+			}
 		} else {
 			/*
 			 * scsi got underrun, this is an error
 			 */
+			FC_FCP_DBG(fsp, "Returning DID_ERROR to scsi-ml "
+				   "due to FC_DATA_UNDRUN (scsi)\n");
 			CMD_RESID_LEN(sc_cmd) = fsp->scsi_resid;
 			sc_cmd->result = (DID_ERROR << 16) | fsp->cdb_status;
 		}
@@ -1937,9 +1946,13 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp)
 		/*
 		 * overrun is an error
 		 */
+		FC_FCP_DBG(fsp, "Returning DID_ERROR to scsi-ml "
+			   "due to FC_DATA_OVRRUN\n");
 		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;
 		break;
 	case FC_CMD_RECOVERY:
@@ -1952,6 +1965,8 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp)
 		sc_cmd->result = (DID_NO_CONNECT << 16);
 		break;
 	default:
+		FC_FCP_DBG(fsp, "Returning DID_ERROR to scsi-ml "
+			   "due to unknown error\n");
 		sc_cmd->result = (DID_ERROR << 16);
 		break;
 	}

[PATCH 16/16] libfc, libfcoe, fcoe: use smp_processor_id() only when preempt disabled

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

When the kernel is configured for preemption, using smp_processor_id()
when preemption is enabled causes a warning backtrace and is wrong
since we could move off of that CPU as soon as we get the ID,
and we would be referencing the wrong CPU, and possibly an invalid one
if it could be hotswapped out.

Remove the fc_lport_get_stats() function and explicitly use per_cpu_ptr()
to get the statistics.  Where preemption has been disabled by holding
a _bh lock continue to use smp_processor_id(), but otherwise use
get_cpu()/put_cpu().

In fcoe_recv_frame() also changed the cases where we return in the
middle to do a goto to the code which bumps ErrorFrames and does
a put_cpu().  Two of these cases didn't bump ErrorFrames before, but
doing so is harmless because they "can't happen", due to prior length
checks.

Also rearranged code in fcoe_recv_frame() to have only one call to
fc_exch_recv().  It's just as efficient and saves a call to put_cpu().

In fc_fcp.c, adjusted a FIXME comment for code which doesn't need fixing.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---

 drivers/scsi/fcoe/fcoe.c     |   51 +++++++++++++++++++++---------------------
 drivers/scsi/fcoe/libfcoe.c  |   19 +++++++++++-----
 drivers/scsi/libfc/fc_exch.c |    3 ++
 drivers/scsi/libfc/fc_fcp.c  |    8 ++++---
 include/scsi/libfc.h         |    9 -------
 5 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index feddb53..c9bf473 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1074,7 +1074,7 @@ static void fcoe_percpu_thread_destroy(unsigned int cpu)
 	struct sk_buff *skb;
 #ifdef CONFIG_SMP
 	struct fcoe_percpu_s *p0;
-	unsigned targ_cpu = smp_processor_id();
+	unsigned targ_cpu = get_cpu();
 #endif /* CONFIG_SMP */
 
 	FCOE_DBG("Destroying receive thread for CPU %d\n", cpu);
@@ -1130,6 +1130,7 @@ static void fcoe_percpu_thread_destroy(unsigned int cpu)
 			kfree_skb(skb);
 		spin_unlock_bh(&p->fcoe_rx_list.lock);
 	}
+	put_cpu();
 #else
 	/*
 	 * This a non-SMP scenario where the singular Rx thread is
@@ -1298,8 +1299,8 @@ int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
 
 	return 0;
 err:
-	fc_lport_get_stats(lport)->ErrorFrames++;
-
+	per_cpu_ptr(lport->dev_stats, get_cpu())->ErrorFrames++;
+	put_cpu();
 err2:
 	kfree_skb(skb);
 	return -1;
@@ -1528,9 +1529,10 @@ int fcoe_xmit(struct fc_lport *lport, struct fc_frame *fp)
 		skb_shinfo(skb)->gso_size = 0;
 	}
 	/* update tx stats: regardless if LLD fails */
-	stats = fc_lport_get_stats(lport);
+	stats = per_cpu_ptr(lport->dev_stats, get_cpu());
 	stats->TxFrames++;
 	stats->TxWords += wlen;
+	put_cpu();
 
 	/* send down to lld */
 	fr_dev(fp) = lport;
@@ -1594,7 +1596,7 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 	hp = (struct fcoe_hdr *) skb_network_header(skb);
 	fh = (struct fc_frame_header *) skb_transport_header(skb);
 
-	stats = fc_lport_get_stats(lport);
+	stats = per_cpu_ptr(lport->dev_stats, get_cpu());
 	if (unlikely(FC_FCOE_DECAPS_VER(hp) != FC_FCOE_VER)) {
 		if (stats->ErrorFrames < 5)
 			printk(KERN_WARNING "fcoe: FCoE version "
@@ -1603,9 +1605,7 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 			       "initiator supports version "
 			       "%x\n", FC_FCOE_DECAPS_VER(hp),
 			       FC_FCOE_VER);
-		stats->ErrorFrames++;
-		kfree_skb(skb);
-		return;
+		goto drop;
 	}
 
 	skb_pull(skb, sizeof(struct fcoe_hdr));
@@ -1620,16 +1620,12 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 	fr_sof(fp) = hp->fcoe_sof;
 
 	/* Copy out the CRC and EOF trailer for access */
-	if (skb_copy_bits(skb, fr_len, &crc_eof, sizeof(crc_eof))) {
-		kfree_skb(skb);
-		return;
-	}
+	if (skb_copy_bits(skb, fr_len, &crc_eof, sizeof(crc_eof)))
+		goto drop;
 	fr_eof(fp) = crc_eof.fcoe_eof;
 	fr_crc(fp) = crc_eof.fcoe_crc32;
-	if (pskb_trim(skb, fr_len)) {
-		kfree_skb(skb);
-		return;
-	}
+	if (pskb_trim(skb, fr_len))
+		goto drop;
 
 	/*
 	 * We only check CRC if no offload is available and if it is
@@ -1643,25 +1639,27 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 		fr_flags(fp) |= FCPHF_CRC_UNCHECKED;
 
 	fh = fc_frame_header_get(fp);
-	if (fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA &&
-	    fh->fh_type == FC_TYPE_FCP) {
-		fc_exch_recv(lport, fp);
-		return;
-	}
-	if (fr_flags(fp) & FCPHF_CRC_UNCHECKED) {
+	if ((fh->fh_r_ctl != FC_RCTL_DD_SOL_DATA ||
+	    fh->fh_type != FC_TYPE_FCP) &&
+	    (fr_flags(fp) & FCPHF_CRC_UNCHECKED)) {
 		if (le32_to_cpu(fr_crc(fp)) !=
 		    ~crc32(~0, skb->data, fr_len)) {
 			if (stats->InvalidCRCCount < 5)
 				printk(KERN_WARNING "fcoe: dropping "
 				       "frame with CRC error\n");
 			stats->InvalidCRCCount++;
-			stats->ErrorFrames++;
-			fc_frame_free(fp);
-			return;
+			goto drop;
 		}
 		fr_flags(fp) &= ~FCPHF_CRC_UNCHECKED;
 	}
+	put_cpu();
 	fc_exch_recv(lport, fp);
+	return;
+
+drop:
+	stats->ErrorFrames++;
+	put_cpu();
+	kfree_skb(skb);
 }
 
 /**
@@ -1834,8 +1832,9 @@ static int fcoe_device_notification(struct notifier_block *notifier,
 	if (link_possible && !fcoe_link_ok(lport))
 		fcoe_ctlr_link_up(&fcoe->ctlr);
 	else if (fcoe_ctlr_link_down(&fcoe->ctlr)) {
-		stats = fc_lport_get_stats(lport);
+		stats = per_cpu_ptr(lport->dev_stats, get_cpu());
 		stats->LinkFailureCount++;
+		put_cpu();
 		fcoe_clean_pending_queue(lport);
 	}
 out:
diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c
index d60f332..626edd6 100644
--- a/drivers/scsi/fcoe/libfcoe.c
+++ b/drivers/scsi/fcoe/libfcoe.c
@@ -549,7 +549,7 @@ EXPORT_SYMBOL(fcoe_ctlr_els_send);
  * fcoe_ctlr_age_fcfs() - Reset and free all old FCFs for a controller
  * @fip: The FCoE controller to free FCFs on
  *
- * Called with lock held.
+ * Called with lock held and preemption disabled.
  *
  * An FCF is considered old if we have missed three advertisements.
  * That is, there have been no valid advertisement from it for three
@@ -566,17 +566,20 @@ static void fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
 	struct fcoe_fcf *next;
 	unsigned long sel_time = 0;
 	unsigned long mda_time = 0;
+	struct fcoe_dev_stats *stats;
 
 	list_for_each_entry_safe(fcf, next, &fip->fcfs, list) {
 		mda_time = fcf->fka_period + (fcf->fka_period >> 1);
 		if ((fip->sel_fcf == fcf) &&
 		    (time_after(jiffies, fcf->time + mda_time))) {
 			mod_timer(&fip->timer, jiffies + mda_time);
-			fc_lport_get_stats(fip->lp)->MissDiscAdvCount++;
+			stats = per_cpu_ptr(fip->lp->dev_stats,
+					    smp_processor_id());
+			stats->MissDiscAdvCount++;
 			printk(KERN_INFO "libfcoe: host%d: Missing Discovery "
 			       "Advertisement for fab %llx count %lld\n",
 			       fip->lp->host->host_no, fcf->fabric_name,
-			       fc_lport_get_stats(fip->lp)->MissDiscAdvCount);
+			       stats->MissDiscAdvCount);
 		}
 		if (time_after(jiffies, fcf->time + fcf->fka_period * 3 +
 			       msecs_to_jiffies(FIP_FCF_FUZZ * 3))) {
@@ -586,7 +589,9 @@ static void fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
 			WARN_ON(!fip->fcf_count);
 			fip->fcf_count--;
 			kfree(fcf);
-			fc_lport_get_stats(fip->lp)->VLinkFailureCount++;
+			stats = per_cpu_ptr(fip->lp->dev_stats,
+					    smp_processor_id());
+			stats->VLinkFailureCount++;
 		} else if (fcoe_ctlr_mtu_valid(fcf) &&
 			   (!sel_time || time_before(sel_time, fcf->time))) {
 			sel_time = fcf->time;
@@ -899,9 +904,10 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 	fr_eof(fp) = FC_EOF_T;
 	fr_dev(fp) = lport;
 
-	stats = fc_lport_get_stats(lport);
+	stats = per_cpu_ptr(lport->dev_stats, get_cpu());
 	stats->RxFrames++;
 	stats->RxWords += skb->len / FIP_BPW;
+	put_cpu();
 
 	fc_exch_recv(lport, fp);
 	return;
@@ -999,7 +1005,8 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
 		LIBFCOE_FIP_DBG(fip, "performing Clear Virtual Link\n");
 
 		spin_lock_bh(&fip->lock);
-		fc_lport_get_stats(lport)->VLinkFailureCount++;
+		per_cpu_ptr(lport->dev_stats,
+			    smp_processor_id())->VLinkFailureCount++;
 		fcoe_ctlr_reset(fip);
 		spin_unlock_bh(&fip->lock);
 
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index fe1ceb3..549657a 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -676,9 +676,10 @@ static struct fc_exch *fc_exch_em_alloc(struct fc_lport *lport,
 	}
 	memset(ep, 0, sizeof(*ep));
 
-	cpu = smp_processor_id();
+	cpu = get_cpu();
 	pool = per_cpu_ptr(mp->pool, cpu);
 	spin_lock_bh(&pool->lock);
+	put_cpu();
 	index = pool->next_index;
 	/* allocate new exch from pool */
 	while (fc_exch_ptr_get(pool, index)) {
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 5340d00..be6a469 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -483,13 +483,14 @@ static void fc_fcp_recv_data(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 
 		if (~crc != le32_to_cpu(fr_crc(fp))) {
 crc_err:
-			stats = fc_lport_get_stats(lport);
+			stats = per_cpu_ptr(lport->dev_stats, get_cpu());
 			stats->ErrorFrames++;
-			/* FIXME - per cpu count, not total count! */
+			/* per cpu count, not total count, but OK for limit */
 			if (stats->InvalidCRCCount++ < 5)
 				printk(KERN_WARNING "libfc: CRC error on data "
 				       "frame for port (%6x)\n",
 				       fc_host_port_id(lport->host));
+			put_cpu();
 			/*
 			 * Assume the frame is total garbage.
 			 * We may have copied it over the good part
@@ -1818,7 +1819,7 @@ int fc_queuecommand(struct scsi_cmnd *sc_cmd, void (*done)(struct scsi_cmnd *))
 	/*
 	 * setup the data direction
 	 */
-	stats = fc_lport_get_stats(lport);
+	stats = per_cpu_ptr(lport->dev_stats, get_cpu());
 	if (sc_cmd->sc_data_direction == DMA_FROM_DEVICE) {
 		fsp->req_flags = FC_SRB_READ;
 		stats->InputRequests++;
@@ -1831,6 +1832,7 @@ int fc_queuecommand(struct scsi_cmnd *sc_cmd, void (*done)(struct scsi_cmnd *))
 		fsp->req_flags = 0;
 		stats->ControlRequests++;
 	}
+	put_cpu();
 
 	fsp->tgt_flags = rpriv->flags;
 
diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h
index 4b912ee..8d0d1b2 100644
--- a/include/scsi/libfc.h
+++ b/include/scsi/libfc.h
@@ -918,15 +918,6 @@ static inline void fc_lport_free_stats(struct fc_lport *lport)
 }
 
 /**
- * fc_lport_get_stats() - Get a local port's statistics
- * @lport: The local port whose statistics are to be retreived
- */
-static inline struct fcoe_dev_stats *fc_lport_get_stats(struct fc_lport *lport)
-{
-	return per_cpu_ptr(lport->dev_stats, smp_processor_id());
-}
-
-/**
  * lport_priv() - Return the private data from a local port
  * @lport: The local port whose private data is to be retreived
  */