[PATCH 0/8] Open-FCoE fixes for 2.6.30 RC

[PATCH 0/8] Open-FCoE fixes for 2.6.30 RC

[Robert Love]

The following series implements general fixes to libfc, libfcoe and fcoe. 

Note: One patch adds the FIP ehtertype to if_ether.h. As discussed on netdev,
this change was pre-acked and it was requested that the change go through
the SCSI tree.

---

Chris Leech (1):
      fcoe: use ETH_P_FIP for skb->protocol of FIP frames

Joe Eykholt (2):
      net, libfcoe: Add the FCoE Initialization Protocol ethertype
      libfcoe: fip: fix non-FIP-mode FLOGI state after reset.

Mike Christie (1):
      libfc: use DID_ERROR when we have internall aborted command

Steve Ma (1):
      libfc: Check if exchange is completed when receiving a sequence

Vasu Dev (3):
      fcoe: removes reserving memory for vlan_ethdr on tx path
      fcoe: removes fcoe_watchdog
      fcoe: reduces lock cost when adding a new skb to fcoe_pending_queue


 drivers/scsi/fcoe/fcoe.c     |   90 +++++++++++++++---------------------------
 drivers/scsi/fcoe/fcoe.h     |    1 
 drivers/scsi/fcoe/libfcoe.c  |   21 ++++++----
 drivers/scsi/libfc/fc_exch.c |    4 ++
 drivers/scsi/libfc/fc_fcp.c  |    2 -
 include/linux/if_ether.h     |    1 
 include/scsi/fc/fc_fip.h     |    7 ---
 7 files changed, 53 insertions(+), 73 deletions(-)

-- 
//Rob

[PATCH 1/8] libfcoe: fip: fix non-FIP-mode FLOGI state after reset.

[Robert Love]

From: Joe Eykholt <jeykholt@cisco.com>

When a reset is sent using fcoeadm on a non-FIP mode NIC,
there's no link flap, so the fcoe_ctlr stays in non-FIP mode.

In that case, FIP wasn't setting the flogi_oxid or map_dest flag,
causing the FLOGI to be sent with the both wrong source MAC and
the wrong destination MAC address, causing it to fail.

This leads to a non-functioning HBA until a link flap or
instance delete/create.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
---

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

diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c
index 62ba0f3..a7ecafb 100644
--- a/drivers/scsi/fcoe/libfcoe.c
+++ b/drivers/scsi/fcoe/libfcoe.c
@@ -447,14 +447,10 @@ int fcoe_ctlr_els_send(struct fcoe_ctlr *fip, struct sk_buff *skb)
 	u16 old_xid;
 	u8 op;
 
-	if (fip->state == FIP_ST_NON_FIP)
-		return 0;
-
 	fh = (struct fc_frame_header *)skb->data;
 	op = *(u8 *)(fh + 1);
 
