[PATCH 0/3] libfc, libfcoe and fcoe updates for scsi-misc

[PATCH 0/3] libfc, libfcoe and fcoe updates for scsi-misc

[Robert Love]

Here are a few bug fixes for libfc and fcoe.
Additionally there is a documentation fix for sysfs-bus-fcoe included.

---

Neerav Parikh (1):
      fcoe: Fix write errors on NPIV ports

Robert Love (1):
      Documentation: Add lesb/ to path for LESB attributes in FCoE bus documentation

Yi Zou (1):
      libfc: fix lun reset failure bugs in fc_fcp_resp handling of FCP_RSP_INFO


 Documentation/ABI/testing/sysfs-bus-fcoe |   12 ++++++------
 drivers/scsi/fcoe/fcoe.c                 |   14 +++++---------
 drivers/scsi/fcoe/fcoe.h                 |    2 ++
 drivers/scsi/libfc/fc_fcp.c              |    3 ++-
 include/scsi/fc/fc_fcp.h                 |    6 ++++++
 include/scsi/libfcoe.h                   |    2 --
 6 files changed, 21 insertions(+), 18 deletions(-)

-- 
Thanks, //Rob

[PATCH 1/3] fcoe: Fix write errors on NPIV ports

[Robert Love]

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

SCSI errors were generated while writing to LUNs
connected via NPIV ports.

Debugging this it was found that the FCoE packets
transmitted via the NPIV ports were not tagged with
correct user priority as negotiated with peer by DCB
agent. This resulted in FCoE traffic going with priority
zero(0) that did not have priority flow control (PFC)
enabled for it. The initiator after transferring data
to the target never saw any reply indicating the transfer
was complete. This resulted in error recovery (ABTS) and
SCSI command retries by the scsi-mid layer; eventually
resulting in I/O errors.

This patch fixes this issue by keeping the FCoE user
priority information in the fcoe_interface instance
that is common for both the physical port as well as
NPIV ports connected to that physical port; instead
of storing it in fcoe_port structure that has a per
port instance.

Signed-off-by: Neerav Parikh <Neerav.Parikh@intel.com>
Acked-by: Yi Zou <yi.zou@intel.com>
Acked-by: John Fastabend <john.r.fastabend@intel.com>
Tested-by: Marcus Dennis <marcusx.e.dennis@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe.c |   14 +++++---------
 drivers/scsi/fcoe/fcoe.h |    2 ++
 include/scsi/libfcoe.h   |    2 --
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 078d262..666b7ac 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1643,7 +1643,7 @@ static int fcoe_xmit(struct fc_lport *lport, struct fc_frame *fp)
 	skb_reset_network_header(skb);
 	skb->mac_len = elen;
 	skb->protocol = htons(ETH_P_FCOE);
-	skb->priority = port->priority;
+	skb->priority = fcoe->priority;
 
 	if (fcoe->netdev->priv_flags & IFF_802_1Q_VLAN &&
 	    fcoe->realdev->features & NETIF_F_HW_VLAN_TX) {
@@ -1917,7 +1917,6 @@ static int fcoe_dcb_app_notification(struct notifier_block *notifier,
 	struct fcoe_ctlr *ctlr;
 	struct fcoe_interface *fcoe;
 	struct net_device *netdev;
-	struct fcoe_port *port;
 	int prio;
 
 	if (entry->app.selector != DCB_APP_IDTYPE_ETHTYPE)
@@ -1946,10 +1945,8 @@ static int fcoe_dcb_app_notification(struct notifier_block *notifier,
 	    entry->app.protocol == ETH_P_FCOE)
 		ctlr->priority = prio;
 
-	if (entry->app.protocol == ETH_P_FCOE) {
-		port = lport_priv(ctlr->lp);
-		port->priority = prio;
-	}
+	if (entry->app.protocol == ETH_P_FCOE)
+		fcoe->priority = prio;
 
 	return NOTIFY_OK;
 }
@@ -2180,7 +2177,6 @@ static void fcoe_dcb_create(struct fcoe_interface *fcoe)
 	u8 fup, up;
 	struct net_device *netdev = fcoe->realdev;
 	struct fcoe_ctlr *ctlr = fcoe_to_ctlr(fcoe);
-	struct fcoe_port *port = lport_priv(ctlr->lp);
 	struct dcb_app app = {
 				.priority = 0,
 				.protocol = ETH_P_FCOE
@@ -2202,8 +2198,8 @@ static void fcoe_dcb_create(struct fcoe_interface *fcoe)
 			fup = dcb_getapp(netdev, &app);
 		}
 
-		port->priority = ffs(up) ? ffs(up) - 1 : 0;
-		ctlr->priority = ffs(fup) ? ffs(fup) - 1 : port->priority;
+		fcoe->priority = ffs(up) ? ffs(up) - 1 : 0;
+		ctlr->priority = ffs(fup) ? ffs(fup) - 1 : fcoe->priority;
 	}
 #endif
 }
