Skip to content

Commit

Permalink
[hyperv] Assume that VMBus xfer page ranges correspond to RNDIS messages
Browse files Browse the repository at this point in the history
The (undocumented) VMBus protocol seems to allow for transfer
page-based packets where the data payload is split into an arbitrary
set of ranges within the transfer page set.

The RNDIS protocol includes a length field within the header of each
message, and it is known from observation that multiple RNDIS messages
can be concatenated into a single VMBus message.

iPXE currently assumes that the transfer page range boundaries are
entirely arbitrary, and uses the RNDIS header length to determine the
RNDIS message boundaries.

Windows Server 2012 R2 generates an RNDIS_INDICATE_STATUS_MSG for an
undocumented and unknown status code (0x40020006) with a malformed
RNDIS header length: the length does not cover the StatusBuffer
portion of the message.  This causes iPXE to report a malformed RNDIS
message and to discard any further RNDIS messages within the same
VMBus message.

The Linux Hyper-V driver assumes that the transfer page range
boundaries correspond to RNDIS message boundaries, and so does not
notice the malformed length field in the RNDIS header.

Match the behaviour of the Linux Hyper-V driver: assume that the
transfer page range boundaries correspond to the RNDIS message
boundaries and ignore the RNDIS header length.  This avoids triggering
the "malformed packet" error and also avoids unnecessary data copying:
since we now have one I/O buffer per RNDIS message, there is no longer
any need to use iob_split().

Signed-off-by: Michael Brown <mcb30@ipxe.org>
  • Loading branch information