-	switch (op) {
-	case ELS_FLOGI:
+	if (op == ELS_FLOGI) {
 		old_xid = fip->flogi_oxid;
 		fip->flogi_oxid = ntohs(fh->fh_ox_id);
 		if (fip->state == FIP_ST_AUTO) {
@@ -466,6 +462,15 @@ int fcoe_ctlr_els_send(struct fcoe_ctlr *fip, struct sk_buff *skb)
 			fip->map_dest = 1;
 			return 0;
 		}
+		if (fip->state == FIP_ST_NON_FIP)
+			fip->map_dest = 1;
+	}
+
+	if (fip->state == FIP_ST_NON_FIP)
+		return 0;
+
+	switch (op) {
+	case ELS_FLOGI:
 		op = FIP_DT_FLOGI;
 		break;
 	case ELS_FDISC:

[PATCH 2/8] fcoe: use ETH_P_FIP for skb->protocol of FIP frames

[Robert Love]

From: Chris Leech <christopher.leech@intel.com>

FIP frames should leave the fcoe layer with skb->protocol set to
ETH_P_FIP, not ETH_P_802_3.

Signed-off-by: Chris Leech <christopher.leech@intel.com>
---

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

diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c
index a7ecafb..9294118 100644
--- a/drivers/scsi/fcoe/libfcoe.c
+++ b/drivers/scsi/fcoe/libfcoe.c
@@ -213,7 +213,7 @@ static void fcoe_ctlr_solicit(struct fcoe_ctlr *fip, struct fcoe_fcf *fcf)
 	sol->desc.size.fd_size = htons(fcoe_size);
 
 	skb_put(skb, sizeof(*sol));
-	skb->protocol = htons(ETH_P_802_3);
+	skb->protocol = htons(ETH_P_FIP);
 	skb_reset_mac_header(skb);
 	skb_reset_network_header(skb);
 	fip->send(fip, skb);
@@ -365,7 +365,7 @@ static void fcoe_ctlr_send_keep_alive(struct fcoe_ctlr *fip, int ports, u8 *sa)
 	}
 
 	skb_put(skb, len);
-	skb->protocol = htons(ETH_P_802_3);
+	skb->protocol = htons(ETH_P_FIP);
 	skb_reset_mac_header(skb);
 	skb_reset_network_header(skb);
 	fip->send(fip, skb);
@@ -424,7 +424,7 @@ static int fcoe_ctlr_encaps(struct fcoe_ctlr *fip,
 	if (dtype != ELS_FLOGI)
 		memcpy(mac->fd_mac, fip->data_src_addr, ETH_ALEN);
 
-	skb->protocol = htons(ETH_P_802_3);
+	skb->protocol = htons(ETH_P_FIP);
 	skb_reset_mac_header(skb);
 	skb_reset_network_header(skb);
 	return 0;

[PATCH 3/8] libfc: use DID_ERROR when we have internall aborted command

[Robert Love]

From: Mike Christie <michaelc@cs.wisc.edu>

If we aborted a command, because it timed out we should not use
DID_ABORT. It will fail the command right away back to the upper
layer. We want to use something that indicated that the problem
did not complete normally, but it was not a fatal problem.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---

 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 521f996..ad8b747 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -1896,7 +1896,7 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp)
 		sc_cmd->result = (DID_ERROR << 16) | fsp->cdb_status;
 		break;
 	case FC_CMD_ABORTED:
-		sc_cmd->result = (DID_ABORT << 16) | fsp->io_status;
+		sc_cmd->result = (DID_ERROR << 16) | fsp->io_status;
 		break;
 	case FC_CMD_TIME_OUT:
 		sc_cmd->result = (DID_BUS_BUSY << 16) | fsp->io_status;

[PATCH 4/8] libfc: Check if exchange is completed when receiving a sequence

[Robert Love]

From: Steve Ma <steve.ma@intel.com>

When a sequence is received in response to an exchange we issued previously,
we should check to see if the exchange has completed. If yes, the sequence
should be discarded. Since the exchange might be still in the completion
process, it should be untouched.

Signed-off-by: Steve Ma <steve.ma@intel.com>
---

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

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 992af05..7af9bce 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1159,6 +1159,10 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
 		atomic_inc(&mp->stats.xid_not_found);
 		goto out;
 	}
