[PATCH 00/10] libfc, libfcoe and fcoe updates for scsi-misc

[PATCH 00/10] libfc, libfcoe and fcoe updates for scsi-misc

[Robert Love]

This series consists of updates for the libfc, libfcoe
and fcoe modules. There are no features in this series,
only bug fixes.

It would be great if this code could be merged for the
3.1 merge window.

---

Hillf Danton (4):
      libfc: use FC_MAX_ERROR_CNT
      libfc: release exchg cache
      libfc: two minor changes in comments
      libfc: cleanup sending SRR request

Neerav Parikh (1):
      libfc: Remove the reference to FCP packet from scsi_cmnd in case of error

Vasu Dev (4):
      libfc, fcoe: ignore rx frame with wrong xid info
      libfc: fix warn on in lport retry
      fcoe: add fip retry to avoid missing critical keep alive
      fcoe: cleanup cpu selection for incoming requests

Yi Zou (1):
      fcoe: remove unused ptype field in fcoe_rcv_info


 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    1 -
 drivers/scsi/fcoe/fcoe.c          |   69 +++++++++++++++++--------------------
 drivers/scsi/libfc/fc_exch.c      |    9 +++--
 drivers/scsi/libfc/fc_fcp.c       |    9 ++---
 drivers/scsi/libfc/fc_lport.c     |    1 -
 include/scsi/fc_frame.h           |    1 -
 6 files changed, 41 insertions(+), 49 deletions(-)

-- 
Thanks, //Rob

[PATCH 01/10] fcoe: remove unused ptype field in fcoe_rcv_info

[Robert Love]

From: Yi Zou <yi.zou@intel.com>

There is no need to cache the ptype in fcoe_rcv_info struct as it is never
used anywhere.

Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    1 -
 drivers/scsi/fcoe/fcoe.c          |    1 -
 include/scsi/fc_frame.h           |    1 -
 3 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 65561fc..39c1602 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -418,7 +418,6 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev,
 
 	fr = fcoe_dev_from_skb(skb);
 	fr->fr_dev = lport;
-	fr->ptype = ptype;
 
 	bg = &bnx2fc_global;
 	spin_lock_bh(&bg->fcoe_rx_list.lock);
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 204fa8d..f7547fb 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1350,7 +1350,6 @@ int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
 
 	fr = fcoe_dev_from_skb(skb);
 	fr->fr_dev = lport;
-	fr->ptype = ptype;
 
 	/*
 	 * In case the incoming frame's exchange is originated from
diff --git a/include/scsi/fc_frame.h b/include/scsi/fc_frame.h
index 4ad0204..8225d80 100644
--- a/include/scsi/fc_frame.h
+++ b/include/scsi/fc_frame.h
@@ -78,7 +78,6 @@ struct fc_frame {
 };
 
 struct fcoe_rcv_info {
-	struct packet_type  *ptype;
 	struct fc_lport	*fr_dev;	/* transport layer private pointer */
 	struct fc_seq	*fr_seq;	/* for use with exchange manager */
 	struct fc_fcp_pkt *fr_fsp;	/* for the corresponding fcp I/O */

[PATCH 02/10] libfc: use FC_MAX_ERROR_CNT

[Robert Love]

From: Hillf Danton <dhillf@gmail.com>

Though defined, FC_MAX_ERROR_CNT is not used. It is used now for CRC error in
the path of receiving FCP frame.

Signed-off-by: Hillf Danton <dhillf@gmail.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_fcp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 9cd2149..41f7070 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -498,7 +498,7 @@ crc_err:
 			stats = per_cpu_ptr(lport->dev_stats, get_cpu());
 			stats->ErrorFrames++;
 			/* per cpu count, not total count, but OK for limit */
-			if (stats->InvalidCRCCount++ < 5)
+			if (stats->InvalidCRCCount++ < FC_MAX_ERROR_CNT)
 				printk(KERN_WARNING "libfc: CRC error on data "
 				       "frame for port (%6.6x)\n",
 				       lport->port_id);

[PATCH 03/10] libfc: release exchg cache

[Robert Love]

From: Hillf Danton <dhillf@gmail.com>

If fail to create workqueue, the newly created cache for exchg has to be
released.

