[PATCH DOCDAY v2] netif.h: describe request/response structures in terms of binary layout

[PATCH DOCDAY v2] netif.h: describe request/response structures in terms of binary layout

[Ian Campbell]

In RFC style, rather than relying on the implicit assumptions of a
particular C ABI.

I have also confirmed, using the Python gdb extension technique in
[0], that the struct offsets (in a Linux binary at least) are the same
as described here.

I took the opportunity to also confirm that x86_32, x86_64, arm32 and
arm64 are all the same.

This highlighted that struct netif_rx_request was missing some
explicit padding, which is added here.

Lastly, fixup some struct names to allow the generated docs to
properly hyperlink, mainly by adding the _t to type names where
appropriate, but also s/netif_tx_extra/netif_extra_info_t/.

[0] http://stackoverflow.com/questions/9788679/how-to-get-the-relative-adress-of-a-field-in-a-structure-dump-c

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v2: typoes
---
 xen/include/public/io/netif.h |  137 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 127 insertions(+), 10 deletions(-)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index 61e9aea..353eab7 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -137,13 +137,129 @@
 
 /*
  * This is the 'wire' format for packets:
- *  Request 1: netif_tx_request -- NETTXF_* (any flags)
- * [Request 2: netif_tx_extra]  (only if request 1 has NETTXF_extra_info)
- * [Request 3: netif_tx_extra]  (only if request 2 has XEN_NETIF_EXTRA_MORE)
- *  Request 4: netif_tx_request -- NETTXF_more_data
- *  Request 5: netif_tx_request -- NETTXF_more_data
+ *  Request 1: netif_tx_request_t -- NETTXF_* (any flags)
+ * [Request 2: netif_extra_info_t] (only if request 1 has NETTXF_extra_info)
+ * [Request 3: netif_extra_info_t] (only if request 2 has XEN_NETIF_EXTRA_MORE)
+ *  Request 4: netif_tx_request_t -- NETTXF_more_data
+ *  Request 5: netif_tx_request_t -- NETTXF_more_data
  *  ...
- *  Request N: netif_tx_request -- 0
+ *  Request N: netif_tx_request_t -- 0
+ */
+
+/*
+ * Guest transmit
+ * ==============
+ *
+ * Ring slot size is 12 octets, however not all request/response
+ * structs use the full size.
+ *
+ * tx request data (netif_tx_request_t)
+ * ------------------------------------
+ *
+ *    0     1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * | grant ref             | offset    | flags     |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * | id        | size      |
+ * +-----+-----+-----+-----+
+ *
+ * grant ref: Reference to buffer page.
+ * offset: Offset within buffer page.
+ * flags: NETTXF_*.
+ * id: request identifier, echoed in response.
+ * size: packet size in bytes.
+ *
+ * tx response (netif_tx_response_t)
+ * ---------------------------------
+ *
+ *    0     1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * | id        | status    | unused                |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * | unused                |
+ * +-----+-----+-----+-----+
+ *
+ * id: reflects id in transmit request
+ * status: NETIF_RSP_*
+ *
+ * Guest receive
+ * =============
+ *
+ * Ring slot size is 8 octets.
+ *
+ * rx request (netif_rx_request_t)
+ * -------------------------------
+ *
+ *    0     1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * | id        | pad       | gref                  |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * id: request identifier, echoed in response.
+ * gref: reference to incoming granted frame.
+ *
+ * rx response (netif_rx_response_t)
+ * ---------------------------------
+ *
+ *    0     1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * | id        | offset    | flags     | status    |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * id: reflects id in receive request
+ * offset: offset in page of start of received packet
+ * flags: NETRXF_*
+ * status: -ve: NETIF_RSP_*; +ve: Rx'ed pkt size.
+ *
+ * Extra Info
+ * ==========
+ *
+ * Can be present if initial request has NET{T,R}XF_extra_info, or
+ * previous extra request has XEN_NETIF_EXTRA_MORE.
+ *
+ * The struct therefore needs to fit into either a tx or rx slot and
+ * is therefore limited to 8 octets.
+ *
+ * extra info (netif_extra_info_t)
+ * -------------------------------
+ *
+ * General format:
+ *
+ *    0     1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |type |flags| type specfic data                 |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * | padding for tx        |
+ * +-----+-----+-----+-----+
+ *
+ * type: XEN_NETIF_EXTRA_TYPE_*
+ * flags: XEN_NETIF_EXTRA_FLAG_*
+ * padding for tx: present only in the tx case due to 8 octet limit
+ *     from rx case. Not shown in type specific entries below.
+ *
+ * XEN_NETIF_EXTRA_TYPE_GSO:
+ *
+ *    0     1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |type |flags| size      |type | pad | features  |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * type: Must be XEN_NETIF_EXTRA_TYPE_GSO
+ * flags: XEN_NETIF_EXTRA_FLAG_*
+ * size: Maximum payload size of each segment.
+ * type: XEN_NETIF_GSO_TYPE_*
+ * features: EN_NETIF_GSO_FEAT_*
+ *
+ * XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}:
+ *
+ *    0     1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |type |flags| addr                              |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * type: Must be XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}
+ * flags: XEN_NETIF_EXTRA_FLAG_*
+ * addr: address to add/remove
  */
 
 /* Protocol checksum field is blank in the packet (hardware offload)? */