+	if (ep->esb_stat & ESB_ST_COMPLETE) {
+		atomic_inc(&mp->stats.xid_not_found);
+		goto out;
+	}
 	if (ep->rxid == FC_XID_UNKNOWN)
 		ep->rxid = ntohs(fh->fh_rx_id);
 	if (ep->sid != 0 && ep->sid != ntoh24(fh->fh_d_id)) {

[PATCH 5/8] fcoe: reduces lock cost when adding a new skb to fcoe_pending_queue

[Robert Love]

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

Currently fcoe_pending_queue.lock held twice for every new skb
adding to this queue when already least one pkt is pending in this
queue and that is not uncommon once skb pkts starts getting queued
here upon fcoe_start_io => dev_queue_xmit failure.

This patch moves most fcoe_pending_queue logic to fcoe_check_wait_queue
function, this new logic grabs fcoe_pending_queue.lock only once to
add a new skb instead twice as used to be.

I think after this patch call flow around fcoe_check_wait_queue
calling in fcoe_xmit is bit simplified with modified
fcoe_check_wait_queue function taking care of adding and
removing pending skb in one function.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
---

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

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 03e1926..7d44a49 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -71,7 +71,7 @@ static struct fc_lport *fcoe_hostlist_lookup(const struct net_device *);
 static int fcoe_hostlist_add(const struct fc_lport *);
 static int fcoe_hostlist_remove(const struct fc_lport *);
 
-static int fcoe_check_wait_queue(struct fc_lport *);
+static void fcoe_check_wait_queue(struct fc_lport *, struct sk_buff *);
 static int fcoe_device_notification(struct notifier_block *, ulong, void *);
 static void fcoe_dev_setup(void);
 static void fcoe_dev_cleanup(void);
@@ -988,7 +988,7 @@ u32 fcoe_fc_crc(struct fc_frame *fp)
  */
 int fcoe_xmit(struct fc_lport *lp, struct fc_frame *fp)
 {
-	int wlen, rc = 0;
+	int wlen;
 	u32 crc;
 	struct ethhdr *eh;
 	struct fcoe_crc_eof *cp;
@@ -1107,18 +1107,9 @@ int fcoe_xmit(struct fc_lport *lp, struct fc_frame *fp)
 	/* send down to lld */
 	fr_dev(fp) = lp;
 	if (fc->fcoe_pending_queue.qlen)
-		rc = fcoe_check_wait_queue(lp);
-
-	if (rc == 0)
-		rc = fcoe_start_io(skb);
-
-	if (rc) {
-		spin_lock_bh(&fc->fcoe_pending_queue.lock);
-		__skb_queue_tail(&fc->fcoe_pending_queue, skb);
-		spin_unlock_bh(&fc->fcoe_pending_queue.lock);
-		if (fc->fcoe_pending_queue.qlen > FCOE_MAX_QUEUE_DEPTH)
-			lp->qfull = 1;
-	}
+		fcoe_check_wait_queue(lp, skb);
+	else if (fcoe_start_io(skb))
+		fcoe_check_wait_queue(lp, skb);
 
 	return 0;
 }
@@ -1284,7 +1275,7 @@ void fcoe_watchdog(ulong vp)
 	read_lock(&fcoe_hostlist_lock);
 	list_for_each_entry(fc, &fcoe_hostlist, list) {
 		if (fc->ctlr.lp)
-			fcoe_check_wait_queue(fc->ctlr.lp);
+			fcoe_check_wait_queue(fc->ctlr.lp, NULL);
 	}
 	read_unlock(&fcoe_hostlist_lock);
 
@@ -1305,16 +1296,17 @@ void fcoe_watchdog(ulong vp)
  * The wait_queue is used when the skb transmit fails. skb will go
  * in the wait_queue which will be emptied by the timer function or
  * by the next skb transmit.
- *
- * Returns: 0 for success
  */
-static int fcoe_check_wait_queue(struct fc_lport *lp)
+static void fcoe_check_wait_queue(struct fc_lport *lp, struct sk_buff *skb)
 {
 	struct fcoe_softc *fc = lport_priv(lp);
-	struct sk_buff *skb;
-	int rc = -1;
+	int rc;
 
 	spin_lock_bh(&fc->fcoe_pending_queue.lock);
+
+	if (skb)
+		__skb_queue_tail(&fc->fcoe_pending_queue, skb);
+
 	if (fc->fcoe_pending_queue_active)
 		goto out;
 	fc->fcoe_pending_queue_active = 1;
@@ -1341,10 +1333,11 @@ static int fcoe_check_wait_queue(struct fc_lport *lp)
 	if (fc->fcoe_pending_queue.qlen < FCOE_LOW_QUEUE_DEPTH)
 		lp->qfull = 0;
 	fc->fcoe_pending_queue_active = 0;
-	rc = fc->fcoe_pending_queue.qlen;
 out:
+	if (fc->fcoe_pending_queue.qlen > FCOE_MAX_QUEUE_DEPTH)
+		lp->qfull = 1;
 	spin_unlock_bh(&fc->fcoe_pending_queue.lock);
-	return rc;
+	return;
 }
 
 /**

[PATCH 6/8] fcoe: removes fcoe_watchdog

[Robert Love]

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

Removes periodic fcoe_watchdog timer used across all fcoe interface
maintained in fcoe_hostlist instead added new fcoe_queue_timer
per fcoe interface.

Added timer is armed only when some pending skb need to be flushed
as oppose to periodic 1 second fcoe_watchdog, since now
fcoe_queue_timer is used on demand thus set this to 2 jiffies.

Now fcoe_queue_timer is much simple than fcoe_watchdog using lock to
process all fcoe interface from fcoe_hostlist.

I noticed +ve performance result with using 2 jiffies timer as
this helps flushing fcoe_pending_queue quickly.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
---

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

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 7d44a49..cf62e99 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -54,7 +54,6 @@ MODULE_LICENSE("GPL v2");
 /* fcoe host list */
 LIST_HEAD(fcoe_hostlist);
 DEFINE_RWLOCK(fcoe_hostlist_lock);
-DEFINE_TIMER(fcoe_timer, NULL, 0, 0);
 DEFINE_PER_CPU(struct fcoe_percpu_s, fcoe_percpu);
 
 /* Function Prototypes */
@@ -167,6 +166,18 @@ static int fcoe_lport_config(struct fc_lport *lp)
 }
 
 /**
+ * fcoe_queue_timer() - fcoe queue timer
+ * @lp: the fc_lport pointer
+ *
+ * Calls fcoe_check_wait_queue on timeout
+ *
+ */
+static void fcoe_queue_timer(ulong lp)
+{
+	fcoe_check_wait_queue((struct fc_lport *)lp, NULL);
+}
+
+/**
  * fcoe_netdev_config() - Set up netdev for SW FCoE
  * @lp : ptr to the fc_lport
  * @netdev : ptr to the associated netdevice struct
@@ -236,6 +247,7 @@ static int fcoe_netdev_config(struct fc_lport *lp, struct net_device *netdev)
 	}
 	skb_queue_head_init(&fc->fcoe_pending_queue);
 	fc->fcoe_pending_queue_active = 0;
+	setup_timer(&fc->timer, fcoe_queue_timer, (unsigned long)lp);
 
 	/* setup Source Mac Address */
 	memcpy(fc->ctlr.ctl_src_addr, fc->real_dev->dev_addr,
@@ -386,6 +398,9 @@ static int fcoe_if_destroy(struct net_device *netdev)
 	/* Free existing skbs */
 	fcoe_clean_pending_queue(lp);
 
+	/* Stop the timer */
+	del_timer_sync(&fc->timer);
+
 	/* Free memory used by statistical counters */
 	fc_lport_free_stats(lp);
 
@@ -1259,32 +1274,6 @@ int fcoe_percpu_receive_thread(void *arg)
 }
 
 /**
- * fcoe_watchdog() - fcoe timer callback
- * @vp:
- *
- * This checks the pending queue length for fcoe and set lport qfull
- * if the FCOE_MAX_QUEUE_DEPTH is reached. This is done for all fc_lport on the
- * fcoe_hostlist.
- *
- * Returns: 0 for success
- */
-void fcoe_watchdog(ulong vp)
-{
-	struct fcoe_softc *fc;
-
-	read_lock(&fcoe_hostlist_lock);
-	list_for_each_entry(fc, &fcoe_hostlist, list) {
-		if (fc->ctlr.lp)
-			fcoe_check_wait_queue(fc->ctlr.lp, NULL);
-	}
-	read_unlock(&fcoe_hostlist_lock);
-
-	fcoe_timer.expires = jiffies + (1 * HZ);
-	add_timer(&fcoe_timer);
-}
-
-
-/**
  * fcoe_check_wait_queue() - attempt to clear the transmit backlog
  * @lp: the fc_lport
  *
@@ -1332,6 +1321,8 @@ static void fcoe_check_wait_queue(struct fc_lport *lp, struct sk_buff *skb)
 
 	if (fc->fcoe_pending_queue.qlen < FCOE_LOW_QUEUE_DEPTH)
 		lp->qfull = 0;
+	if (fc->fcoe_pending_queue.qlen && !timer_pending(&fc->timer))
+		mod_timer(&fc->timer, jiffies + 2);
 	fc->fcoe_pending_queue_active = 0;
 out:
 	if (fc->fcoe_pending_queue.qlen > FCOE_MAX_QUEUE_DEPTH)
@@ -1808,10 +1799,6 @@ static int __init fcoe_init(void)
 	/* Setup link change notification */
 	fcoe_dev_setup();
 