Signed-off-by: Hillf Danton <dhillf@gmail.com>
Reviewed-by: Vasu Dev <vasu.dev@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index f5a0665..7baf223 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -2465,8 +2465,11 @@ int fc_setup_exch_mgr(void)
 
 	fc_exch_workqueue = create_singlethread_workqueue("fc_exch_workqueue");
 	if (!fc_exch_workqueue)
-		return -ENOMEM;
+		goto err;
 	return 0;
+err:
+	kmem_cache_destroy(fc_em_cachep);
+	return -ENOMEM;
 }
 
 /**

[PATCH 04/10] libfc, fcoe: ignore rx frame with wrong xid info

[Robert Love]

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

Drop the rx frame having xid with wrong cpu info
or received with xid  not matching to our xid.

Not dropping such frame is causing panic as
that causes accessing data struct beyond their
bounds.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe.c     |    4 ++++
 drivers/scsi/libfc/fc_exch.c |    4 +---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index f7547fb..945df21 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1373,6 +1373,10 @@ int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
 		} else
 			cpu = smp_processor_id();
 	}
+
+	if (cpu >= nr_cpu_ids)
+		goto err;
+
 	fps = &per_cpu(fcoe_percpu, cpu);
 	spin_lock_bh(&fps->fcoe_rx_list.lock);
 	if (unlikely(!fps->thread)) {
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 7baf223..01ff082 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -802,10 +802,8 @@ static struct fc_exch *fc_exch_find(struct fc_exch_mgr *mp, u16 xid)
 		pool = per_cpu_ptr(mp->pool, xid & fc_cpu_mask);
 		spin_lock_bh(&pool->lock);
 		ep = fc_exch_ptr_get(pool, (xid - mp->min_xid) >> fc_cpu_order);
-		if (ep) {
+		if (ep && ep->xid == xid)
 			fc_exch_hold(ep);
-			WARN_ON(ep->xid != xid);
-		}
 		spin_unlock_bh(&pool->lock);
 	}
 	return ep;

[PATCH 05/10] libfc: two minor changes in comments

[Robert Love]

From: Hillf Danton <dhillf@gmail.com>

One change is to cleanup typo in comment for fc_fcp_recv(), another corrects
the misleading comment for fc_fcp_abts_resp().

[ Patch reworked by Robert Love due to invalid patch format ]

Signed-off-by: Hillf Danton <dhillf@gmail.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_fcp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 41f7070..57311d0 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -690,7 +690,7 @@ static int fc_fcp_send_data(struct fc_fcp_pkt *fsp, struct fc_seq *seq,
 }
 
 /**
- * fc_fcp_abts_resp() - Send an ABTS response
+ * fc_fcp_abts_resp() - Receive an ABTS response
  * @fsp: The FCP packet that is being aborted
  * @fp:	 The response frame
  */
