Skip to contentSkip to conversation

libfc, libfcoe and fcoe updates for 2.6.38

PATCH New Robert Love 2 participants 29 messages Dec 1, 2010

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

cover libfc, libfcoe and fcoe updates for 2.6.38

The following series contains a variety of fixes to libfc, libfcoe and fcoe for the 2.6.38 merge window.

The "drivers/scsi/fcoe: Update WARN uses" patch was pulled in from another series submitted to linux-scsi.

---

Hillf Danton (8):
libfc: remove define of fc_seq_exch in fc_exch.c
libfc: fix mem leak in fc_exch_recv_seq_resp()
libfc: tune fc_exch_em_alloc() to be O(2)
libfc: fix mem leak in fc_seq_assign()
libfc: fix stats computation in fc_queuecommand()
libfc: fix memory leakage in local port
libfc: fix memory leakage in local port
libfc: fix memory leakage in remote port

Joe Eykholt (8):
libfcoe: update FIP FCF announcements
libfcoe: move some timer code to make it reusable.
libfcoe: fix checking of conflicting fabrics in fcoe_ctlr_select()
libfcoe: retry rejected FLOGI to another FCF if possible
libfcoe: add debug message for FCF destination MAC
libfcoe: reorder FCF list to put latest advertiser first
libfcoe: change fip_select to return new FCF
libfc: fix statistics for FCP input/output megabytes

Joe Perches (1):
drivers/scsi/fcoe: Update WARN uses

Robert Love (1):
libfc: Fix incorrect locking and unlocking in FCP

Yi Zou (6):
libfc: fix NULL pointer dereference bug in fc_fcp_pkt_release
libfc: add print of exchange id for debugging fc_fcp
libfc: do not fc_io_compl on fsp w/o any scsi_cmnd associated
libfc: fix exchange being deleted when the abort itself is timed out
libfc: the timeout for the REC itself is 2 * R_A_TOV_els
libfc: fix fc_tm_done not freeing the allocated fsp pkt

john fastabend (3):
libfc: incorrect scsi host byte codes returned to scsi-ml
libfc: use rport timeout values for fcp recovery
libfc: remove tgt_flags from fc_fcp_pkt struct

 drivers/scsi/fcoe/fcoe.c      |    4 -
 drivers/scsi/fcoe/libfcoe.c   |  300 +++++++++++++++++++++++++++++++++--------
 drivers/scsi/libfc/fc_exch.c  |   48 +++++--
 drivers/scsi/libfc/fc_fcp.c   |  147 +++++++++++++-------
 drivers/scsi/libfc/fc_libfc.h |   16 ++
 drivers/scsi/libfc/fc_lport.c |   16 ++
 drivers/scsi/libfc/fc_rport.c |    3 
 include/scsi/libfc.h          |   10 +
 include/scsi/libfcoe.h        |    8 +
 9 files changed, 422 insertions(+), 130 deletions(-)
Signature · 1 line
Thanks, //Rob
#1 lore raw

PATCH 1/27 libfc: remove define of fc_seq_exch in fc_exch.c

From: Hillf Danton <[email protected]>

The define for fc_seq_exch is unnecessary, since it also appears in scsi/libfc.h

---
 drivers/scsi/libfc/fc_exch.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_exch.c +0 -1
index ec2a1ae..1dec593 100644
@@ -108,7 +108,6 @@ struct fc_exch_mgr {
 		atomic_t non_bls_resp;
 	} stats;
 };
-#define	fc_seq_exch(sp) container_of(sp, struct fc_exch, seq)
 
 /**
  * struct fc_exch_mgr_anchor - primary structure for list of EMs
#2 lore raw

PATCH 2/27 libfc: fix NULL pointer dereference bug in fc_fcp_pkt_release

In reply to Robert Love

From: Yi Zou <[email protected]>

This happens when then tearing down the fcoe interface with active I/O. The back trace shows dead000000200200 in RAX, i.e., LIST_POISON2, indicating that the fsp is already being dequeued, which is probably why no complaining was seen in fc_fcp_destroy() about outstanding fsp not freed, since we dequeue it in the end of fc_io_compl() before releasing it. The bug is due to the fact that we have already destroyed lport's scsi_pkt_pool while on-going i/o is still accessing it through fc_fcp_pkt_release(), like this trace or the similar code path from scsi-ml to fc_eh_abort, etc. This is fixed by moving the fc_fcp_destroy() after lport is detached from scsi-ml since fc_fcp_destroy is supposed to called only once where no lport lock is taken, otherwise the fc_fcp_pkt_release() would have to grab the lport lock.

 BUG: unable to handle kernel NULL pointer dereference at (null)
 .......
 RIP: 0010:[<0000000000000000>]
 [<(null)>] (null)
 RSP: 0018:ffff8803270f7b88  EFLAGS: 00010282
 RAX: dead000000200200 RBX: ffff880197d2fbc0 RCX: 0000000000005908
 RDX: ffff880195ea6d08 RSI: 0000000000000282 RDI: ffff880180f4fec0
 RBP: ffff8803270f7bc0 R08: ffff880197d2fbe0 R09: 0000000000000000
 R10: ffff88032867f090 R11: 0000000000000000 R12: ffff880195ea6d08
 R13: 0000000000000282 R14: ffff880180f4fec0 R15: 0000000000000000
 FS:  0000000000000000(0000) GS:ffff8801b5820000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
 CR2: 0000000000000000 CR3: 00000001a6eae000 CR4: 00000000000006e0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
 Process fc_rport_eq (pid: 5278, threadinfo ffff8803270f6000, task ffff880326254ab0)
 Stack:
 ffffffffa02c39ca ffff8803270f7ba0 ffff88019331cbc0 ffff880197d2fbc0
 0000000000000000 ffff8801a8c895e0 ffff8801a8c895e0 ffff8803270f7c10
 ffffffffa02c4962 ffff8803270f7be0 ffffffff814c94ab ffff8803270f7c10
 Call Trace:
 [<ffffffffa02c39ca>] ? fc_io_compl+0x10a/0x530 [libfc]
 [<ffffffffa02c4962>] fc_fcp_complete_locked+0x72/0x150 [libfc]
 [<ffffffff814c94ab>] ? _spin_unlock_bh+0x1b/0x20
 [<ffffffffa02b98ff>] ? fc_exch_done+0x3f/0x60 [libfc]
 [<ffffffffa02c4a8f>] fc_fcp_retry_cmd+0x4f/0x60 [libfc]
 [<ffffffffa02c6150>] fc_fcp_recv+0x9b0/0xc30 [libfc]
 [<ffffffff8106ba7a>] ? _call_console_drivers+0x4a/0x80
 [<ffffffff8107d5ec>] ? lock_timer_base+0x3c/0x70
 [<ffffffff8107e06b>] ? try_to_del_timer_sync+0x7b/0xe0
 [<ffffffffa02b9dcf>] fc_exch_mgr_reset+0x1df/0x250 [libfc]
 [<ffffffffa02c57a0>] ? fc_fcp_recv+0x0/0xc30 [libfc]
 [<ffffffffa02c1042>] fc_rport_work+0xf2/0x4e0 [libfc]
 [<ffffffff8109203e>] ? prepare_to_wait+0x4e/0x80
 [<ffffffffa02c0f50>] ? fc_rport_work+0x0/0x4e0 [libfc]
 [<ffffffff8108c6c0>] worker_thread+0x170/0x2a0
 [<ffffffff81091d50>] ? autoremove_wake_function+0x0/0x40
 [<ffffffff8108c550>] ? worker_thread+0x0/0x2a0
 [<ffffffff810919e6>] kthread+0x96/0xa0
 [<ffffffff810141ca>] child_rip+0xa/0x20
 [<ffffffff81091950>] ? kthread+0x0/0xa0
 [<ffffffff810141c0>] ? child_rip+0x0/0x20
 Code:
 Bad RIP value.

RIP
[<(null)>] (null)
RSP <ffff8803270f7b88>
CR2: 0000000000000000

---
 drivers/scsi/fcoe/fcoe.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/scsi/fcoe/fcoe.c +3 -1
index d23a538..9f9600b 100644
@@ -854,7 +854,6 @@ static void fcoe_if_destroy(struct fc_lport *lport)
 
 	/* Cleanup the fc_lport */
 	fc_lport_destroy(lport);
-	fc_fcp_destroy(lport);
 
 	/* Stop the transmit retry timer */
 	del_timer_sync(&port->timer);
@@ -876,6 +875,9 @@ static void fcoe_if_destroy(struct fc_lport *lport)
 	fc_remove_host(lport->host);
 	scsi_remove_host(lport->host);
 
+	/* Destroy lport scsi_priv */
+	fc_fcp_destroy(lport);
+
 	/* There are no more rports or I/O, free the EM */
 	fc_exch_mgr_free(lport);
 
#3 lore raw

PATCH 3/27 libfc: fix mem leak in fc_exch_recv_seq_resp()

In reply to Robert Love

From: Hillf Danton <[email protected]>

There seems that ep should get released, or it will no longer get freed.

---
 drivers/scsi/libfc/fc_exch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_exch.c +1 -1
index 1dec593..d0df1b2 100644
@@ -1342,7 +1342,7 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
 	}
 	if (ep->esb_stat & ESB_ST_COMPLETE) {
 		atomic_inc(&mp->stats.xid_not_found);
-		goto out;
+		goto rel;
 	}
 	if (ep->rxid == FC_XID_UNKNOWN)
 		ep->rxid = ntohs(fh->fh_rx_id);
#4 lore raw

PATCH 4/27 libfc: tune fc_exch_em_alloc() to be O(2)

In reply to Robert Love

From: Hillf Danton <[email protected]>

For allocating new exch from pool, scanning for free slot in exch array fluctuates when exch pool is close to exhaustion.

The fluctuation is smoothed, and the scan looks to be O(2).

---
 drivers/scsi/libfc/fc_exch.c |   36 +++++++++++++++++++++++++++++++++---
 1 files changed, 33 insertions(+), 3 deletions(-)
drivers/scsi/libfc/fc_exch.c +33 -3
index d0df1b2..46973d6 100644
@@ -67,6 +67,11 @@ struct workqueue_struct *fc_exch_workqueue;
 struct fc_exch_pool {
 	u16		 next_index;
 	u16		 total_exches;
+
+	/* two cache of free slot in exch array */
+	u16		 left;
+	u16		 right;
+
 	spinlock_t	 lock;
 	struct list_head ex_list;
 };