-	setup_timer(&fcoe_timer, fcoe_watchdog, 0);
-
-	mod_timer(&fcoe_timer, jiffies + (10 * HZ));
-
 	fcoe_if_init();
 
 	return 0;
@@ -1837,9 +1824,6 @@ static void __exit fcoe_exit(void)
 
 	fcoe_dev_cleanup();
 
-	/* Stop the timer */
-	del_timer_sync(&fcoe_timer);
-
 	/* releases the associated fcoe hosts */
 	list_for_each_entry_safe(fc, tmp, &fcoe_hostlist, list)
 		fcoe_if_destroy(fc->real_dev);
diff --git a/drivers/scsi/fcoe/fcoe.h b/drivers/scsi/fcoe/fcoe.h
index 917aae8..a1eb8c1 100644
--- a/drivers/scsi/fcoe/fcoe.h
+++ b/drivers/scsi/fcoe/fcoe.h
@@ -61,6 +61,7 @@ struct fcoe_softc {
 	struct packet_type  fip_packet_type;
 	struct sk_buff_head fcoe_pending_queue;
 	u8	fcoe_pending_queue_active;
+	struct timer_list timer;		/* queue timer */
 	struct fcoe_ctlr ctlr;
 };
 

[PATCH 7/8] fcoe: removes reserving memory for vlan_ethdr on tx path

