iscsi-target: remove support for obsolete markers

Support for markers is currently broken because of a bug in
iscsi_enforce_integrity_rules(): the "IFMarkInt_Reject" and
"OFMarkInt_Reject" variables are always equal to 1 in
iscsi_enforce_integrity_rules().

Moreover, fixed interval markers keys (IFMarker, OFMarker, IFMarkInt
and OFMarkInt) are obsolete according to iSCSI RFC 7143:

>From http://tools.ietf.org/html/rfc7143#section-13.25:

   13.25.  Obsoleted Keys

   This document obsoletes the following keys defined in [RFC3720]:
   IFMarker, OFMarker, OFMarkInt, and IFMarkInt.  However, iSCSI
   implementations compliant to this document may still receive these
   obsoleted keys -- i.e., in a responder role -- in a text negotiation.

   When an IFMarker or OFMarker key is received, a compliant iSCSI
   implementation SHOULD respond with the constant "Reject" value.  The
   implementation MAY alternatively respond with a "No" value.

   However, the implementation MUST NOT respond with a "NotUnderstood"
   value for either of these keys.

   When an IFMarkInt or OFMarkInt key is received, a compliant iSCSI
   implementation MUST respond with the constant "Reject" value.  The
   implementation MUST NOT respond with a "NotUnderstood" value for
   either of these keys.

This patch disables markers by turning the corresponding parameters to
read-only. The default value of IFMarker and OFMarker remains "No" but
the user cannot change it to "Yes" anymore. The new value of IFMarkInt
and OFMarkInt is "Reject".

(Drop left-over iscsi_get_value_from_number_range + make configfs
 parameters attrs R/W nops - nab)
Signed-off-by: Christophe Vu-Brugier <cvubrugier@fastmail.fm>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

drivers/target/iscsi/iscsi_target.c

@@ -2736,11 +2736,7 @@ static int iscsit_send_datain(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
 	cmd->iov_data_count = iov_count;
 	cmd->tx_size = tx_size;
 
-	/* sendpage is preferred but can't insert markers */
-	if (!conn->conn_ops->IFMarker)
-		ret = iscsit_fe_sendpage_sg(cmd, conn);
-	else
-		ret = iscsit_send_tx_data(cmd, conn, 0);
+	ret = iscsit_fe_sendpage_sg(cmd, conn);
 
 	iscsit_unmap_iovec(cmd);
 
@@ -4072,17 +4068,9 @@ static int iscsi_target_rx_opcode(struct iscsi_conn *conn, unsigned char *buf)
 			" opcode while ERL=0, closing iSCSI connection.\n");
 			return -1;
 		}
-		if (!conn->conn_ops->OFMarker) {
-			pr_err("Unable to recover from unknown"
-			" opcode while OFMarker=No, closing iSCSI"
-				" connection.\n");
-			return -1;
-		}
-		if (iscsit_recover_from_unknown_opcode(conn) < 0) {
-			pr_err("Unable to recover from unknown"
-				" opcode, closing iSCSI connection.\n");
-			return -1;
-		}
+		pr_err("Unable to recover from unknown opcode while OFMarker=No,"
+		       " closing iSCSI connection.\n");
+		ret = -1;
 		break;
 	}
 

drivers/target/iscsi/iscsi_target_erl0.c

@@ -956,56 +956,3 @@ void iscsit_take_action_for_connection_exit(struct iscsi_conn *conn)
 
 	iscsit_handle_connection_cleanup(conn);
 }
-
-/*
- *	This is the simple function that makes the magic of
- *	sync and steering happen in the follow paradoxical order:
- *
- *	0) Receive conn->of_marker (bytes left until next OFMarker)
- *	   bytes into an offload buffer.  When we pass the exact number
- *	   of bytes in conn->of_marker, iscsit_dump_data_payload() and hence
- *	   rx_data() will automatically receive the identical u32 marker
- *	   values and store it in conn->of_marker_offset;
- *	1) Now conn->of_marker_offset will contain the offset to the start
- *	   of the next iSCSI PDU.  Dump these remaining bytes into another
- *	   offload buffer.
- *	2) We are done!
- *	   Next byte in the TCP stream will contain the next iSCSI PDU!
- *	   Cool Huh?!
- */
-int iscsit_recover_from_unknown_opcode(struct iscsi_conn *conn)
-{
-	/*
-	 * Make sure the remaining bytes to next maker is a sane value.
-	 */
-	if (conn->of_marker > (conn->conn_ops->OFMarkInt * 4)) {
-		pr_err("Remaining bytes to OFMarker: %u exceeds"
-			" OFMarkInt bytes: %u.\n", conn->of_marker,
-				conn->conn_ops->OFMarkInt * 4);
-		return -1;
-	}
-
-	pr_debug("Advancing %u bytes in TCP stream to get to the"
-			" next OFMarker.\n", conn->of_marker);
-
-	if (iscsit_dump_data_payload(conn, conn->of_marker, 0) < 0)
-		return -1;
-
-	/*
-	 * Make sure the offset marker we retrived is a valid value.
-	 */
-	if (conn->of_marker_offset > (ISCSI_HDR_LEN + (ISCSI_CRC_LEN * 2) +
-	    conn->conn_ops->MaxRecvDataSegmentLength)) {
-		pr_err("OfMarker offset value: %u exceeds limit.\n",
-			conn->of_marker_offset);
-		return -1;
-	}
-
-	pr_debug("Discarding %u bytes of TCP stream to get to the"
-			" next iSCSI Opcode.\n", conn->of_marker_offset);
-
-	if (iscsit_dump_data_payload(conn, conn->of_marker_offset, 0) < 0)
-		return -1;
-
-	return 0;
-}

drivers/target/iscsi/iscsi_target_erl0.h

@@ -10,6 +10,5 @@ extern void iscsit_connection_reinstatement_rcfr(struct iscsi_conn *);
 extern void iscsit_cause_connection_reinstatement(struct iscsi_conn *, int);
 extern void iscsit_fall_back_to_erl0(struct iscsi_session *);
 extern void iscsit_take_action_for_connection_exit(struct iscsi_conn *);
-extern int iscsit_recover_from_unknown_opcode(struct iscsi_conn *);
 
 #endif   /*** ISCSI_TARGET_ERL0_H ***/

drivers/target/iscsi/iscsi_target_login.c

@@ -410,8 +410,6 @@ static int iscsi_login_zero_tsih_s2(
 	if (iscsi_change_param_sprintf(conn, "ErrorRecoveryLevel=%d", na->default_erl))
 		return -1;
 
-	if (iscsi_login_disable_FIM_keys(conn->param_list, conn) < 0)
-		return -1;
 	/*
 	 * Set RDMAExtensions=Yes by default for iSER enabled network portals
 	 */
@@ -477,59 +475,6 @@ static int iscsi_login_zero_tsih_s2(
 	return 0;
 }
 
-/*
- * Remove PSTATE_NEGOTIATE for the four FIM related keys.
- * The Initiator node will be able to enable FIM by proposing them itself.
- */
-int iscsi_login_disable_FIM_keys(
-	struct iscsi_param_list *param_list,
-	struct iscsi_conn *conn)
-{
-	struct iscsi_param *param;
-
-	param = iscsi_find_param_from_key("OFMarker", param_list);
-	if (!param) {
-		pr_err("iscsi_find_param_from_key() for"
-				" OFMarker failed\n");
-		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
-				ISCSI_LOGIN_STATUS_NO_RESOURCES);
-		return -1;
-	}
-	param->state &= ~PSTATE_NEGOTIATE;
-
-	param = iscsi_find_param_from_key("OFMarkInt", param_list);
-	if (!param) {
-		pr_err("iscsi_find_param_from_key() for"
-				" IFMarker failed\n");
-		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
-				ISCSI_LOGIN_STATUS_NO_RESOURCES);
-		return -1;
-	}
-	param->state &= ~PSTATE_NEGOTIATE;
-
-	param = iscsi_find_param_from_key("IFMarker", param_list);
-	if (!param) {
-		pr_err("iscsi_find_param_from_key() for"
-				" IFMarker failed\n");
-		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
-				ISCSI_LOGIN_STATUS_NO_RESOURCES);
-		return -1;
-	}
-	param->state &= ~PSTATE_NEGOTIATE;
-
-	param = iscsi_find_param_from_key("IFMarkInt", param_list);
-	if (!param) {
-		pr_err("iscsi_find_param_from_key() for"
-				" IFMarker failed\n");
-		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
-				ISCSI_LOGIN_STATUS_NO_RESOURCES);
-		return -1;
-	}
-	param->state &= ~PSTATE_NEGOTIATE;
-
-	return 0;
-}
-
 static int iscsi_login_non_zero_tsih_s1(
 	struct iscsi_conn *conn,
 	unsigned char *buf)
@@ -616,7 +561,7 @@ static int iscsi_login_non_zero_tsih_s2(
 	if (iscsi_change_param_sprintf(conn, "TargetPortalGroupTag=%hu", sess->tpg->tpgt))
 		return -1;
 
-	return iscsi_login_disable_FIM_keys(conn->param_list, conn);
+	return 0;
 }
 
 int iscsi_login_post_auth_non_zero_tsih(
@@ -765,7 +710,6 @@ int iscsi_post_login_handler(
 	conn->conn_state = TARG_CONN_STATE_LOGGED_IN;
 
 	iscsi_set_connection_parameters(conn->conn_ops, conn->param_list);
-	iscsit_set_sync_and_steering_values(conn);
 	/*
 	 * SCSI Initiator -> SCSI Target Port Mapping
 	 */

drivers/target/iscsi/iscsi_target_login.h

@@ -16,6 +16,5 @@ extern int iscsi_post_login_handler(struct iscsi_np *, struct iscsi_conn *, u8);
 extern void iscsi_target_login_sess_out(struct iscsi_conn *, struct iscsi_np *,
 				bool, bool);
 extern int iscsi_target_login_thread(void *);
-extern int iscsi_login_disable_FIM_keys(struct iscsi_param_list *, struct iscsi_conn *);
 
 #endif   /*** ISCSI_TARGET_LOGIN_H ***/

drivers/target/iscsi/iscsi_target_parameters.h

@@ -138,8 +138,8 @@ extern void iscsi_set_session_parameters(struct iscsi_sess_ops *,
 #define INITIAL_SESSIONTYPE			NORMAL
 #define INITIAL_IFMARKER			NO
 #define INITIAL_OFMARKER			NO
-#define INITIAL_IFMARKINT			"2048~65535"
-#define INITIAL_OFMARKINT			"2048~65535"
+#define INITIAL_IFMARKINT			REJECT
+#define INITIAL_OFMARKINT			REJECT
 
 /*
  * Initial values for iSER parameters following RFC-5046 Section 6
@@ -239,10 +239,9 @@ extern void iscsi_set_session_parameters(struct iscsi_sess_ops *,
 #define TYPERANGE_AUTH			0x0200
 #define TYPERANGE_DIGEST		0x0400
 #define TYPERANGE_ISCSINAME		0x0800
-#define TYPERANGE_MARKINT		0x1000
-#define TYPERANGE_SESSIONTYPE		0x2000
-#define TYPERANGE_TARGETADDRESS		0x4000
-#define TYPERANGE_UTF8			0x8000
+#define TYPERANGE_SESSIONTYPE		0x1000
+#define TYPERANGE_TARGETADDRESS		0x2000
+#define TYPERANGE_UTF8			0x4000
 
 #define IS_TYPERANGE_0_TO_2(p)		((p)->type_range & TYPERANGE_0_TO_2)
 #define IS_TYPERANGE_0_TO_3600(p)	((p)->type_range & TYPERANGE_0_TO_3600)

drivers/target/iscsi/iscsi_target_util.c

@@ -809,54 +809,6 @@ void iscsit_inc_session_usage_count(struct iscsi_session *sess)
 	spin_unlock_bh(&sess->session_usage_lock);
 }
 
-/*
- *	Setup conn->if_marker and conn->of_marker values based upon
- *	the initial marker-less interval. (see iSCSI v19 A.2)
- */
-int iscsit_set_sync_and_steering_values(struct iscsi_conn *conn)
-{
-	int login_ifmarker_count = 0, login_ofmarker_count = 0, next_marker = 0;
-	/*
-	 * IFMarkInt and OFMarkInt are negotiated as 32-bit words.
-	 */
-	u32 IFMarkInt = (conn->conn_ops->IFMarkInt * 4);
-	u32 OFMarkInt = (conn->conn_ops->OFMarkInt * 4);
-
-	if (conn->conn_ops->OFMarker) {
-		/*
-		 * Account for the first Login Command received not
-		 * via iscsi_recv_msg().
-		 */
-		conn->of_marker += ISCSI_HDR_LEN;
-		if (conn->of_marker <= OFMarkInt) {
-			conn->of_marker = (OFMarkInt - conn->of_marker);
-		} else {
-			login_ofmarker_count = (conn->of_marker / OFMarkInt);
-			next_marker = (OFMarkInt * (login_ofmarker_count + 1)) +
-					(login_ofmarker_count * MARKER_SIZE);
-			conn->of_marker = (next_marker - conn->of_marker);
-		}
-		conn->of_marker_offset = 0;
-		pr_debug("Setting OFMarker value to %u based on Initial"
-			" Markerless Interval.\n", conn->of_marker);
-	}
-
-	if (conn->conn_ops->IFMarker) {
-		if (conn->if_marker <= IFMarkInt) {
-			conn->if_marker = (IFMarkInt - conn->if_marker);
-		} else {
-			login_ifmarker_count = (conn->if_marker / IFMarkInt);
-			next_marker = (IFMarkInt * (login_ifmarker_count + 1)) +
-					(login_ifmarker_count * MARKER_SIZE);
-			conn->if_marker = (next_marker - conn->if_marker);
-		}
-		pr_debug("Setting IFMarker value to %u based on Initial"
-			" Markerless Interval.\n", conn->if_marker);
-	}
-
-	return 0;
-}
-
 struct iscsi_conn *iscsit_get_conn_from_cid(struct iscsi_session *sess, u16 cid)
 {
 	struct iscsi_conn *conn;

drivers/target/iscsi/iscsi_target_util.h

@@ -34,7 +34,6 @@ extern void iscsit_free_cmd(struct iscsi_cmd *, bool);
 extern int iscsit_check_session_usage_count(struct iscsi_session *);
 extern void iscsit_dec_session_usage_count(struct iscsi_session *);
 extern void iscsit_inc_session_usage_count(struct iscsi_session *);
-extern int iscsit_set_sync_and_steering_values(struct iscsi_conn *);
 extern struct iscsi_conn *iscsit_get_conn_from_cid(struct iscsi_session *, u16);
 extern struct iscsi_conn *iscsit_get_conn_from_cid_rcfr(struct iscsi_session *, u16);
 extern void iscsit_check_conn_usage_count(struct iscsi_conn *);

include/target/iscsi/iscsi_target_core.h

@@ -248,10 +248,6 @@ struct iscsi_conn_ops {
 	u8	DataDigest;			/* [0,1] == [None,CRC32C] */
 	u32	MaxRecvDataSegmentLength;	/* [512..2**24-1] */
 	u32	MaxXmitDataSegmentLength;	/* [512..2**24-1] */
-	u8	OFMarker;			/* [0,1] == [No,Yes] */
-	u8	IFMarker;			/* [0,1] == [No,Yes] */
-	u32	OFMarkInt;			/* [1..65535] */
-	u32	IFMarkInt;			/* [1..65535] */
 	/*
 	 * iSER specific connection parameters
 	 */
@@ -532,12 +528,6 @@ struct iscsi_conn {
 	u32			exp_statsn;
 	/* Per connection status sequence number */
 	u32			stat_sn;
-	/* IFMarkInt's Current Value */
-	u32			if_marker;
-	/* OFMarkInt's Current Value */
-	u32			of_marker;
-	/* Used for calculating OFMarker offset to next PDU */
-	u32			of_marker_offset;
 #define IPV6_ADDRESS_SPACE				48
 	unsigned char		login_ip[IPV6_ADDRESS_SPACE];
 	unsigned char		local_ip[IPV6_ADDRESS_SPACE];