* [PATCH 1/8] libfcoe: Incorrect CVL handling for NPIV ports
2011-05-16 23:45 [PATCH 0/8] libfc, libfcoe and fcoe patches for 2.6.40 Robert Love
@ 2011-05-16 23:45 ` Robert Love
2011-05-16 23:45 ` [PATCH 2/8] fcoe: Prevent creation of an NPIV port with duplicate WWPN Robert Love
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Robert Love @ 2011-05-16 23:45 UTC (permalink / raw)
To: linux-scsi; +Cc: Bhanu Prakash Gollapudi
From: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
Host doesnt handle CVL to NPIV instantiated ports correctly.
- As per FC-BB-5 Rev 2 CVLs with no VN_Port descriptors shall be treated as
implicit logout of ALL vn_ports.
- CVL for NPIV ports should be handled before physical port even if descriptor
for physical port appears before NPIV ports
Signed-off-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/fcoe/fcoe_ctlr.c | 133 ++++++++++++++++++++++++++++-------------
1 files changed, 90 insertions(+), 43 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 229e4af..c74c4b8 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -1173,7 +1173,9 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
struct fc_lport *lport = fip->lp;
struct fc_lport *vn_port = NULL;
u32 desc_mask;
- int is_vn_port = 0;
+ int num_vlink_desc;
+ int reset_phys_port = 0;
+ struct fip_vn_desc **vlink_desc_arr = NULL;
LIBFCOE_FIP_DBG(fip, "Clear Virtual Link received\n");
@@ -1183,70 +1185,73 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
/*
* mask of required descriptors. Validating each one clears its bit.
*/
- desc_mask = BIT(FIP_DT_MAC) | BIT(FIP_DT_NAME) | BIT(FIP_DT_VN_ID);
+ desc_mask = BIT(FIP_DT_MAC) | BIT(FIP_DT_NAME);
rlen = ntohs(fh->fip_dl_len) * FIP_BPW;
desc = (struct fip_desc *)(fh + 1);
+
+ /*
+ * Actually need to subtract 'sizeof(*mp) - sizeof(*wp)' from 'rlen'
+ * before determining max Vx_Port descriptor but a buggy FCF could have
+ * omited either or both MAC Address and Name Identifier descriptors
+ */
+ num_vlink_desc = rlen / sizeof(*vp);
+ if (num_vlink_desc)
+ vlink_desc_arr = kmalloc(sizeof(vp) * num_vlink_desc,
+ GFP_ATOMIC);
+ if (!vlink_desc_arr)
+ return;
+ num_vlink_desc = 0;
+
while (rlen >= sizeof(*desc)) {
dlen = desc->fip_dlen * FIP_BPW;
if (dlen > rlen)
- return;
+ goto err;
/* Drop CVL if there are duplicate critical descriptors */
if ((desc->fip_dtype < 32) &&
+ (desc->fip_dtype != FIP_DT_VN_ID) &&
!(desc_mask & 1U << desc->fip_dtype)) {
LIBFCOE_FIP_DBG(fip, "Duplicate Critical "
"Descriptors in FIP CVL\n");
- return;
+ goto err;
}
switch (desc->fip_dtype) {
case FIP_DT_MAC:
mp = (struct fip_mac_desc *)desc;
if (dlen < sizeof(*mp))
- return;
+ goto err;
if (compare_ether_addr(mp->fd_mac, fcf->fcf_mac))
- return;
+ goto err;
desc_mask &= ~BIT(FIP_DT_MAC);
break;
case FIP_DT_NAME:
wp = (struct fip_wwn_desc *)desc;
if (dlen < sizeof(*wp))
- return;
+ goto err;
if (get_unaligned_be64(&wp->fd_wwn) != fcf->switch_name)
- return;
+ goto err;
desc_mask &= ~BIT(FIP_DT_NAME);
break;
case FIP_DT_VN_ID:
vp = (struct fip_vn_desc *)desc;
if (dlen < sizeof(*vp))
- return;
- if (compare_ether_addr(vp->fd_mac,
- fip->get_src_addr(lport)) == 0 &&
- get_unaligned_be64(&vp->fd_wwpn) == lport->wwpn &&
- ntoh24(vp->fd_fc_id) == lport->port_id) {
- desc_mask &= ~BIT(FIP_DT_VN_ID);
- break;
+ goto err;
+ vlink_desc_arr[num_vlink_desc++] = vp;
+ vn_port = fc_vport_id_lookup(lport,
+ ntoh24(vp->fd_fc_id));
+ if (vn_port && (vn_port == lport)) {
+ mutex_lock(&fip->ctlr_mutex);
+ per_cpu_ptr(lport->dev_stats,
+ get_cpu())->VLinkFailureCount++;
+ put_cpu();
+ fcoe_ctlr_reset(fip);
+ mutex_unlock(&fip->ctlr_mutex);
}
- /* check if clr_vlink is for NPIV port */
- mutex_lock(&lport->lp_mutex);
- list_for_each_entry(vn_port, &lport->vports, list) {
- if (compare_ether_addr(vp->fd_mac,
- fip->get_src_addr(vn_port)) == 0 &&
- (get_unaligned_be64(&vp->fd_wwpn)
- == vn_port->wwpn) &&
- (ntoh24(vp->fd_fc_id) ==
- fc_host_port_id(vn_port->host))) {
- desc_mask &= ~BIT(FIP_DT_VN_ID);
- is_vn_port = 1;
- break;
- }
- }
- mutex_unlock(&lport->lp_mutex);
-
break;
default:
/* standard says ignore unknown descriptors >= 128 */
if (desc->fip_dtype < FIP_DT_VENDOR_BASE)
- return;
+ goto err;
break;
}
desc = (struct fip_desc *)((char *)desc + dlen);
@@ -1256,26 +1261,68 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
/*
* reset only if all required descriptors were present and valid.
*/
- if (desc_mask) {
+ if (desc_mask)
LIBFCOE_FIP_DBG(fip, "missing descriptors mask %x\n",
desc_mask);
+ else if (!num_vlink_desc) {
+ LIBFCOE_FIP_DBG(fip, "CVL: no Vx_Port descriptor found\n");
+ /*
+ * No Vx_Port description. Clear all NPIV ports,
+ * followed by physical port
+ */
+ mutex_lock(&lport->lp_mutex);
+ list_for_each_entry(vn_port, &lport->vports, list)
+ fc_lport_reset(vn_port);
+ mutex_unlock(&lport->lp_mutex);
+
+ mutex_lock(&fip->ctlr_mutex);
+ per_cpu_ptr(lport->dev_stats,
+ get_cpu())->VLinkFailureCount++;
+ put_cpu();
+ fcoe_ctlr_reset(fip);
+ mutex_unlock(&fip->ctlr_mutex);
+
+ fc_lport_reset(fip->lp);
+ fcoe_ctlr_solicit(fip, NULL);
} else {
- LIBFCOE_FIP_DBG(fip, "performing Clear Virtual Link\n");
+ int i;
- if (is_vn_port)
- fc_lport_reset(vn_port);
- else {
- mutex_lock(&fip->ctlr_mutex);
- per_cpu_ptr(lport->dev_stats,
- get_cpu())->VLinkFailureCount++;
- put_cpu();
- fcoe_ctlr_reset(fip);
- mutex_unlock(&fip->ctlr_mutex);
+ LIBFCOE_FIP_DBG(fip, "performing Clear Virtual Link\n");
+ for (i = 0; i < num_vlink_desc; i++) {
+ vp = vlink_desc_arr[i];
+ vn_port = fc_vport_id_lookup(lport,
+ ntoh24(vp->fd_fc_id));
+ if (!vn_port)
+ continue;
+
+ /*
+ * 'port_id' is already validated, check MAC address and
+ * wwpn
+ */
+ if (compare_ether_addr(fip->get_src_addr(vn_port),
+ vp->fd_mac) != 0 ||
+ get_unaligned_be64(&vp->fd_wwpn) !=
+ vn_port->wwpn)
+ continue;
+
+ if (vn_port == lport)
+ /*
+ * Physical port, defer processing till all
+ * listed NPIV ports are cleared
+ */
+ reset_phys_port = 1;
+ else /* NPIV port */
+ fc_lport_reset(vn_port);
+ }
+ if (reset_phys_port) {
fc_lport_reset(fip->lp);
fcoe_ctlr_solicit(fip, NULL);
}
}
+
+err:
+ kfree(vlink_desc_arr);
}
/**
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/8] fcoe: Prevent creation of an NPIV port with duplicate WWPN
2011-05-16 23:45 [PATCH 0/8] libfc, libfcoe and fcoe patches for 2.6.40 Robert Love
2011-05-16 23:45 ` [PATCH 1/8] libfcoe: Incorrect CVL handling for NPIV ports Robert Love
@ 2011-05-16 23:45 ` Robert Love
2011-05-16 23:45 ` [PATCH 3/8] libfc: fix mm leak in handling incoming request for target discovery Robert Love
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Robert Love @ 2011-05-16 23:45 UTC (permalink / raw)
To: linux-scsi; +Cc: Ross Brattain, Neerav Parikh
From: Neerav Parikh <Neerav.Parikh@intel.com>
This patch adds a validation step before allowing creation of a new NPIV port.
It checks whether the WWPN passed for the new NPIV port to be created is unique
for the given physical port.
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/fcoe/fcoe.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/scsi/fcoe/fcoe.h | 10 ++++++++
2 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 5d3700d..1c55a61 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -137,6 +137,7 @@ static int fcoe_vport_create(struct fc_vport *, bool disabled);
static int fcoe_vport_disable(struct fc_vport *, bool disable);
static void fcoe_set_vport_symbolic_name(struct fc_vport *);
static void fcoe_set_port_id(struct fc_lport *, u32, struct fc_frame *);
+static int fcoe_validate_vport_create(struct fc_vport *);
static struct libfc_function_template fcoe_libfc_fcn_templ = {
.frame_send = fcoe_xmit,
@@ -2348,6 +2349,17 @@ static int fcoe_vport_create(struct fc_vport *vport, bool disabled)
struct fcoe_interface *fcoe = port->priv;
struct net_device *netdev = fcoe->netdev;
struct fc_lport *vn_port;
+ int rc;
+ char buf[32];
+
+ rc = fcoe_validate_vport_create(vport);
+ if (rc) {
+ wwn_to_str(vport->port_name, buf, sizeof(buf));
+ printk(KERN_ERR "fcoe: Failed to create vport, "
+ "WWPN (0x%s) already exists\n",
+ buf);
+ return rc;
+ }
mutex_lock(&fcoe_config_mutex);
vn_port = fcoe_if_create(fcoe, &vport->dev, 1);
@@ -2494,3 +2506,49 @@ static void fcoe_set_port_id(struct fc_lport *lport,
if (fp && fc_frame_payload_op(fp) == ELS_FLOGI)
fcoe_ctlr_recv_flogi(&fcoe->ctlr, lport, fp);
}
+
+/**
+ * fcoe_validate_vport_create() - Validate a vport before creating it
+ * @vport: NPIV port to be created
+ *
+ * This routine is meant to add validation for a vport before creating it
+ * via fcoe_vport_create().
+ * Current validations are:
+ * - WWPN supplied is unique for given lport
+ *
+ *
+*/
+static int fcoe_validate_vport_create(struct fc_vport *vport)
+{
+ struct Scsi_Host *shost = vport_to_shost(vport);
+ struct fc_lport *n_port = shost_priv(shost);
+ struct fc_lport *vn_port;
+ int rc = 0;
+ char buf[32];
+
+ mutex_lock(&n_port->lp_mutex);
+
+ wwn_to_str(vport->port_name, buf, sizeof(buf));
+ /* Check if the wwpn is not same as that of the lport */
+ if (!memcmp(&n_port->wwpn, &vport->port_name, sizeof(u64))) {
+ FCOE_DBG("vport WWPN 0x%s is same as that of the "
+ "base port WWPN\n", buf);
+ rc = -EINVAL;
+ goto out;
+ }
+
+ /* Check if there is any existing vport with same wwpn */
+ list_for_each_entry(vn_port, &n_port->vports, list) {
+ if (!memcmp(&vn_port->wwpn, &vport->port_name, sizeof(u64))) {
+ FCOE_DBG("vport with given WWPN 0x%s already "
+ "exists\n", buf);
+ rc = -EINVAL;
+ break;
+ }
+ }
+
+out:
+ mutex_unlock(&n_port->lp_mutex);
+
+ return rc;
+}
diff --git a/drivers/scsi/fcoe/fcoe.h b/drivers/scsi/fcoe/fcoe.h
index 408a6fd..c4a9399 100644
--- a/drivers/scsi/fcoe/fcoe.h
+++ b/drivers/scsi/fcoe/fcoe.h
@@ -99,4 +99,14 @@ static inline struct net_device *fcoe_netdev(const struct fc_lport *lport)
((struct fcoe_port *)lport_priv(lport))->priv)->netdev;
}
+static inline void wwn_to_str(u64 wwn, char *buf, int len)
+{
+ u8 wwpn[8];
+
+ u64_to_wwn(wwn, wwpn);
+ snprintf(buf, len, "%02x%02x%02x%02x%02x%02x%02x%02x",
+ wwpn[0], wwpn[1], wwpn[2], wwpn[3],
+ wwpn[4], wwpn[5], wwpn[6], wwpn[7]);
+}
+
#endif /* _FCOE_H_ */
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/8] libfc: fix mm leak in handling incoming request for target discovery
2011-05-16 23:45 [PATCH 0/8] libfc, libfcoe and fcoe patches for 2.6.40 Robert Love
2011-05-16 23:45 ` [PATCH 1/8] libfcoe: Incorrect CVL handling for NPIV ports Robert Love
2011-05-16 23:45 ` [PATCH 2/8] fcoe: Prevent creation of an NPIV port with duplicate WWPN Robert Love
@ 2011-05-16 23:45 ` Robert Love
2011-05-16 23:45 ` [PATCH 4/8] libfc: release DDP context if frame_send() fails Robert Love
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Robert Love @ 2011-05-16 23:45 UTC (permalink / raw)
To: linux-scsi; +Cc: Hillf Danton
From: Hillf Danton <dhillf@gmail.com>
When handling incoming request, if the operation code carried by the
received frame is not RSCN, the frame should be freed as in the RSCN
case, or there is memory leakage.
Signed-off-by: Hillf Danton <dhillf@gmail.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/libfc/fc_disc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/libfc/fc_disc.c b/drivers/scsi/libfc/fc_disc.c
index 911b273..b9cb814 100644
--- a/drivers/scsi/libfc/fc_disc.c
+++ b/drivers/scsi/libfc/fc_disc.c
@@ -205,6 +205,7 @@ static void fc_disc_recv_req(struct fc_lport *lport, struct fc_frame *fp)
default:
FC_DISC_DBG(disc, "Received an unsupported request, "
"the opcode is (%x)\n", op);
+ fc_frame_free(fp);
break;
}
}
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/8] libfc: release DDP context if frame_send() fails
2011-05-16 23:45 [PATCH 0/8] libfc, libfcoe and fcoe patches for 2.6.40 Robert Love
` (2 preceding siblings ...)
2011-05-16 23:45 ` [PATCH 3/8] libfc: fix mm leak in handling incoming request for target discovery Robert Love
@ 2011-05-16 23:45 ` Robert Love
2011-05-16 23:45 ` [PATCH 5/8] libfc: don't call resp handler after FC_EX_TIMEOUT Robert Love
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Robert Love @ 2011-05-16 23:45 UTC (permalink / raw)
To: linux-scsi; +Cc: Yi Zou
From: Yi Zou <yi.zou@intel.com>
In case frame_send() fails, make sure to let the underlying HW release the DDP
context that has already been set up before calling frame_send().
Signed-off-by: Yi Zou <yi.zou@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/libfc/fc_exch.c | 1 +
drivers/scsi/libfc/fc_fcp.c | 2 +-
drivers/scsi/libfc/fc_libfc.h | 1 +
3 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 77035a7..4d2994d 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1978,6 +1978,7 @@ static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
spin_unlock_bh(&ep->ex_lock);
return sp;
err:
+ fc_fcp_ddp_done(fr_fsp(fp));
rc = fc_exch_done_locked(ep);
spin_unlock_bh(&ep->ex_lock);
if (!rc)
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 2a3a472..f880d40 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -312,7 +312,7 @@ void fc_fcp_ddp_setup(struct fc_fcp_pkt *fsp, u16 xid)
* DDP related resources for a fcp_pkt
* @fsp: The FCP packet that DDP had been used on
*/
-static void fc_fcp_ddp_done(struct fc_fcp_pkt *fsp)
+void fc_fcp_ddp_done(struct fc_fcp_pkt *fsp)
{
struct fc_lport *lport;
diff --git a/drivers/scsi/libfc/fc_libfc.h b/drivers/scsi/libfc/fc_libfc.h
index fedc819..c7d0712 100644
--- a/drivers/scsi/libfc/fc_libfc.h
+++ b/drivers/scsi/libfc/fc_libfc.h
@@ -108,6 +108,7 @@ extern struct fc4_prov fc_rport_fcp_init; /* FCP initiator provider */
* Set up direct-data placement for this I/O request
*/
void fc_fcp_ddp_setup(struct fc_fcp_pkt *fsp, u16 xid);
+void fc_fcp_ddp_done(struct fc_fcp_pkt *fsp);
/*
* Module setup functions
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/8] libfc: don't call resp handler after FC_EX_TIMEOUT
2011-05-16 23:45 [PATCH 0/8] libfc, libfcoe and fcoe patches for 2.6.40 Robert Love
` (3 preceding siblings ...)
2011-05-16 23:45 ` [PATCH 4/8] libfc: release DDP context if frame_send() fails Robert Love
@ 2011-05-16 23:45 ` Robert Love
2011-05-16 23:45 ` [PATCH 6/8] libfc: fix race in SRR response Robert Love
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Robert Love @ 2011-05-16 23:45 UTC (permalink / raw)
To: linux-scsi; +Cc: Ross Brattain, Vasu Dev
From: Vasu Dev <vasu.dev@intel.com>
In cases exch is already timed out then exch layer could
end up calling resp handler again for its response frame
received after timeout, though in this case fc_exch_timeout
handler would have already called resp with FC_EX_TIMEOUT.
This would cause REC response handler to release its
fsp pkt hold twice instead once and possibly similar issues
with other ELS exchanges in this race.
To avoid this race have resp updated under exch lock
in rx path, the resp would get set to NULL in case
of FC_EX_TIMEOUT under the same lock to prevent resp
callback after FC_EX_TIMEOUT.
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_exch.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 4d2994d..3b8a645 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1434,6 +1434,7 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
(f_ctl & (FC_FC_LAST_SEQ | FC_FC_END_SEQ)) ==
(FC_FC_LAST_SEQ | FC_FC_END_SEQ)) {
spin_lock_bh(&ep->ex_lock);
+ resp = ep->resp;
rc = fc_exch_done_locked(ep);
WARN_ON(fc_seq_exch(sp) != ep);
spin_unlock_bh(&ep->ex_lock);
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/8] libfc: fix race in SRR response
2011-05-16 23:45 [PATCH 0/8] libfc, libfcoe and fcoe patches for 2.6.40 Robert Love
` (4 preceding siblings ...)
2011-05-16 23:45 ` [PATCH 5/8] libfc: don't call resp handler after FC_EX_TIMEOUT Robert Love
@ 2011-05-16 23:45 ` Robert Love
2011-05-16 23:45 ` [PATCH 7/8] libfc: do not immediately retry the cmd when seq_send fails in fc_fcp_send_data Robert Love
2011-05-16 23:46 ` [PATCH 8/8] libfcoe: Remove unnecessary module state checks Robert Love
7 siblings, 0 replies; 9+ messages in thread
From: Robert Love @ 2011-05-16 23:45 UTC (permalink / raw)
To: linux-scsi; +Cc: Ross Brattain, Vasu Dev
From: Vasu Dev <vasu.dev@intel.com>
In this case fsp was freed before error handler was invoked,
this is fixed by having SRR fsp reference freed by exch
destructor so that fsp will be always held until it exch
is freed.
Also don't reset fsp->recov_seq since this is needed by
SRR error handler to do exch done.
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_fcp.c | 11 ++++-------
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index f880d40..57704e8 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -1673,7 +1673,8 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset)
FC_FCTL_REQ, 0);
rec_tov = get_fsp_rec_tov(fsp);
- seq = lport->tt.exch_seq_send(lport, fp, fc_fcp_srr_resp, NULL,
+ seq = lport->tt.exch_seq_send(lport, fp, fc_fcp_srr_resp,
+ fc_fcp_pkt_destroy,
fsp, jiffies_to_msecs(rec_tov));
if (!seq)
goto retry;
@@ -1720,7 +1721,6 @@ static void fc_fcp_srr_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
return;
}
- fsp->recov_seq = NULL;
switch (fc_frame_payload_op(fp)) {
case ELS_LS_ACC:
fsp->recov_retry = 0;
@@ -1732,10 +1732,9 @@ static void fc_fcp_srr_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
break;
}
fc_fcp_unlock_pkt(fsp);
- fsp->lp->tt.exch_done(seq);
out:
+ fsp->lp->tt.exch_done(seq);
fc_frame_free(fp);
- fc_fcp_pkt_release(fsp); /* drop hold for outstanding SRR */
}
/**
@@ -1747,8 +1746,6 @@ static void fc_fcp_srr_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
{
if (fc_fcp_lock_pkt(fsp))
goto out;
- fsp->lp->tt.exch_done(fsp->recov_seq);
- fsp->recov_seq = NULL;
switch (PTR_ERR(fp)) {
case -FC_EX_TIMEOUT:
if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY)
@@ -1764,7 +1761,7 @@ static void fc_fcp_srr_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
}
fc_fcp_unlock_pkt(fsp);
out:
- fc_fcp_pkt_release(fsp); /* drop hold for outstanding SRR */
+ fsp->lp->tt.exch_done(fsp->recov_seq);
}
/**
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 7/8] libfc: do not immediately retry the cmd when seq_send fails in fc_fcp_send_data
2011-05-16 23:45 [PATCH 0/8] libfc, libfcoe and fcoe patches for 2.6.40 Robert Love
` (5 preceding siblings ...)
2011-05-16 23:45 ` [PATCH 6/8] libfc: fix race in SRR response Robert Love
@ 2011-05-16 23:45 ` Robert Love
2011-05-16 23:46 ` [PATCH 8/8] libfcoe: Remove unnecessary module state checks Robert Love
7 siblings, 0 replies; 9+ messages in thread
From: Robert Love @ 2011-05-16 23:45 UTC (permalink / raw)
To: linux-scsi; +Cc: Ross Brattain, Yi Zou
From: Yi Zou <yi.zou@intel.com>
Currently, when seq_send() fails in fc_fcp_send_data(), fc_fcp_retry_cmd()
would complete this failed I/O directly and let scsi-ml retry. However, target side
is not notified which may hang the target. Instead, we should just bail out from
from fc_fcp_send_data and let scsi-ml times it out and aborts this I/O instead.
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 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 57704e8..9cd2149 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -681,8 +681,7 @@ static int fc_fcp_send_data(struct fc_fcp_pkt *fsp, struct fc_seq *seq,
error = lport->tt.seq_send(lport, seq, fp);
if (error) {
WARN_ON(1); /* send error should be rare */
- fc_fcp_retry_cmd(fsp);
- return 0;
+ return error;
}
fp = NULL;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 8/8] libfcoe: Remove unnecessary module state checks
2011-05-16 23:45 [PATCH 0/8] libfc, libfcoe and fcoe patches for 2.6.40 Robert Love
` (6 preceding siblings ...)
2011-05-16 23:45 ` [PATCH 7/8] libfc: do not immediately retry the cmd when seq_send fails in fc_fcp_send_data Robert Love
@ 2011-05-16 23:46 ` Robert Love
7 siblings, 0 replies; 9+ messages in thread
From: Robert Love @ 2011-05-16 23:46 UTC (permalink / raw)
To: linux-scsi; +Cc: Ross Brattain
libfcoe's interface consists of create, destroy, enable,
disable and create_vn2vn. These are currently module
paramaters added durring the module initialization. A
concern arose that the module parameters were being added
with write permissions before the module had completed
initialization. The following code was added to each
sysfs store file.
* Make sure the module has been initialized, and is not about to be
* removed. Module parameter sysfs files are writable before the
* module_init function is called and after module_exit.
*/
if (THIS_MODULE->state != MODULE_STATE_LIVE)
goto out_nodev;
This check was called out as unhelpful as the module can
go dead at any time and therefore its state isn't a reliable
thing to look at as a sign of stability and initialization
completion. Also, that functional interfaces like these
should be added after module initialization.
This patch removes the unnecessary checks and hopes to
disprove the concern about initialization ordering.
Recent fcoe transport rework changes now require fcoe
transports to register with libfcoe before any operation
can take place. libfcoe may access some static variables
but nothing that could cause a problem. Once a fcoe transport
is registered, libfcoe is usable and any interface calls will
be functional.
Signed-off-by: Robert Love <robert.w.love@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
---
drivers/scsi/fcoe/fcoe_transport.c | 40 ------------------------------------
1 files changed, 0 insertions(+), 40 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c
index f81f77c..41068e8 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -544,16 +544,6 @@ static int fcoe_transport_create(const char *buffer, struct kernel_param *kp)
struct fcoe_transport *ft = NULL;
enum fip_state fip_mode = (enum fip_state)(long)kp->arg;
-#ifdef CONFIG_LIBFCOE_MODULE
- /*
- * Make sure the module has been initialized, and is not about to be
- * removed. Module parameter sysfs files are writable before the
- * module_init function is called and after module_exit.
- */
- if (THIS_MODULE->state != MODULE_STATE_LIVE)
- goto out_nodev;
-#endif
-
mutex_lock(&ft_mutex);
netdev = fcoe_if_to_netdev(buffer);
@@ -618,16 +608,6 @@ static int fcoe_transport_destroy(const char *buffer, struct kernel_param *kp)
struct net_device *netdev = NULL;
struct fcoe_transport *ft = NULL;
-#ifdef CONFIG_LIBFCOE_MODULE
- /*
- * Make sure the module has been initialized, and is not about to be
- * removed. Module parameter sysfs files are writable before the
- * module_init function is called and after module_exit.
- */
- if (THIS_MODULE->state != MODULE_STATE_LIVE)
- goto out_nodev;
-#endif
-
mutex_lock(&ft_mutex);
netdev = fcoe_if_to_netdev(buffer);
@@ -672,16 +652,6 @@ static int fcoe_transport_disable(const char *buffer, struct kernel_param *kp)
struct net_device *netdev = NULL;
struct fcoe_transport *ft = NULL;
-#ifdef CONFIG_LIBFCOE_MODULE
- /*
- * Make sure the module has been initialized, and is not about to be
- * removed. Module parameter sysfs files are writable before the
- * module_init function is called and after module_exit.
- */
- if (THIS_MODULE->state != MODULE_STATE_LIVE)
- goto out_nodev;
-#endif
-
mutex_lock(&ft_mutex);
netdev = fcoe_if_to_netdev(buffer);
@@ -720,16 +690,6 @@ static int fcoe_transport_enable(const char *buffer, struct kernel_param *kp)
struct net_device *netdev = NULL;
struct fcoe_transport *ft = NULL;
-#ifdef CONFIG_LIBFCOE_MODULE
- /*
- * Make sure the module has been initialized, and is not about to be
- * removed. Module parameter sysfs files are writable before the
- * module_init function is called and after module_exit.
- */
- if (THIS_MODULE->state != MODULE_STATE_LIVE)
- goto out_nodev;
-#endif
-
mutex_lock(&ft_mutex);
netdev = fcoe_if_to_netdev(buffer);
^ permalink raw reply related [flat|nested] 9+ messages in thread