[Robert Love]

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

This is not required as VLAN header is added by device
interface driver, this was causing bad FC_CRC in FCoE pkts when
using VLAN interface.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
---

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

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index cf62e99..b8db2a8 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1036,8 +1036,7 @@ int fcoe_xmit(struct fc_lport *lp, struct fc_frame *fp)
 	sof = fr_sof(fp);
 	eof = fr_eof(fp);
 
-	elen = (fc->real_dev->priv_flags & IFF_802_1Q_VLAN) ?
-		sizeof(struct vlan_ethhdr) : sizeof(struct ethhdr);
+	elen = sizeof(struct ethhdr);
 	hlen = sizeof(struct fcoe_hdr);
 	tlen = sizeof(struct fcoe_crc_eof);
 	wlen = (skb->len - tlen + sizeof(crc)) / FCOE_WORD_TO_BYTE;

Re: [PATCH 5/8] fcoe: reduces lock cost when adding a new skb to fcoe_pending_queue

[James Bottomley]

On Wed, 2009-05-06 at 10:52 -0700, Robert Love wrote:
> From: Vasu Dev <vasu.dev@intel.com>
> 
> Currently fcoe_pending_queue.lock held twice for every new skb
> adding to this queue when already least one pkt is pending in this
> queue and that is not uncommon once skb pkts starts getting queued
> here upon fcoe_start_io => dev_queue_xmit failure.
> 
> This patch moves most fcoe_pending_queue logic to fcoe_check_wait_queue
> function, this new logic grabs fcoe_pending_queue.lock only once to
> add a new skb instead twice as used to be.
> 
> I think after this patch call flow around fcoe_check_wait_queue
> calling in fcoe_xmit is bit simplified with modified
> fcoe_check_wait_queue function taking care of adding and
> removing pending skb in one function.