@@ -730,7 +730,7 @@ static void fc_fcp_abts_resp(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 }
 
 /**
- * fc_fcp_recv() - Reveive an FCP frame
+ * fc_fcp_recv() - Receive an FCP frame
  * @seq: The sequence the frame is on
  * @fp:	 The received frame
  * @arg: The related FCP packet

[PATCH 06/10] libfc: cleanup sending SRR request

[Robert Love]

From: Hillf Danton <dhillf@gmail.com>

The variable on stack, namely cdb_op, is not used but removed.

[ Patch reworked by Robert Love due to invalid patch format ]

Signed-off-by: Hillf Danton <dhillf@gmail.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_fcp.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 57311d0..14dd121 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -1645,12 +1645,10 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset)
 	struct fc_seq *seq;
 	struct fcp_srr *srr;
 	struct fc_frame *fp;
-	u8 cdb_op;
 	unsigned int rec_tov;
 
 	rport = fsp->rport;
 	rpriv = rport->dd_data;
-	cdb_op = fsp->cdb_cmd.fc_cdb[0];
 
 	if (!(rpriv->flags & FC_RP_FLAGS_RETRY) ||
 	    rpriv->rp_state != RPORT_ST_READY)

[PATCH 07/10] libfc: Remove the reference to FCP packet from scsi_cmnd in case of error

[Robert Love]

From: Neerav Parikh <Neerav.Parikh@intel.com>

fc_queuecommand() allocates an FCP packet for each SCSI command and sends
it out on the wire. In the process it stores the reference to the FCP packet
in the scsi_cmnd structure.
Now, in case under stress testing the libfc exchange layer runs out of
exchanges the fc_queuecommand() may not be able to send out commands out on
the wire. In such a scenario if there is an error in sending the FCP packet
out the wire; fc_queuecommand() deletes the FCP packet from internal queue,
releases the FCP packet and returns a SCSI_MLQUEUE_HOST_BUSY status to the
scsi-ml. But, the reference to the FCP packet set in the scsi_cmnd is not
removed from the scsi_cmnd in this code path.

This might lead to a crash under stress testing where the scsi_cmnd failed by
fc_queuecommand() comes up to fc_eh_abort() via scsi eh thread. fc_eh_abort()
will get reference to the FCP packet to be aborted from the scsi_cmnd for
further FCP abort related processing and then try to release the FCP packet
that has already been released.

This patch removes the FCP packet reference from the scsi_cmnd before returning
back from fc_queuecommand() in case of an error in sending out the FCP packet.

Signed-off-by: Neerav Parikh <Neerav.Parikh@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_fcp.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 14dd121..afb63c8 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -1084,6 +1084,7 @@ static int fc_fcp_pkt_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp)
 	rc = lport->tt.fcp_cmd_send(lport, fsp, fc_fcp_recv);
 	if (unlikely(rc)) {
 		spin_lock_irqsave(&si->scsi_queue_lock, flags);
+		fsp->cmd->SCp.ptr = NULL;
 		list_del(&fsp->list);
 		spin_unlock_irqrestore(&si->scsi_queue_lock, flags);
 	}

[PATCH 08/10] libfc: fix warn on in lport retry

[Robert Love]

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

The lport retry timer hits warn on in case
it has become ready in response from fip
login from fcoe_ctlr_flogi_send(), this is
possible but safe code path, therefore
removing this warn on.

Jun 22 03:16:30 10.0.16.6 [488198.316517] host3: Assigned Port ID 180f02
Jun 22 03:16:32 10.0.16.6 [488200.091561] ------------[ cut here ]------------
Jun 22 03:16:32 10.0.16.6 [488200.091586] WARNING: at
drivers/scsi/libfc/fc_lport.c:1355 fc_lport_timeout+0xd9/0xe0 [libfc]()

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_lport.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index e008b16..e55ed9c 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -1352,7 +1352,6 @@ static void fc_lport_timeout(struct work_struct *work)
 		WARN_ON(1);
 		break;
 	case LPORT_ST_READY:
-		WARN_ON(1);
 		break;
 	case LPORT_ST_RESET:
 		break;

[PATCH 09/10] fcoe: add fip retry to avoid missing critical keep alive

[Robert Love]

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

Use pending queue to retry FIP frame in case its tx
fails and use common pending queue for both fcoe
and fip frames using fcoe_port_send.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 945df21..528b86b 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -487,6 +487,19 @@ static int fcoe_fip_recv(struct sk_buff *skb, struct net_device *netdev,
 }
 
 /**
+ * fcoe_port_send() - Send an Ethernet-encapsulated FIP/FCoE frame
+ * @port: The FCoE port
+ * @skb: The FIP/FCoE packet to be sent
+ */
+static void fcoe_port_send(struct fcoe_port *port, struct sk_buff *skb)
+{
+	if (port->fcoe_pending_queue.qlen)
+		fcoe_check_wait_queue(port->lport, skb);
+	else if (fcoe_start_io(skb))
+		fcoe_check_wait_queue(port->lport, skb);
+}
+
+/**
  * fcoe_fip_send() - Send an Ethernet-encapsulated FIP frame
  * @fip: The FCoE controller
  * @skb: The FIP packet to be sent
@@ -494,7 +507,7 @@ static int fcoe_fip_recv(struct sk_buff *skb, struct net_device *netdev,
 static void fcoe_fip_send(struct fcoe_ctlr *fip, struct sk_buff *skb)
 {
 	skb->dev = fcoe_from_ctlr(fip)->netdev;
-	dev_queue_xmit(skb);
+	fcoe_port_send(lport_priv(fip->lp), skb);
 }
 
 /**
@@ -1575,11 +1588,7 @@ int fcoe_xmit(struct fc_lport *lport, struct fc_frame *fp)
 
 	/* send down to lld */
 	fr_dev(fp) = lport;
