Hi Michael and List,
We realized that sending the padding bytes to align the data segment to 4byte
boundaries as a separate segment causes an issue.
When the full featured login phase starts the BHS segment is sent followed by
the data segment. Once the data segment is received EQL targets send login
success but are still waiting for the padding bytes as ipxe sends the padding
in a separate segment.
Note that since the data segment length field does not include padding bytes
the target can't make a decision on if the padding bytes will arrive at all.
So, it bases its decision on receiving the next PDU. If the next PDU is
received on a non-aligned boundary it will close the connection.
The reason the padding bytes are never received by the target is because the
initiator changes login state on receiving login SUCCESS from the target and
starts SCSI initiatialization and since it doesn't allow concurrent tx threads
the padding segment is never sent.
The target is not expecting the next set of SCSI initialization TCP segments
because it has not yet received the padding bytes and hence closes connection.
Older EQL firmware would respond with login SUCCESS a little slower and that
allowed the initiator to send the padding bytes in a new TCP segment before the
LOGIN SUCCESS response was sent by the target and thus avoid this situation.
Newer EQL firmwares respond quickly with login SUCCESS before the padding bytes
are received and hence this issue.
Looking further seems like this could be an issue for all PDUs as padding
should be done such that each segment is 4byte aligned and sending them
separately when we don't allow concurrent tx threads may become a problem in
other situations.
rfc 3720 Section 10.2. PDU Template, Header, and Opcodes states-
“
All PDU segments and digests are padded to the closest integer number
of four byte words. For example, all PDU segments and digests start
at a four byte word boundary and the padding ranges from 0 to 3
bytes. The padding bytes SHOULD be sent as 0.
"
The following changes were done in the fix.
1) Removed fns iscsi_tx_data_padding and the corresponding data_padding state
machine in fn iscsi_tx_step.
2) Added padding bytes before performing tx of the data segment.
3) Removed ISCSI_TX_DATA_PADDING enum.
Thanks,
Shyam Iyer
---
src/include/ipxe/iscsi.h | 2 -
src/net/tcp/iscsi.c | 55 ++++++++++++++++++++--------------------------
2 files changed, 24 insertions(+), 33 deletions(-)
diff --git a/src/include/ipxe/iscsi.h b/src/include/ipxe/iscsi.h
index 5d3d73b..b4de793 100644
--- a/src/include/ipxe/iscsi.h
+++ b/src/include/ipxe/iscsi.h
@@ -515,8 +515,6 @@ enum iscsi_tx_state {
ISCSI_TX_AHS,
/** Sending the data segment */
ISCSI_TX_DATA,
- /** Sending the data segment padding */
- ISCSI_TX_DATA_PADDING,
};
/** State of an iSCSI RX engine */
diff --git a/src/net/tcp/iscsi.c b/src/net/tcp/iscsi.c
index fa1bb39..1ae4da7 100644
--- a/src/net/tcp/iscsi.c
+++ b/src/net/tcp/iscsi.c
@@ -568,23 +568,32 @@ static void iscsi_data_out_done ( struct iscsi_session
*iscsi ) {
static int iscsi_tx_data_out ( struct iscsi_session *iscsi ) {
struct iscsi_bhs_data_out *data_out = &iscsi->tx_bhs.data_out;
struct io_buffer *iobuf;
+ static const char pad[] = { '\0', '\0', '\0' };
unsigned long offset;
- size_t len;
+ size_t len, pad_len;
offset = ntohl ( data_out->offset );
len = ISCSI_DATA_LEN ( data_out->lengths );
+ pad_len = ISCSI_DATA_PAD_LEN ( data_out->lengths );
assert ( iscsi->command != NULL );
assert ( iscsi->command->data_out );
assert ( ( offset + len ) <= iscsi->command->data_out_len );
- iobuf = xfer_alloc_iob ( &iscsi->socket, len );
+ iobuf = xfer_alloc_iob ( &iscsi->socket, len + pad_len );
if ( ! iobuf )
return -ENOMEM;
copy_from_user ( iob_put ( iobuf, len ),
iscsi->command->data_out, offset, len );
+ /*Send padding bytes along*/
+ if ( pad_len ) {
+ memcpy( iob_put( iobuf, pad_len ), pad, pad_len );
+ DBGC ( iscsi, "iSCSI %p Adding padding %d bytes \n",
+ iscsi, pad_len );
+ }
+
return xfer_deliver_iob ( &iscsi->socket, iobuf );
}
@@ -799,15 +808,25 @@ static void iscsi_login_request_done ( struct
iscsi_session *iscsi ) {
*/
static int iscsi_tx_login_request ( struct iscsi_session *iscsi ) {
struct iscsi_bhs_login_request *request = &iscsi->tx_bhs.login_request;
+ static const char pad[] = { '\0', '\0', '\0' };
struct io_buffer *iobuf;
- size_t len;
+ size_t len, pad_len;
len = ISCSI_DATA_LEN ( request->lengths );
- iobuf = xfer_alloc_iob ( &iscsi->socket, len );
+ pad_len = ISCSI_DATA_PAD_LEN ( request->lengths );
+ iobuf = xfer_alloc_iob ( &iscsi->socket, len + pad_len );
if ( ! iobuf )
return -ENOMEM;
iob_put ( iobuf, len );
iscsi_build_login_request_strings ( iscsi, iobuf->data, len );
+
+ /*Send padding bytes along*/
+ if ( pad_len ) {
+ memcpy( iob_put( iobuf, pad_len ), pad, pad_len );
+ DBGC ( iscsi, "iSCSI %p Adding padding %d bytes \n",
+ iscsi, pad_len );
+ }
+
return xfer_deliver_iob ( &iscsi->socket, iobuf );
}
@@ -1416,27 +1435,6 @@ static int iscsi_tx_data ( struct iscsi_session *iscsi )
{
}
/**
- * Transmit data padding of an iSCSI PDU
- *
- * @v iscsi iSCSI session
- * @ret rc Return status code
- *
- * Handle transmission of any data padding in a PDU data segment.
- * iscsi::tx_bhs will be valid when this is called.
- */
-static int iscsi_tx_data_padding ( struct iscsi_session *iscsi ) {
- static const char pad[] = { '\0', '\0', '\0' };
- struct iscsi_bhs_common *common = &iscsi->tx_bhs.common;
- size_t pad_len;
-
- pad_len = ISCSI_DATA_PAD_LEN ( common->lengths );
- if ( ! pad_len )
- return 0;
-
- return xfer_deliver_raw ( &iscsi->socket, pad, pad_len );
-}
-
-/**
* Complete iSCSI PDU transmission
*
* @v iscsi iSCSI session
@@ -1494,12 +1492,7 @@ static void iscsi_tx_step ( struct iscsi_session *iscsi
) {
case ISCSI_TX_DATA:
tx = iscsi_tx_data;
tx_len = ISCSI_DATA_LEN ( common->lengths );
- next_state = ISCSI_TX_DATA_PADDING;
- break;
- case ISCSI_TX_DATA_PADDING:
- tx = iscsi_tx_data_padding;
- tx_len = ISCSI_DATA_PAD_LEN ( common->lengths );
- next_state = ISCSI_TX_IDLE;
+ next_state= ISCSI_TX_IDLE;
break;
case ISCSI_TX_IDLE:
/* Nothing to do; pause processing */
--
1.7.6.5
_______________________________________________
ipxe-devel mailing list
[email protected]
https://lists.ipxe.org/mailman/listinfo/ipxe-devel