This isn't really a -rc4 bug fix, is it?  It reads a lot more like a
feature enhancement that was given a bug like description.

James

RE: [PATCH 5/8] fcoe: reduces lock cost when adding a new skb to fcoe_pending_queue

[Love, Robert W]

James Bottomley wrote:
> On Wed, 2009-05-06 at 10:52 -0700, Robert Love wrote:
>> From: Vasu Dev <vasu.dev@intel.com>
>> 
>> Currently fcoe_pending_queue.lock held twice for every new skb
>> adding to this queue when already least one pkt is pending in this
>> queue and that is not uncommon once skb pkts starts getting queued
>> here upon fcoe_start_io => dev_queue_xmit failure.
>> 
>> This patch moves most fcoe_pending_queue logic to
>> fcoe_check_wait_queue function, this new logic grabs
>> fcoe_pending_queue.lock only once to add a new skb instead twice as
>> used to be. 
>> 
>> I think after this patch call flow around fcoe_check_wait_queue
>> calling in fcoe_xmit is bit simplified with modified
>> fcoe_check_wait_queue function taking care of adding and
>> removing pending skb in one function.
> 
> This isn't really a -rc4 bug fix, is it?  It reads a lot more like a
> feature enhancement that was given a bug like description.
> 
You're right- it is an enhancement and doesn't fix a bug (that I'm aware
of). I've pulled it out of the set (locally) and re-tested; there are
no dependency issues with removing it from this update.

