svn commit: r276613 - in stable/10: sys/cam/ctl sys/dev/iscsi usr.sbin/ctld usr.sbin/iscsid

Alexander Motin mav at FreeBSD.org
Sat Jan 3 13:08:11 UTC 2015


Author: mav
Date: Sat Jan  3 13:08:08 2015
New Revision: 276613
URL: https://svnweb.freebsd.org/changeset/base/276613

Log:
  MFC r275864: Make sequence numbers checks more strict.
  
  While we don't support MCS, hole in received sequence numbers may mean
  only PDU loss.  While we don't support lost PDU recovery, terminate the
  connection to avoid stuck commands.
  
  While there, improve handling of sequence numbers wrap after 2^32 PDUs.

Modified:
  stable/10/sys/cam/ctl/ctl_frontend_iscsi.c
  stable/10/sys/cam/ctl/ctl_frontend_iscsi.h
  stable/10/sys/dev/iscsi/iscsi.c
  stable/10/sys/dev/iscsi/iscsi_proto.h
  stable/10/usr.sbin/ctld/discovery.c
  stable/10/usr.sbin/ctld/login.c
  stable/10/usr.sbin/iscsid/discovery.c
  stable/10/usr.sbin/iscsid/login.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/cam/ctl/ctl_frontend_iscsi.c
==============================================================================
--- stable/10/sys/cam/ctl/ctl_frontend_iscsi.c	Sat Jan  3 11:52:43 2015	(r276612)
+++ stable/10/sys/cam/ctl/ctl_frontend_iscsi.c	Sat Jan  3 13:08:08 2015	(r276613)
@@ -237,19 +237,34 @@ cfiscsi_pdu_update_cmdsn(const struct ic
 	}
 #endif
 
-	/*
-	 * The target MUST silently ignore any non-immediate command outside
-	 * of this range.
-	 */
-	if (cmdsn < cs->cs_cmdsn || cmdsn > cs->cs_cmdsn + maxcmdsn_delta) {
-		CFISCSI_SESSION_UNLOCK(cs);
-		CFISCSI_SESSION_WARN(cs, "received PDU with CmdSN %d, "
-		    "while expected CmdSN was %d", cmdsn, cs->cs_cmdsn);
-		return (true);
-	}
+	if ((request->ip_bhs->bhs_opcode & ISCSI_BHS_OPCODE_IMMEDIATE) == 0) {
+		/*
+		 * The target MUST silently ignore any non-immediate command
+		 * outside of this range.
+		 */
+		if (ISCSI_SNLT(cmdsn, cs->cs_cmdsn) ||
+		    ISCSI_SNGT(cmdsn, cs->cs_cmdsn + maxcmdsn_delta)) {
+			CFISCSI_SESSION_UNLOCK(cs);
+			CFISCSI_SESSION_WARN(cs, "received PDU with CmdSN %u, "
+			    "while expected %u", cmdsn, cs->cs_cmdsn);
+			return (true);
+		}
 
-	if ((request->ip_bhs->bhs_opcode & ISCSI_BHS_OPCODE_IMMEDIATE) == 0)
+		/*
+		 * We don't support multiple connections now, so any
+		 * discontinuity in CmdSN means lost PDUs.  Since we don't
+		 * support PDU retransmission -- terminate the connection.
+		 */
+		if (cmdsn != cs->cs_cmdsn) {
+			CFISCSI_SESSION_UNLOCK(cs);
+			CFISCSI_SESSION_WARN(cs, "received PDU with CmdSN %u, "
+			    "while expected %u; dropping connection",
+			    cmdsn, cs->cs_cmdsn);
+			cfiscsi_session_terminate(cs);
+			return (true);
+		}
 		cs->cs_cmdsn++;
+	}
 
 	CFISCSI_SESSION_UNLOCK(cs);
 
@@ -896,6 +911,16 @@ cfiscsi_pdu_handle_data_out(struct icl_p
 		return;
 	}
 
