Skip to contentSkip to conversation

libfc, libfcoe and fcoe fixes

PATCH New Robert Love 1 participant 18 messages Jun 11, 2010

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover libfc, libfcoe and fcoe fixes

This patch series fixes a variety of issues in libfc, libfcoe and fcoe. The patches apply against scsi-misc.

The "fnic: drivers/scsi/fnic/fnic_scsi.c: clean up" patch was brought in from a series posted to linux-scsi from Roel Kluin.

---

Bhanu Prakash Gollapudi (8):
libfcoe: Avoid hang when receiving non-critical descriptors
libfcoe: No solicitation if adv is dropped
libfc: Retry a rejected PRLI request
libfc: Honor LS_ACC response codes for PRLI
libfc: Handle unsolicited PRLO request
libfcoe: Handle duplicate critical descriptors
libfcoe: Host doesnt handle CVL to NPIV ports
libfcoe: Check for order and missing critical descriptors for FIP ELS requests

Joe Eykholt (7):
libfcoe: FIP link keep-alive should continue while logged off
fcoe: clean up TBD comments in FCoE prototype header
libfcoe: fix lenient aging of FCF advertisements
libfcoe: Use fka_period as periodic timeouts to age out fcf if
libfcoe: update FIP FCF D flag from advertisments
libfc: Fix remote port restart problem
libfc: fix indefinite rport restart

Roel Kluin (1):
fnic: drivers/scsi/fnic/fnic_scsi.c: clean up

Yi Zou (1):
libfc: lport state is enum not bit mask

 drivers/scsi/fcoe/libfcoe.c   |  221 ++++++++++++++++++++++++++++++-----------
 drivers/scsi/fnic/fnic_scsi.c |   22 +---
 drivers/scsi/libfc/fc_fcp.c   |    2 
 drivers/scsi/libfc/fc_rport.c |  175 ++++++++++++++++++++++----------
 include/scsi/fc/fc_els.h      |    9 ++
 include/scsi/fc/fc_fcoe.h     |   15 +--
 include/scsi/libfc.h          |    7 +
 7 files changed, 307 insertions(+), 144 deletions(-)
Signature · 1 line
Thanks, //Rob
#1 lore raw

PATCH 1/17 libfcoe: FIP link keep-alive should continue while logged off

From: Joe Eykholt <[email protected]>

A check in fcoe_ctlr_send_keep_alive() returns if there's no port_id for the local port. This could miss a keep alive if we just did a host reset and have logged off and will log back in.

Return only if we are doing the port keep alive, in which case we need to be logged in.

---
 drivers/scsi/fcoe/libfcoe.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/fcoe/libfcoe.c +1 -1
index 50aaa4b..cc5e886 100644
@@ -343,7 +343,7 @@ static void fcoe_ctlr_send_keep_alive(struct fcoe_ctlr *fip,
 
 	fcf = fip->sel_fcf;
 	lp = fip->lp;
-	if (!fcf || !lp->port_id)
+	if (!fcf || (ports && !lp->port_id))
 		return;
 
 	len = sizeof(*kal) + ports * sizeof(*vn);
#2 lore raw

PATCH 2/17 libfcoe: Avoid hang when receiving non-critical descriptors

In reply to Robert Love

From: Bhanu Prakash Gollapudi <[email protected]>

Avoid infinite loop while processing FIP ELS or discovery advertisement with non-critical descriptors.

---
 drivers/scsi/fcoe/libfcoe.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/fcoe/libfcoe.c +2 -2
index cc5e886..66120f1 100644
@@ -703,7 +703,7 @@ static int fcoe_ctlr_parse_adv(struct fcoe_ctlr *fip,
 			/* standard says ignore unknown descriptors >= 128 */
 			if (desc->fip_dtype < FIP_DT_VENDOR_BASE)
 				return -EINVAL;
-			continue;
+			break;
 		}
 		desc = (struct fip_desc *)((char *)desc + dlen);
 		rlen -= dlen;
@@ -885,7 +885,7 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 			/* standard says ignore unknown descriptors >= 128 */
 			if (desc->fip_dtype < FIP_DT_VENDOR_BASE)
 				goto drop;
-			continue;
+			break;
 		}
 		desc = (struct fip_desc *)((char *)desc + dlen);
 		rlen -= dlen;
#3 lore raw

PATCH 3/17 libfcoe: No solicitation if adv is dropped

In reply to Robert Love

From: Bhanu Prakash Gollapudi <[email protected]>

Host does not send discovery solicitation messages if Disc. Adv from FCF are dropped. It restarts sending solicitation only after receiving a Discovery Adv. from FCF. Fix is to restart solicitation immediately after CVL processing.

---
 drivers/scsi/fcoe/libfcoe.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/scsi/fcoe/libfcoe.c +4 -1
index 66120f1..1d5b949 100644
@@ -1232,8 +1232,11 @@ static void fcoe_ctlr_timer_work(struct work_struct *work)
 	fip->reset_req = 0;
 	spin_unlock_bh(&fip->lock);
 
-	if (reset)
+	if (reset) {
 		fc_lport_reset(fip->lp);
+		/* restart things with a solicitation */
+		fcoe_ctlr_solicit(fip, NULL);
+	}
 
 	if (fip->send_ctlr_ka) {
 		fip->send_ctlr_ka = 0;
#4 lore raw

PATCH 4/17 libfc: Retry a rejected PRLI request

In reply to Robert Love

From: Bhanu Prakash Gollapudi <[email protected]>

Retry upto max_rport_retry_count when a target responds with LS_RJT for a PRLI request.

---
 drivers/scsi/libfc/fc_rport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_rport.c +1 -1
index 39e440f..3ee497a 100644
@@ -739,7 +739,7 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp,
 
 	} else {
 		FC_RPORT_DBG(rdata, "Bad ELS response for PRLI command\n");
-		fc_rport_enter_delete(rdata, RPORT_EV_FAILED);
+		fc_rport_error_retry(rdata, fp);
 	}
 
 out:
#5 lore raw

PATCH 5/17 libfc: Honor LS_ACC response codes for PRLI

In reply to Robert Love

From: Bhanu Prakash Gollapudi <[email protected]>

As per FC-LS Rev 1.62 table 46, response codes are handled as follows:

1. If the Req executed is true, PRLI is accepted.
2. If Req executed is not set, if resp code is 5, PRLI is not retried and port is logged out.
3. If resp code is anything apart from 1 or 5, PRLI is retired upto max retry count.

---
 drivers/scsi/libfc/fc_rport.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)
drivers/scsi/libfc/fc_rport.c +19 -4
index 3ee497a..e33c5c7 100644
@@ -698,6 +698,7 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp,
 	u32 roles = FC_RPORT_ROLE_UNKNOWN;
 	u32 fcp_parm = 0;
 	u8 op;
+	u8 resp_code = 0;
 
 	mutex_lock(&rdata->rp_mutex);
 