@@ -396,13 +401,23 @@ static inline void fc_exch_ptr_set(struct fc_exch_pool *pool, u16 index,
 static void fc_exch_delete(struct fc_exch *ep)
 {
 	struct fc_exch_pool *pool;
+	u16 index;
 
 	pool = ep->pool;
 	spin_lock_bh(&pool->lock);
 	WARN_ON(pool->total_exches <= 0);
 	pool->total_exches--;
-	fc_exch_ptr_set(pool, (ep->xid - ep->em->min_xid) >> fc_cpu_order,
-			NULL);
+
+	/* update cache of free slot */
+	index = (ep->xid - ep->em->min_xid) >> fc_cpu_order;
+	if (pool->left == FC_XID_UNKNOWN)
+		pool->left = index;
+	else if (pool->right == FC_XID_UNKNOWN)
+		pool->right = index;
+	else
+		pool->next_index = index;
+
+	fc_exch_ptr_set(pool, index, NULL);
 	list_del(&ep->ex_list);
 	spin_unlock_bh(&pool->lock);
 	fc_exch_release(ep);	/* drop hold for exch in mp */
@@ -678,6 +693,19 @@ static struct fc_exch *fc_exch_em_alloc(struct fc_lport *lport,
 	pool = per_cpu_ptr(mp->pool, cpu);
 	spin_lock_bh(&pool->lock);
 	put_cpu();
+
+	/* peek cache of free slot */
+	if (pool->left != FC_XID_UNKNOWN) {
+		index = pool->left;
+		pool->left = FC_XID_UNKNOWN;
+		goto hit;
+	}
+	if (pool->right != FC_XID_UNKNOWN) {
+		index = pool->right;
+		pool->right = FC_XID_UNKNOWN;
+		goto hit;
+	}
+
 	index = pool->next_index;
 	/* allocate new exch from pool */
 	while (fc_exch_ptr_get(pool, index)) {
@@ -686,7 +714,7 @@ static struct fc_exch *fc_exch_em_alloc(struct fc_lport *lport,
 			goto err;
 	}
 	pool->next_index = index == mp->pool_max_index ? 0 : index + 1;
-
+hit:
 	fc_exch_hold(ep);	/* hold for exch in mp */
 	spin_lock_init(&ep->ex_lock);
 	/*
@@ -2180,6 +2208,8 @@ struct fc_exch_mgr *fc_exch_mgr_alloc(struct fc_lport *lport,
 		goto free_mempool;
 	for_each_possible_cpu(cpu) {
 		pool = per_cpu_ptr(mp->pool, cpu);
+		pool->left = FC_XID_UNKNOWN;
+		pool->right = FC_XID_UNKNOWN;
 		spin_lock_init(&pool->lock);
 		INIT_LIST_HEAD(&pool->ex_list);
 	}
#5 lore raw

PATCH 5/27 libfc: Fix incorrect locking and unlocking in FCP

In reply to Robert Love

The error handler grabs the si->scsi_queue_lock, but in the case where the fsp pointer is NULL it releases the scsi_host lock. This can lead to a variety of system hangs depending on which is used first- the scsi_host lock or the scsi_queue_lock.

This patch simply unlocks the correct lock when fcp is NULL.

---
 drivers/scsi/libfc/fc_fcp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_fcp.c +1 -1
index e340373..e82ff6e 100644
@@ -2002,7 +2002,7 @@ int fc_eh_abort(struct scsi_cmnd *sc_cmd)
 	fsp = CMD_SP(sc_cmd);
 	if (!fsp) {
 		/* command completed while scsi eh was setting up */
-		spin_unlock_irqrestore(lport->host->host_lock, flags);
+		spin_unlock_irqrestore(&si->scsi_queue_lock, flags);
 		return SUCCESS;
 	}
 	/* grab a ref so the fsp and sc_cmd cannot be relased from under us */
#6 lore raw

PATCH 6/27 libfc: fix mem leak in fc_seq_assign()

In reply to Robert Love

From: Hillf Danton <[email protected]>

There is a typo cleaned, which triggers memory leakage.

---
 drivers/scsi/libfc/fc_exch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_exch.c +1 -1
index 46973d6..8bfb421 100644
@@ -1274,7 +1274,7 @@ static struct fc_seq *fc_seq_assign(struct fc_lport *lport, struct fc_frame *fp)
 
 	list_for_each_entry(ema, &lport->ema_list, ema_list)
 		if ((!ema->match || ema->match(fp)) &&
-		    fc_seq_lookup_recip(lport, ema->mp, fp) != FC_RJT_NONE)
+		    fc_seq_lookup_recip(lport, ema->mp, fp) == FC_RJT_NONE)
 			break;
 	return fr_seq(fp);
 }
#7 lore raw

PATCH 7/27 libfc: fix stats computation in fc_queuecommand()

In reply to Robert Love

From: Hillf Danton <[email protected]>

There seems accumulation needed.

---
 drivers/scsi/libfc/fc_fcp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/libfc/fc_fcp.c +2 -2
index e82ff6e..eb3fe37 100644
@@ -1820,11 +1820,11 @@ int fc_queuecommand(struct scsi_cmnd *sc_cmd, void (*done)(struct scsi_cmnd *))
 	if (sc_cmd->sc_data_direction == DMA_FROM_DEVICE) {
 		fsp->req_flags = FC_SRB_READ;
 		stats->InputRequests++;
-		stats->InputMegabytes = fsp->data_len;
+		stats->InputMegabytes += fsp->data_len;
 	} else if (sc_cmd->sc_data_direction == DMA_TO_DEVICE) {
 		fsp->req_flags = FC_SRB_WRITE;
 		stats->OutputRequests++;
-		stats->OutputMegabytes = fsp->data_len;
+		stats->OutputMegabytes += fsp->data_len;
 	} else {
 		fsp->req_flags = 0;
 		stats->ControlRequests++;
#8 lore raw

PATCH 8/27 libfc: incorrect scsi host byte codes returned to scsi-ml

In reply to Robert Love

From: john fastabend <[email protected]>

The fcp packet recovery handler fc_fcp_recover() is called when errors occurr in a fcp session. Currently it is generically setting the status code to FC_CMD_RECOVERY for all error types. This results in DID_BUS_BUSY errors being returned to the scsi-ml.

DID_BUS_BUSY errors indicate "BUS stayed busy through time out period" according to scsi.h. Many of the error reported by fc_rcp_recovery() are pkt errors. Here we update fc_fcp_recovery to use better host byte codes.

With certain FAST FAIL flags set DID_BUS_BUSY and DID_ERROR will have different behaviors this was causing dm multipath to fail quickly in some cases where a retry would be a better action.

---
 drivers/scsi/libfc/fc_fcp.c |   61 +++++++++++++++++++++++++++++--------------
 1 files changed, 41 insertions(+), 20 deletions(-)
drivers/scsi/libfc/fc_fcp.c +41 -20
index eb3fe37..87361cc 100644
@@ -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_fcp_recovery(struct fc_fcp_pkt *);
+static void fc_fcp_recovery(struct fc_fcp_pkt *, u8 code);
 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,8 @@ 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_RECOVERY		10
+#define FC_CRC_ERROR		10
+#define FC_TIMED_OUT		11
 
 /*
  * Error recovery timeout values.
@@ -438,6 +439,7 @@ static void fc_fcp_recv_data(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 	void *buf;
 	struct scatterlist *sg;
 	u32 nents;
+	u8 host_bcode = FC_COMPLETE;
 
 	fh = fc_frame_header_get(fp);
 	offset = ntohl(fh->fh_parm_offset);
@@ -446,13 +448,16 @@ static void fc_fcp_recv_data(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 	buf = fc_frame_payload_get(fp, 0);
 
 	/*
-	 * 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 this I/O is ddped then clear it and initiate recovery since data
+	 * frames are expected to be placed directly in that case.
+	 *
+	 * Indicate error to scsi-ml because something went wrong with the
+	 * ddp handling to get us here.
 	 */
 	if (fsp->xfer_ddp != FC_XID_UNKNOWN) {
 		fc_fcp_ddp_done(fsp);
+		FC_FCP_DBG(fsp, "DDP I/O in fc_fcp_recv_data set ERROR\n");
+		host_bcode = FC_ERROR;
 		goto err;
 	}
 	if (offset + len > fsp->data_len) {
@@ -462,6 +467,9 @@ 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);
+
+		/* Data is corrupted indicate scsi-ml should retry */
+		host_bcode = FC_DATA_OVRRUN;
 		goto err;
 	}
 	if (offset != fsp->xfer_len)
@@ -498,8 +506,10 @@ crc_err:
 			 * If so, we need to retry the entire operation.
 			 * Otherwise, ignore it.
 			 */
-			if (fsp->state & FC_SRB_DISCONTIG)
+			if (fsp->state & FC_SRB_DISCONTIG) {
+				host_bcode = FC_CRC_ERROR;
 				goto err;
+			}
 			return;
 		}
 	}
@@ -517,7 +527,7 @@ crc_err:
 		fc_fcp_complete_locked(fsp);
 	return;
 err:
-	fc_fcp_recovery(fsp);
+	fc_fcp_recovery(fsp, host_bcode);
 }
 
 /**
@@ -1347,7 +1357,7 @@ static void fc_fcp_timeout(unsigned long data)
 	else if (fsp->state & FC_SRB_RCV_STATUS)
 		fc_fcp_complete_locked(fsp);
 	else
-		fc_fcp_recovery(fsp);
+		fc_fcp_recovery(fsp, FC_TIMED_OUT);
 	fsp->state &= ~FC_SRB_FCP_PROCESSING_TMO;
 unlock:
 	fc_fcp_unlock_pkt(fsp);
@@ -1391,7 +1401,7 @@ retry:
 	if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY)
 		fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV);
 	else
-		fc_fcp_recovery(fsp);
+		fc_fcp_recovery(fsp, FC_TIMED_OUT);
 }
 
 /**
@@ -1460,7 +1470,7 @@ static void fc_fcp_rec_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
 				fc_fcp_retry_cmd(fsp);
 				break;
 			}
-			fc_fcp_recovery(fsp);
+			fc_fcp_recovery(fsp, FC_ERROR);
 			break;
 		}
 	} else if (opcode == ELS_LS_ACC) {
@@ -1575,7 +1585,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_fcp_recovery(fsp);
+			fc_fcp_recovery(fsp, FC_ERROR);
 		break;
 	}
 	fc_fcp_unlock_pkt(fsp);
@@ -1587,9 +1597,9 @@ out:
  * fc_fcp_recovery() - Handler for fcp_pkt recovery
  * @fsp: The FCP pkt that needs to be aborted
  */