+	if (cdw->cdw_datasn != ntohl(bhsdo->bhsdo_datasn)) {
+		CFISCSI_SESSION_WARN(cs, "received Data-Out PDU with "
+		    "DataSN %u, while expected %u; dropping connection",
+		    ntohl(bhsdo->bhsdo_datasn), cdw->cdw_datasn);
+		icl_pdu_free(request);
+		cfiscsi_session_terminate(cs);
+		return;
+	}
+	cdw->cdw_datasn++;
+
 	io = cdw->cdw_ctl_io;
 	KASSERT((io->io_hdr.flags & CTL_FLAG_DATA_MASK) != CTL_FLAG_DATA_IN,
 	    ("CTL_FLAG_DATA_IN"));
@@ -2654,6 +2679,7 @@ cfiscsi_datamove_out(union ctl_io *io)
 	cdw->cdw_target_transfer_tag = target_transfer_tag;
 	cdw->cdw_initiator_task_tag = bhssc->bhssc_initiator_task_tag;
 	cdw->cdw_r2t_end = io->scsiio.kern_data_len;
+	cdw->cdw_datasn = 0;
 
 	/* Set initial data pointer for the CDW respecting ext_data_filled. */
 	if (io->scsiio.kern_sg_entries > 0) {

Modified: stable/10/sys/cam/ctl/ctl_frontend_iscsi.h
==============================================================================
--- stable/10/sys/cam/ctl/ctl_frontend_iscsi.h	Sat Jan  3 11:52:43 2015	(r276612)
+++ stable/10/sys/cam/ctl/ctl_frontend_iscsi.h	Sat Jan  3 13:08:08 2015	(r276613)
@@ -58,6 +58,7 @@ struct cfiscsi_data_wait {
 	char				*cdw_sg_addr;
 	size_t				cdw_sg_len;
 	uint32_t			cdw_r2t_end;
+	uint32_t			cdw_datasn;
 };
 
 #define CFISCSI_SESSION_STATE_INVALID		0

Modified: stable/10/sys/dev/iscsi/iscsi.c
==============================================================================
--- stable/10/sys/dev/iscsi/iscsi.c	Sat Jan  3 11:52:43 2015	(r276612)
+++ stable/10/sys/dev/iscsi/iscsi.c	Sat Jan  3 13:08:08 2015	(r276613)
@@ -197,7 +197,7 @@ iscsi_pdu_prepare(struct icl_pdu *reques
 	 * Data-Out PDU does not contain CmdSN.
 	 */
 	if (bhssc->bhssc_opcode != ISCSI_BHS_OPCODE_SCSI_DATA_OUT) {
-		if (is->is_cmdsn > is->is_maxcmdsn &&
+		if (ISCSI_SNGT(is->is_cmdsn, is->is_maxcmdsn) &&
 		    (bhssc->bhssc_opcode & ISCSI_BHS_OPCODE_IMMEDIATE) == 0) {
 			/*
 			 * Current MaxCmdSN prevents us from sending any more
@@ -206,8 +206,10 @@ iscsi_pdu_prepare(struct icl_pdu *reques
 			 * or by maintenance thread.
 			 */
 #if 0
-			ISCSI_SESSION_DEBUG(is, "postponing send, CmdSN %d, ExpCmdSN %d, MaxCmdSN %d, opcode 0x%x",
-			    is->is_cmdsn, is->is_expcmdsn, is->is_maxcmdsn, bhssc->bhssc_opcode);
+			ISCSI_SESSION_DEBUG(is, "postponing send, CmdSN %u, "
+			    "ExpCmdSN %u, MaxCmdSN %u, opcode 0x%x",
+			    is->is_cmdsn, is->is_expcmdsn, is->is_maxcmdsn,
+			    bhssc->bhssc_opcode);
 #endif
 			return (true);
 		}
@@ -616,7 +618,7 @@ iscsi_pdu_update_statsn(const struct icl
 {
 	const struct iscsi_bhs_data_in *bhsdi;
 	struct iscsi_session *is;
-	uint32_t expcmdsn, maxcmdsn;
+	uint32_t expcmdsn, maxcmdsn, statsn;
 
 	is = PDU_SESSION(response);
 
@@ -635,26 +637,27 @@ iscsi_pdu_update_statsn(const struct icl
 	 */
 	if (bhsdi->bhsdi_opcode != ISCSI_BHS_OPCODE_SCSI_DATA_IN ||
 	    (bhsdi->bhsdi_flags & BHSDI_FLAGS_S) != 0) {
-		if (ntohl(bhsdi->bhsdi_statsn) < is->is_statsn) {
-			ISCSI_SESSION_WARN(is,
-			    "PDU StatSN %d >= session StatSN %d, opcode 0x%x",
-			    is->is_statsn, ntohl(bhsdi->bhsdi_statsn),
-			    bhsdi->bhsdi_opcode);
+		statsn = ntohl(bhsdi->bhsdi_statsn);
+		if (statsn != is->is_statsn && statsn != (is->is_statsn + 1)) {
+			/* XXX: This is normal situation for MCS */
+			ISCSI_SESSION_WARN(is, "PDU 0x%x StatSN %u != "
+			    "session ExpStatSN %u (or + 1); reconnecting",
+			    bhsdi->bhsdi_opcode, statsn, is->is_statsn);
+			iscsi_session_reconnect(is);
 		}
-		is->is_statsn = ntohl(bhsdi->bhsdi_statsn);
+		if (ISCSI_SNGT(statsn, is->is_statsn))
+			is->is_statsn = statsn;
 	}
 
 	expcmdsn = ntohl(bhsdi->bhsdi_expcmdsn);
 	maxcmdsn = ntohl(bhsdi->bhsdi_maxcmdsn);
 
-	/*
-	 * XXX: Compare using Serial Arithmetic Sense.
-	 */
-	if (maxcmdsn + 1 < expcmdsn) {
-		ISCSI_SESSION_DEBUG(is, "PDU MaxCmdSN %d + 1 < PDU ExpCmdSN %d; ignoring",
+	if (ISCSI_SNLT(maxcmdsn + 1, expcmdsn)) {
+		ISCSI_SESSION_DEBUG(is,
+		    "PDU MaxCmdSN %u + 1 < PDU ExpCmdSN %u; ignoring",
 		    maxcmdsn, expcmdsn);
 	} else {
-		if (maxcmdsn > is->is_maxcmdsn) {
+		if (ISCSI_SNGT(maxcmdsn, is->is_maxcmdsn)) {
 			is->is_maxcmdsn = maxcmdsn;
 
 			/*
@@ -663,15 +666,19 @@ iscsi_pdu_update_statsn(const struct icl
 			 */
 			if (!STAILQ_EMPTY(&is->is_postponed))
 				cv_signal(&is->is_maintenance_cv);
-		} else if (maxcmdsn < is->is_maxcmdsn) {
-			ISCSI_SESSION_DEBUG(is, "PDU MaxCmdSN %d < session MaxCmdSN %d; ignoring",
+		} else if (ISCSI_SNLT(maxcmdsn, is->is_maxcmdsn)) {
+			/* XXX: This is normal situation for MCS */
+			ISCSI_SESSION_DEBUG(is,
+			    "PDU MaxCmdSN %u < session MaxCmdSN %u; ignoring",
 			    maxcmdsn, is->is_maxcmdsn);
 		}
 
-		if (expcmdsn > is->is_expcmdsn) {
+		if (ISCSI_SNGT(expcmdsn, is->is_expcmdsn)) {
 			is->is_expcmdsn = expcmdsn;
-		} else if (expcmdsn < is->is_expcmdsn) {
-			ISCSI_SESSION_DEBUG(is, "PDU ExpCmdSN %d < session ExpCmdSN %d; ignoring",
+		} else if (ISCSI_SNLT(expcmdsn, is->is_expcmdsn)) {
+			/* XXX: This is normal situation for MCS */
+			ISCSI_SESSION_DEBUG(is,
+			    "PDU ExpCmdSN %u < session ExpCmdSN %u; ignoring",
 			    expcmdsn, is->is_expcmdsn);
 		}
 	}

Modified: stable/10/sys/dev/iscsi/iscsi_proto.h
==============================================================================
--- stable/10/sys/dev/iscsi/iscsi_proto.h	Sat Jan  3 11:52:43 2015	(r276612)
+++ stable/10/sys/dev/iscsi/iscsi_proto.h	Sat Jan  3 13:08:08 2015	(r276613)
@@ -38,6 +38,9 @@
 #define __CTASSERT(x, y)	typedef char __assert_ ## y [(x) ? 1 : -1]
 #endif
 
+#define	ISCSI_SNGT(x, y)	((int32_t)(x) - (int32_t)(y) > 0)
+#define	ISCSI_SNLT(x, y)	((int32_t)(x) - (int32_t)(y) < 0)
+
 #define	ISCSI_BHS_SIZE			48
 #define	ISCSI_HEADER_DIGEST_SIZE	4
 #define	ISCSI_DATA_DIGEST_SIZE		4

Modified: stable/10/usr.sbin/ctld/discovery.c
==============================================================================
--- stable/10/usr.sbin/ctld/discovery.c	Sat Jan  3 11:52:43 2015	(r276612)
+++ stable/10/usr.sbin/ctld/discovery.c	Sat Jan  3 13:08:08 2015	(r276613)
@@ -65,13 +65,13 @@ text_receive(struct connection *conn)
 	 */
 	if ((bhstr->bhstr_flags & BHSTR_FLAGS_CONTINUE) != 0)
 		log_errx(1, "received Text PDU with unsupported \"C\" flag");
-	if (ntohl(bhstr->bhstr_cmdsn) < conn->conn_cmdsn) {
+	if (ISCSI_SNLT(ntohl(bhstr->bhstr_cmdsn), conn->conn_cmdsn)) {
 		log_errx(1, "received Text PDU with decreasing CmdSN: "
-		    "was %d, is %d", conn->conn_cmdsn, ntohl(bhstr->bhstr_cmdsn));
+		    "was %u, is %u", conn->conn_cmdsn, ntohl(bhstr->bhstr_cmdsn));
 	}
 	if (ntohl(bhstr->bhstr_expstatsn) != conn->conn_statsn) {
 		log_errx(1, "received Text PDU with wrong StatSN: "
-		    "is %d, should be %d", ntohl(bhstr->bhstr_expstatsn),
+		    "is %u, should be %u", ntohl(bhstr->bhstr_expstatsn),
 		    conn->conn_statsn);
 	}
 	conn->conn_cmdsn = ntohl(bhstr->bhstr_cmdsn);
@@ -120,14 +120,14 @@ logout_receive(struct connection *conn)
 	if ((bhslr->bhslr_reason & 0x7f) != BHSLR_REASON_CLOSE_SESSION)
 		log_debugx("received Logout PDU with invalid reason 0x%x; "
 		    "continuing anyway", bhslr->bhslr_reason & 0x7f);
-	if (ntohl(bhslr->bhslr_cmdsn) < conn->conn_cmdsn) {
+	if (ISCSI_SNLT(ntohl(bhslr->bhslr_cmdsn), conn->conn_cmdsn)) {
 		log_errx(1, "received Logout PDU with decreasing CmdSN: "
-		    "was %d, is %d", conn->conn_cmdsn,
+		    "was %u, is %u", conn->conn_cmdsn,
 		    ntohl(bhslr->bhslr_cmdsn));
 	}
 	if (ntohl(bhslr->bhslr_expstatsn) != conn->conn_statsn) {
 		log_errx(1, "received Logout PDU with wrong StatSN: "
-		    "is %d, should be %d", ntohl(bhslr->bhslr_expstatsn),
+		    "is %u, should be %u", ntohl(bhslr->bhslr_expstatsn),
 		    conn->conn_statsn);
 	}
 	conn->conn_cmdsn = ntohl(bhslr->bhslr_cmdsn);

Modified: stable/10/usr.sbin/ctld/login.c
==============================================================================
--- stable/10/usr.sbin/ctld/login.c	Sat Jan  3 11:52:43 2015	(r276612)
+++ stable/10/usr.sbin/ctld/login.c	Sat Jan  3 13:08:08 2015	(r276613)
@@ -127,17 +127,17 @@ login_receive(struct connection *conn, b
 		log_errx(1, "received Login PDU with unsupported "
 		    "Version-min 0x%x", bhslr->bhslr_version_min);
 	}
-	if (ntohl(bhslr->bhslr_cmdsn) < conn->conn_cmdsn) {
+	if (ISCSI_SNLT(ntohl(bhslr->bhslr_cmdsn), conn->conn_cmdsn)) {
 		login_send_error(request, 0x02, 0x05);
 		log_errx(1, "received Login PDU with decreasing CmdSN: "
-		    "was %d, is %d", conn->conn_cmdsn,
+		    "was %u, is %u", conn->conn_cmdsn,
 		    ntohl(bhslr->bhslr_cmdsn));
 	}
 	if (initial == false &&
 	    ntohl(bhslr->bhslr_expstatsn) != conn->conn_statsn) {
 		login_send_error(request, 0x02, 0x05);
 		log_errx(1, "received Login PDU with wrong ExpStatSN: "
-		    "is %d, should be %d", ntohl(bhslr->bhslr_expstatsn),
+		    "is %u, should be %u", ntohl(bhslr->bhslr_expstatsn),
 		    conn->conn_statsn);
 	}
 	conn->conn_cmdsn = ntohl(bhslr->bhslr_cmdsn);

Modified: stable/10/usr.sbin/iscsid/discovery.c
==============================================================================
--- stable/10/usr.sbin/iscsid/discovery.c	Sat Jan  3 11:52:43 2015	(r276612)
+++ stable/10/usr.sbin/iscsid/discovery.c	Sat Jan  3 13:08:08 2015	(r276613)
@@ -66,7 +66,7 @@ text_receive(struct connection *conn)
 		log_errx(1, "received Text PDU with unsupported \"C\" flag");
 	if (ntohl(bhstr->bhstr_statsn) != conn->conn_statsn + 1) {
 		log_errx(1, "received Text PDU with wrong StatSN: "
-		    "is %d, should be %d", ntohl(bhstr->bhstr_statsn),
+		    "is %u, should be %u", ntohl(bhstr->bhstr_statsn),
 		    conn->conn_statsn + 1);
 	}
 	conn->conn_statsn = ntohl(bhstr->bhstr_statsn);
@@ -112,7 +112,7 @@ logout_receive(struct connection *conn)
 		    ntohs(bhslr->bhslr_response));
 	if (ntohl(bhslr->bhslr_statsn) != conn->conn_statsn + 1) {
 		log_errx(1, "received Logout PDU with wrong StatSN: "
-		    "is %d, should be %d", ntohl(bhslr->bhslr_statsn),
+		    "is %u, should be %u", ntohl(bhslr->bhslr_statsn),
 		    conn->conn_statsn + 1);
 	}
 	conn->conn_statsn = ntohl(bhslr->bhslr_statsn);

Modified: stable/10/usr.sbin/iscsid/login.c
==============================================================================
--- stable/10/usr.sbin/iscsid/login.c	Sat Jan  3 11:52:43 2015	(r276612)
+++ stable/10/usr.sbin/iscsid/login.c	Sat Jan  3 13:08:08 2015	(r276613)
@@ -257,7 +257,7 @@ login_receive(struct connection *conn)
 		 * to be bug in NetBSD iSCSI target.
 		 */
 		log_warnx("received Login PDU with wrong StatSN: "
-		    "is %d, should be %d", ntohl(bhslr->bhslr_statsn),
+		    "is %u, should be %u", ntohl(bhslr->bhslr_statsn),
 		    conn->conn_statsn + 1);
 	}
 	conn->conn_tsih = ntohs(bhslr->bhslr_tsih);


More information about the svn-src-all mailing list