@@ -722,11 +723,25 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp,
 	op = fc_frame_payload_op(fp);
 	if (op == ELS_LS_ACC) {
 		pp = fc_frame_payload_get(fp, sizeof(*pp));
-		if (pp && pp->prli.prli_spp_len >= sizeof(pp->spp)) {
-			fcp_parm = ntohl(pp->spp.spp_params);
-			if (fcp_parm & FCP_SPPF_RETRY)
-				rdata->flags |= FC_RP_FLAGS_RETRY;
+		if (!pp)
+			goto out;
+
+		resp_code = (pp->spp.spp_flags & FC_SPP_RESP_MASK);
+		FC_RPORT_DBG(rdata, "PRLI spp_flags = 0x%x\n",
+			     pp->spp.spp_flags);
+		if (resp_code != FC_SPP_RESP_ACK) {
+			if (resp_code == FC_SPP_RESP_CONF)
+				fc_rport_error(rdata, fp);
+			else
+				fc_rport_error_retry(rdata, fp);
+			goto out;
 		}
+		if (pp->prli.prli_spp_len < sizeof(pp->spp))
+			goto out;
+
+		fcp_parm = ntohl(pp->spp.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)
#6 lore raw

PATCH 6/17 fcoe: clean up TBD comments in FCoE prototype header

In reply to Robert Love

From: Joe Eykholt <[email protected]>

Some old comments in fc_fcoe.h say TBD long after the standard has been passed by T11. Clean them up.

---
 include/scsi/fc/fc_fcoe.h |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)
include/scsi/fc/fc_fcoe.h +4 -11
index e6ad3d2..d5dcd60 100644
@@ -22,23 +22,18 @@
 
 /*
  * FCoE - Fibre Channel over Ethernet.
+ * See T11 FC-BB-5 Rev 2.00 (09-056v5.pdf)
  */
 
 /*
- * FC_FCOE_OUI hasn't been standardized yet.   XXX TBD.
+ * Default FC_FCOE_OUI / FC-MAP value.
  */
-#ifndef FC_FCOE_OUI
-#define	FC_FCOE_OUI	0x0efc00	/* upper 24 bits of FCOE dest MAC TBD */
-#endif
+#define	FC_FCOE_OUI	0x0efc00	/* upper 24 bits of FCOE MAC */
 
 /*
- * The destination MAC address for the fabric login may get a different OUI.
- * This isn't standardized yet.
+ * Fabric Login (FLOGI) MAC for non-FIP use.  Non-FIP use is deprecated.
  */
-#ifndef FC_FCOE_FLOGI_MAC
-/* gateway MAC - TBD */
 #define	FC_FCOE_FLOGI_MAC { 0x0e, 0xfc, 0x00, 0xff, 0xff, 0xfe }
-#endif
 
 #define	FC_FCOE_VER	0			/* version */
 
@@ -51,8 +46,6 @@
 
 /*
  * FCoE frame header - 14 bytes
- *
- * This is the August 2007 version of the FCoE header as defined by T11.
  * This follows the VLAN header, which includes the ethertype.
  */
 struct fcoe_hdr {
#7 lore raw

PATCH 7/17 libfc: Handle unsolicited PRLO request

In reply to Robert Love

From: Bhanu Prakash Gollapudi <[email protected]>

Resubmitting after incorporating Joe's review comment.

Unsolicited PRLO request is now handled by sending LS_ACC, and then relogin to the remote port if an N-port login session exists for that remote port.

Note that this patch should be applied on top of Joe Eykholt's "Fix remote port restart problem" patch.

---
 drivers/scsi/libfc/fc_rport.c |   71 ++++++++++++++++++++++++++++++++++++-----
 include/scsi/fc/fc_els.h      |    9 +++++
 2 files changed, 72 insertions(+), 8 deletions(-)
drivers/scsi/libfc/fc_rport.c +63 -8
index e33c5c7..df85e19 100644
@@ -1573,30 +1573,85 @@ drop:
  * fc_rport_recv_prlo_req() - Handler for process logout (PRLO) requests
  * @rdata: The remote port that sent the PRLO request
  * @sp:	   The sequence that the PRLO was on
- * @fp:	   The PRLO request frame
+ * @rx_fp: The PRLO request frame
  *
  * Locking Note: The rport lock is exected to be held before calling
  * this function.
  */
 static void fc_rport_recv_prlo_req(struct fc_rport_priv *rdata,
 				   struct fc_seq *sp,
-				   struct fc_frame *fp)
+				   struct fc_frame *rx_fp)
 {
 	struct fc_lport *lport = rdata->local_port;
-
 	struct fc_frame_header *fh;
+	struct fc_exch *ep;
+	struct fc_frame *fp;
+	struct {
+		struct fc_els_prlo prlo;
+		struct fc_els_spp spp;
+	} *pp;
+	struct fc_els_spp *rspp;	/* request service param page */
+	struct fc_els_spp *spp;		/* response spp */
+	unsigned int len;
+	unsigned int plen;
+	u32 f_ctl;
 	struct fc_seq_els_data rjt_data;
 
-	fh = fc_frame_header_get(fp);
+	rjt_data.fp = NULL;
+	fh = fc_frame_header_get(rx_fp);
 
 	FC_RPORT_DBG(rdata, "Received PRLO request while in state %s\n",
 		     fc_rport_state(rdata));
 
-	rjt_data.fp = NULL;
-	rjt_data.reason = ELS_RJT_UNAB;
-	rjt_data.explan = ELS_EXPL_NONE;
+	len = fr_len(rx_fp) - sizeof(*fh);
+	pp = fc_frame_payload_get(rx_fp, sizeof(*pp));
+	if (!pp)
+		goto reject_len;
+	plen = ntohs(pp->prlo.prlo_len);
+	if (plen != 20)
+		goto reject_len;
+	if (plen < len)
+		len = plen;
+
+	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;
+	}
+
+	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->prlo.prlo_cmd = ELS_LS_ACC;
+	pp->prlo.prlo_obs = 0x10;
+	pp->prlo.prlo_len = htons(len);
+	spp = &pp->spp;
+	spp->spp_type = rspp->spp_type;
+	spp->spp_type_ext = rspp->spp_type_ext;
+	spp->spp_flags = FC_SPP_RESP_ACK;
+
+	fc_rport_enter_delete(rdata, RPORT_EV_LOGO);
+
+	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);
+	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);
-	fc_frame_free(fp);
+drop:
+	fc_frame_free(rx_fp);
 }
 
 /**
include/scsi/fc/fc_els.h +9 -0
index f943281..70a7e92 100644
@@ -405,6 +405,15 @@ struct fc_els_prli {
 };
 
 /*
+ * ELS_PRLO - Process logout request and response.
+ */
+struct fc_els_prlo {
+	__u8            prlo_cmd;       /* command */
+	__u8            prlo_obs;       /* obsolete, but shall be set to 10h */
+	__be16          prlo_len;       /* payload length */
+};
+
+/*
  * ELS_ADISC payload
  */
 struct fc_els_adisc {
#8 lore raw

PATCH 8/17 libfcoe: fix lenient aging of FCF advertisements

In reply to Robert Love

From: Joe Eykholt <[email protected]>

[This patch has several improvements to the code in the fip timers. It hasn't been tested yet. I'm sending it out for review. Vasu, perhaps you can merge this with your patch and test it together.]

The current code allows an advertisement to be used even if it has been 3 times the FCF keep-alive advertisement period (FKA) since one was received from that FCF. The spec. calls for 2.5 times FKA.

Fix this and make sure we detect missed keep-alives promptly.

---
 drivers/scsi/fcoe/libfcoe.c |   76 ++++++++++++++++++++++++-------------------
 1 files changed, 42 insertions(+), 34 deletions(-)
drivers/scsi/fcoe/libfcoe.c +42 -34
index 1d5b949..3ab3db3 100644
@@ -557,38 +557,44 @@ EXPORT_SYMBOL(fcoe_ctlr_els_send);
  *
  * 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
- * times its keep-alive period including fuzz.
+ * An FCF is considered old if we have missed two advertisements.
+ * That is, there have been no valid advertisement from it for 2.5
+ * times its keep-alive period.
  *
  * In addition, determine the time when an FCF selection can occur.
  *
  * Also, increment the MissDiscAdvCount when no advertisement is received
  * for the corresponding FCF for 1.5 * FKA_ADV_PERIOD (FC-BB-5 LESB).
+ *
+ * Returns the time in jiffies for the next call.
  */
-static void fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
+static unsigned long fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
 {
 	struct fcoe_fcf *fcf;
 	struct fcoe_fcf *next;
+	unsigned long next_timer = jiffies + msecs_to_jiffies(FIP_VN_KA_PERIOD);
+	unsigned long deadline;
 	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);
-			stats = per_cpu_ptr(fip->lp->dev_stats,
-					    smp_processor_id());
-			stats->MissDiscAdvCount++;
-			printk(KERN_INFO "libfcoe: host%d: Missing Discovery "
-			       "Advertisement for fab %16.16llx count %lld\n",
-			       fip->lp->host->host_no, fcf->fabric_name,
-			       stats->MissDiscAdvCount);
+		deadline = fcf->time + fcf->fka_period + fcf->fka_period / 2;
+		if (fip->sel_fcf == fcf) {
+			if (time_after(jiffies, deadline)) {
+				stats = per_cpu_ptr(fip->lp->dev_stats,
+						    smp_processor_id());
+				stats->MissDiscAdvCount++;
+				printk(KERN_INFO "libfcoe: host%d: "
+				       "Missing Discovery Advertisement "
+				       "for fab %16.16llx count %lld\n",
+				       fip->lp->host->host_no, fcf->fabric_name,
+				       stats->MissDiscAdvCount);
+			} else if (time_after(next_timer, deadline))
+				next_timer = deadline;
 		}
-		if (time_after(jiffies, fcf->time + fcf->fka_period * 3 +
-			       msecs_to_jiffies(FIP_FCF_FUZZ * 3))) {
+
+		deadline += fcf->fka_period;
+		if (time_after(jiffies, deadline)) {
 			if (fip->sel_fcf == fcf)
 				fip->sel_fcf = NULL;
 			list_del(&fcf->list);
@@ -598,19 +604,21 @@ static void fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
 			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;
+		} else {
+			if (time_after(next_timer, deadline))
+				next_timer = deadline;
+			if (fcoe_ctlr_mtu_valid(fcf) &&
+			    (!sel_time || time_before(sel_time, fcf->time)))
+				sel_time = fcf->time;
 		}
 	}
 	if (sel_time) {
 		sel_time += msecs_to_jiffies(FCOE_CTLR_START_DELAY);
 		fip->sel_time = sel_time;
-		if (time_before(sel_time, fip->timer.expires))
-			mod_timer(&fip->timer, sel_time);
 	} else {
 		fip->sel_time = 0;
 	}
+	return next_timer;
 }
 
 /**
@@ -1148,7 +1156,7 @@ static void fcoe_ctlr_timeout(unsigned long arg)
 	struct fcoe_ctlr *fip = (struct fcoe_ctlr *)arg;
 	struct fcoe_fcf *sel;
 	struct fcoe_fcf *fcf;
-	unsigned long next_timer = jiffies + msecs_to_jiffies(FIP_VN_KA_PERIOD);
+	unsigned long next_timer;
 
 	spin_lock_bh(&fip->lock);
 	if (fip->state == FIP_ST_DISABLED) {
@@ -1157,13 +1165,16 @@ static void fcoe_ctlr_timeout(unsigned long arg)
 	}
 
 	fcf = fip->sel_fcf;
-	fcoe_ctlr_age_fcfs(fip);
+	next_timer = fcoe_ctlr_age_fcfs(fip);
 
 	sel = fip->sel_fcf;
-	if (!sel && fip->sel_time && time_after_eq(jiffies, fip->sel_time)) {
-		fcoe_ctlr_select(fip);
-		sel = fip->sel_fcf;
-		fip->sel_time = 0;
+	if (!sel && fip->sel_time) {
+		if (time_after_eq(jiffies, fip->sel_time)) {
+			fcoe_ctlr_select(fip);
+			sel = fip->sel_fcf;
+			fip->sel_time = 0;
+		} else if (time_after(next_timer, fip->sel_time))
+			next_timer = fip->sel_time;
 	}
 
 	if (sel != fcf) {
@@ -1201,12 +1212,9 @@ static void fcoe_ctlr_timeout(unsigned long arg)
 		}
 		if (time_after(next_timer, fip->port_ka_time))
 			next_timer = fip->port_ka_time;
-		mod_timer(&fip->timer, next_timer);
-	} else if (fip->sel_time) {
-		next_timer = fip->sel_time +
-			msecs_to_jiffies(FCOE_CTLR_START_DELAY);
-		mod_timer(&fip->timer, next_timer);
 	}
+	if (!list_empty(&fip->fcfs))
+		mod_timer(&fip->timer, next_timer);
 	if (fip->send_ctlr_ka || fip->send_port_ka)
 		schedule_work(&fip->timer_work);
 	spin_unlock_bh(&fip->lock);
#9 lore raw

PATCH 9/17 libfcoe: Use fka_period as periodic timeouts to age out fcf if

In reply to Robert Love

From: Joe Eykholt <[email protected]>

keep alives are disabled due to fd_flags set and also stop updating keep alive values in that case.

Update select fcf time only if fcf is not already selected or select time is not already determined from parse adv, and then have select time cleared only once after fcf is selected.

Changed deadline check to time_after_eq() from time_after() since now next timeout will be on exact 2.5 times FKA followed by first advertisement.

---
 drivers/scsi/fcoe/libfcoe.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)
drivers/scsi/fcoe/libfcoe.c +7 -6
index 3ab3db3..4893098 100644
@@ -594,7 +594,7 @@ static unsigned long fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
 		}
 
 		deadline += fcf->fka_period;
-		if (time_after(jiffies, deadline)) {
+		if (time_after_eq(jiffies, deadline)) {
 			if (fip->sel_fcf == fcf)
 				fip->sel_fcf = NULL;
 			list_del(&fcf->list);
@@ -612,12 +612,11 @@ static unsigned long fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
 				sel_time = fcf->time;
 		}
 	}
-	if (sel_time) {
+	if (sel_time && !fip->sel_fcf && !fip->sel_time) {
 		sel_time += msecs_to_jiffies(FCOE_CTLR_START_DELAY);
 		fip->sel_time = sel_time;
-	} else {
-		fip->sel_time = 0;
 	}
+
 	return next_timer;
 }
 
@@ -775,7 +774,7 @@ static void fcoe_ctlr_recv_adv(struct fcoe_ctlr *fip, struct sk_buff *skb)
 		 * ignored after a usable solicited advertisement
 		 * has been received.
 		 */
