[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>

diff --git a/src/include/ipxe/iscsi.h b/src/include/ipxe/iscsi.h
index f336c7cc0e3e9dfa9c5577f9f8ae010cb995794b..2202a9d871d6beb42ff2c3d9db6d25070778e1e1 100644 (file)
--- a/src/include/ipxe/iscsi.h
+++ b/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 ) ];
 };
 

diff --git a/src/net/tcp/iscsi.c b/src/net/tcp/iscsi.c
index 17cdaba24aa90e3215f24fab0d5bc402a57e5f7c..6ef4c8968fc681388876341053aeebfa49fe6c8c 100644 (file)
--- a/src/net/tcp/iscsi.c
+++ b/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;