mcb30 committed Dec 20, 2014
1 parent a69c995 commit 639632b
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 94 deletions.
20 changes: 14 additions & 6 deletions src/drivers/net/netvsc.c
Expand Up @@ -298,15 +298,17 @@ static int netvsc_recv_control ( struct vmbus_device *vmdev, uint64_t xid,
* @v xid Transaction ID
* @v data Data
* @v len Length of data
* @v iobuf I/O buffer, or NULL if allocation failed
* @v list List of I/O buffers
* @ret rc Return status code
*/
static int netvsc_recv_data ( struct vmbus_device *vmdev, uint64_t xid,
const void *data, size_t len,
struct io_buffer *iobuf ) {
struct list_head *list ) {
struct rndis_device *rndis = vmbus_get_drvdata ( vmdev );
struct netvsc_device *netvsc = rndis->priv;
const struct netvsc_rndis_message *msg = data;
struct io_buffer *iobuf;
struct io_buffer *tmp;
int rc;

/* Sanity check */
Expand All @@ -324,21 +326,27 @@ static int netvsc_recv_data ( struct vmbus_device *vmdev, uint64_t xid,
goto err_sanity;
}

/* Send completion back to host (even if I/O buffer was missing) */
/* Send completion back to host */
if ( ( rc = vmbus_send_completion ( vmdev, xid, NULL, 0 ) ) != 0 ) {
DBGC ( netvsc, "NETVSC %s could not send completion: %s\n",
netvsc->name, strerror ( rc ) );
goto err_completion;
}

/* Hand off to RNDIS (even if I/O buffer was missing) */
rndis_rx ( rndis, iob_disown ( iobuf ) );
/* Hand off to RNDIS */
list_for_each_entry_safe ( iobuf, tmp, list, list ) {
list_del ( &iobuf->list );
rndis_rx ( rndis, iob_disown ( iobuf ) );
}

return 0;

err_completion:
err_sanity:
free_iob ( iobuf );
list_for_each_entry_safe ( iobuf, tmp, list, list ) {
list_del ( &iobuf->list );
free_iob ( iobuf );
}
return rc;
}

Expand Down
9 changes: 2 additions & 7 deletions src/include/ipxe/vmbus.h
Expand Up @@ -403,21 +403,16 @@ struct vmbus_channel_operations {
* @v xid Transaction ID
* @v data Data
* @v len Length of data
* @v iobuf I/O buffer, or NULL if allocation failed
* @v list List of I/O buffers
* @ret rc Return status code
*
* This function takes ownership of the I/O buffer. It should
* eventually call vmbus_send_completion() to indicate to the
* host that the buffer can be reused.
*
* Note that this function will be called even if we failed to
* allocate or populate the I/O buffer; this is to allow for a
* completion to be sent even in the event of a transient
* memory shortage failure.
*/
int ( * recv_data ) ( struct vmbus_device *vmdev, uint64_t xid,
const void *data, size_t len,
struct io_buffer *iobuf );
struct list_head *list );
/**
* Handle received completion packet
*
Expand Down
74 changes: 43 additions & 31 deletions src/interface/hyperv/vmbus.c
Expand Up @@ -807,20 +807,21 @@ static struct vmbus_xfer_pages * vmbus_xfer_pages ( struct vmbus_device *vmdev,
}

/**
* Construct I/O buffer from transfer pages
* Construct I/O buffer list from transfer pages
*
* @v vmdev VMBus device
* @v header Transfer page header
* @ret iobuf I/O buffer, or NULL on error
* @v list I/O buffer list to populate
* @ret rc Return status code
*/
static struct io_buffer *
vmbus_xfer_page_iobuf ( struct vmbus_device *vmdev,
struct vmbus_packet_header *header ) {
static int vmbus_xfer_page_iobufs ( struct vmbus_device *vmdev,
struct vmbus_packet_header *header,
struct list_head *list ) {
struct vmbus_xfer_page_header *page_header =
container_of ( header, struct vmbus_xfer_page_header, header );
struct vmbus_xfer_pages *pages;
struct io_buffer *iobuf;
size_t total_len;
struct io_buffer *tmp;
size_t len;
size_t offset;
unsigned int range_count;
Expand All @@ -832,28 +833,32 @@ vmbus_xfer_page_iobuf ( struct vmbus_device *vmdev,

/* Locate page set */
pages = vmbus_xfer_pages ( vmdev, page_header->pageset );
if ( ! pages )
if ( ! pages ) {
rc = -ENOENT;
goto err_pages;

/* Determine total length */
range_count = le32_to_cpu ( page_header->range_count );
for ( total_len = 0, i = 0 ; i < range_count ; i++ ) {
len = le32_to_cpu ( page_header->range[i].len );
total_len += len;
}

/* Allocate I/O buffer */
iobuf = alloc_iob ( total_len );
if ( ! iobuf ) {
DBGC ( vmdev, "VMBUS %s could not allocate %zd-byte I/O "
"buffer\n", vmdev->dev.name, total_len );
goto err_alloc;
}

/* Populate I/O buffer */
/* Allocate and populate I/O buffers */
range_count = le32_to_cpu ( page_header->range_count );
for ( i = 0 ; i < range_count ; i++ ) {

/* Parse header */
len = le32_to_cpu ( page_header->range[i].len );
offset = le32_to_cpu ( page_header->range[i].offset );

/* Allocate I/O buffer */
iobuf = alloc_iob ( len );
if ( ! iobuf ) {
DBGC ( vmdev, "VMBUS %s could not allocate %zd-byte "
"I/O buffer\n", vmdev->dev.name, len );
rc = -ENOMEM;
goto err_alloc;
}

/* Add I/O buffer to list */
list_add ( &iobuf->list, list );

/* Populate I/O buffer */
if ( ( rc = pages->op->copy ( pages, iob_put ( iobuf, len ),
offset, len ) ) != 0 ) {
DBGC ( vmdev, "VMBUS %s could not populate I/O buffer "
Expand All @@ -863,13 +868,16 @@ vmbus_xfer_page_iobuf ( struct vmbus_device *vmdev,
}
}

return iobuf;
return 0;

err_copy:
free_iob ( iobuf );
err_alloc:
list_for_each_entry_safe ( iobuf, tmp, list, list ) {
list_del ( &iobuf->list );
free_iob ( iobuf );
}
err_pages:
return NULL;
return rc;
}

/**
Expand All @@ -880,7 +888,7 @@ vmbus_xfer_page_iobuf ( struct vmbus_device *vmdev,
*/
int vmbus_poll ( struct vmbus_device *vmdev ) {
struct vmbus_packet_header *header = vmdev->packet;
struct io_buffer *iobuf;
struct list_head list;
void *data;
size_t header_len;
size_t len;
Expand Down Expand Up @@ -929,6 +937,14 @@ int vmbus_poll ( struct vmbus_device *vmdev ) {
DBGC2_HDA ( vmdev, old_cons, header, ring_len );
assert ( ( ( cons - old_cons ) & ( vmdev->in_len - 1 ) ) == ring_len );

/* Allocate I/O buffers, if applicable */
INIT_LIST_HEAD ( &list );
if ( header->type == cpu_to_le16 ( VMBUS_DATA_XFER_PAGES ) ) {
if ( ( rc = vmbus_xfer_page_iobufs ( vmdev, header,
&list ) ) != 0 )
return rc;
}

/* Update producer index */
rmb();
vmdev->in->cons = cpu_to_le32 ( cons );
Expand All @@ -948,12 +964,8 @@ int vmbus_poll ( struct vmbus_device *vmdev ) {
break;

case cpu_to_le16 ( VMBUS_DATA_XFER_PAGES ) :
iobuf = vmbus_xfer_page_iobuf ( vmdev, header );
/* Call recv_data() even if I/O buffer allocation
* failed, to allow for completions to be sent.
*/
if ( ( rc = vmdev->op->recv_data ( vmdev, xid, data, len,
iob_disown ( iobuf ) ) )!=0){
&list ) ) != 0 ) {
DBGC ( vmdev, "VMBUS %s could not handle data packet: "
"%s\n", vmdev->dev.name, strerror ( rc ) );
return rc;
Expand Down
63 changes: 13 additions & 50 deletions src/net/rndis.c
Expand Up @@ -759,67 +759,30 @@ static void rndis_rx_message ( struct rndis_device *rndis,
* Receive packet from underlying transport layer
*
* @v rndis RNDIS device
* @v iobuf I/O buffer, or NULL if allocation failed
* @v iobuf I/O buffer
*/
void rndis_rx ( struct rndis_device *rndis, struct io_buffer *iobuf ) {
struct net_device *netdev = rndis->netdev;
struct rndis_header *header;
struct io_buffer *msg;
size_t len;
unsigned int type;
int rc;

/* Record dropped packet if I/O buffer is missing */
if ( ! iobuf ) {
DBGC2 ( rndis, "RNDIS %s received dropped packet\n",
rndis->name );
rc = -ENOBUFS;
/* Sanity check */
if ( iob_len ( iobuf ) < sizeof ( *header ) ) {
DBGC ( rndis, "RNDIS %s received underlength packet:\n",
rndis->name );
DBGC_HDA ( rndis, 0, iobuf->data, iob_len ( iobuf ) );
rc = -EINVAL;
goto drop;
}
header = iobuf->data;

/* Split packet into messages */
while ( iobuf ) {

/* Sanity check */
if ( iob_len ( iobuf ) < sizeof ( *header ) ) {
DBGC ( rndis, "RNDIS %s received underlength packet:\n",
rndis->name );
DBGC_HDA ( rndis, 0, iobuf->data, iob_len ( iobuf ) );
rc = -EINVAL;
goto drop;
}
header = iobuf->data;

/* Parse and check header */
type = le32_to_cpu ( header->type );
len = le32_to_cpu ( header->len );
if ( ( len < sizeof ( *header ) ) ||
( len > iob_len ( iobuf ) ) ) {
DBGC ( rndis, "RNDIS %s received malformed packet:\n",
rndis->name );
DBGC_HDA ( rndis, 0, iobuf->data, iob_len ( iobuf ) );
rc = -EINVAL;
goto drop;
}

/* Split buffer if required */
if ( len < iob_len ( iobuf ) ) {
msg = iob_split ( iobuf, len );
if ( ! msg ) {
rc = -ENOMEM;
goto drop;
}
} else {
msg = iobuf;
iobuf = NULL;
}

/* Strip header */
iob_pull ( msg, sizeof ( *header ) );
/* Parse and strip header */
type = le32_to_cpu ( header->type );
iob_pull ( iobuf, sizeof ( *header ) );

/* Handle message */
rndis_rx_message ( rndis, iob_disown ( msg ), type );
}
/* Handle message */
rndis_rx_message ( rndis, iob_disown ( iobuf ), type );

return;

Expand Down

0 comments on commit 639632b

Please sign in to comment.