-		if (fcf == fip->sel_fcf) {
+		if (fcf == fip->sel_fcf && !fcf->fd_flags) {
 			fip->ctlr_ka_time -= fcf->fka_period;
 			fip->ctlr_ka_time += new.fka_period;
 			if (time_before(fip->ctlr_ka_time, fip->timer.expires))
@@ -813,7 +812,7 @@ static void fcoe_ctlr_recv_adv(struct fcoe_ctlr *fip, struct sk_buff *skb)
 	 * If this is the first validated FCF, note the time and
 	 * set a timer to trigger selection.
 	 */
-	if (mtu_valid && !fip->sel_time && fcoe_ctlr_fcf_usable(fcf)) {
+	if (mtu_valid && !fip->sel_fcf && fcoe_ctlr_fcf_usable(fcf)) {
 		fip->sel_time = jiffies +
 			msecs_to_jiffies(FCOE_CTLR_START_DELAY);
 		if (!timer_pending(&fip->timer) ||
@@ -1187,6 +1186,8 @@ static void fcoe_ctlr_timeout(unsigned long arg)
 			fip->port_ka_time = jiffies +
 				msecs_to_jiffies(FIP_VN_KA_PERIOD);
 			fip->ctlr_ka_time = jiffies + sel->fka_period;
+			if (time_after(next_timer, fip->ctlr_ka_time))
+				next_timer = fip->ctlr_ka_time;
 		} else {
 			printk(KERN_NOTICE "libfcoe: host%d: "
 			       "FIP Fibre-Channel Forwarder timed out.	"
#10 lore raw

PATCH 10/17 libfcoe: update FIP FCF D flag from advertisments

In reply to Robert Love

From: Joe Eykholt <[email protected]>

Allow the D flag (indicating that keep-alives are not needed) to be updated dynamically from received FIP advertisements.

---
 drivers/scsi/fcoe/libfcoe.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)
drivers/scsi/fcoe/libfcoe.c +9 -6
index 4893098..27c21ca 100644
@@ -769,18 +769,21 @@ static void fcoe_ctlr_recv_adv(struct fcoe_ctlr *fip, struct sk_buff *skb)
 		list_add(&fcf->list, &fip->fcfs);
 	} else {
 		/*
-		 * Flags in advertisements are ignored once the FCF is
-		 * selected.  Flags in unsolicited advertisements are
-		 * ignored after a usable solicited advertisement
-		 * has been received.
+		 * Update the FCF's keep-alive descriptor flags.
+		 * Other flag changes from new advertisements are
+		 * ignored after a solicited advertisement is
+		 * received and the FCF is selectable (usable).
 		 */
+		fcf->fd_flags = new.fd_flags;
+		if (!fcoe_ctlr_fcf_usable(fcf))
+			fcf->flags = new.flags;
+
 		if (fcf == fip->sel_fcf && !fcf->fd_flags) {
 			fip->ctlr_ka_time -= fcf->fka_period;
 			fip->ctlr_ka_time += new.fka_period;
 			if (time_before(fip->ctlr_ka_time, fip->timer.expires))
 				mod_timer(&fip->timer, fip->ctlr_ka_time);
-		} else if (!fcoe_ctlr_fcf_usable(fcf))
-			fcf->flags = new.flags;
+		}
 		fcf->fka_period = new.fka_period;
 		memcpy(fcf->fcf_mac, new.fcf_mac, ETH_ALEN);
 	}
#11 lore raw

PATCH 11/17 libfcoe: Handle duplicate critical descriptors

In reply to Robert Love

From: Bhanu Prakash Gollapudi <[email protected]>

As per FC-BB-5 rev 2, section 7.8.6.2, malformed FIP frame shall be discarded. Drop discovery adv, ELS and CLV's with duplicate critical descriptors.

[Resending after incorporating Joe's review comments]

---
 drivers/scsi/fcoe/libfcoe.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)
drivers/scsi/fcoe/libfcoe.c +41 -0
index 27c21ca..db7185c 100644
@@ -640,6 +640,7 @@ static int fcoe_ctlr_parse_adv(struct fcoe_ctlr *fip,
 	unsigned long t;
 	size_t rlen;
 	size_t dlen;
+	u32 desc_mask;
 
 	memset(fcf, 0, sizeof(*fcf));
 	fcf->fka_period = msecs_to_jiffies(FCOE_CTLR_DEF_FKA);
@@ -647,6 +648,12 @@ static int fcoe_ctlr_parse_adv(struct fcoe_ctlr *fip,
 	fiph = (struct fip_header *)skb->data;
 	fcf->flags = ntohs(fiph->fip_flags);
 
+	/*
+	 * mask of required descriptors. validating each one clears its bit.
+	 */
+	desc_mask = BIT(FIP_DT_PRI) | BIT(FIP_DT_MAC) | BIT(FIP_DT_NAME) |
+			BIT(FIP_DT_FAB) | BIT(FIP_DT_FKA);
+
 	rlen = ntohs(fiph->fip_dl_len) * 4;
 	if (rlen + sizeof(*fiph) > skb->len)
 		return -EINVAL;
@@ -656,11 +663,19 @@ static int fcoe_ctlr_parse_adv(struct fcoe_ctlr *fip,
 		dlen = desc->fip_dlen * FIP_BPW;
 		if (dlen < sizeof(*desc) || dlen > rlen)
 			return -EINVAL;
+		/* Drop Adv if there are duplicate critical descriptors */
+		if ((desc->fip_dtype < 32) &&
+		    !(desc_mask & 1U << desc->fip_dtype)) {
+			LIBFCOE_FIP_DBG(fip, "Duplicate Critical "
+					"Descriptors in FIP adv\n");
+			return -EINVAL;
+		}
 		switch (desc->fip_dtype) {
 		case FIP_DT_PRI:
 			if (dlen != sizeof(struct fip_pri_desc))
 				goto len_err;
 			fcf->pri = ((struct fip_pri_desc *)desc)->fd_pri;
+			desc_mask &= ~BIT(FIP_DT_PRI);
 			break;
 		case FIP_DT_MAC:
 			if (dlen != sizeof(struct fip_mac_desc))
@@ -673,12 +688,14 @@ static int fcoe_ctlr_parse_adv(struct fcoe_ctlr *fip,
 						"in FIP adv\n");
 				return -EINVAL;
 			}
+			desc_mask &= ~BIT(FIP_DT_MAC);
 			break;
 		case FIP_DT_NAME:
 			if (dlen != sizeof(struct fip_wwn_desc))
 				goto len_err;
 			wwn = (struct fip_wwn_desc *)desc;
 			fcf->switch_name = get_unaligned_be64(&wwn->fd_wwn);
+			desc_mask &= ~BIT(FIP_DT_NAME);
 			break;
 		case FIP_DT_FAB:
 			if (dlen != sizeof(struct fip_fab_desc))
@@ -687,6 +704,7 @@ static int fcoe_ctlr_parse_adv(struct fcoe_ctlr *fip,
 			fcf->fabric_name = get_unaligned_be64(&fab->fd_wwn);
 			fcf->vfid = ntohs(fab->fd_vfid);
 			fcf->fc_map = ntoh24(fab->fd_map);
+			desc_mask &= ~BIT(FIP_DT_FAB);
 			break;
 		case FIP_DT_FKA:
 			if (dlen != sizeof(struct fip_fka_desc))
@@ -697,6 +715,7 @@ static int fcoe_ctlr_parse_adv(struct fcoe_ctlr *fip,
 			t = ntohl(fka->fd_fka_period);
 			if (t >= FCOE_CTLR_MIN_FKA)
 				fcf->fka_period = msecs_to_jiffies(t);
+			desc_mask &= ~BIT(FIP_DT_FKA);
 			break;
 		case FIP_DT_MAP_OUI:
 		case FIP_DT_FCOE_SIZE:
@@ -719,6 +738,11 @@ static int fcoe_ctlr_parse_adv(struct fcoe_ctlr *fip,
 		return -EINVAL;
 	if (!fcf->switch_name || !fcf->fabric_name)
 		return -EINVAL;
+	if (desc_mask) {
+		LIBFCOE_FIP_DBG(fip, "adv missing descriptors mask %x\n",
+				desc_mask);
+		return -EINVAL;
+	}
 	return 0;
 
 len_err:
@@ -847,6 +871,7 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 	size_t els_len = 0;
 	size_t rlen;
 	size_t dlen;
+	u32 dupl_desc = 0;
 
 	fiph = (struct fip_header *)skb->data;
 	sub = fiph->fip_subcode;
@@ -862,6 +887,15 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 		dlen = desc->fip_dlen * FIP_BPW;
 		if (dlen < sizeof(*desc) || dlen > rlen)
 			goto drop;
+		/* Drop ELS if there are duplicate critical descriptors */
+		if (desc->fip_dtype < 32) {
+			if (dupl_desc & 1U << desc->fip_dtype) {
+				LIBFCOE_FIP_DBG(fip, "Duplicate Critical "
+						"Descriptors in FIP ELS\n");
+				goto drop;
+			}
+			dupl_desc |= (1 << desc->fip_dtype);
+		}
 		switch (desc->fip_dtype) {
 		case FIP_DT_MAC:
 			if (dlen != sizeof(struct fip_mac_desc))
@@ -973,6 +1007,13 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
 		dlen = desc->fip_dlen * FIP_BPW;
 		if (dlen > rlen)
 			return;
+		/* Drop CVL if there are duplicate critical descriptors */
+		if ((desc->fip_dtype < 32) &&
+		    !(desc_mask & 1U << desc->fip_dtype)) {
+			LIBFCOE_FIP_DBG(fip, "Duplicate Critical "
+					"Descriptors in FIP CVL\n");
+			return;
+		}
 		switch (desc->fip_dtype) {
 		case FIP_DT_MAC:
 			mp = (struct fip_mac_desc *)desc;
#12 lore raw

PATCH 12/17 libfcoe: Host doesnt handle CVL to NPIV ports

In reply to Robert Love

From: Bhanu Prakash Gollapudi <[email protected]>

Clear virtual link for NPIV ports is now handled by resetting the matching vnport.

---
 drivers/scsi/fcoe/libfcoe.c |   42 +++++++++++++++++++++++++++++++++---------
 1 files changed, 33 insertions(+), 9 deletions(-)
drivers/scsi/fcoe/libfcoe.c +33 -9
index db7185c..ce651ca 100644
@@ -989,7 +989,9 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
 	size_t dlen;
 	struct fcoe_fcf *fcf = fip->sel_fcf;
 	struct fc_lport *lport = fip->lp;
-	u32	desc_mask;
+	struct fc_lport *vn_port = NULL;
+	u32 desc_mask;
+	int is_vn_port = 0;
 
 	LIBFCOE_FIP_DBG(fip, "Clear Virtual Link received\n");
 
@@ -1038,8 +1040,26 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
 			if (compare_ether_addr(vp->fd_mac,
 					       fip->get_src_addr(lport)) == 0 &&
 			    get_unaligned_be64(&vp->fd_wwpn) == lport->wwpn &&
-			    ntoh24(vp->fd_fc_id) == lport->port_id)
+			    ntoh24(vp->fd_fc_id) == lport->port_id) {
 				desc_mask &= ~BIT(FIP_DT_VN_ID);
+				break;
+			}
+			/* check if clr_vlink is for NPIV port */
+			mutex_lock(&lport->lp_mutex);
+			list_for_each_entry(vn_port, &lport->vports, list) {
+				if (compare_ether_addr(vp->fd_mac,
+				    fip->get_src_addr(vn_port)) == 0 &&
+				    (get_unaligned_be64(&vp->fd_wwpn)
+							== vn_port->wwpn) &&
+				    (ntoh24(vp->fd_fc_id) ==
+					    fc_host_port_id(vn_port->host))) {
+					desc_mask &= ~BIT(FIP_DT_VN_ID);
+					is_vn_port = 1;
+					break;
+				}
+			}
+			mutex_unlock(&lport->lp_mutex);
+
 			break;
 		default:
 			/* standard says ignore unknown descriptors >= 128 */
@@ -1060,14 +1080,18 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
 	} else {
 		LIBFCOE_FIP_DBG(fip, "performing Clear Virtual Link\n");
 
-		spin_lock_bh(&fip->lock);
-		per_cpu_ptr(lport->dev_stats,
-			    smp_processor_id())->VLinkFailureCount++;
-		fcoe_ctlr_reset(fip);
-		spin_unlock_bh(&fip->lock);
+		if (is_vn_port)
+			fc_lport_reset(vn_port);
+		else {
+			spin_lock_bh(&fip->lock);
+			per_cpu_ptr(lport->dev_stats,
+				    smp_processor_id())->VLinkFailureCount++;
+			fcoe_ctlr_reset(fip);
+			spin_unlock_bh(&fip->lock);
 
-		fc_lport_reset(fip->lp);
-		fcoe_ctlr_solicit(fip, NULL);
+			fc_lport_reset(fip->lp);
+			fcoe_ctlr_solicit(fip, NULL);
+		}
 	}
 }
 
#13 lore raw

PATCH 13/17 libfcoe: Check for order and missing critical descriptors for FIP ELS requests

In reply to Robert Love

From: Bhanu Prakash Gollapudi <[email protected]>

As per FC-BB-5 rev.2, section 7.8.7.1, strict ordering of FIP descriptors is required for ELS requests. Also, look for missing and duplicate critical descriptors.

---
 drivers/scsi/fcoe/libfcoe.c |   31 ++++++++++++++++++++++++++++---
 1 files changed, 28 insertions(+), 3 deletions(-)
drivers/scsi/fcoe/libfcoe.c +28 -3
index ce651ca..f009191 100644
@@ -871,7 +871,8 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 	size_t els_len = 0;
 	size_t rlen;
 	size_t dlen;
-	u32 dupl_desc = 0;
+	u32 desc_mask = 0;
+	u32 desc_cnt = 0;
 
 	fiph = (struct fip_header *)skb->data;
 	sub = fiph->fip_subcode;
@@ -884,20 +885,27 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 
 	desc = (struct fip_desc *)(fiph + 1);
 	while (rlen > 0) {
+		desc_cnt++;
 		dlen = desc->fip_dlen * FIP_BPW;
 		if (dlen < sizeof(*desc) || dlen > rlen)
 			goto drop;
 		/* Drop ELS if there are duplicate critical descriptors */
 		if (desc->fip_dtype < 32) {
-			if (dupl_desc & 1U << desc->fip_dtype) {
+			if (desc_mask & 1U << desc->fip_dtype) {
 				LIBFCOE_FIP_DBG(fip, "Duplicate Critical "
 						"Descriptors in FIP ELS\n");
 				goto drop;
 			}
-			dupl_desc |= (1 << desc->fip_dtype);
+			desc_mask |= (1 << desc->fip_dtype);
 		}
 		switch (desc->fip_dtype) {
 		case FIP_DT_MAC:
+			if (desc_cnt == 1) {
+				LIBFCOE_FIP_DBG(fip, "FIP descriptors "
+						"received out of order\n");
+				goto drop;
+			}
+
 			if (dlen != sizeof(struct fip_mac_desc))
 				goto len_err;
 			memcpy(granted_mac,
@@ -914,6 +922,11 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 		case FIP_DT_FDISC:
 		case FIP_DT_LOGO:
 		case FIP_DT_ELP:
+			if (desc_cnt != 1) {
+				LIBFCOE_FIP_DBG(fip, "FIP descriptors "
+						"received out of order\n");
+				goto drop;
+			}
 			if (fh)
 				goto drop;
 			if (dlen < sizeof(*els) + sizeof(*fh) + 1)
@@ -929,6 +942,11 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 			/* standard says ignore unknown descriptors >= 128 */
 			if (desc->fip_dtype < FIP_DT_VENDOR_BASE)
 				goto drop;
+			if (desc_cnt <= 2) {
+				LIBFCOE_FIP_DBG(fip, "FIP descriptors "
+						"received out of order\n");
+				goto drop;
+			}
 			break;
 		}
 		desc = (struct fip_desc *)((char *)desc + dlen);
@@ -944,6 +962,13 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 	    els_op == ELS_LS_ACC && is_valid_ether_addr(granted_mac))
 		fip->flogi_oxid = FC_XID_UNKNOWN;
 
+	if ((desc_cnt == 0) || ((els_op != ELS_LS_RJT) &&
+	    (!(1U << FIP_DT_MAC & desc_mask)))) {
+		LIBFCOE_FIP_DBG(fip, "Missing critical descriptors "
+				"in FIP ELS\n");
+		goto drop;
+	}
+
 	/*
 	 * Convert skb into an fc_frame containing only the ELS.
 	 */
#14 lore raw

PATCH 14/17 libfc: lport state is enum not bit mask

In reply to Robert Love

From: Yi Zou <[email protected]>

lport state is enum not bit mask.

---
 drivers/scsi/libfc/fc_fcp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_fcp.c +1 -1
index ec1f66c..8914f1e 100644
@@ -745,7 +745,7 @@ 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)
 		goto out;
 	if (fc_fcp_lock_pkt(fsp))
 		goto out;
#15 lore raw

PATCH 15/17 fnic: drivers/scsi/fnic/fnic_scsi.c: clean up

In reply to Robert Love

From: Roel Kluin <[email protected]>

In fnic_abort_cmd() and fnic_device_reset() assign `rport' earlier to make FNIC_SCSI_DBG() calls cleaner.

In fnic_clean_pending_aborts() `rport' is not used.

---
 drivers/scsi/fnic/fnic_scsi.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)
drivers/scsi/fnic/fnic_scsi.c +8 -14
index 3cc47c6..198cbab 100644
@@ -1246,11 +1246,10 @@ int fnic_abort_cmd(struct scsi_cmnd *sc)
 	lp = shost_priv(sc->device->host);
 
 	fnic = lport_priv(lp);
-	FNIC_SCSI_DBG(KERN_DEBUG,
-		      fnic->lport->host,
-		      "Abort Cmd called FCID 0x%x, LUN 0x%x TAG %d\n",
-		      (starget_to_rport(scsi_target(sc->device)))->port_id,
-		      sc->device->lun, sc->request->tag);
+	rport = starget_to_rport(scsi_target(sc->device));
+	FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+			"Abort Cmd called FCID 0x%x, LUN 0x%x TAG %d\n",
+			rport->port_id, sc->device->lun, sc->request->tag);
 
 	if (lp->state != LPORT_ST_READY || !(lp->link_up)) {
 		ret = FAILED;
@@ -1299,7 +1298,6 @@ int fnic_abort_cmd(struct scsi_cmnd *sc)
 	 * port is up, then send abts to the remote port to terminate
 	 * the IO. Else, just locally terminate the IO in the firmware
 	 */
-	rport = starget_to_rport(scsi_target(sc->device));
 	if (fc_remote_port_chkready(rport) == 0)
 		task_req = FCPIO_ITMF_ABT_TASK;
 	else
@@ -1418,7 +1416,6 @@ static int fnic_clean_pending_aborts(struct fnic *fnic,
 	unsigned long flags;
 	int ret = 0;
 	struct scsi_cmnd *sc;
-	struct fc_rport *rport;
 	struct scsi_lun fc_lun;
 	struct scsi_device *lun_dev = lr_sc->device;
 	DECLARE_COMPLETION_ONSTACK(tm_done);
@@ -1458,7 +1455,6 @@ static int fnic_clean_pending_aborts(struct fnic *fnic,
 
 		/* Now queue the abort command to firmware */
 		int_to_scsilun(sc->device->lun, &fc_lun);
-		rport = starget_to_rport(scsi_target(sc->device));
 
 		if (fnic_queue_abort_io_req(fnic, tag,
 					    FCPIO_ITMF_ABT_TASK_TERM,
@@ -1528,18 +1524,16 @@ int fnic_device_reset(struct scsi_cmnd *sc)
 	lp = shost_priv(sc->device->host);
 
 	fnic = lport_priv(lp);
-	FNIC_SCSI_DBG(KERN_DEBUG,
-		      fnic->lport->host,
-		      "Device reset called FCID 0x%x, LUN 0x%x\n",
-		      (starget_to_rport(scsi_target(sc->device)))->port_id,
-		      sc->device->lun);
 
+	rport = starget_to_rport(scsi_target(sc->device));
+	FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+			"Device reset called FCID 0x%x, LUN 0x%x\n",
+			rport->port_id, sc->device->lun);
 
 	if (lp->state != LPORT_ST_READY || !(lp->link_up))
 		goto fnic_device_reset_end;
 
 	/* Check if remote port up */
-	rport = starget_to_rport(scsi_target(sc->device));
 	if (fc_remote_port_chkready(rport))
 		goto fnic_device_reset_end;
 
#16 lore raw

PATCH 16/17 libfc: Fix remote port restart problem

In reply to Robert Love

From: Joe Eykholt <[email protected]>

This patch somewhat combines two fixes to remote port handing in libfc.

The first problem was that rport work could be queued on a deleted and freed rport. This is handled by not resetting rdata->event ton NONE if the rdata is about to be deleted.

However, that fix led to the second problem, described by Bhanu Gollapudi, as follows:

Quoted 13 lines

Here is the sequence of events. T1 is first LOGO receive thread, T2 is fc_rport_work() scheduled by T1 and T3 is second LOGO receive thread and T4 is fc_rport_work scheduled by T3.

1. (T1)Received 1st LOGO in state Ready
2. (T1)Delete port & enter to RESTART state.
3. (T1)schdule event_work, since event is RPORT_EV_NONE.
4. (T1)set event = RPORT_EV_LOGO
5. (T1)Enter RESTART state as disc_id is set.
6. (T2)remember to PLOGI, and set event = RPORT_EV_NONE
6. (T3)Received 2nd LOGO
7. (T3)Delete Port & enter to RESTART state.
8. (T3)schedule event_work, since event is RPORT_EV_NONE.

Quoted 8 lines

9. (T3)Enter RESTART state as disc_id is set.
9. (T3)set event = RPORT_EV_LOGO
10.(T2)work restart, enter PLOGI state and issues PLOGI 11.(T4)Since state is not RESTART anymore, restart is not set, and the event is not reset to RPORT_EV_NONE. (current event is RPORT_EV_LOGO).
12. Now, PLOGI succeeds and fc_rport_enter_ready() will not schedule event_work, and hence the rport will never be created, eventually losing the target after dev_loss_tmo.

So, the problem here is that we were tracking the desire for the rport be restarted by state RESTART, which was otherwise equivalent to DELETE. A contributing factor is that we dropped the lock between steps 6 and 10 in thread T2, which allows the state to change, and we didn't completely re-evaluate then.

This is hopefully corrected by the following minor redesign:

Simplify the rport restart logic by making the decision to restart after deleting the transport rport. That decision is based on a new STARTED flag that indicates fc_rport_login() has been called and fc_rport_logoff() has not been called since then. This replaces the need for the RESTART state.

Only restart if the rdata is still in DELETED state and only if it still has the STARTED flag set.

Also now, since we clear the event code much later in the work thread, allow for the possibility that the rport may have become READY again via incoming PLOGI, and if so, queue another event to handle that.

In the problem scenario, the second LOGO received will cause the LOGO event to occur again.

---
 drivers/scsi/libfc/fc_rport.c |   75 +++++++++++++++++------------------------
 include/scsi/libfc.h          |    5 +--
 2 files changed, 33 insertions(+), 47 deletions(-)
drivers/scsi/libfc/fc_rport.c +31 -44
index df85e19..d385efc 100644
@@ -89,7 +89,6 @@ static const char *fc_rport_state_names[] = {
 	[RPORT_ST_LOGO] = "LOGO",
 	[RPORT_ST_ADISC] = "ADISC",
 	[RPORT_ST_DELETE] = "Delete",
-	[RPORT_ST_RESTART] = "Restart",
 };
 
 /**
@@ -246,7 +245,6 @@ static void fc_rport_work(struct work_struct *work)
 	struct fc_rport_operations *rport_ops;
 	struct fc_rport_identifiers ids;
 	struct fc_rport *rport;
-	int restart = 0;
 
 	mutex_lock(&rdata->rp_mutex);
 	event = rdata->event;
@@ -298,24 +296,6 @@ static void fc_rport_work(struct work_struct *work)
 		port_id = rdata->ids.port_id;
 		mutex_unlock(&rdata->rp_mutex);
 
-		if (port_id != FC_FID_DIR_SERV) {
-			/*
-			 * We must drop rp_mutex before taking disc_mutex.
-			 * Re-evaluate state to allow for restart.
-			 * A transition to RESTART state must only happen
-			 * while disc_mutex is held and rdata is on the list.
-			 */
-			mutex_lock(&lport->disc.disc_mutex);
-			mutex_lock(&rdata->rp_mutex);
-			if (rdata->rp_state == RPORT_ST_RESTART)
-				restart = 1;
-			else
-				list_del(&rdata->peers);
-			rdata->event = RPORT_EV_NONE;
-			mutex_unlock(&rdata->rp_mutex);
-			mutex_unlock(&lport->disc.disc_mutex);
-		}
-
 		if (rport_ops && rport_ops->event_callback) {
 			FC_RPORT_DBG(rdata, "callback ev %d\n", event);
 			rport_ops->event_callback(lport, rdata, event);
@@ -336,13 +316,34 @@ static void fc_rport_work(struct work_struct *work)
 			mutex_unlock(&rdata->rp_mutex);
 			fc_remote_port_delete(rport);
 		}
-		if (restart) {
-			mutex_lock(&rdata->rp_mutex);
-			FC_RPORT_DBG(rdata, "work restart\n");
-			fc_rport_enter_plogi(rdata);
+
+		mutex_lock(&lport->disc.disc_mutex);
+		mutex_lock(&rdata->rp_mutex);
+		if (rdata->rp_state == RPORT_ST_DELETE) {
+			if (port_id == FC_FID_DIR_SERV) {
+				rdata->event = RPORT_EV_NONE;
+				mutex_unlock(&rdata->rp_mutex);
+			} else if (rdata->flags & FC_RP_STARTED) {
+				rdata->event = RPORT_EV_NONE;
+				FC_RPORT_DBG(rdata, "work restart\n");
+				fc_rport_enter_plogi(rdata);
+				mutex_unlock(&rdata->rp_mutex);
+			} else {
+				FC_RPORT_DBG(rdata, "work delete\n");
+				list_del(&rdata->peers);
+				mutex_unlock(&rdata->rp_mutex);
+				kref_put(&rdata->kref, lport->tt.rport_destroy);
+			}
+		} else {
+			/*
+			 * Re-open for events.  Reissue READY event if ready.
+			 */
+			rdata->event = RPORT_EV_NONE;
+			if (rdata->rp_state == RPORT_ST_READY)
+				fc_rport_enter_ready(rdata);
 			mutex_unlock(&rdata->rp_mutex);
-		} else
-			kref_put(&rdata->kref, lport->tt.rport_destroy);
+		}
+		mutex_unlock(&lport->disc.disc_mutex);
 		break;
 
 	default:
@@ -367,16 +368,14 @@ int fc_rport_login(struct fc_rport_priv *rdata)
 {
 	mutex_lock(&rdata->rp_mutex);
 
+	rdata->flags |= FC_RP_STARTED;
 	switch (rdata->rp_state) {
 	case RPORT_ST_READY:
 		FC_RPORT_DBG(rdata, "ADISC port\n");
 		fc_rport_enter_adisc(rdata);
 		break;
-	case RPORT_ST_RESTART:
-		break;
 	case RPORT_ST_DELETE:
 		FC_RPORT_DBG(rdata, "Restart deleted port\n");
-		fc_rport_state_enter(rdata, RPORT_ST_RESTART);
 		break;
 	default:
 		FC_RPORT_DBG(rdata, "Login to port\n");
@@ -431,15 +430,12 @@ int fc_rport_logoff(struct fc_rport_priv *rdata)
 
 	FC_RPORT_DBG(rdata, "Remove port\n");
 
+	rdata->flags &= ~FC_RP_STARTED;
 	if (rdata->rp_state == RPORT_ST_DELETE) {
 		FC_RPORT_DBG(rdata, "Port in Delete state, not removing\n");
 		goto out;
 	}
-
-	if (rdata->rp_state == RPORT_ST_RESTART)
-		FC_RPORT_DBG(rdata, "Port in Restart state, deleting\n");
-	else
-		fc_rport_enter_logo(rdata);
+	fc_rport_enter_logo(rdata);
 
 	/*
 	 * Change the state to Delete so that we discard
@@ -503,7 +499,6 @@ static void fc_rport_timeout(struct work_struct *work)
 	case RPORT_ST_READY:
 	case RPORT_ST_INIT:
 	case RPORT_ST_DELETE:
-	case RPORT_ST_RESTART:
 		break;
 	}
 
@@ -527,6 +522,7 @@ static void fc_rport_error(struct fc_rport_priv *rdata, struct fc_frame *fp)
 	switch (rdata->rp_state) {
 	case RPORT_ST_PLOGI:
 	case RPORT_ST_LOGO:
+		rdata->flags &= ~FC_RP_STARTED;
 		fc_rport_enter_delete(rdata, RPORT_EV_FAILED);
 		break;
 	case RPORT_ST_RTV:
@@ -537,7 +533,6 @@ static void fc_rport_error(struct fc_rport_priv *rdata, struct fc_frame *fp)
 		fc_rport_enter_logo(rdata);
 		break;
 	case RPORT_ST_DELETE:
-	case RPORT_ST_RESTART:
 	case RPORT_ST_READY:
 	case RPORT_ST_INIT:
 		break;
@@ -1392,7 +1387,6 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport,
 		break;
 	case RPORT_ST_DELETE:
 	case RPORT_ST_LOGO:
-	case RPORT_ST_RESTART:
 		FC_RPORT_DBG(rdata, "Received PLOGI in state %s - send busy\n",
 			     fc_rport_state(rdata));
 		mutex_unlock(&rdata->rp_mutex);
@@ -1684,13 +1678,6 @@ static void fc_rport_recv_logo_req(struct fc_lport *lport,
 			     fc_rport_state(rdata));
 
 		fc_rport_enter_delete(rdata, RPORT_EV_LOGO);
-
-		/*
-		 * If the remote port was created due to discovery, set state
-		 * to log back in.  It may have seen a stale RSCN about us.
-		 */
-		if (rdata->disc_id)
-			fc_rport_state_enter(rdata, RPORT_ST_RESTART);
 		mutex_unlock(&rdata->rp_mutex);
 	} else
 		FC_RPORT_ID_DBG(lport, sid,
include/scsi/libfc.h +2 -3
index 7495c0b..db54c4a 100644
@@ -104,7 +104,6 @@ enum fc_disc_event {
  * @RPORT_ST_LOGO:    Remote port logout (LOGO) sent
  * @RPORT_ST_ADISC:   Discover Address sent
  * @RPORT_ST_DELETE:  Remote port being deleted
- * @RPORT_ST_RESTART: Remote port being deleted and will restart
 */
 enum fc_rport_state {
 	RPORT_ST_INIT,
@@ -115,7 +114,6 @@ enum fc_rport_state {
 	RPORT_ST_LOGO,
 	RPORT_ST_ADISC,
 	RPORT_ST_DELETE,
-	RPORT_ST_RESTART,
 };
 
 /**
@@ -173,6 +171,7 @@ struct fc_rport_libfc_priv {
 	u16			   flags;
 	#define FC_RP_FLAGS_REC_SUPPORTED	(1 << 0)
 	#define FC_RP_FLAGS_RETRY		(1 << 1)
+	#define FC_RP_STARTED			(1 << 2)
 	unsigned int	           e_d_tov;
 	unsigned int	           r_a_tov;
 };
@@ -185,7 +184,7 @@ struct fc_rport_libfc_priv {
  * @rp_state:       Enumeration that tracks progress of PLOGI, PRLI,
  *                  and RTV exchanges
  * @ids:            The remote port identifiers and roles
- * @flags:          REC and RETRY supported flags
+ * @flags:          STARTED, REC and RETRY_SUPPORTED flags
  * @max_seq:        Maximum number of concurrent sequences
  * @disc_id:        The discovery identifier
  * @maxframe_size:  The maximum frame size
#17 lore raw

PATCH 17/17 libfc: fix indefinite rport restart

In reply to Robert Love

From: Joe Eykholt <[email protected]>

Remote ports were restarting indefinitely after getting rejects in PRLI.

Fix by adding a counter of restarts and limiting that with the port login retry limit as well.

---
 drivers/scsi/libfc/fc_rport.c |    6 +++++-
 include/scsi/libfc.h          |    2 ++
 2 files changed, 7 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_rport.c +5 -1
index d385efc..363cde3 100644
@@ -257,6 +257,7 @@ static void fc_rport_work(struct work_struct *work)
 	case RPORT_EV_READY:
 		ids = rdata->ids;
 		rdata->event = RPORT_EV_NONE;
+		rdata->major_retries = 0;
 		kref_get(&rdata->kref);
 		mutex_unlock(&rdata->rp_mutex);
 
@@ -323,7 +324,10 @@ static void fc_rport_work(struct work_struct *work)
 			if (port_id == FC_FID_DIR_SERV) {
 				rdata->event = RPORT_EV_NONE;
 				mutex_unlock(&rdata->rp_mutex);
-			} else if (rdata->flags & FC_RP_STARTED) {
+			} else if ((rdata->flags & FC_RP_STARTED) &&
+				   rdata->major_retries <
+				   lport->max_rport_retry_count) {
+				rdata->major_retries++;
 				rdata->event = RPORT_EV_NONE;
 				FC_RPORT_DBG(rdata, "work restart\n");
 				fc_rport_enter_plogi(rdata);
include/scsi/libfc.h +2 -0
index db54c4a..6d78df7 100644
@@ -189,6 +189,7 @@ struct fc_rport_libfc_priv {
  * @disc_id:        The discovery identifier
  * @maxframe_size:  The maximum frame size
  * @retries:        The retry count for the current state
+ * @major_retries:  The retry count for the entire PLOGI/PRLI state machine
  * @e_d_tov:        Error detect timeout value (in msec)
  * @r_a_tov:        Resource allocation timeout value (in msec)
  * @rp_mutex:       The mutex that protects the remote port
@@ -206,6 +207,7 @@ struct fc_rport_priv {
 	u16			    disc_id;
 	u16			    maxframe_size;
 	unsigned int	            retries;
+	unsigned int	            major_retries;
 	unsigned int	            e_d_tov;
 	unsigned int	            r_a_tov;
 	struct mutex                rp_mutex;