[Open-FCoE] [RFC PATCH] libfc: fixed a read IO data integrity issue when a IO data frame lost

Vasu Dev vasu.dev at intel.com
Thu Jan 22 18:57:25 UTC 2009

The fc_fcp_resp was detecting lost data frame correctly during read IO but
this function set FC_SRB_RCV_STATUS in fsp->state before detecting and causing
REC/SRR to recover lost data frame.

This early FC_SRB_RCV_STATUS setting led to passing up SCSI command with good
status though cmd data was incomplete due lost data frame if target
doesn't support REC, in this the REC RJT response handling cleared the
FC_RP_FLAGS_REC_SUPPORTED and started fcp timer and in turn call to
fc_fcp_complete_locked from fc_fcp_timeout passed up SCSI command with good
status since FC_SRB_RCV_STATUS was already set and FC_RP_FLAGS_REC_SUPPORTED

This patch moved setting of FC_SRB_RCV_STATUS after checks for lost data
in fc_fcp_resp, to prevent above mentioned call to fc_fcp_complete_locked
instead enable call to fc_timeout_error from fc_fcp_timeout when above
mentioned REC is rejected after a read data frame is lost.

Removed clearing of FC_SRB_RCV_STATUS in fc_fcp_srr since after this patch
it won't be set when calling fc_fcp_srr.

I'm looking into why this didn't got caught by fc_fcp_complete_locked and
perhaps fc_fcp_complete_locked should have fix for this as well.

After this patch I don't see any more data integrity issue if a data frame is
lost and I see data frame lost case led to scsi-ml retries to get good READ IO

Signed-off-by: Vasu Dev <vasu.dev at 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 f440aac..a94bab2 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -799,7 +799,6 @@ static void fc_fcp_resp(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
-	fsp->state |= FC_SRB_RCV_STATUS;
 	 * Check for missing or extra data frames.
@@ -820,6 +819,8 @@ static void fc_fcp_resp(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 		       fsp->xfer_len, expected_len, fsp->data_len);
+	fsp->state |= FC_SRB_RCV_STATUS;
@@ -1540,7 +1541,6 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset)
 	fsp->recov_seq = seq;
 	fsp->xfer_len = offset;
 	fsp->xfer_contig_end = offset;
-	fsp->state &= ~FC_SRB_RCV_STATUS;
 	fc_fcp_pkt_hold(fsp);		/* hold for outstanding SRR */

More information about the devel mailing list