@@ -179,7 +295,7 @@ typedef struct netif_tx_request netif_tx_request_t;
 #define XEN_NETIF_EXTRA_TYPE_MCAST_DEL (3)  /* u.mcast */
 #define XEN_NETIF_EXTRA_TYPE_MAX       (4)
 
-/* netif_extra_info flags. */
+/* netif_extra_info_t flags. */
 #define _XEN_NETIF_EXTRA_FLAG_MORE (0)
 #define XEN_NETIF_EXTRA_FLAG_MORE  (1U<<_XEN_NETIF_EXTRA_FLAG_MORE)
 
@@ -189,8 +305,8 @@ typedef struct netif_tx_request netif_tx_request_t;
 #define XEN_NETIF_GSO_TYPE_TCPV6        (2)
 
 /*
- * This structure needs to fit within both netif_tx_request and
- * netif_rx_response for compatibility.
+ * This structure needs to fit within both netif_tx_request_t and
+ * netif_rx_response_t for compatibility.
  */
 struct netif_extra_info {
     uint8_t type;  /* XEN_NETIF_EXTRA_TYPE_* */
@@ -251,6 +367,7 @@ typedef struct netif_tx_response netif_tx_response_t;
 
 struct netif_rx_request {
     uint16_t    id;        /* Echoed in response message.        */
+    uint16_t    pad;
     grant_ref_t gref;      /* Reference to incoming granted frame */
 };
 typedef struct netif_rx_request netif_rx_request_t;
@@ -289,7 +406,7 @@ DEFINE_RING_TYPES(netif_rx, struct netif_rx_request, struct netif_rx_response);
 #define NETIF_RSP_DROPPED         -2
 #define NETIF_RSP_ERROR           -1
 #define NETIF_RSP_OKAY             0
-/* No response: used for auxiliary requests (e.g., netif_tx_extra). */
+/* No response: used for auxiliary requests (e.g., netif_extra_info_t). */
 #define NETIF_RSP_NULL             1
 
 #endif
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH DOCDAY v2] netif.h: describe request/response structures in terms of binary layout

[David Vrabel]

On 25/02/15 13:39, Ian Campbell wrote:
> 
> + * Guest transmit
> + * ==============
> + *
> + * Ring slot size is 12 octets, however not all request/response
> + * structs use the full size.
> + *
> + * tx request data (netif_tx_request_t)
> + * ------------------------------------
> + *
> + *    0     1     2     3     4     5     6     7  octet
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * | grant ref             | offset    | flags     |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * | id        | size      |
> + * +-----+-----+-----+-----+
> + *
> + * grant ref: Reference to buffer page.
> + * offset: Offset within buffer page.
> + * flags: NETTXF_*.
> + * id: request identifier, echoed in response.
> + * size: packet size in bytes.

