iSCSI writes seem to be working (at least, the ethereal trace shows no

errors; still need to verify data integrity).

SCSI response PDUs are handled: status and sense data (if available) are
returned via the scsi_command structure.

Updated iSCSI session parameter usage.

src/include/gpxe/iscsi.h

@@ -81,33 +81,6 @@ struct iscsi_bhs_common {
 /** Final PDU of a sequence */
 #define ISCSI_FLAG_FINAL 0x80
 
-/**
- * iSCSI basic header segment common request fields
- *
- */
-struct iscsi_bhs_common_request {
-	/** Opcode */
-	uint8_t opcode;
-	/** Flags */
-	uint8_t flags;
-	/** Fields specific to the PDU type */
-	uint8_t other_a[2];
-	/** Segment lengths */
-	union iscsi_segment_lengths lengths;
-	/** Fields specific to the PDU type */
-	uint8_t other_b[8];
-	/** Initiator Task Tag */
-	uint32_t itt;
-	/** Fields specific to the PDU type */
-	uint8_t other_c[4];
-	/** Command sequence number */
-	uint32_t cmdsn;
-	/** Expected status sequence number */
-	uint32_t expstatsn;
-	/** Fields specific to the PDU type */
-	uint8_t other_d[16];
-};
-
 /**
  * iSCSI basic header segment common request fields
  *
@@ -328,8 +301,16 @@ struct iscsi_bhs_scsi_response {
 /** SCSI target failure */
 #define ISCSI_RESPONSE_TARGET_FAILURE 0x01
 
+/** SCSI sense response code offset
+ *
+ * The SCSI response may contain unsolicited sense data in the data
+ * segment.  If it does, this is the offset to the sense response code
+ * byte, which is the only byte we care about.
+ */
+#define ISCSI_SENSE_RESPONSE_CODE_OFFSET 2
+
 /**
- * iSCSI data in basic header segment
+ * iSCSI data-in basic header segment
  *
  */
 struct iscsi_bhs_data_in {
@@ -363,7 +344,7 @@ struct iscsi_bhs_data_in {
 	uint32_t residual_count;
 };
 
-/** Data in opcode */
+/** Data-in opcode */
 #define ISCSI_OPCODE_DATA_IN 0x25
 
 /** Data requires acknowledgement */
@@ -375,21 +356,94 @@ struct iscsi_bhs_data_in {
 /** Data underflow occurred */
 #define ISCSI_DATA_FLAG_UNDERFLOW 0x02
 
-/** SCSI status code and verflow/underflow flags are valid */
+/** SCSI status code and overflow/underflow flags are valid */
 #define ISCSI_DATA_FLAG_STATUS 0x01
 
+/**
+ * iSCSI data-out basic header segment
+ *
+ */
+struct iscsi_bhs_data_out {
+	/** Opcode */
+	uint8_t opcode;
+	/** Flags */
+	uint8_t flags;
+	/** Reserved */
+	uint16_t reserved_a;
+	/** Segment lengths */
+	union iscsi_segment_lengths lengths;
+	/** Logical Unit Number */
+	uint64_t lun;
+	/** Initiator Task Tag */
+	uint32_t itt;
+	/** Target Transfer Tag */
+	uint32_t ttt;
+	/** Reserved */
+	uint32_t reserved_b;
+	/** Expected status sequence number */
+	uint32_t expstatsn;
+	/** Reserved */
+	uint32_t reserved_c;
+	/** Data sequence number */
+	uint32_t datasn;
+	/** Buffer offset */
+	uint32_t offset;
+	/** Reserved */
+	uint32_t reserved_d;
+};
+
+/** Data-out opcode */
+#define ISCSI_OPCODE_DATA_OUT 0x05
+
+/**
+ * iSCSI request to transfer basic header segment
+ *
+ */
+struct iscsi_bhs_r2t {
+	/** Opcode */
+	uint8_t opcode;
+	/** Flags */
+	uint8_t flags;
+	/** Reserved */
+	uint16_t reserved_a;
+	/** Segment lengths */
+	union iscsi_segment_lengths lengths;
+	/** Logical Unit Number */
+	uint64_t 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;
+	/** R2T sequence number */
+	uint32_t r2tsn;
+	/** Buffer offset */
+	uint32_t offset;
+	/** Desired data transfer length */
+	uint32_t len;
+};
+
+/** R2T opcode */
+#define ISCSI_OPCODE_R2T 0x31
+
 /**
  * An iSCSI basic header segment
  */
 union iscsi_bhs {
 	struct iscsi_bhs_common common;
-	struct iscsi_bhs_common_request common_request;
 	struct iscsi_bhs_common_response common_response;
 	struct iscsi_bhs_login_request login_request;
 	struct iscsi_bhs_login_response login_response;
 	struct iscsi_bhs_scsi_command scsi_command;
 	struct iscsi_bhs_scsi_response scsi_response;
 	struct iscsi_bhs_data_in data_in;
+	struct iscsi_bhs_data_out data_out;
+	struct iscsi_bhs_r2t r2t;
 	unsigned char bytes[ sizeof ( struct iscsi_bhs_common ) ];
 };
 
@@ -452,9 +506,29 @@ struct iscsi_session {
 	/** Initiator task tag
 	 *
 	 * This is the tag of the current command.  It is incremented
-	 * whenever a final response PDU is received.
+	 * whenever a new command is started.
 	 */
 	uint32_t itt;
+	/** Target transfer tag
+	 *
+	 * This is the tag attached to a sequence of data-out PDUs in
+	 * response to an R2T.
+	 */
+	uint32_t ttt;
+	/**
+	 * Transfer offset
+	 *
+	 * This is the offset for an in-progress sequence of data-out
+	 * PDUs in response to an R2T.
+	 */
+	uint32_t transfer_offset;
+	/**
+	 * Transfer length
+	 *
+	 * This is the length for an in-progress sequence of data-out
+	 * PDUs in response to an R2T.
+	 */
+	uint32_t transfer_len;
 	/** Command sequence number
 	 *
 	 * This is the sequence number of the current command, used to

src/include/gpxe/scsi.h

@@ -136,6 +136,10 @@ struct scsi_command {
 	 * Must be zero if @c data_in is NULL
 	 */
 	size_t data_in_len;
+	/** SCSI statua code */
+	uint8_t status;
+	/** SCSI sense response code */
+	uint8_t sense_response;
 };
 
 /** A SCSI device */
@@ -154,6 +158,12 @@ struct scsi_device {
 	 * @v scsi		SCSI device
 	 * @v command		SCSI command
 	 * @ret rc		Return status code
+	 *
+	 * Note that a successful return status code indicates only
+	 * that the SCSI command completed.  The caller must check the
+	 * status field in the command structure to see if, for
+	 * example, the device returned CHECK CONDITION or some other
+	 * non-success status code.
 	 */
 	int ( * command ) ( struct scsi_device *scsi,
 			    struct scsi_command *command );

src/net/tcp/iscsi.c

@@ -33,6 +33,8 @@
  */
 
 static void iscsi_start_tx ( struct iscsi_session *iscsi );
+static void iscsi_start_data_out ( struct iscsi_session *iscsi,
+				   unsigned int datasn );
 
 /****************************************************************************
  *
@@ -44,6 +46,11 @@ static void iscsi_start_tx ( struct iscsi_session *iscsi );
  * Build iSCSI SCSI command BHS
  *
  * @v iscsi		iSCSI session
+ *
+ * We don't currently support bidirectional commands (i.e. with both
+ * Data-In and Data-Out segments); these would require providing code
+ * to generate an AHS, and there doesn't seem to be any need for it at
+ * the moment.
  */
 static void iscsi_start_command ( struct iscsi_session *iscsi ) {
 	struct iscsi_bhs_scsi_command *command = &iscsi->tx_bhs.scsi_command;
@@ -59,21 +66,49 @@ static void iscsi_start_command ( struct iscsi_session *iscsi ) {
 		command->flags |= ISCSI_COMMAND_FLAG_READ;
 	if ( iscsi->command->data_out )
 		command->flags |= ISCSI_COMMAND_FLAG_WRITE;
-	ISCSI_SET_LENGTHS ( command->lengths, 0, iscsi->command->data_out_len);
+	/* lengths left as zero */
 	command->lun = iscsi->lun;
-	command->itt = htonl ( iscsi->itt );
-	command->exp_len = htonl ( iscsi->command->data_in_len );
+	command->itt = htonl ( ++iscsi->itt );
+	command->exp_len = htonl ( iscsi->command->data_in_len |
+				   iscsi->command->data_out_len );
+	command->cmdsn = htonl ( iscsi->cmdsn );
+	command->expstatsn = htonl ( iscsi->statsn + 1 );
 	memcpy ( &command->cdb, &iscsi->command->cdb, sizeof ( command->cdb ));
 }
 
 /**
- * Send iSCSI SCSI command data
+ * Receive data segment of an iSCSI SCSI response PDU
  *
  * @v iscsi		iSCSI session
+ * @v data		Received data
+ * @v len		Length of received data
+ * @v remaining		Data remaining after this data
+ * 
  */
-static void iscsi_tx_command ( struct iscsi_session *iscsi ) {
-	tcp_send ( &iscsi->tcp, iscsi->command->data_out + iscsi->tx_offset,
-		   iscsi->command->data_out_len - iscsi->tx_offset );
+static void iscsi_rx_scsi_response ( struct iscsi_session *iscsi, void *data,
+				     size_t len, size_t remaining ) {
+	struct iscsi_bhs_scsi_response *response
+		= &iscsi->rx_bhs.scsi_response;
+	int sense_offset;
+
+	/* Capture the sense response code as it floats past, if present */
+	sense_offset = ISCSI_SENSE_RESPONSE_CODE_OFFSET - iscsi->rx_offset;
+	if ( ( sense_offset >= 0 ) && len ) {
+		iscsi->command->sense_response =
+			* ( ( char * ) data + sense_offset );
+	}
+
+	/* Wait for whole SCSI response to arrive */
+	if ( remaining )
+		return;
+	
+	/* Record SCSI status code */
+	iscsi->command->status = response->status;
+
+	/* Mark as completed, with error if applicable */
+	iscsi->status |= ISCSI_STATUS_DONE;
+	if ( response->response != ISCSI_RESPONSE_COMMAND_COMPLETE )
+		iscsi->status |= ISCSI_STATUS_ERR;
 }
 
 /**
@@ -86,7 +121,7 @@ static void iscsi_tx_command ( struct iscsi_session *iscsi ) {
  * 
  */
 static void iscsi_rx_data_in ( struct iscsi_session *iscsi, void *data,
-			       size_t len, size_t remaining ) {
+			       size_t len, size_t remaining __unused ) {
 	struct iscsi_bhs_data_in *data_in = &iscsi->rx_bhs.data_in;
 	unsigned long offset;
 
@@ -97,9 +132,109 @@ static void iscsi_rx_data_in ( struct iscsi_session *iscsi, void *data,
 	assert ( ( offset + len ) <= iscsi->command->data_in_len );
 	memcpy ( ( iscsi->command->data_in + offset ), data, len );
 
+	/* Record SCSI status, if present */
+	if ( data_in->flags & ISCSI_DATA_FLAG_STATUS )
+		iscsi->command->status = data_in->status;
+
 	/* If this is the end, flag as complete */
-	if ( ( data_in->flags & ISCSI_FLAG_FINAL ) && ( remaining == 0 ) )
+	if ( ( offset + len ) == iscsi->command->data_in_len ) {
+		assert ( data_in->flags & ISCSI_FLAG_FINAL );
+		assert ( remaining == 0 );
 		iscsi->status |= ISCSI_STATUS_DONE;
+	}
+}
+
+/**
+ * Receive data segment of an iSCSI R2T PDU
+ *
+ * @v iscsi		iSCSI session
+ * @v data		Received data
+ * @v len		Length of received data
+ * @v remaining		Data remaining after this data
+ * 
+ */
+static void iscsi_rx_r2t ( struct iscsi_session *iscsi, void *data __unused,
+			   size_t len __unused, size_t remaining __unused ) {
+	struct iscsi_bhs_r2t *r2t = &iscsi->rx_bhs.r2t;
+
+	/* Record transfer parameters and trigger first data-out */
+	iscsi->ttt = ntohl ( r2t->ttt );
+	iscsi->transfer_offset = ntohl ( r2t->offset );
+	iscsi->transfer_len = ntohl ( r2t->len );
+	iscsi_start_data_out ( iscsi, 0 );
+}
+
+/**
+ * Build iSCSI data-out BHS
+ *
+ * @v iscsi		iSCSI session
+ * @v datasn		Data sequence number within the transfer
+ *
+ */
+static void iscsi_start_data_out ( struct iscsi_session *iscsi,
+				   unsigned int datasn ) {
+	struct iscsi_bhs_data_out *data_out = &iscsi->tx_bhs.data_out;
+	unsigned long offset;
+	unsigned long remaining;
+	unsigned long len;
+
+	/* We always send 512-byte Data-Out PDUs; this removes the
+	 * need to worry about the target's MaxRecvDataSegmentLength.
+	 */
+	offset = datasn * 512;
+	remaining = iscsi->transfer_len - offset;
+	len = remaining;
+	if ( len > 512 )
+		len = 512;
+
+	/* Construct BHS and initiate transmission */
+	iscsi_start_tx ( iscsi );
+	data_out->opcode = ISCSI_OPCODE_DATA_OUT;
+	if ( len == remaining )
+		data_out->flags = ( ISCSI_FLAG_FINAL );
+	ISCSI_SET_LENGTHS ( data_out->lengths, 0, len );
+	data_out->lun = iscsi->lun;
+	data_out->itt = htonl ( iscsi->itt );
+	data_out->ttt = htonl ( iscsi->ttt );
+	data_out->expstatsn = htonl ( iscsi->statsn + 1 );
+	data_out->datasn = htonl ( datasn );
+	data_out->offset = htonl ( iscsi->transfer_offset + offset );
+}
+
+/**
+ * Complete iSCSI data-out PDU transmission
+ *
+ * @v iscsi		iSCSI session
+ *
+ */
+static void iscsi_data_out_done ( struct iscsi_session *iscsi ) {
+	struct iscsi_bhs_data_out *data_out = &iscsi->tx_bhs.data_out;
+
+	/* If we haven't reached the end of the sequence, start
+	 * sending the next data-out PDU.
+	 */
+	if ( ! ( data_out->flags & ISCSI_FLAG_FINAL ) )
+		iscsi_start_data_out ( iscsi, ntohl ( data_out->datasn ) + 1 );
+}
+
+/**
+ * Send iSCSI data-out data segment
+ *
+ * @v iscsi		iSCSI session
+ */
+static void iscsi_tx_data_out ( struct iscsi_session *iscsi ) {
+	struct iscsi_bhs_data_out *data_out = &iscsi->tx_bhs.data_out;
+	unsigned long offset;
+	unsigned long len;
+
+	offset = ( iscsi->transfer_offset + ntohl ( data_out->offset ) +
+		   iscsi->tx_offset );
+	len = ( ISCSI_DATA_LEN ( data_out->lengths ) - iscsi->tx_offset );
+	assert ( iscsi->command != NULL );
+	assert ( iscsi->command->data_out != NULL );
+	assert ( ( offset + len ) <= iscsi->command->data_out_len );
+	
+	tcp_send ( &iscsi->tcp, iscsi->command->data_out + offset, len );
 }
 
 /****************************************************************************
@@ -121,11 +256,11 @@ static int iscsi_build_login_request_strings ( struct iscsi_session *iscsi,
 	return snprintf ( data, len,
 			  "InitiatorName=%s%c"
 			  "TargetName=%s%c"
-			  "MaxRecvDataSegmentLength=512%c"
 			  "SessionType=Normal%c"
 			  "DataDigest=None%c"
 			  "HeaderDigest=None%c"
-			  "ErrorRecoveryLevel=0%c",
+			  "DefaultTime2Wait=0%c"
+			  "DefaultTime2Retain=0%c",
 			  iscsi->initiator, 0, iscsi->target, 0,
 			  0, 0, 0, 0, 0 );
 }
@@ -134,7 +269,7 @@ static int iscsi_build_login_request_strings ( struct iscsi_session *iscsi,
  * Build iSCSI login request BHS
  *
  * @v iscsi		iSCSI session
- * @v first		Login request is the first request of a session
+ * @v first		Login request is the first in a sequence
  */
 static void iscsi_start_login ( struct iscsi_session *iscsi, int first ) {
 	struct iscsi_bhs_login_request *request = &iscsi->tx_bhs.login_request;
@@ -156,8 +291,12 @@ static void iscsi_start_login ( struct iscsi_session *iscsi, int first ) {
 					IANA_EN_FEN_SYSTEMS );
 	/* isid_iana_qual left as zero */
 	request->tsih = htons ( iscsi->tsih );
-	/* itt left as zero */
+	if ( first )
+		iscsi->itt++;
+	request->itt = htonl ( iscsi->itt );
 	/* cid left as zero */
+	request->cmdsn = htonl ( iscsi->cmdsn );
+	request->expstatsn = htonl ( iscsi->statsn + 1 );
 }
 
 /**
@@ -240,8 +379,6 @@ static void iscsi_start_tx ( struct iscsi_session *iscsi ) {
 	
 	/* Initialise TX BHS */
 	memset ( &iscsi->tx_bhs, 0, sizeof ( iscsi->tx_bhs ) );
-	iscsi->tx_bhs.common_request.cmdsn = htonl ( iscsi->cmdsn );
-	iscsi->tx_bhs.common_request.expstatsn = htonl ( iscsi->statsn + 1 );
 
 	/* Flag TX engine to start transmitting */
 	iscsi->tx_state = ISCSI_TX_BHS;
@@ -260,8 +397,8 @@ static void iscsi_tx_data ( struct iscsi_session *iscsi ) {
 	struct iscsi_bhs_common *common = &iscsi->tx_bhs.common;
 
 	switch ( common->opcode & ISCSI_OPCODE_MASK ) {
-	case ISCSI_OPCODE_SCSI_COMMAND:
-		iscsi_tx_command ( iscsi );
+	case ISCSI_OPCODE_DATA_OUT:
+		iscsi_tx_data_out ( iscsi );
 		break;
 	case ISCSI_OPCODE_LOGIN_REQUEST:
 		iscsi_tx_login_request ( iscsi );
@@ -272,6 +409,27 @@ static void iscsi_tx_data ( struct iscsi_session *iscsi ) {
 	}
 }
 
+/**
+ * Complete iSCSI PDU transmission
+ *
+ * @v iscsi		iSCSI session
+ *
+ * Called when a PDU has been completely transmitted and the TX state
+ * machine is about to enter the idle state.  iscsi::tx_bhs will be
+ * valid for the just-completed PDU when this is called.
+ */
+static void iscsi_tx_done ( struct iscsi_session *iscsi ) {
+	struct iscsi_bhs_common *common = &iscsi->tx_bhs.common;
+
+	switch ( common->opcode & ISCSI_OPCODE_MASK ) {
+	case ISCSI_OPCODE_DATA_OUT:
+		iscsi_data_out_done ( iscsi );
+	default:
+		/* No action */
+		break;
+	}
+}
+
 /**
  * Handle TCP ACKs
  *
@@ -318,9 +476,14 @@ static void iscsi_acked ( struct tcp_connection *conn, size_t len ) {
 		 */
 		if ( iscsi->tx_offset != max_tx_offset )
 			return;
-		
+
+		/* Move to next state.  Call iscsi_tx_done() when PDU
+		 * transmission is complete.
+		 */
 		iscsi->tx_state = next_state;
 		iscsi->tx_offset = 0;
+		if ( next_state == ISCSI_TX_IDLE )
+			iscsi_tx_done ( iscsi );
 	}
 }
 
@@ -381,17 +544,19 @@ static void iscsi_rx_data ( struct iscsi_session *iscsi, void *data,
 	iscsi->cmdsn = ntohl ( response->expcmdsn );
 	iscsi->statsn = ntohl ( response->statsn );
 
-	/* Increment itt when we receive a final response */
-	if ( response->flags & ISCSI_FLAG_FINAL )
-		iscsi->itt++;
-
 	switch ( response->opcode & ISCSI_OPCODE_MASK ) {
 	case ISCSI_OPCODE_LOGIN_RESPONSE:
 		iscsi_rx_login_response ( iscsi, data, len, remaining );
 		break;
+	case ISCSI_OPCODE_SCSI_RESPONSE:
+		iscsi_rx_scsi_response ( iscsi, data, len, remaining );
+		break;
 	case ISCSI_OPCODE_DATA_IN:
 		iscsi_rx_data_in ( iscsi, data, len, remaining );
 		break;
+	case ISCSI_OPCODE_R2T:
+		iscsi_rx_r2t ( iscsi, data, len, remaining );
+		break;
 	default:
 		printf ( "Unknown iSCSI opcode %02x\n", response->opcode );
 		iscsi->status |= ( ISCSI_STATUS_DONE | ISCSI_STATUS_ERR );
@@ -521,6 +686,7 @@ static void iscsi_closed ( struct tcp_connection *conn, int status __unused ) {
 	if ( ++iscsi->retry_count <= ISCSI_MAX_RETRIES ) {
 		tcp_connect ( conn );
 	} else {
+		printf ( "iSCSI retry count exceeded\n" );
 		iscsi->status |= ( ISCSI_STATUS_DONE | ISCSI_STATUS_ERR );
 	}
 }