-static void fc_fcp_recovery(struct fc_fcp_pkt *fsp)
+static void fc_fcp_recovery(struct fc_fcp_pkt *fsp, u8 code)
 {
-	fsp->status_code = FC_CMD_RECOVERY;
+	fsp->status_code = code;
 	fsp->cdb_status = 0;
 	fsp->io_status = 0;
 	/*
@@ -1695,7 +1705,7 @@ static void fc_fcp_srr_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
 		break;
 	case ELS_LS_RJT:
 	default:
-		fc_fcp_recovery(fsp);
+		fc_fcp_recovery(fsp, FC_ERROR);
 		break;
 	}
 	fc_fcp_unlock_pkt(fsp);
@@ -1721,7 +1731,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_fcp_recovery(fsp);
+			fc_fcp_recovery(fsp, FC_TIMED_OUT);
 		break;
 	case -FC_EX_CLOSED:			/* e.g., link failure */
 		/* fall through */
@@ -1944,18 +1954,29 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp)
 		break;
 	case FC_CMD_ABORTED:
 		FC_FCP_DBG(fsp, "Returning DID_ERROR to scsi-ml "
-			   "due to FC_CMD_ABORTED\n");
+			  "due to FC_CMD_ABORTED\n");
 		sc_cmd->result = (DID_ERROR << 16) | fsp->io_status;
 		break;
-	case FC_CMD_RECOVERY:
-		sc_cmd->result = (DID_BUS_BUSY << 16) | fsp->io_status;
-		break;
 	case FC_CMD_RESET:
+		FC_FCP_DBG(fsp, "Returning DID_RESET to scsi-ml "
+			   "due to FC_CMD_RESET\n");
 		sc_cmd->result = (DID_RESET << 16);
 		break;
 	case FC_HRD_ERROR:
+		FC_FCP_DBG(fsp, "Returning DID_NO_CONNECT to scsi-ml "
+			   "due to FC_HRD_ERROR\n");
 		sc_cmd->result = (DID_NO_CONNECT << 16);
 		break;
+	case FC_CRC_ERROR:
+		FC_FCP_DBG(fsp, "Returning DID_PARITY to scsi-ml "
+			   "due to FC_CRC_ERROR\n");
+		sc_cmd->result = (DID_PARITY << 16);
+		break;
+	case FC_TIMED_OUT:
+		FC_FCP_DBG(fsp, "Returning DID_BUS_BUSY to scsi-ml "
+			   "due to FC_TIMED_OUT\n");
+		sc_cmd->result = (DID_BUS_BUSY << 16) | fsp->io_status;
+		break;
 	default:
 		FC_FCP_DBG(fsp, "Returning DID_ERROR to scsi-ml "
 			   "due to unknown error\n");
#9 lore raw

PATCH 9/27 libfc: use rport timeout values for fcp recovery

In reply to Robert Love

From: john fastabend <[email protected]>

Use the rport value for rec_tov for timeout values when sending fcp commands. Currently, defaults are being used which may or may not match the advertised values.

The default may cause i/o to timeout on networks that set this value larger then the default value. To make the timeout more configurable in the non-REC mode we remove the FC_SCSI_ER_TIMEOUT completely allowing the scsi-ml to do the timeout. This removes an unneeded timer and allows the i/o timeout to be configured using the scsi-ml knobs.

---
 drivers/scsi/libfc/fc_fcp.c |   58 +++++++++++++++++++++++++++++++------------
 1 files changed, 42 insertions(+), 16 deletions(-)
drivers/scsi/libfc/fc_fcp.c +42 -16
index 87361cc..3cf81b9 100644
@@ -57,6 +57,9 @@ struct kmem_cache *scsi_pkt_cachep;
 #define FC_SRB_READ		(1 << 1)
 #define FC_SRB_WRITE		(1 << 0)
 
+/* constant added to e_d_tov timeout to get rec_tov value */
+#define REC_TOV_CONST		1
+
 /*
  * The SCp.ptr should be tested and set under the scsi_pkt_queue lock
  */
@@ -126,9 +129,7 @@ static void fc_fcp_srr_error(struct fc_fcp_pkt *, struct fc_frame *);
 /*
  * Error recovery timeout values.
  */
-#define FC_SCSI_ER_TIMEOUT	(10 * HZ)
 #define FC_SCSI_TM_TOV		(10 * HZ)
-#define FC_SCSI_REC_TOV		(2 * HZ)
 #define FC_HOST_RESET_TIMEOUT	(30 * HZ)
 #define FC_CAN_QUEUE_PERIOD	(60 * HZ)
 
@@ -1083,6 +1084,21 @@ static int fc_fcp_pkt_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp)
 }
 
 /**
+ * get_fsp_rec_tov() - Helper function to get REC_TOV
+ * @fsp: the FCP packet
+ */
+static inline unsigned int get_fsp_rec_tov(struct fc_fcp_pkt *fsp)
+{
+	struct fc_rport *rport;
+	struct fc_rport_libfc_priv *rpriv;
+
+	rport = fsp->rport;
+	rpriv = rport->dd_data;
+
+	return rpriv->e_d_tov + REC_TOV_CONST;
+}
+
+/**
  * fc_fcp_cmd_send() - Send a FCP command
  * @lport: The local port to send the command on
  * @fsp:   The FCP packet the command is on
@@ -1099,6 +1115,7 @@ static int fc_fcp_cmd_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp,
 	struct fc_rport_libfc_priv *rpriv;
 	const size_t len = sizeof(fsp->cdb_cmd);
 	int rc = 0;
+	unsigned int rec_tov;
 
 	if (fc_fcp_lock_pkt(fsp))
 		return 0;
@@ -1129,10 +1146,12 @@ static int fc_fcp_cmd_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp,
 	fsp->seq_ptr = seq;
 	fc_fcp_pkt_hold(fsp);	/* hold for fc_fcp_pkt_destroy */
 
+	rec_tov = get_fsp_rec_tov(fsp);
+
 	setup_timer(&fsp->timer, fc_fcp_timeout, (unsigned long)fsp);
-	fc_fcp_timer_set(fsp,
-			 (fsp->tgt_flags & FC_RP_FLAGS_REC_SUPPORTED) ?
-			 FC_SCSI_REC_TOV : FC_SCSI_ER_TIMEOUT);
+
+	if (fsp->tgt_flags & FC_RP_FLAGS_REC_SUPPORTED)
+		fc_fcp_timer_set(fsp, rec_tov);
 unlock:
 	fc_fcp_unlock_pkt(fsp);
 	return rc;
@@ -1207,13 +1226,16 @@ static void fc_lun_reset_send(unsigned long data)
 {
 	struct fc_fcp_pkt *fsp = (struct fc_fcp_pkt *)data;
 	struct fc_lport *lport = fsp->lp;
+	unsigned int rec_tov;
+
 	if (lport->tt.fcp_cmd_send(lport, fsp, fc_tm_done)) {
 		if (fsp->recov_retry++ >= FC_MAX_RECOV_RETRY)
 			return;
 		if (fc_fcp_lock_pkt(fsp))
 			return;
+		rec_tov = get_fsp_rec_tov(fsp);
 		setup_timer(&fsp->timer, fc_lun_reset_send, (unsigned long)fsp);
-		fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV);
+		fc_fcp_timer_set(fsp, rec_tov);
 		fc_fcp_unlock_pkt(fsp);
 	}
 }
@@ -1351,9 +1373,6 @@ static void fc_fcp_timeout(unsigned long data)
 
 	if (rpriv->flags & FC_RP_FLAGS_REC_SUPPORTED)
 		fc_fcp_rec(fsp);
-	else if (time_after_eq(fsp->last_pkt_time + (FC_SCSI_ER_TIMEOUT / 2),
-			       jiffies))
-		fc_fcp_timer_set(fsp, FC_SCSI_ER_TIMEOUT);
 	else if (fsp->state & FC_SRB_RCV_STATUS)
 		fc_fcp_complete_locked(fsp);
 	else
@@ -1373,6 +1392,7 @@ static void fc_fcp_rec(struct fc_fcp_pkt *fsp)
 	struct fc_frame *fp;
 	struct fc_rport *rport;
 	struct fc_rport_libfc_priv *rpriv;
+	unsigned int rec_tov;
 
 	lport = fsp->lp;
 	rport = fsp->rport;
@@ -1383,6 +1403,9 @@ static void fc_fcp_rec(struct fc_fcp_pkt *fsp)
 		fc_fcp_complete_locked(fsp);
 		return;
 	}
+
+	rec_tov = get_fsp_rec_tov(fsp);
+
 	fp = fc_fcp_frame_alloc(lport, sizeof(struct fc_els_rec));
 	if (!fp)
 		goto retry;
@@ -1393,13 +1416,13 @@ static void fc_fcp_rec(struct fc_fcp_pkt *fsp)
 		       FC_FCTL_REQ, 0);
 	if (lport->tt.elsct_send(lport, rport->port_id, fp, ELS_REC,
 				 fc_fcp_rec_resp, fsp,
-				 jiffies_to_msecs(FC_SCSI_REC_TOV))) {
+				 jiffies_to_msecs(rec_tov))) {
 		fc_fcp_pkt_hold(fsp);		/* hold while REC outstanding */
 		return;
 	}
 retry:
 	if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY)
-		fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV);
+		fc_fcp_timer_set(fsp, rec_tov);
 	else
 		fc_fcp_recovery(fsp, FC_TIMED_OUT);
 }
@@ -1455,7 +1478,6 @@ static void fc_fcp_rec_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
 			 * making progress.
 			 */
 			rpriv->flags &= ~FC_RP_FLAGS_REC_SUPPORTED;
-			fc_fcp_timer_set(fsp, FC_SCSI_ER_TIMEOUT);
 			break;
 		case ELS_RJT_LOGIC:
 		case ELS_RJT_UNAB:
@@ -1508,12 +1530,12 @@ static void fc_fcp_rec_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
 			}
 			fc_fcp_srr(fsp, r_ctl, offset);
 		} else if (e_stat & ESB_ST_SEQ_INIT) {
-
+			unsigned int rec_tov = get_fsp_rec_tov(fsp);
 			/*
 			 * The remote port has the initiative, so just
 			 * keep waiting for it to complete.
 			 */
-			fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV);
+			fc_fcp_timer_set(fsp, rec_tov);
 		} else {
 
 			/*
@@ -1626,6 +1648,7 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset)
 	struct fcp_srr *srr;
 	struct fc_frame *fp;
 	u8 cdb_op;
+	unsigned int rec_tov;
 
 	rport = fsp->rport;
 	rpriv = rport->dd_data;
@@ -1650,8 +1673,9 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset)
 		       rpriv->local_port->port_id, FC_TYPE_FCP,
 		       FC_FCTL_REQ, 0);
 
+	rec_tov = get_fsp_rec_tov(fsp);
 	seq = lport->tt.exch_seq_send(lport, fp, fc_fcp_srr_resp, NULL,
-				      fsp, jiffies_to_msecs(FC_SCSI_REC_TOV));
+				      fsp, jiffies_to_msecs(rec_tov));
 	if (!seq)
 		goto retry;
 
@@ -1675,6 +1699,7 @@ static void fc_fcp_srr_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
 {
 	struct fc_fcp_pkt *fsp = arg;
 	struct fc_frame_header *fh;
+	unsigned int rec_tov;
 
 	if (IS_ERR(fp)) {
 		fc_fcp_srr_error(fsp, fp);
@@ -1701,7 +1726,8 @@ static void fc_fcp_srr_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
 	switch (fc_frame_payload_op(fp)) {
 	case ELS_LS_ACC:
 		fsp->recov_retry = 0;
-		fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV);
+		rec_tov = get_fsp_rec_tov(fsp);
+		fc_fcp_timer_set(fsp, rec_tov);
 		break;
 	case ELS_LS_RJT:
 	default:
#10 lore raw

PATCH 10/27 libfc: remove tgt_flags from fc_fcp_pkt struct

In reply to Robert Love

From: john fastabend <[email protected]>

We can easily remove the tgt_flags from fc_fcp_pkt struct and use rpriv->tgt_flags directly where needed.

---
 drivers/scsi/libfc/fc_fcp.c |    5 ++---
 include/scsi/libfc.h        |    2 --
 2 files changed, 2 insertions(+), 5 deletions(-)
drivers/scsi/libfc/fc_fcp.c +2 -3
index 3cf81b9..c12994b 100644
@@ -1150,8 +1150,9 @@ static int fc_fcp_cmd_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp,
 
 	setup_timer(&fsp->timer, fc_fcp_timeout, (unsigned long)fsp);
 
-	if (fsp->tgt_flags & FC_RP_FLAGS_REC_SUPPORTED)
+	if (rpriv->flags & FC_RP_FLAGS_REC_SUPPORTED)
 		fc_fcp_timer_set(fsp, rec_tov);
+
 unlock:
 	fc_fcp_unlock_pkt(fsp);
 	return rc;
@@ -1867,8 +1868,6 @@ int fc_queuecommand(struct scsi_cmnd *sc_cmd, void (*done)(struct scsi_cmnd *))
 	}
 	put_cpu();
 
-	fsp->tgt_flags = rpriv->flags;
-
 	init_timer(&fsp->timer);
 	fsp->timer.data = (unsigned long)fsp;
 
include/scsi/libfc.h +0 -2
index f986ab7..1463660 100644
@@ -263,7 +263,6 @@ struct fc_seq_els_data {
  * struct fc_fcp_pkt - FCP request structure (one for each scsi_cmnd request)
  * @lp:              The associated local port
  * @state:           The state of the I/O
- * @tgt_flags:       Target's flags
  * @ref_cnt:         Reference count
  * @scsi_pkt_lock:   Lock to protect the SCSI packet (must be taken before the
  *                   host_lock if both are to be held at the same time)
@@ -298,7 +297,6 @@ struct fc_fcp_pkt {
 	/* Housekeeping information */
 	struct fc_lport   *lp;
 	u16		  state;
-	u16		  tgt_flags;
 	atomic_t	  ref_cnt;
 	spinlock_t	  scsi_pkt_lock;
 
#11 lore raw

PATCH 11/27 libfc: fix memory leakage in local port

In reply to Robert Love

From: Hillf Danton <[email protected]>

There seems info should get freed when error encountered.

---
 drivers/scsi/libfc/fc_lport.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_lport.c +3 -1
index 9be63ed..9c1d6b8 100644
@@ -1762,8 +1762,10 @@ static int fc_lport_ct_request(struct fc_bsg_job *job,
 	info->sg = job->reply_payload.sg_list;
 
 	if (!lport->tt.exch_seq_send(lport, fp, fc_lport_bsg_resp,
-				     NULL, info, tov))
+				     NULL, info, tov)) {
+		kfree(info);
 		return -ECOMM;
+	}
 	return 0;
 }
 
#12 lore raw

PATCH 12/27 libfc: fix memory leakage in local port

In reply to Robert Love

From: Hillf Danton <[email protected]>

There seems info should get freed when error encountered.

---
 drivers/scsi/libfc/fc_lport.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_lport.c +3 -1
index 9c1d6b8..b91a11e 100644
@@ -1703,8 +1703,10 @@ static int fc_lport_els_request(struct fc_bsg_job *job,
 	info->sg = job->reply_payload.sg_list;
 
 	if (!lport->tt.exch_seq_send(lport, fp, fc_lport_bsg_resp,
-				     NULL, info, tov))
+				     NULL, info, tov)) {
+		kfree(info);
 		return -ECOMM;
+	}
 	return 0;
 }
 
#13 lore raw

PATCH 13/27 libfc: fix memory leakage in remote port

In reply to Robert Love

From: Hillf Danton <[email protected]>

There seems rdata should get put before return.

---
 drivers/scsi/libfc/fc_rport.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_rport.c +2 -1
index a84ef13..a7175ad 100644
@@ -652,7 +652,7 @@ void fc_rport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp,
 	FC_RPORT_DBG(rdata, "Received a FLOGI %s\n", fc_els_resp_type(fp));
 
 	if (fp == ERR_PTR(-FC_EX_CLOSED))
-		return;
+		goto put;
 
 	mutex_lock(&rdata->rp_mutex);
 
@@ -689,6 +689,7 @@ out:
 	fc_frame_free(fp);
 err:
 	mutex_unlock(&rdata->rp_mutex);
+put:
 	kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy);
 	return;
 bad:
#14 lore raw

PATCH 14/27 drivers/scsi/fcoe: Update WARN uses

In reply to Robert Love

From: Joe Perches <[email protected]>

Add missing newlines.

---
 drivers/scsi/fcoe/libfcoe.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/fcoe/libfcoe.c +1 -1
index bc17c71..58ad3c7 100644
@@ -2475,7 +2475,7 @@ static void fcoe_ctlr_vn_timeout(struct fcoe_ctlr *fip)
 	case FIP_ST_LINK_WAIT:
 		goto unlock;
 	default:
-		WARN(1, "unexpected state %d", fip->state);
+		WARN(1, "unexpected state %d\n", fip->state);
 		goto unlock;
 	}
 	mod_timer(&fip->timer, next_time);
#15 lore raw

PATCH 15/27 libfc: add print of exchange id for debugging fc_fcp

In reply to Robert Love

From: Yi Zou <[email protected]>

This is very helpful to match up the corresponding exchange to the actual I/O described by the fsp, particularly when you do a side-by-side comparison of the syslog with your trace.

---
 drivers/scsi/libfc/fc_libfc.h |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)
drivers/scsi/libfc/fc_libfc.h +14 -2
index 16d2162..eea0c35 100644
@@ -66,9 +66,21 @@ extern unsigned int fc_debug_logging;
 
 #define FC_FCP_DBG(pkt, fmt, args...)					\
 	FC_CHECK_LOGGING(FC_FCP_LOGGING,				\
-			 printk(KERN_INFO "host%u: fcp: %6.6x: " fmt,	\
+	{								\
+		if ((pkt)->seq_ptr) {					\
+			struct fc_exch *_ep = NULL;			\
+			_ep = fc_seq_exch((pkt)->seq_ptr);		\
+			printk(KERN_INFO "host%u: fcp: %6.6x: "		\
+				"xid %04x-%04x: " fmt,			\
 				(pkt)->lp->host->host_no,		\
-				pkt->rport->port_id, ##args))
+				(pkt)->rport->port_id,			\
+				(_ep)->oxid, (_ep)->rxid, ##args);	\
+		} else {						\
+			printk(KERN_INFO "host%u: fcp: %6.6x: " fmt,	\
+				(pkt)->lp->host->host_no,		\
+				(pkt)->rport->port_id, ##args);		\
+		}							\
+	})
 
 #define FC_EXCH_DBG(exch, fmt, args...)					\
 	FC_CHECK_LOGGING(FC_EXCH_LOGGING,				\
#16 lore raw

PATCH 16/27 libfc: do not fc_io_compl on fsp w/o any scsi_cmnd associated

In reply to Robert Love

From: Yi Zou <[email protected]>

Do not call fc_io_compl() on fsp w/o any scsi_cmnd, e.g., lun reset is built inside fc_fcp, not from a scsi command from queuecommnd from scsi-ml, so in in case target is buggy that is invalid flags in the FCP_RSP, as we have seen in some SAN Blaze target where all bits in flags are 0, we do not want to call io_compl on this fsp.

[ Comment block added by Robert Love ]

---
 drivers/scsi/libfc/fc_fcp.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
drivers/scsi/libfc/fc_fcp.c +7 -1
index c12994b..088ea2a 100644
@@ -973,7 +973,13 @@ static void fc_fcp_complete_locked(struct fc_fcp_pkt *fsp)
 		}
 		lport->tt.exch_done(seq);
 	}
-	fc_io_compl(fsp);
+	/*
+	 * Some resets driven by SCSI are not I/Os and do not have
+	 * SCSI commands associated with the requests. We should not
+	 * call I/O completion if we do not have a SCSI command.
+	 */
+	if (fsp->cmd)
+		fc_io_compl(fsp);
 }
 
 /**
#17 lore raw

PATCH 17/27 libfc: fix exchange being deleted when the abort itself is timed out

In reply to Robert Love

From: Yi Zou <[email protected]>

Should not continue when the abort itself is being timeout since in that case the exchange will be deleted and relesased. We still want to call the associated response handler to let the layer, e.g., fcp, know the exchange itself is being timed out.

---
 drivers/scsi/libfc/fc_exch.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
drivers/scsi/libfc/fc_exch.c +5 -2
index 8bfb421..d21367d 100644
@@ -650,10 +650,13 @@ static void fc_exch_timeout(struct work_struct *work)
 		if (e_stat & ESB_ST_ABNORMAL)
 			rc = fc_exch_done_locked(ep);
 		spin_unlock_bh(&ep->ex_lock);
-		if (!rc)
-			fc_exch_delete(ep);
 		if (resp)
 			resp(sp, ERR_PTR(-FC_EX_TIMEOUT), arg);
+		if (!rc) {
+			/* delete the exchange if it's already being aborted */
+			fc_exch_delete(ep);
+			return;
+		}
 		fc_seq_exch_abort(sp, 2 * ep->r_a_tov);
 		goto done;
 	}