What about the "6/8 removes fcoe_watchdog" patch? I think your reasoning
could apply to that one as well. It's replacing the global fcoe watchdog
with a per-interface one, which could be clasified as an enhancement
as well. (I've removed this patch also (locally) and tested, so if you
think it should be pushed later, removing it won't cause any issues.)

RE: [PATCH 5/8] fcoe: reduces lock cost when adding a new skb to fcoe_pending_queue

[James Bottomley]

On Wed, 2009-05-06 at 13:28 -0700, Love, Robert W wrote:
> James Bottomley wrote:
> > On Wed, 2009-05-06 at 10:52 -0700, Robert Love wrote:
> >> From: Vasu Dev <vasu.dev@intel.com>
> >> 
> >> Currently fcoe_pending_queue.lock held twice for every new skb
> >> adding to this queue when already least one pkt is pending in this
> >> queue and that is not uncommon once skb pkts starts getting queued
> >> here upon fcoe_start_io => dev_queue_xmit failure.
> >> 
> >> This patch moves most fcoe_pending_queue logic to
> >> fcoe_check_wait_queue function, this new logic grabs
> >> fcoe_pending_queue.lock only once to add a new skb instead twice as
> >> used to be. 
> >> 
> >> I think after this patch call flow around fcoe_check_wait_queue
> >> calling in fcoe_xmit is bit simplified with modified
> >> fcoe_check_wait_queue function taking care of adding and
> >> removing pending skb in one function.
> > 
> > This isn't really a -rc4 bug fix, is it?  It reads a lot more like a
> > feature enhancement that was given a bug like description.
> > 
> You're right- it is an enhancement and doesn't fix a bug (that I'm aware
> of). I've pulled it out of the set (locally) and re-tested; there are
> no dependency issues with removing it from this update.
> 
> What about the "6/8 removes fcoe_watchdog" patch? I think your reasoning
> could apply to that one as well. It's replacing the global fcoe watchdog
> with a per-interface one, which could be clasified as an enhancement
> as well. (I've removed this patch also (locally) and tested, so if you
> think it should be pushed later, removing it won't cause any issues.)--

That was next in my sights ... I was just waiting to see whether you
noticed.

James

RE: [PATCH 0/8] Open-FCoE fixes for 2.6.30 RC

[Love, Robert W]

Robert Love wrote:
> The following series implements general fixes to libfc, libfcoe and
> fcoe. 
> 
> Note: One patch adds the FIP ehtertype to if_ether.h. As discussed on
> netdev, this change was pre-acked and it was requested that the
> change go through 
> the SCSI tree.
> 
James, aside of the two patches that weren't appropriate for -rc will
the other 6 in this set be pushed upstream for -rc?

The two bad ones were:

fcoe: removes fcoe_watchdog
fcoe: reduces lock cost when adding a new skb to fcoe_pending_queue

The other 6 are good fixes and I tested them after removing the two
bad ones from the set, so they should be fine to push.

If you need a repost, I'd be glad to provide one.

> ---
> 
> Chris Leech (1):
>       fcoe: use ETH_P_FIP for skb->protocol of FIP frames
> 
> Joe Eykholt (2):
>       net, libfcoe: Add the FCoE Initialization Protocol ethertype
>       libfcoe: fip: fix non-FIP-mode FLOGI state after reset.
> 
> Mike Christie (1):
>       libfc: use DID_ERROR when we have internall aborted command
> 
> Steve Ma (1):
>       libfc: Check if exchange is completed when receiving a sequence
> 
> Vasu Dev (3):
>       fcoe: removes reserving memory for vlan_ethdr on tx path
>       fcoe: removes fcoe_watchdog
>       fcoe: reduces lock cost when adding a new skb to
> fcoe_pending_queue 
> 
> 
>  drivers/scsi/fcoe/fcoe.c     |   90
>  +++++++++++++++--------------------------- drivers/scsi/fcoe/fcoe.h 
>  |    1 drivers/scsi/fcoe/libfcoe.c  |   21 ++++++----
>  drivers/scsi/libfc/fc_exch.c |    4 ++
>  drivers/scsi/libfc/fc_fcp.c  |    2 -
>  include/linux/if_ether.h     |    1
>  include/scsi/fc/fc_fip.h     |    7 ---
>  7 files changed, 53 insertions(+), 73 deletions(-)
> 
> --
> //Rob

RE: [PATCH 0/8] Open-FCoE fixes for 2.6.30 RC

[James Bottomley]

On Thu, 2009-05-28 at 15:22 -0700, Love, Robert W wrote:
> Robert Love wrote:
> > The following series implements general fixes to libfc, libfcoe and
> > fcoe. 
> > 
> > Note: One patch adds the FIP ehtertype to if_ether.h. As discussed on
> > netdev, this change was pre-acked and it was requested that the
> > change go through 
> > the SCSI tree.
> > 
> James, aside of the two patches that weren't appropriate for -rc will
> the other 6 in this set be pushed upstream for -rc?

I sort of lost track of which the other 6 were.  If I look at the
patches, the bug fixes seem to be: 1-4 and 7.  Then for non bug fixes: 5
is a lock optimisation, 6 is a watchdog optimisation and 8 is moving
header contents ... and can all wait for the merge window.  Is that
about right?

James

RE: [PATCH 0/8] Open-FCoE fixes for 2.6.30 RC

[Love, Robert W]

James Bottomley wrote:
> On Thu, 2009-05-28 at 15:22 -0700, Love, Robert W wrote:
>> Robert Love wrote:
>>> The following series implements general fixes to libfc, libfcoe and
>>> fcoe. 
>>> 
>>> Note: One patch adds the FIP ehtertype to if_ether.h. As discussed
>>> on netdev, this change was pre-acked and it was requested that the
>>> change go through the SCSI tree.
>>> 
>> James, aside of the two patches that weren't appropriate for -rc will
>> the other 6 in this set be pushed upstream for -rc?
> 
> I sort of lost track of which the other 6 were.  If I look at the
> patches, the bug fixes seem to be: 1-4 and 7.  Then for non bug
> fixes: 5 is a lock optimisation, 6 is a watchdog optimisation and 8
> is moving header contents ... and can all wait for the merge window. 
> Is that about right?
> 
Yes, that is correct.