[PATCH v2 0/4] Updates to public/io/netif.h

[PATCH v2 0/4] Updates to public/io/netif.h

[Paul Durrant]

This series makes several modifications to netif.h in anticipation of
implementing NDIS RSS support in the Windows frontend driver.

Patch #1 documents the (sad) reality of the netif_rx_request/response id
field, which has been long overdue.

Patch #2 adds a definition of the NETRXF_gso_prefix flag which has been
present in the Linux variant of the header for several years

Patch #3 adds documentation for how negotiation of a hash algortithm to
be used on the frontend receive side should be done.

Patch #4 adds a new netif_extra_info type for passing hash values between
backend and frontend (or vice versa).

[PATCH v2 1/4] public/io/netif.h: document the reality of netif_rx_request/reponse id

[Paul Durrant]

The id field of the netif_rx_request_t abd netif_rx_response_t structures
is actually useless.

Because GSO metadata is passed from backend to frontend using
netif_extra_info segments, which do not carry information stating which
netif_rx_request_t was consumed to free up their slot, frontends assume
that the consumed request is the one that previously occupied that same
slot in the shared ring. Also, whilst theoretically possible for other
responses to be re-ordered, they never have been and that assumption has
always been baked into Linux xen-netfront.

Hence this patch documents that the request id and the response id must
be equal to the ring slot index modulo the ring size.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/include/public/io/netif.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index 5c31ae3..0fd413a 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -210,7 +210,7 @@
  * | id        | pad       | gref                  |
  * +-----+-----+-----+-----+-----+-----+-----+-----+
  *
- * id: request identifier, echoed in response.
+ * id: must be identical to ring slot index modulo ring size.
  * gref: reference to incoming granted frame.
  *
  * rx response (netif_rx_response_t)
@@ -221,11 +221,18 @@
  * | id        | offset    | flags     | status    |
  * +-----+-----+-----+-----+-----+-----+-----+-----+
  *
- * id: reflects id in receive request
+ * id: must be identical to ring slot index modulo ring size.
  * offset: offset in page of start of received packet
  * flags: NETRXF_*
  * status: -ve: NETIF_RSP_*; +ve: Rx'ed pkt size.
  *
+ * NOTE: The reason that id must be identical to ring slot index
+ *       modulo ring size is because extra info segments (see below)
+ *       carry no indication of the netif_rx_request_t that was
+ *       consumed to make their slot available. The only way a
+ *       frontend can determine which netif_rx_request_t was consumed
+ *       is using the id -> slot identity relation.
+ *
  * Extra Info
  * ==========
  *
-- 
2.1.4

[PATCH v2 2/4] public/io/netif.h: add definition of gso_prefix flag

[Paul Durrant]

This flag is defined here only for compatibility with the Linux variant of
this header. The feature has never been documented and should be
considered deprecated.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/include/public/io/netif.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index 0fd413a..57eb903 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -402,6 +402,10 @@ typedef struct netif_rx_request netif_rx_request_t;
 #define _NETRXF_extra_info     (3)
 #define  NETRXF_extra_info     (1U<<_NETRXF_extra_info)
 