#18 lore raw

PATCH 18/27 libfc: the timeout for the REC itself is 2 * R_A_TOV_els

In reply to Robert Love

From: Yi Zou <[email protected]>

The timeout for the exchange carrying REC itself is 2 * R_A_TOV_els.

---
 drivers/scsi/libfc/fc_fcp.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)
drivers/scsi/libfc/fc_fcp.c +2 -5
index 088ea2a..a25057d 100644
@@ -1399,7 +1399,6 @@ static void fc_fcp_rec(struct fc_fcp_pkt *fsp)
 	struct fc_frame *fp;
 	struct fc_rport *rport;
 	struct fc_rport_libfc_priv *rpriv;
-	unsigned int rec_tov;
 
 	lport = fsp->lp;
 	rport = fsp->rport;
@@ -1411,8 +1410,6 @@ static void fc_fcp_rec(struct fc_fcp_pkt *fsp)
 		return;
 	}
 
-	rec_tov = get_fsp_rec_tov(fsp);
-
 	fp = fc_fcp_frame_alloc(lport, sizeof(struct fc_els_rec));
 	if (!fp)
 		goto retry;
@@ -1423,13 +1420,13 @@ static void fc_fcp_rec(struct fc_fcp_pkt *fsp)
 		       FC_FCTL_REQ, 0);
 	if (lport->tt.elsct_send(lport, rport->port_id, fp, ELS_REC,
 				 fc_fcp_rec_resp, fsp,
-				 jiffies_to_msecs(rec_tov))) {
+				 2 * lport->r_a_tov)) {
 		fc_fcp_pkt_hold(fsp);		/* hold while REC outstanding */
 		return;
 	}
 retry:
 	if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY)
-		fc_fcp_timer_set(fsp, rec_tov);
+		fc_fcp_timer_set(fsp, get_fsp_rec_tov(fsp));
 	else
 		fc_fcp_recovery(fsp, FC_TIMED_OUT);
 }
#19 lore raw

PATCH 19/27 libfc: fix fc_tm_done not freeing the allocated fsp pkt

In reply to Robert Love

From: Yi Zou <[email protected]>

Frame should be freed in fc_tm_done, this is an updated patch on the one initially submitted by Hillf Danton.

---
 drivers/scsi/libfc/fc_fcp.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
drivers/scsi/libfc/fc_fcp.c +7 -7
index a25057d..12b639d 100644
@@ -1321,27 +1321,27 @@ static void fc_tm_done(struct fc_seq *seq, struct fc_frame *fp, void *arg)
 		 *
 		 * scsi-eh will escalate for when either happens.
 		 */
-		return;
+		goto out;
 	}
 
 	if (fc_fcp_lock_pkt(fsp))
-		return;
+		goto out;
 
 	/*
 	 * raced with eh timeout handler.
 	 */
-	if (!fsp->seq_ptr || !fsp->wait_for_comp) {
-		spin_unlock_bh(&fsp->scsi_pkt_lock);
-		return;
-	}
+	if (!fsp->seq_ptr || !fsp->wait_for_comp)
+		goto out_unlock;
 
 	fh = fc_frame_header_get(fp);
 	if (fh->fh_type != FC_TYPE_BLS)
 		fc_fcp_resp(fsp, fp);
 	fsp->seq_ptr = NULL;
 	fsp->lp->tt.exch_done(seq);
-	fc_frame_free(fp);
+out_unlock:
 	fc_fcp_unlock_pkt(fsp);
+out:
+	fc_frame_free(fp);
 }
 
 /**
#20 lore raw

PATCH 20/27 libfcoe: update FIP FCF announcements

In reply to Robert Love

From: Joe Eykholt <[email protected]>

Move the announcement code to a separate function for reuse in a forthcoming patch.

For messages regarding FCF timeout and selection, use the previously-announced FCF MAC address (dest_addr) in the fcoe_ctlr struct.
Only print (announce) the FCF if it is new. Print MAC for
timed-out or deselected FCFs.

---
 drivers/scsi/fcoe/libfcoe.c |   40 +++++++++++++++++++++++++++++-----------
 1 files changed, 29 insertions(+), 11 deletions(-)
drivers/scsi/fcoe/libfcoe.c +29 -11
index 58ad3c7..26381f0 100644
@@ -231,6 +231,33 @@ void fcoe_ctlr_destroy(struct fcoe_ctlr *fip)
 EXPORT_SYMBOL(fcoe_ctlr_destroy);
 
 /**
+ * fcoe_ctlr_announce() - announce new selection
+ * @fip: The FCoE controller
+ *
+ * Also sets the destination MAC for FCoE and control packets
+ */
+static void fcoe_ctlr_announce(struct fcoe_ctlr *fip)
+{
+	struct fcoe_fcf *sel = fip->sel_fcf;
+
+	if (sel && !compare_ether_addr(sel->fcf_mac, fip->dest_addr))
+		return;
+	if (!is_zero_ether_addr(fip->dest_addr)) {
+		printk(KERN_NOTICE "libfcoe: host%d: "
+		       "FIP Fibre-Channel Forwarder MAC %pM deselected\n",
+		       fip->lp->host->host_no, fip->dest_addr);
+		memset(fip->dest_addr, 0, ETH_ALEN);
+	}
+	if (sel) {
+		printk(KERN_INFO "libfcoe: host%d: FIP selected "
+		       "Fibre-Channel Forwarder MAC %pM\n",
+		       fip->lp->host->host_no, sel->fcf_mac);
+		memcpy(fip->dest_addr, sel->fcf_mac, ETH_ALEN);
+		fip->map_dest = 0;
+	}
+}
+
+/**
  * fcoe_ctlr_fcoe_size() - Return the maximum FCoE size required for VN_Port
  * @fip: The FCoE controller to get the maximum FCoE size from
  *
@@ -1420,24 +1447,15 @@ static void fcoe_ctlr_timer_work(struct work_struct *work)
 
 	if (sel != fcf) {
 		fcf = sel;		/* the old FCF may have been freed */
+		fcoe_ctlr_announce(fip);
 		if (sel) {
-			printk(KERN_INFO "libfcoe: host%d: FIP selected "
-			       "Fibre-Channel Forwarder MAC %pM\n",
-			       fip->lp->host->host_no, sel->fcf_mac);
-			memcpy(fip->dest_addr, sel->fcf_mac, ETH_ALEN);
-			fip->map_dest = 0;
 			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.	"
-			       "Starting FCF discovery.\n",
-			       fip->lp->host->host_no);
+		} else
 			reset = 1;
-		}
 	}
 
 	if (sel && !sel->fd_flags) {
#21 lore raw

PATCH 21/27 libfcoe: move some timer code to make it reusable.

In reply to Robert Love

From: Joe Eykholt <[email protected]>

Move some of the code in fcoe_ctlr_timer_work() to fcoe_ctlr_select() so that it can be shared with another function in a forthcoming patch.

---
 drivers/scsi/fcoe/libfcoe.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
drivers/scsi/fcoe/libfcoe.c +7 -3
index 26381f0..4d0be20 100644
@@ -1391,6 +1391,13 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 			best = fcf;
 	}
 	fip->sel_fcf = best;
+	if (best) {
+		fip->port_ka_time = jiffies +
+			msecs_to_jiffies(FIP_VN_KA_PERIOD);
+		fip->ctlr_ka_time = jiffies + best->fka_period;
+		if (time_before(fip->ctlr_ka_time, fip->timer.expires))
+			mod_timer(&fip->timer, fip->ctlr_ka_time);
+	}
 }
 
 /**
@@ -1449,9 +1456,6 @@ static void fcoe_ctlr_timer_work(struct work_struct *work)
 		fcf = sel;		/* the old FCF may have been freed */
 		fcoe_ctlr_announce(fip);
 		if (sel) {
-			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
#22 lore raw

PATCH 22/27 libfcoe: fix checking of conflicting fabrics in fcoe_ctlr_select()

In reply to Robert Love

From: Joe Eykholt <[email protected]>

The check for conflicting fabrics in fcoe_ctlr_select() ignores any FCFs that aren't usable. This is a minor problem now but becomes more pronounced after later patches.

---
 drivers/scsi/fcoe/libfcoe.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)
drivers/scsi/fcoe/libfcoe.c +10 -7
index 4d0be20..826c260 100644
@@ -1361,12 +1361,22 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 {
 	struct fcoe_fcf *fcf;
 	struct fcoe_fcf *best = NULL;
+	struct fcoe_fcf *first;
+
+	first = list_first_entry(&fip->fcfs, struct fcoe_fcf, list);
 
 	list_for_each_entry(fcf, &fip->fcfs, list) {
 		LIBFCOE_FIP_DBG(fip, "consider FCF for fab %16.16llx "
 				"VFID %d map %x val %d\n",
 				fcf->fabric_name, fcf->vfid,
 				fcf->fc_map, fcoe_ctlr_mtu_valid(fcf));
+		if (fcf->fabric_name != first->fabric_name ||
+		    fcf->vfid != first->vfid ||
+		    fcf->fc_map != first->fc_map) {
+			LIBFCOE_FIP_DBG(fip, "Conflicting fabric, VFID, "
+					"or FC-MAP\n");
+			return NULL;
+		}
 		if (!fcoe_ctlr_fcf_usable(fcf)) {
 			LIBFCOE_FIP_DBG(fip, "FCF for fab %16.16llx "
 					"map %x %svalid %savailable\n",
@@ -1380,13 +1390,6 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 			best = fcf;
 			continue;
 		}
-		if (fcf->fabric_name != best->fabric_name ||
-		    fcf->vfid != best->vfid ||
-		    fcf->fc_map != best->fc_map) {
-			LIBFCOE_FIP_DBG(fip, "Conflicting fabric, VFID, "
-					"or FC-MAP\n");
-			return;
-		}
 		if (fcf->pri < best->pri)
 			best = fcf;
 	}
#23 lore raw

PATCH 23/27 libfcoe: retry rejected FLOGI to another FCF if possible

In reply to Robert Love

From: Joe Eykholt <[email protected]>

Switches using multiple-FCFs may reject FLOGI in order to balance the load between multiple FCFs. Even though the FCF was available, it may have more load at the point we actually send the FLOGI.

If the FLOGI fails, select a different FCF
if possible, among those with the same priority. If no other FCF is available, just deliver the reject to libfc for retry.

---
 drivers/scsi/fcoe/libfcoe.c |  207 +++++++++++++++++++++++++++++++++++++------
 include/scsi/libfcoe.h      |    8 ++
 2 files changed, 185 insertions(+), 30 deletions(-)
drivers/scsi/fcoe/libfcoe.c +177 -30
index 826c260..c90622c 100644
@@ -54,6 +54,8 @@ MODULE_LICENSE("GPL v2");
 static void fcoe_ctlr_timeout(unsigned long);
 static void fcoe_ctlr_timer_work(struct work_struct *);
 static void fcoe_ctlr_recv_work(struct work_struct *);
+static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *);
+static void fcoe_ctlr_select(struct fcoe_ctlr *);
 
 static void fcoe_ctlr_vn_start(struct fcoe_ctlr *);
 static int fcoe_ctlr_vn_recv(struct fcoe_ctlr *, struct sk_buff *);
@@ -176,6 +178,7 @@ void fcoe_ctlr_init(struct fcoe_ctlr *fip, enum fip_state mode)
 	fip->mode = mode;
 	INIT_LIST_HEAD(&fip->fcfs);
 	mutex_init(&fip->ctlr_mutex);
+	spin_lock_init(&fip->ctlr_lock);
 	fip->flogi_oxid = FC_XID_UNKNOWN;
 	setup_timer(&fip->timer, fcoe_ctlr_timeout, (unsigned long)fip);
 	INIT_WORK(&fip->timer_work, fcoe_ctlr_timer_work);
@@ -231,17 +234,31 @@ void fcoe_ctlr_destroy(struct fcoe_ctlr *fip)
 EXPORT_SYMBOL(fcoe_ctlr_destroy);
 
 /**
- * fcoe_ctlr_announce() - announce new selection
+ * fcoe_ctlr_announce() - announce new FCF selection
  * @fip: The FCoE controller
  *
  * Also sets the destination MAC for FCoE and control packets
+ *
+ * Called with neither ctlr_mutex nor ctlr_lock held.
  */
 static void fcoe_ctlr_announce(struct fcoe_ctlr *fip)
 {
-	struct fcoe_fcf *sel = fip->sel_fcf;
+	struct fcoe_fcf *sel;
+	struct fcoe_fcf *fcf;
+
+	mutex_lock(&fip->ctlr_mutex);
+	spin_lock_bh(&fip->ctlr_lock);
+
+	kfree_skb(fip->flogi_req);
+	fip->flogi_req = NULL;
+	list_for_each_entry(fcf, &fip->fcfs, list)
+		fcf->flogi_sent = 0;
+
+	spin_unlock_bh(&fip->ctlr_lock);
+	sel = fip->sel_fcf;
 
 	if (sel && !compare_ether_addr(sel->fcf_mac, fip->dest_addr))
-		return;
+		goto unlock;
 	if (!is_zero_ether_addr(fip->dest_addr)) {
 		printk(KERN_NOTICE "libfcoe: host%d: "
 		       "FIP Fibre-Channel Forwarder MAC %pM deselected\n",
@@ -255,6 +272,8 @@ static void fcoe_ctlr_announce(struct fcoe_ctlr *fip)
 		memcpy(fip->dest_addr, sel->fcf_mac, ETH_ALEN);
 		fip->map_dest = 0;
 	}
+unlock:
+	mutex_unlock(&fip->ctlr_mutex);
 }
 
 /**
@@ -591,6 +610,9 @@ static int fcoe_ctlr_encaps(struct fcoe_ctlr *fip, struct fc_lport *lport,
  * The caller must check that the length is a multiple of 4.
  * The SKB must have enough headroom (28 bytes) and tailroom (8 bytes).
  * The the skb must also be an fc_frame.
+ *
+ * This is called from the lower-level driver with spinlocks held,
+ * so we must not take a mutex here.
  */
 int fcoe_ctlr_els_send(struct fcoe_ctlr *fip, struct fc_lport *lport,
 		       struct sk_buff *skb)
@@ -628,7 +650,15 @@ int fcoe_ctlr_els_send(struct fcoe_ctlr *fip, struct fc_lport *lport,
 	switch (op) {
 	case ELS_FLOGI:
 		op = FIP_DT_FLOGI;
-		break;
+		if (fip->mode == FIP_MODE_VN2VN)
+			break;
+		spin_lock_bh(&fip->ctlr_lock);
+		kfree_skb(fip->flogi_req);
+		fip->flogi_req = skb;
+		fip->flogi_req_send = 1;
+		spin_unlock_bh(&fip->ctlr_lock);
+		schedule_work(&fip->timer_work);
+		return -EINPROGRESS;
 	case ELS_FDISC:
 		if (ntoh24(fh->fh_s_id))
 			return 0;
@@ -1088,18 +1118,24 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip, struct sk_buff *skb)
 	els_op = *(u8 *)(fh + 1);
 
 	if ((els_dtype == FIP_DT_FLOGI || els_dtype == FIP_DT_FDISC) &&
-	    sub == FIP_SC_REP && els_op == ELS_LS_ACC &&
-	    fip->mode != FIP_MODE_VN2VN) {
-		if (!is_valid_ether_addr(granted_mac)) {
-			LIBFCOE_FIP_DBG(fip,
-				"Invalid MAC address %pM in FIP ELS\n",
-				granted_mac);
-			goto drop;
-		}
-		memcpy(fr_cb(fp)->granted_mac, granted_mac, ETH_ALEN);
+	    sub == FIP_SC_REP && fip->mode != FIP_MODE_VN2VN) {
+		if (els_op == ELS_LS_ACC) {
+			if (!is_valid_ether_addr(granted_mac)) {
+				LIBFCOE_FIP_DBG(fip,
+					"Invalid MAC address %pM in FIP ELS\n",
+					granted_mac);
+				goto drop;
+			}
+			memcpy(fr_cb(fp)->granted_mac, granted_mac, ETH_ALEN);
 
-		if (fip->flogi_oxid == ntohs(fh->fh_ox_id))
-			fip->flogi_oxid = FC_XID_UNKNOWN;
+			if (fip->flogi_oxid == ntohs(fh->fh_ox_id)) {
+				fip->flogi_oxid = FC_XID_UNKNOWN;
+				if (els_dtype == FIP_DT_FLOGI)
+					fcoe_ctlr_announce(fip);
+			}
+		} else if (els_dtype == FIP_DT_FLOGI &&
+			   !fcoe_ctlr_flogi_retry(fip))
+			goto drop;	/* retrying FLOGI so drop reject */
 	}
 
 	if ((desc_cnt == 0) || ((els_op != ELS_LS_RJT) &&
@@ -1355,12 +1391,15 @@ drop:
  *
  * If there are conflicting advertisements, no FCF can be chosen.
  *
+ * If there is already a selected FCF, this will choose a better one or
+ * an equivalent one that hasn't already been sent a FLOGI.
+ *
  * Called with lock held.
  */
 static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 {
 	struct fcoe_fcf *fcf;
-	struct fcoe_fcf *best = NULL;
+	struct fcoe_fcf *best = fip->sel_fcf;
 	struct fcoe_fcf *first;
 
 	first = list_first_entry(&fip->fcfs, struct fcoe_fcf, list);
@@ -1377,6 +1416,8 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 					"or FC-MAP\n");
 			return NULL;
 		}
+		if (fcf->flogi_sent)
+			continue;
 		if (!fcoe_ctlr_fcf_usable(fcf)) {
 			LIBFCOE_FIP_DBG(fip, "FCF for fab %16.16llx "
 					"map %x %svalid %savailable\n",
@@ -1386,11 +1427,7 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 					"" : "un");
 			continue;
 		}
-		if (!best) {
-			best = fcf;
-			continue;
-		}
-		if (fcf->pri < best->pri)
+		if (!best || fcf->pri < best->pri || best->flogi_sent)
 			best = fcf;
 	}
 	fip->sel_fcf = best;
@@ -1404,6 +1441,121 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 }
 
 /**
+ * fcoe_ctlr_flogi_send_locked() - send FIP-encapsulated FLOGI to current FCF
+ * @fip: The FCoE controller
+ *
+ * Returns non-zero error if it could not be sent.
+ *
+ * Called with ctlr_mutex and ctlr_lock held.
+ * Caller must verify that fip->sel_fcf is not NULL.
+ */
+static int fcoe_ctlr_flogi_send_locked(struct fcoe_ctlr *fip)
+{
+	struct sk_buff *skb;
+	struct sk_buff *skb_orig;
+	struct fc_frame_header *fh;
+	int error;
+
+	skb_orig = fip->flogi_req;
+	if (!skb_orig)
+		return -EINVAL;
+
+	/*
+	 * Clone and send the FLOGI request.  If clone fails, use original.
+	 */
+	skb = skb_clone(skb_orig, GFP_ATOMIC);
+	if (!skb) {
+		skb = skb_orig;
+		fip->flogi_req = NULL;
+	}
+	fh = (struct fc_frame_header *)skb->data;
+	error = fcoe_ctlr_encaps(fip, fip->lp, FIP_DT_FLOGI, skb,
+				 ntoh24(fh->fh_d_id));
+	if (error) {
+		kfree_skb(skb);
+		return error;
+	}
+	fip->send(fip, skb);
+	fip->sel_fcf->flogi_sent = 1;
+	return 0;
+}
+
+/**
+ * fcoe_ctlr_flogi_retry() - resend FLOGI request to a new FCF if possible
+ * @fip: The FCoE controller
+ *
+ * Returns non-zero error code if there's no FLOGI request to retry or
+ * no alternate FCF available.
+ */
+static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *fip)
+{
+	struct fcoe_fcf *fcf;
+	int error;
+
+	mutex_lock(&fip->ctlr_mutex);
+	spin_lock_bh(&fip->ctlr_lock);
+	LIBFCOE_FIP_DBG(fip, "re-sending FLOGI - reselect\n");
+	fcoe_ctlr_select(fip);
+	fcf = fip->sel_fcf;
+	if (!fcf || fcf->flogi_sent) {
+		kfree_skb(fip->flogi_req);
+		fip->flogi_req = NULL;
+		error = -ENOENT;
+	} else {
+		fcoe_ctlr_solicit(fip, NULL);
+		error = fcoe_ctlr_flogi_send_locked(fip);
+	}
+	spin_unlock_bh(&fip->ctlr_lock);
+	mutex_unlock(&fip->ctlr_mutex);
+	return error;
+}
+
+
+/**
+ * fcoe_ctlr_flogi_send() - Handle sending of FIP FLOGI.
+ * @fip: The FCoE controller that timed out
+ *
+ * Done here because fcoe_ctlr_els_send() can't get mutex.
+ *
+ * Called with ctlr_mutex held.  The caller must not hold ctlr_lock.
+ */
+static void fcoe_ctlr_flogi_send(struct fcoe_ctlr *fip)
+{
+	struct fcoe_fcf *fcf;
+
+	spin_lock_bh(&fip->ctlr_lock);
+	fcf = fip->sel_fcf;
+	if (!fcf || !fip->flogi_req_send)
+		goto unlock;
+
+	LIBFCOE_FIP_DBG(fip, "sending FLOGI\n");
+
+	/*
+	 * If this FLOGI is being sent due to a timeout retry
+	 * to the same FCF as before, select a different FCF if possible.
+	 */
+	if (fcf->flogi_sent) {
+		LIBFCOE_FIP_DBG(fip, "sending FLOGI - reselect\n");
+		fcoe_ctlr_select(fip);
+		fcf = fip->sel_fcf;
+		if (!fcf || fcf->flogi_sent) {
+			LIBFCOE_FIP_DBG(fip, "sending FLOGI - clearing\n");
+			list_for_each_entry(fcf, &fip->fcfs, list)
+				fcf->flogi_sent = 0;
+			fcoe_ctlr_select(fip);
+			fcf = fip->sel_fcf;
+		}
+	}
+	if (fcf) {
+		fcoe_ctlr_flogi_send_locked(fip);
+		fip->flogi_req_send = 0;
+	} else /* XXX */
+		LIBFCOE_FIP_DBG(fip, "No FCF selected - defer send\n");
+unlock:
+	spin_unlock_bh(&fip->ctlr_lock);
+}
+
+/**
  * fcoe_ctlr_timeout() - FIP timeout handler
  * @arg: The FCoE controller that timed out
  */
@@ -1455,15 +1607,10 @@ static void fcoe_ctlr_timer_work(struct work_struct *work)
 			next_timer = fip->sel_time;
 	}
 