This size field should be described better since it's a bit weird when a
packet it split across multiple requests -- I have to look at the
implementations every time.

David

Re: [PATCH DOCDAY v2] netif.h: describe request/response structures in terms of binary layout

[Ian Campbell]

On Wed, 2015-02-25 at 13:57 +0000, David Vrabel wrote:
> On 25/02/15 13:39, Ian Campbell wrote:
> > 
> > + * Guest transmit
> > + * ==============
> > + *
> > + * Ring slot size is 12 octets, however not all request/response
> > + * structs use the full size.
> > + *
> > + * tx request data (netif_tx_request_t)
> > + * ------------------------------------
> > + *
> > + *    0     1     2     3     4     5     6     7  octet
> > + * +-----+-----+-----+-----+-----+-----+-----+-----+
> > + * | grant ref             | offset    | flags     |
> > + * +-----+-----+-----+-----+-----+-----+-----+-----+
> > + * | id        | size      |
> > + * +-----+-----+-----+-----+
> > + *
> > + * grant ref: Reference to buffer page.
> > + * offset: Offset within buffer page.
> > + * flags: NETTXF_*.
> > + * id: request identifier, echoed in response.
> > + * size: packet size in bytes.
> 
> This size field should be described better since it's a bit weird when a
> packet it split across multiple requests -- I have to look at the
> implementations every time.

Me too :-/

My intention with this patch though was to make the binary layout
clearer, so the field descriptions themselves are the same as before.
Improving those will have to wait for another time.

Ian.

Re: [PATCH DOCDAY v2] netif.h: describe request/response structures in terms of binary layout

[David Vrabel]

On 02/03/15 17:08, Ian Campbell wrote:
> On Wed, 2015-02-25 at 13:57 +0000, David Vrabel wrote:
>> On 25/02/15 13:39, Ian Campbell wrote:
>>>
>>> + * Guest transmit
>>> + * ==============
>>> + *
>>> + * Ring slot size is 12 octets, however not all request/response
>>> + * structs use the full size.
>>> + *
>>> + * tx request data (netif_tx_request_t)
>>> + * ------------------------------------
>>> + *
>>> + *    0     1     2     3     4     5     6     7  octet
>>> + * +-----+-----+-----+-----+-----+-----+-----+-----+
>>> + * | grant ref             | offset    | flags     |
>>> + * +-----+-----+-----+-----+-----+-----+-----+-----+
>>> + * | id        | size      |
>>> + * +-----+-----+-----+-----+
>>> + *
>>> + * grant ref: Reference to buffer page.
>>> + * offset: Offset within buffer page.
>>> + * flags: NETTXF_*.
>>> + * id: request identifier, echoed in response.
>>> + * size: packet size in bytes.
>>
>> This size field should be described better since it's a bit weird when a
>> packet it split across multiple requests -- I have to look at the
>> implementations every time.
> 
> Me too :-/
> 
> My intention with this patch though was to make the binary layout
> clearer, so the field descriptions themselves are the same as before.
> Improving those will have to wait for another time.

Fair enough.

David

Re: [PATCH DOCDAY v2] netif.h: describe request/response structures in terms of binary layout

[Ian Campbell]

On Wed, 2015-02-25 at 13:39 +0000, Ian Campbell wrote:
> In RFC style, rather than relying on the implicit assumptions of a
> particular C ABI.
> 
> I have also confirmed, using the Python gdb extension technique in
> [0], that the struct offsets (in a Linux binary at least) are the same
> as described here.
> 
> I took the opportunity to also confirm that x86_32, x86_64, arm32 and
> arm64 are all the same.
> 
> This highlighted that struct netif_rx_request was missing some
> explicit padding, which is added here.
> 
> Lastly, fixup some struct names to allow the generated docs to
> properly hyperlink, mainly by adding the _t to type names where
> appropriate, but also s/netif_tx_extra/netif_extra_info_t/.
> 
> [0] http://stackoverflow.com/questions/9788679/how-to-get-the-relative-adress-of-a-field-in-a-structure-dump-c
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Applied, thanks.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel