[iscsi] Accept NOP-In PDUs sent by the target

Some iSCSI targets (observed with a Synology DS207+ NAS) send
unsolicited NOP-Ins to the initiator.  RFC 3720 is remarkably unclear
and possibly self-contradictory on how NOPs are supposed to work, but
it seems as though we can legitimately just ignore any unsolicited
NOP-In PDU.

Reported-by: Marc Lecuyer <marc@maxiscreen.com>
Originally-implemented-by: Thomas Miletich <thomas.miletich@gmail.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/include/ipxe/iscsi.h

@@ -93,6 +93,9 @@ struct iscsi_bhs_common {
 /** iSCSI tag magic marker */
 #define ISCSI_TAG_MAGIC 0x18ae0000
 
+/** iSCSI reserved tag value */
+#define ISCSI_TAG_RESERVED 0xffffffff
+
 /**
  * iSCSI basic header segment common request fields
  *
@@ -455,6 +458,36 @@ struct iscsi_bhs_r2t {
 /** R2T opcode */
 #define ISCSI_OPCODE_R2T 0x31
 
+/**
+ * iSCSI NOP-In basic header segment
+ *
+ */
+struct iscsi_nop_in {
+	/** Opcode */
+	uint8_t opcode;
+	/** Reserved */
+	uint8_t reserved_a[3];
+	/** Segment lengths */
+	union iscsi_segment_lengths lengths;
+	/** Logical Unit Number */
+	struct scsi_lun lun;
+	/** Initiator Task Tag */
+	uint32_t itt;
+	/** Target Transfer Tag */
+	uint32_t ttt;
+	/** Status sequence number */
+	uint32_t statsn;
+	/** Expected command sequence number */
+	uint32_t expcmdsn;
+	/** Maximum command sequence number */
+	uint32_t maxcmdsn;
+	/** Reserved */
+	uint8_t reserved_b[12];
+};
+
+/** NOP-In opcode */
+#define ISCSI_OPCODE_NOP_IN 0x20
+
 /**
  * An iSCSI basic header segment
  */
@@ -468,6 +501,7 @@ union iscsi_bhs {
 	struct iscsi_bhs_data_in data_in;
 	struct iscsi_bhs_data_out data_out;
 	struct iscsi_bhs_r2t r2t;
+	struct iscsi_nop_in nop_in;
 	unsigned char bytes[ sizeof ( struct iscsi_bhs_common ) ];
 };
 

src/net/tcp/iscsi.c

@@ -123,6 +123,10 @@ FEATURE ( FEATURE_PROTOCOL, "iSCSI", DHCP_EB_FEATURE_ISCSI, 1 );
 	__einfo_error ( EINFO_EPROTO_INVALID_CHAP_RESPONSE )
 #define EINFO_EPROTO_INVALID_CHAP_RESPONSE \
 	__einfo_uniqify ( EINFO_EPROTO, 0x04, "Invalid CHAP response" )
+#define EPROTO_INVALID_NOP_IN \
+	__einfo_error ( EINFO_EPROTO_INVALID_NOP_IN )
+#define EINFO_EPROTO_INVALID_NOP_IN \
+	__einfo_uniqify ( EINFO_EPROTO, 0x05, "Invalid NOP-In received" )
 
 /** iSCSI initiator name (explicitly specified) */
 static char *iscsi_explicit_initiator_iqn;
@@ -589,6 +593,50 @@ static int iscsi_tx_data_out ( struct iscsi_session *iscsi ) {
 	return xfer_deliver_iob ( &iscsi->socket, iobuf );
 }
 
+/**
+ * Receive data segment of an iSCSI NOP-In
+ *
+ * @v iscsi		iSCSI session
+ * @v data		Received data
+ * @v len		Length of received data
+ * @v remaining		Data remaining after this data
+ * @ret rc		Return status code
+ */
+static int iscsi_rx_nop_in ( struct iscsi_session *iscsi,
+			     const void *data __unused, size_t len __unused,
+			     size_t remaining __unused ) {
+	struct iscsi_nop_in *nop_in = &iscsi->rx_bhs.nop_in;
+
+	DBGC2 ( iscsi, "iSCSI %p received NOP-In\n", iscsi );
+
+	/* RFC 3720 section 10.19 states that "when a target sends a
+	 * NOP-In that is not a response to a Nop-Out received from
+	 * the initiator, the Initiator Task Tag MUST be set to
+	 * 0xffffffff", and section 10.18 states that "upon receipt of
+	 * a NOP-In with the Target Transfer Tag set to a valid value
+	 * (not the reserved 0xffffffff), the initiator MUST respond
+	 * with a NOP-Out".  Since we never send unsolicited NOP-Outs,
+	 * my reading of this is that we can handle all permitted
+	 * NOP-Ins (which must have ITT set to 0xffffffff) by simply
+	 * ignoring them.
+	 *
+	 * There is some ambiguity in the RFC, since there are other
+	 * places that suggest that a target is supposed to be able to
+	 * send an unsolicited NOP-In and expect a NOP-Out response.
+	 * We catch any apparent attempts to use this immediately, so
+	 * that the relevant error gets reported to the iPXE user,
+	 * rather than just having the target drop the connection when
+	 * it times out waiting for the NOP-Out response.
+	 */
+	if ( nop_in->itt != htonl ( ISCSI_TAG_RESERVED ) ) {
+		DBGC ( iscsi, "iSCSI %p received invalid NOP-In with ITT "
+		       "%08x\n", iscsi, ntohl ( nop_in->itt ) );
+		return -EPROTO_INVALID_NOP_IN;
+	}
+
+	return 0;
+}
+
 /****************************************************************************
  *
  * iSCSI login
@@ -1539,6 +1587,8 @@ static int iscsi_rx_data ( struct iscsi_session *iscsi, const void *data,
 		return iscsi_rx_data_in ( iscsi, data, len, remaining );
 	case ISCSI_OPCODE_R2T:
 		return iscsi_rx_r2t ( iscsi, data, len, remaining );
+	case ISCSI_OPCODE_NOP_IN:
+		return iscsi_rx_nop_in ( iscsi, data, len, remaining );
 	default:
 		if ( remaining )
 			return 0;