+/* Packet has GSO prefix. Deprecated but included for compatibility */
+#define _NETRXF_gso_prefix     (4)
+#define  NETRXF_gso_prefix     (1U<<_NETRXF_gso_prefix)
+
 struct netif_rx_response {
     uint16_t id;
     uint16_t offset;       /* Offset in page of start of received packet  */
-- 
2.1.4

Re: [PATCH v2 2/4] public/io/netif.h: add definition of gso_prefix flag

[Paul Durrant]

> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 16 October 2015 13:41
> To: xen-devel@lists.xenproject.org
> Cc: Paul Durrant; Ian Campbell; Ian Jackson; Jan Beulich; Keir (Xen.org); Tim
> (Xen.org)
> Subject: [PATCH v2 2/4] public/io/netif.h: add definition of gso_prefix flag
> 
> This flag is defined here only for compatibility with the Linux variant of
> this header. The feature has never been documented and should be
> considered deprecated.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

Unfortunately, I found a typo in this patch. I'll send v3 of the series.

  Paul

> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> ---
>  xen/include/public/io/netif.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
> index 0fd413a..57eb903 100644
> --- a/xen/include/public/io/netif.h
> +++ b/xen/include/public/io/netif.h
> @@ -402,6 +402,10 @@ typedef struct netif_rx_request netif_rx_request_t;
>  #define _NETRXF_extra_info     (3)
>  #define  NETRXF_extra_info     (1U<<_NETRXF_extra_info)
> 
> +/* Packet has GSO prefix. Deprecated but included for compatibility */
> +#define _NETRXF_gso_prefix     (4)
> +#define  NETRXF_gso_prefix     (1U<<_NETRXF_gso_prefix)
> +
>  struct netif_rx_response {
>      uint16_t id;
>      uint16_t offset;       /* Offset in page of start of received packet  */
> --
> 2.1.4

[PATCH v2 3/4] public/io/netif.h: add documentation for hash negotiation and mapping

[Paul Durrant]

This patch specifies the xenstore keys that should be used by frontends
and backends to negotiate a particular hash algorithm and queue mapping
to be used for mult-queue packet steering on the guest receive side.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/include/public/io/netif.h | 111 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 106 insertions(+), 5 deletions(-)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index 57eb903..d7c978e 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -114,11 +114,112 @@
  * error. This includes scenarios where more (or fewer) queues were
  * requested than the frontend provided details for.
  *
- * Mapping of packets to queues is considered to be a function of the
- * transmitting system (backend or frontend) and is not negotiated
- * between the two. Guests are free to transmit packets on any queue
- * they choose, provided it has been set up correctly. Guests must be
- * prepared to receive packets on any queue they have requested be set up.
+ * Unless a hash algorithm or mapping of packet hash to queues has been
+ * negotiated (see below), queue selection is considered to be a function of
+ * the transmitting system (backend or frontend) and either end is free to
+ * transmit packets on any queue, provided it has been set up correctly.
+ * Guests must therefore be prepared to receive packets on any queue they
+ * have requested be set up.
+ */
+
+/*
+ * Hash negotiation (only applicable if using multiple queues):
+ *
+ * A backend can advertise a set of hash algorithms that it can perform by
+ * naming it in a space separated list in the "multi-queue-hash-list"
+ * xenstore key. For example, if the backend supports the 'foo' and 'bar'
+ * algorithms it would set:
+ *
+ * /local/domain/X/backend/vif/Y/Z/multi-queue-hash-list = "foo bar"
+ *
+ * Additionally, in supporting a particular algorithm, it may be necessary
+ * for the backend to specify the capabilites of its implementation of
+ * that algorithm, e.g. what sections of packet header it can hash.
+ * To do that it can set algorithm-specific keys under a parent capabilities
+ * key. For example, if the 'bar' algorithm implementation in the backend
+ * is capable of hashing over an IP version 4 header and a TCP header, the
+ * backend might set:
+ *
+ * /local/domain/X/backend/vif/Y/Z/multi-queue-hash-caps-bar/types = "ipv4+tcp"
+ *
+ * The backend should set all such keys before it moves into the initwait
+ * state.
+ *
+ * The frontend can select a hash algorithm at any time after it moves into
+ * the connected state by setting the "multi-queue-hash" key. The backend
+ * must therefore watch this key and be prepared to change hash algorithms
+ * at any time whilst in the connected state. So, for example, if the
+ * frontend wants 'foo' hashing, it should set:
+ *
+ * /local/domain/Y/device/vif/Z/multi-queue-hash = "foo"
+ *
+ * Additionally it may set parameters for that algorithm by setting
+ * algorithm-specific keys under a parent parameters key. For example, if
+ * the 'foo' algorithm implementation in the backend is capable of hashing
+ * over an IP version 4 header, a TCP header or both but the frontend only
+ * wants it to hash over only the IP version 4 header then it might set:
+ *
+ * /local/domain/Y/device/vif/Z/multi-queue-hash-params-foo/types = "ipv4"
+ *
+ * The backend must also watch the parameters key as the frontend may
+ * change the parameters at any time whilst in the connected state.
+ *
+ * (Capabilites and parameters documentation for specific algorithms is
+ * below).
+ *
+ * TOEPLITZ:
+ *
+ * If the backend supports Toeplitz hashing then it should include
+ * the algorithm name 'toeplitz' in its "multi-queue-hash-list" key.
+ * It should also advertise the following capabilities:
+ *
+ * types: a space separated list containing any or all of 'ipv4', 'tcpv4',
+ *        'ipv6', 'tcpv6', indicating over which headers the hash algorithm
+ *        is capable of being performed.
+ *
+ * max-key-length: an integer value indicating the maximum key length (in
+ *                 octets) that the frontend may supply.
+ *
+ * Upon selecting this algorithm, the frontend may supply the following
+ * parameters.
+ *
+ * types: a space separated list containing none, any or all of the type
+ *        names included in the types list in the capabilites.
+ *        When the backend encounters a packet type not in this list it
+ *        will assign a hash value of 0.
+ *
+ * key: a ':' separated list of octets (up to the maximum length specified
+ *      in the capabilities) expressed in hexadecimal indicating the key
+ *      that should be used in the hash calculation.
+ *
+ * For more information on Toeplitz hash calculation see:
+ *
+ * https://msdn.microsoft.com/en-us/library/windows/hardware/ff570725.aspx
+ */
+
+/*
+ * Hash mapping (only applicable if using multiple queues):
+ *
+ * If the backend is not capable, or no mapping is specified by the frontend
+ * then it is assumed that the hash -> queue mapping is done by simple
+ * modular arithmetic.
+ *
+ * To advertise that it is capable of accepting a specific mapping from the
+ * frontend the backend should set the "multi-queue-max-hash-mapping-length"
+ * key to a non-zero value. The frontend may then specify a mapping (up to
+ * the maximum specified length) as a ',' separated list of decimal queue
+ * numbers in the "multi-queue-hash-mapping" key.
+ *
+ * The backend should parse this list into an array and perform the mapping
+ * as follows:
+ *
+ * queue = mapping[hash % length-of-list]
+ *
+ * If any of the queue values specified in the list is not connected then
+ * the backend is free to choose a connected queue arbitrarily.
+ *
+ * The backend must be prepared to handle updates the mapping list at any
+ * time whilst in the connected state.
  */
 
 /*
-- 
2.1.4

[PATCH v2 4/4] public/io/netif.h: add extra info slots for passing hash values

[Paul Durrant]

To properly support NDIS RSS, the Windows frontend PV driver needs the
Toeplitz hash value calculated by the backend (otherwise it would have to
duplicate the calculation).

This patch adds documentation for "feature-hash" and a definition of a
new XEN_NETIF_EXTRA_TYPE_HASH extra info segment.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/include/public/io/netif.h | 52 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index d7c978e..97734b7 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -252,6 +252,12 @@
  */
 
 /*
+ * "feature-hash" advertises the capability to accept extra info slots of
+ * type XEN_NETIF_EXTRA_TYPE_HASH. They will not be sent by either end
+ * unless the other end advertises this feature.
+ */
+
+/*
  * This is the 'wire' format for packets:
  *  Request 1: netif_tx_request_t -- NETTXF_* (any flags)
  * [Request 2: netif_extra_info_t] (only if request 1 has NETTXF_extra_info)
@@ -383,6 +389,18 @@
  * type: Must be XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}
  * flags: XEN_NETIF_EXTRA_FLAG_*
  * addr: address to add/remove
+ *
+ * XEN_NETIF_EXTRA_TYPE_HASH:
+ *
+ *    0     1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |type |flags|htype| pad |LSB ---- value ---- MSB|
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * type: Must be XEN_NETIF_EXTRA_TYPE_HASH
+ * flags: XEN_NETIF_EXTRA_FLAG_*
+ * htype: XEN_NETIF_HASH_TYPE_*
+ * value: Hash value
  */
 
 /* Protocol checksum field is blank in the packet (hardware offload)? */
@@ -412,11 +430,12 @@ struct netif_tx_request {
 typedef struct netif_tx_request netif_tx_request_t;
 
 /* Types of netif_extra_info descriptors. */
-#define XEN_NETIF_EXTRA_TYPE_NONE      (0)  /* Never used - invalid */
-#define XEN_NETIF_EXTRA_TYPE_GSO       (1)  /* u.gso */
-#define XEN_NETIF_EXTRA_TYPE_MCAST_ADD (2)  /* u.mcast */
-#define XEN_NETIF_EXTRA_TYPE_MCAST_DEL (3)  /* u.mcast */
-#define XEN_NETIF_EXTRA_TYPE_MAX       (4)
+#define XEN_NETIF_EXTRA_TYPE_NONE       (0) /* Never used - invalid */
+#define XEN_NETIF_EXTRA_TYPE_GSO        (1) /* u.gso */
+#define XEN_NETIF_EXTRA_TYPE_MCAST_ADD  (2) /* u.mcast */
+#define XEN_NETIF_EXTRA_TYPE_MCAST_DEL  (3) /* u.mcast */
+#define XEN_NETIF_EXTRA_TYPE_MCAST_HASH (4) /* u.hash */
+#define XEN_NETIF_EXTRA_TYPE_MAX        (5)
 
 /* netif_extra_info_t flags. */
 #define _XEN_NETIF_EXTRA_FLAG_MORE (0)
@@ -427,6 +446,13 @@ typedef struct netif_tx_request netif_tx_request_t;
 #define XEN_NETIF_GSO_TYPE_TCPV4        (1)
 #define XEN_NETIF_GSO_TYPE_TCPV6        (2)
 
+/* Hash types */
+#define XEN_NETIF_HASH_TYPE_NONE        (0)
+#define XEN_NETIF_HASH_TYPE_TCPV4       (1)
+#define XEN_NETIF_HASH_TYPE_IPV4        (2)
+#define XEN_NETIF_HASH_TYPE_TCPV6       (3)
+#define XEN_NETIF_HASH_TYPE_IPV6        (4)
+
 /*
  * This structure needs to fit within both netif_tx_request_t and
  * netif_rx_response_t for compatibility.
@@ -469,6 +495,22 @@ struct netif_extra_info {
             uint8_t addr[6]; /* Address to add/remove. */
         } mcast;
 
+        /*
+         * XEN_NETIF_EXTRA_TYPE_HASH:
+         */
+        struct {
+            /* Hash type. This indicates the sections of header over which
+             * the hash has been calculated.
+             */
+            uint8_t type; /* XEN_NETIF_HASH_TYPE_* */
+
+            /* Future expansion. */
+            uint8_t pad;
+
+            /* Hash value. */
+            uint8_t value[4];
+        } hash;
+
         uint16_t pad[3];
     } u;
 };
-- 
2.1.4