-	if (sel != fcf) {
-		fcf = sel;		/* the old FCF may have been freed */
-		fcoe_ctlr_announce(fip);
-		if (sel) {
-			if (time_after(next_timer, fip->ctlr_ka_time))
-				next_timer = fip->ctlr_ka_time;
-		} else
-			reset = 1;
-	}
+	if (sel && fip->flogi_req_send)
+		fcoe_ctlr_flogi_send(fip);
+	else if (!sel && fcf)
+		reset = 1;
 
 	if (sel && !sel->fd_flags) {
 		if (time_after_eq(jiffies, fip->ctlr_ka_time)) {
include/scsi/libfcoe.h +8 -0
index 06f1b5a..feb6a94 100644
@@ -92,10 +92,12 @@ enum fip_state {
  * @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.
+ * @flogi_req:	   clone of FLOGI request sent
  * @rnd_state:	   state for pseudo-random number generator.
  * @port_id:	   proposed or selected local-port ID.
  * @user_mfs:	   configured maximum FC frame size, including FC header.
  * @flogi_oxid:    exchange ID of most recent fabric login.
+ * @flogi_req_send: send of FLOGI requested
  * @flogi_count:   number of FLOGI attempts in AUTO mode.
  * @map_dest:	   use the FC_MAP mode for destination MAC addresses.
  * @spma:	   supports SPMA server-provided MACs mode
@@ -106,6 +108,7 @@ enum fip_state {
  * @update_mac:    LLD-supplied function to handle changes to MAC addresses.
  * @get_src_addr:  LLD-supplied function to supply a source MAC address.
  * @ctlr_mutex:	   lock protecting this structure.
+ * @ctlr_lock:     spinlock covering flogi_req
  *
  * This structure is used by all FCoE drivers.  It contains information
  * needed by all FCoE low-level drivers (LLDs) as well as internal state
@@ -126,12 +129,14 @@ struct fcoe_ctlr {
 	struct work_struct timer_work;
 	struct work_struct recv_work;
 	struct sk_buff_head fip_recv_list;
+	struct sk_buff *flogi_req;
 
 	struct rnd_state rnd_state;
 	u32 port_id;
 
 	u16 user_mfs;
 	u16 flogi_oxid;
+	u8 flogi_req_send;
 	u8 flogi_count;
 	u8 map_dest;
 	u8 spma;
@@ -143,6 +148,7 @@ struct fcoe_ctlr {
 	void (*update_mac)(struct fc_lport *, u8 *addr);
 	u8 * (*get_src_addr)(struct fc_lport *);
 	struct mutex ctlr_mutex;
+	spinlock_t ctlr_lock;
 };
 
 /**
@@ -155,6 +161,7 @@ struct fcoe_ctlr {
  * @fcf_mac:	 Ethernet address of the FCF
  * @vfid:	 virtual fabric ID
  * @pri:	 selection priority, smaller values are better
+ * @flogi_sent:	 current FLOGI sent to this FCF
  * @flags:	 flags received from advertisement
  * @fka_period:	 keep-alive period, in jiffies
  *
@@ -176,6 +183,7 @@ struct fcoe_fcf {
 	u8 fcf_mac[ETH_ALEN];
 
 	u8 pri;
+	u8 flogi_sent;
 	u16 flags;
 	u32 fka_period;
 	u8 fd_flags:1;
#24 lore raw

PATCH 24/27 libfcoe: add debug message for FCF destination MAC

In reply to Robert Love

From: Joe Eykholt <[email protected]>

When multiple FCFs to the same fabric exist, the debug messages
all look alike. Change the message to include the MAC address.

---
 drivers/scsi/fcoe/libfcoe.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)
drivers/scsi/fcoe/libfcoe.c +10 -9
index c90622c..dcccb0b 100644
@@ -979,11 +979,9 @@ static void fcoe_ctlr_recv_adv(struct fcoe_ctlr *fip, struct sk_buff *skb)
 	}
 	mtu_valid = fcoe_ctlr_mtu_valid(fcf);
 	fcf->time = jiffies;
-	if (!found) {
-		LIBFCOE_FIP_DBG(fip, "New FCF for fab %16.16llx "
-				"map %x val %d\n",
-				fcf->fabric_name, fcf->fc_map, mtu_valid);
-	}
+	if (!found)
+		LIBFCOE_FIP_DBG(fip, "New FCF fab %16.16llx mac %pM\n",
+				fcf->fabric_name, fcf->fcf_mac);
 
 	/*
 	 * If this advertisement is not solicited and our max receive size
@@ -1405,10 +1403,12 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 	first = list_first_entry(&fip->fcfs, struct fcoe_fcf, list);
 
 	list_for_each_entry(fcf, &fip->fcfs, list) {
-		LIBFCOE_FIP_DBG(fip, "consider FCF for fab %16.16llx "
-				"VFID %d map %x val %d\n",
-				fcf->fabric_name, fcf->vfid,
-				fcf->fc_map, fcoe_ctlr_mtu_valid(fcf));
+		LIBFCOE_FIP_DBG(fip, "consider FCF fab %16.16llx "
+				"VFID %d mac %pM map %x val %d "
+				"sent %u pri %u\n",
+				fcf->fabric_name, fcf->vfid, fcf->fcf_mac,
+				fcf->fc_map, fcoe_ctlr_mtu_valid(fcf),
+				fcf->flogi_sent, fcf->pri);
 		if (fcf->fabric_name != first->fabric_name ||
 		    fcf->vfid != first->vfid ||
 		    fcf->fc_map != first->fc_map) {
@@ -1432,6 +1432,7 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 	}
 	fip->sel_fcf = best;
 	if (best) {
+		LIBFCOE_FIP_DBG(fip, "using FCF mac %pM\n", best->fcf_mac);
 		fip->port_ka_time = jiffies +
 			msecs_to_jiffies(FIP_VN_KA_PERIOD);
 		fip->ctlr_ka_time = jiffies + best->fka_period;
#25 lore raw

PATCH 25/27 libfcoe: reorder FCF list to put latest advertiser first

In reply to Robert Love

From: Joe Eykholt <[email protected]>

When there are several FCFs to choose from, the one most likely to accept a FLOGI on certian switches is the one that last answered a multicast solicit.

So, when receiving an advertisement, move the FCF to the front of the list so that it gets chosen first among those with the same priority.

Without this, more FLOGIs need to be sent in a test with multiple FCFs and a switch in NPV mode, but it still eventually finds one that accepts the FLOGI.

---
 drivers/scsi/fcoe/libfcoe.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
drivers/scsi/fcoe/libfcoe.c +11 -0
index dcccb0b..1a0bb23 100644
@@ -1000,6 +1000,17 @@ static void fcoe_ctlr_recv_adv(struct fcoe_ctlr *fip, struct sk_buff *skb)
 		fcoe_ctlr_solicit(fip, NULL);
 
 	/*
+	 * Put this FCF at the head of the list for priority among equals.
+	 * This helps in the case of an NPV switch which insists we use
+	 * the FCF that answers multicast solicitations, not the others that
+	 * are sending periodic multicast advertisements.
+	 */
+	if (mtu_valid) {
+		list_del(&fcf->list);
+		list_add(&fcf->list, &fip->fcfs);
+	}
+
+	/*
 	 * If this is the first validated FCF, note the time and
 	 * set a timer to trigger selection.
 	 */
#26 lore raw

PATCH 26/27 libfcoe: change fip_select to return new FCF

In reply to Robert Love

From: Joe Eykholt <[email protected]>

Neaten several calls to fip_select() by having it return the pointer to the new FCF.

---
 drivers/scsi/fcoe/libfcoe.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)
drivers/scsi/fcoe/libfcoe.c +8 -10
index 1a0bb23..625c6be 100644
@@ -55,7 +55,6 @@ static void fcoe_ctlr_timeout(unsigned long);
 static void fcoe_ctlr_timer_work(struct work_struct *);
 static void fcoe_ctlr_recv_work(struct work_struct *);
 static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *);
-static void fcoe_ctlr_select(struct fcoe_ctlr *);
 
 static void fcoe_ctlr_vn_start(struct fcoe_ctlr *);
 static int fcoe_ctlr_vn_recv(struct fcoe_ctlr *, struct sk_buff *);
@@ -1398,6 +1397,8 @@ drop:
  * fcoe_ctlr_select() - Select the best FCF (if possible)
  * @fip: The FCoE controller
  *
+ * Returns the selected FCF, or NULL if none are usable.
+ *
  * If there are conflicting advertisements, no FCF can be chosen.
  *
  * If there is already a selected FCF, this will choose a better one or
@@ -1405,7 +1406,7 @@ drop:
  *
  * Called with lock held.
  */
-static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
+static struct fcoe_fcf *fcoe_ctlr_select(struct fcoe_ctlr *fip)
 {
 	struct fcoe_fcf *fcf;
 	struct fcoe_fcf *best = fip->sel_fcf;
@@ -1450,6 +1451,7 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
 		if (time_before(fip->ctlr_ka_time, fip->timer.expires))
 			mod_timer(&fip->timer, fip->ctlr_ka_time);
 	}
+	return best;
 }
 
 /**
@@ -1507,8 +1509,7 @@ static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *fip)
 	mutex_lock(&fip->ctlr_mutex);
 	spin_lock_bh(&fip->ctlr_lock);
 	LIBFCOE_FIP_DBG(fip, "re-sending FLOGI - reselect\n");
-	fcoe_ctlr_select(fip);
-	fcf = fip->sel_fcf;
+	fcf = fcoe_ctlr_select(fip);
 	if (!fcf || fcf->flogi_sent) {
 		kfree_skb(fip->flogi_req);
 		fip->flogi_req = NULL;
@@ -1548,14 +1549,12 @@ static void fcoe_ctlr_flogi_send(struct fcoe_ctlr *fip)
 	 */
 	if (fcf->flogi_sent) {
 		LIBFCOE_FIP_DBG(fip, "sending FLOGI - reselect\n");
-		fcoe_ctlr_select(fip);
-		fcf = fip->sel_fcf;
+		fcf = fcoe_ctlr_select(fip);
 		if (!fcf || fcf->flogi_sent) {
 			LIBFCOE_FIP_DBG(fip, "sending FLOGI - clearing\n");
 			list_for_each_entry(fcf, &fip->fcfs, list)
 				fcf->flogi_sent = 0;
-			fcoe_ctlr_select(fip);
-			fcf = fip->sel_fcf;
+			fcf = fcoe_ctlr_select(fip);
 		}
 	}
 	if (fcf) {
@@ -1612,8 +1611,7 @@ static void fcoe_ctlr_timer_work(struct work_struct *work)
 	sel = fip->sel_fcf;
 	if (!sel && fip->sel_time) {
 		if (time_after_eq(jiffies, fip->sel_time)) {
-			fcoe_ctlr_select(fip);
-			sel = fip->sel_fcf;
+			sel = fcoe_ctlr_select(fip);
 			fip->sel_time = 0;
 		} else if (time_after(next_timer, fip->sel_time))
 			next_timer = fip->sel_time;
#27 lore raw

PATCH 27/27 libfc: fix statistics for FCP input/output megabytes

In reply to Robert Love

From: Joe Eykholt <[email protected]>

The statistics for InputMegabytes and OutputMegabytes are misnamed. They're accumulating bytes, not megabytes.

The statistic returned via /sys must be in megabytes, however, which is what the HBA-API wants. The FCP code needs to accumulate it in bytes and then divide by 1,000,000 (not 2^20) before it presented via sysfs.

This affects fcoe.ko only, not fnic. The fnic driver correctly by accumulating bytes and then converts to megabytes.

I checked that libhbalinux is using the /sys file directly without conversion.

BTW, qla2xxx does divide by 2^20, which I'm not fixing here.

---
 drivers/scsi/libfc/fc_fcp.c   |    4 ++--
 drivers/scsi/libfc/fc_lport.c |    8 ++++++--
 include/scsi/libfc.h          |    8 ++++----
 3 files changed, 12 insertions(+), 8 deletions(-)
drivers/scsi/libfc/fc_fcp.c +2 -2
index 12b639d..72013d4 100644
@@ -1860,11 +1860,11 @@ int fc_queuecommand(struct scsi_cmnd *sc_cmd, void (*done)(struct scsi_cmnd *))
 	if (sc_cmd->sc_data_direction == DMA_FROM_DEVICE) {
 		fsp->req_flags = FC_SRB_READ;
 		stats->InputRequests++;
-		stats->InputMegabytes += fsp->data_len;
+		stats->InputBytes += fsp->data_len;
 	} else if (sc_cmd->sc_data_direction == DMA_TO_DEVICE) {
 		fsp->req_flags = FC_SRB_WRITE;
 		stats->OutputRequests++;
-		stats->OutputMegabytes += fsp->data_len;
+		stats->OutputBytes += fsp->data_len;
 	} else {
 		fsp->req_flags = 0;
 		stats->ControlRequests++;
drivers/scsi/libfc/fc_lport.c +6 -2
index b91a11e..c5a10f9 100644
@@ -288,6 +288,8 @@ struct fc_host_statistics *fc_get_host_stats(struct Scsi_Host *shost)
 	struct fc_lport *lport = shost_priv(shost);
 	struct timespec v0, v1;
 	unsigned int cpu;
+	u64 fcp_in_bytes = 0;
+	u64 fcp_out_bytes = 0;
 
 	fcoe_stats = &lport->host_stats;
 	memset(fcoe_stats, 0, sizeof(struct fc_host_statistics));
@@ -310,10 +312,12 @@ struct fc_host_statistics *fc_get_host_stats(struct Scsi_Host *shost)
 		fcoe_stats->fcp_input_requests += stats->InputRequests;
 		fcoe_stats->fcp_output_requests += stats->OutputRequests;
 		fcoe_stats->fcp_control_requests += stats->ControlRequests;
-		fcoe_stats->fcp_input_megabytes += stats->InputMegabytes;
-		fcoe_stats->fcp_output_megabytes += stats->OutputMegabytes;
+		fcp_in_bytes += stats->InputBytes;
+		fcp_out_bytes += stats->OutputBytes;
 		fcoe_stats->link_failure_count += stats->LinkFailureCount;
 	}
+	fcoe_stats->fcp_input_megabytes = div_u64(fcp_in_bytes, 1000000);
+	fcoe_stats->fcp_output_megabytes = div_u64(fcp_out_bytes, 1000000);
 	fcoe_stats->lip_count = -1;
 	fcoe_stats->nos_count = -1;
 	fcoe_stats->loss_of_sync_count = -1;
include/scsi/libfc.h +4 -4
index 1463660..8d719e8 100644
@@ -221,8 +221,8 @@ struct fc_rport_priv {
  * @InputRequests:         Number of input requests
  * @OutputRequests:        Number of output requests
  * @ControlRequests:       Number of control requests
- * @InputMegabytes:        Number of received megabytes
- * @OutputMegabytes:       Number of transmitted megabytes
+ * @InputBytes:            Number of received bytes
+ * @OutputBytes:           Number of transmitted bytes
  * @VLinkFailureCount:     Number of virtual link failures
  * @MissDiscAdvCount:      Number of missing FIP discovery advertisement
  */
@@ -241,8 +241,8 @@ struct fcoe_dev_stats {
 	u64		InputRequests;
 	u64		OutputRequests;
 	u64		ControlRequests;
-	u64		InputMegabytes;
-	u64		OutputMegabytes;
+	u64		InputBytes;
+	u64		OutputBytes;
 	u64		VLinkFailureCount;
 	u64		MissDiscAdvCount;
 };
#28 lore raw

PATCH 1/1 scsi: fix libfc sparse warnings

In reply to Robert Love

If not already included elsewhere, this could be added.

---
From: Randy Dunlap <[email protected]>

Fix sparse warning for non-ANSI function declaration. Declare workqueue structs as static.

---
 drivers/scsi/libfc/fc_exch.c  |    6 +++---
 drivers/scsi/libfc/fc_fcp.c   |    6 +++---
 drivers/scsi/libfc/fc_rport.c |    6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)
drivers/scsi/libfc/fc_exch.c +3 -3
@@ -38,7 +38,7 @@ u16	fc_cpu_mask;		/* cpu mask for possib
 EXPORT_SYMBOL(fc_cpu_mask);
 static u16	fc_cpu_order;	/* 2's power to represent total possible cpus */
 static struct kmem_cache *fc_em_cachep;	       /* cache for exchanges */
-struct workqueue_struct *fc_exch_workqueue;
+static struct workqueue_struct *fc_exch_workqueue;
 
 /*
  * Structure and function definitions for managing Fibre Channel Exchanges
@@ -2325,7 +2325,7 @@ EXPORT_SYMBOL(fc_exch_init);
 /**
  * fc_setup_exch_mgr() - Setup an exchange manager
  */
-int fc_setup_exch_mgr()
+int fc_setup_exch_mgr(void)
 {
 	fc_em_cachep = kmem_cache_create("libfc_em", sizeof(struct fc_exch),
 					 0, SLAB_HWCACHE_ALIGN, NULL);
@@ -2363,7 +2363,7 @@ int fc_setup_exch_mgr()
 /**
  * fc_destroy_exch_mgr() - Destroy an exchange manager
  */
-void fc_destroy_exch_mgr()
+void fc_destroy_exch_mgr(void)
 {
 	destroy_workqueue(fc_exch_workqueue);
 	kmem_cache_destroy(fc_em_cachep);
drivers/scsi/libfc/fc_fcp.c +3 -3
@@ -42,7 +42,7 @@
 
 #include "fc_libfc.h"
 
-struct kmem_cache *scsi_pkt_cachep;
+static struct kmem_cache *scsi_pkt_cachep;
 
 /* SRB state definitions */
 #define FC_SRB_FREE		0		/* cmd is free */
@@ -2196,7 +2196,7 @@ void fc_fcp_destroy(struct fc_lport *lpo
 }
 EXPORT_SYMBOL(fc_fcp_destroy);
 
-int fc_setup_fcp()
+int fc_setup_fcp(void)
 {
 	int rc = 0;
 
@@ -2212,7 +2212,7 @@ int fc_setup_fcp()
 	return rc;
 }
 
-void fc_destroy_fcp()
+void fc_destroy_fcp(void)
 {
 	if (scsi_pkt_cachep)
 		kmem_cache_destroy(scsi_pkt_cachep);
drivers/scsi/libfc/fc_rport.c +3 -3
@@ -58,7 +58,7 @@
 
 #include "fc_libfc.h"
 
-struct workqueue_struct *rport_event_queue;
+static struct workqueue_struct *rport_event_queue;
 
 static void fc_rport_enter_flogi(struct fc_rport_priv *);
 static void fc_rport_enter_plogi(struct fc_rport_priv *);
@@ -1888,7 +1888,7 @@ EXPORT_SYMBOL(fc_rport_init);
 /**
  * fc_setup_rport() - Initialize the rport_event_queue
  */
-int fc_setup_rport()
+int fc_setup_rport(void)
 {
 	rport_event_queue = create_singlethread_workqueue("fc_rport_eq");
 	if (!rport_event_queue)
@@ -1899,7 +1899,7 @@ int fc_setup_rport()
 /**
  * fc_destroy_rport() - Destroy the rport_event_queue
  */
-void fc_destroy_rport()
+void fc_destroy_rport(void)
 {
 	destroy_workqueue(rport_event_queue);
 }