diff --git a/drivers/scsi/fcoe/fcoe.h b/drivers/scsi/fcoe/fcoe.h
index a624add..b42dc32 100644
--- a/drivers/scsi/fcoe/fcoe.h
+++ b/drivers/scsi/fcoe/fcoe.h
@@ -71,6 +71,7 @@ do {                                                            	\
  * @oem:	      The offload exchange manager for all local port
  *		      instances associated with this port
  * @removed:	      Indicates fcoe interface removed from net device
+ * @priority:	      Priority for the FCoE packet (DCB)
  * This structure is 1:1 with a net device.
  */
 struct fcoe_interface {
@@ -81,6 +82,7 @@ struct fcoe_interface {
 	struct packet_type fip_packet_type;
 	struct fc_exch_mgr *oem;
 	u8	removed;
+	u8	priority;
 };
 
 #define fcoe_to_ctlr(x)						\
diff --git a/include/scsi/libfcoe.h b/include/scsi/libfcoe.h
index 22b07cc..8742d85 100644
--- a/include/scsi/libfcoe.h
+++ b/include/scsi/libfcoe.h
@@ -327,7 +327,6 @@ struct fcoe_percpu_s {
  * @lport:		       The associated local port
  * @fcoe_pending_queue:	       The pending Rx queue of skbs
  * @fcoe_pending_queue_active: Indicates if the pending queue is active
- * @priority:		       Packet priority (DCB)
  * @max_queue_depth:	       Max queue depth of pending queue
  * @min_queue_depth:	       Min queue depth of pending queue
  * @timer:		       The queue timer
@@ -343,7 +342,6 @@ struct fcoe_port {
 	struct fc_lport	      *lport;
 	struct sk_buff_head   fcoe_pending_queue;
 	u8		      fcoe_pending_queue_active;
-	u8		      priority;
 	u32		      max_queue_depth;
 	u32		      min_queue_depth;
 	struct timer_list     timer;

[PATCH 2/3] libfc: fix lun reset failure bugs in fc_fcp_resp handling of FCP_RSP_INFO

[Robert Love]

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

In LUN RESET testing involving NetApp targets, it is observed that LUN
RESET is failing. The fc_fcp_resp() is not completing the completiong
for the LUN RESET task since fc_fcp_resp assumes that the FCP_RSP_INFO
is 8 bytes with the 4 byte reserved field, where in case of NetApp targets
the FCP_RSP to LUN RESET only has 4 bytes of FCP_RSP_INFO. This leads
fc_fcp_resp to error out w/o completing the task completion, eventually
causing LUN RESET to be escalated to host reset, which is not very nice.

Per FCP-3 r04, clause 9.5.15 and Table 23, the FCP_RSP_INFO field can be either
4 bytes or 8 bytes, with the last 4 bytes as "Reserved (if any)". Therefore it
is valid to have 4 bytes FCP_RSP_INFO like some of the NetApp targets behave.
Fixing this by validating the FCP_RSP_INFO against both the two spec allowed
length.

Reported-by: Frank Zhang <frank_1.zhang@intel.com>
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/libfc/fc_fcp.c |    3 ++-
 include/scsi/fc/fc_fcp.h    |    6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 14243fa..fcb9d0b 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -851,7 +851,8 @@ static void fc_fcp_resp(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 			fc_rp_info = (struct fcp_resp_rsp_info *)(rp_ex + 1);
 			if (flags & FCP_RSP_LEN_VAL) {
 				respl = ntohl(rp_ex->fr_rsp_len);
-				if (respl != sizeof(*fc_rp_info))
+				if ((respl != FCP_RESP_RSP_INFO_LEN4) &&
+				    (respl != FCP_RESP_RSP_INFO_LEN8))
 					goto len_err;
 				if (fsp->wait_for_comp) {
 					/* Abuse cdb_status for rsp code */
diff --git a/include/scsi/fc/fc_fcp.h b/include/scsi/fc/fc_fcp.h
index 0d7d67e..9c87029 100644
--- a/include/scsi/fc/fc_fcp.h
+++ b/include/scsi/fc/fc_fcp.h
@@ -127,6 +127,9 @@ struct fcp_txrdy {
  *
  * All response frames will always contain the fcp_resp template.  Some
  * will also include the fcp_resp_len template.
+ *
+ * From Table 23, the FCP_RSP_INFO can either be 4 bytes or 8 bytes, both
+ * are valid length.
  */
 struct fcp_resp {
 	__u8		_fr_resvd[8];	/* reserved */
@@ -156,6 +159,9 @@ struct fcp_resp_rsp_info {
     __u8      _fr_resvd2[4];      /* reserved */
 };
 
+#define FCP_RESP_RSP_INFO_LEN4    4 /* without reserved field */
+#define FCP_RESP_RSP_INFO_LEN8    8 /* with reserved field */
+
 struct fcp_resp_with_ext {
 	struct fcp_resp resp;
 	struct fcp_resp_ext ext;

[PATCH 3/3] Documentation: Add lesb/ to path for LESB attributes in FCoE bus documentation

[Robert Love]

The Link Error Status Block attributes are incorrectly named as they do
not have the lesb_ prefix, but instead are grouped in the lesb/ attribute
group.

Signed-off-by: Robert Love <robert.w.love@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-fcoe |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-fcoe b/Documentation/ABI/testing/sysfs-bus-fcoe
index 469d09c..50e2a80 100644
--- a/Documentation/ABI/testing/sysfs-bus-fcoe
+++ b/Documentation/ABI/testing/sysfs-bus-fcoe
@@ -9,19 +9,19 @@ Attributes:
 			  this value will change the dev_loss_tmo for all
 			  FCFs discovered by this controller.
 
-	lesb_link_fail:   Link Error Status Block (LESB) link failure count.
+	lesb/link_fail:   Link Error Status Block (LESB) link failure count.
 
-	lesb_vlink_fail:  Link Error Status Block (LESB) virtual link
+	lesb/vlink_fail:  Link Error Status Block (LESB) virtual link
 			  failure count.
 
-	lesb_miss_fka:    Link Error Status Block (LESB) missed FCoE
+	lesb/miss_fka:    Link Error Status Block (LESB) missed FCoE
 			  Initialization Protocol (FIP) Keep-Alives (FKA).
 
-	lesb_symb_err:    Link Error Status Block (LESB) symbolic error count.
+	lesb/symb_err:    Link Error Status Block (LESB) symbolic error count.
 
-	lesb_err_block:   Link Error Status Block (LESB) block error count.
+	lesb/err_block:   Link Error Status Block (LESB) block error count.
 
-	lesb_fcs_error:   Link Error Status Block (LESB) Fibre Channel
+	lesb/fcs_error:   Link Error Status Block (LESB) Fibre Channel
 			  Serivces error count.
 
 Notes: ctlr_X (global increment starting at 0)

Re: [PATCH 2/3] libfc: fix lun reset failure bugs in fc_fcp_resp handling of FCP_RSP_INFO

[Bart Van Assche]

On 09/24/12 20:52, Robert Love wrote:
> In LUN RESET testing involving NetApp targets, it is observed that LUN
> RESET is failing. The fc_fcp_resp() is not completing the completiong

It looks like there is a typo in the patch description (in the last word 
of the second line) ?

Bart.

RE: [PATCH 2/3] libfc: fix lun reset failure bugs in fc_fcp_resp handling of FCP_RSP_INFO

[Zou, Yi]

> 
> On 09/24/12 20:52, Robert Love wrote:
> > In LUN RESET testing involving NetApp targets, it is observed that LUN
> > RESET is failing. The fc_fcp_resp() is not completing the completiong
> 
> It looks like there is a typo in the patch description (in the last word
> of the second line) ?
> 
> Bart.

Yeah, extra 'g' should be removed...meant to say the "completion task"...
Hmm, not sure how my vim spellchecker missed it. 

James, would mind fixing this when it gets applied?

Thanks,
yi