-	if (port->fcoe_pending_queue.qlen)
-		fcoe_check_wait_queue(lport, skb);
-	else if (fcoe_start_io(skb))
-		fcoe_check_wait_queue(lport, skb);
-
+	fcoe_port_send(port, skb);
 	return 0;
 }
 

[PATCH 10/10] fcoe: cleanup cpu selection for incoming requests

[Robert Love]

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

Cleanup to:-

- have selection for all types of frames, not just FCP.
- remove redundant cpu_online check once fcoe_select_cpu called
  as this is not required since later code flow check for offlined
  cpu.
- Simplify fcoe_select_cpu() by removing unnecessary checks to
  skip curr_cpu, this also fixes possibly infinite loop in case
  of curr_cpu is the only cpu while iterating in the loop.

This cleanup mainly applies to target as incoming request are
mostly for target, therefore Kiran has verified the patch
with target also.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Tested-by: Kiran Patil <kiran.patil@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe.c |   43 +++++++++++++------------------------------
 1 files changed, 13 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 528b86b..ba710e3 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1270,30 +1270,20 @@ static int fcoe_cpu_callback(struct notifier_block *nfb,
 /**
  * fcoe_select_cpu() - Selects CPU to handle post-processing of incoming
  *			command.
- * @curr_cpu:   CPU which received request
  *
- * This routine selects next CPU based on cpumask.
+ * This routine selects next CPU based on cpumask to distribute
+ * incoming requests in round robin.
  *
- * Returns: int (CPU number). Caller to verify if returned CPU is online or not.
+ * Returns: int CPU number
  */
-static unsigned int fcoe_select_cpu(unsigned int curr_cpu)
+static inline unsigned int fcoe_select_cpu(void)
 {
 	static unsigned int selected_cpu;
 
-	if (num_online_cpus() == 1)
-		return curr_cpu;
-	/*
-	 * Doing following check, to skip "curr_cpu (smp_processor_id)"
-	 * from selection of CPU is intentional. This is to avoid same CPU
-	 * doing post-processing of command. "curr_cpu" to just receive
-	 * incoming request in case where rx_id is UNKNOWN and all other
-	 * CPU to actually process the command(s)
-	 */
-	do {
-		selected_cpu = cpumask_next(selected_cpu, cpu_online_mask);
-		if (selected_cpu >= nr_cpu_ids)
-			selected_cpu = cpumask_first(cpu_online_mask);
-	} while (selected_cpu == curr_cpu);
+	selected_cpu = cpumask_next(selected_cpu, cpu_online_mask);
+	if (selected_cpu >= nr_cpu_ids)
+		selected_cpu = cpumask_first(cpu_online_mask);
+
 	return selected_cpu;
 }
 
@@ -1368,23 +1358,16 @@ int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
 	 * In case the incoming frame's exchange is originated from
 	 * the initiator, then received frame's exchange id is ANDed
 	 * with fc_cpu_mask bits to get the same cpu on which exchange
-	 * was originated, otherwise just use the current cpu.
+	 * was originated, otherwise select cpu using rx exchange id
+	 * or fcoe_select_cpu().
 	 */
 	if (ntoh24(fh->fh_f_ctl) & FC_FC_EX_CTX)
 		cpu = ntohs(fh->fh_ox_id) & fc_cpu_mask;
 	else {
-		cpu = smp_processor_id();
-
-		if ((fh->fh_type == FC_TYPE_FCP) &&
-		    (ntohs(fh->fh_rx_id) == FC_XID_UNKNOWN)) {
-			do {
-				cpu = fcoe_select_cpu(cpu);
-			} while (!cpu_online(cpu));
-		} else  if ((fh->fh_type == FC_TYPE_FCP) &&
-			    (ntohs(fh->fh_rx_id) != FC_XID_UNKNOWN)) {
+		if (ntohs(fh->fh_rx_id) == FC_XID_UNKNOWN)
+			cpu = fcoe_select_cpu();
+		else
 			cpu = ntohs(fh->fh_rx_id) & fc_cpu_mask;
-		} else
-			cpu = smp_processor_id();
 	}
 
 	if (cpu >= nr_cpu_ids)