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

[PATCH v4 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
protocol, 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 #5 (new in v4) reduces comment duplication and fixes formatting.

[PATCH v5 1/5] public/io/netif.h: document the reality of netif_rx_request/reponse

[Paul Durrant]

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 must
assume some form of identity relation between ring slot and request.
Hence, so that it is able to use GSO metadata, Linux netfront simply
assumes rx responses appear in the same ring slot as their corresponding
request.

This patch documents the assumption made by Linux netfront and the
necessity of the assumption (to support GSO) so that backends are coded
to be compatible.

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>
---

v4:
 - Add slightly more explanation in a comment

v5:
 - Re-word the comments since the restriction is not actually on
   the rx request/response id field, but the slots used by requests
   and responses.
---
 xen/include/public/io/netif.h | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index 5c31ae3..4d2a300 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -226,15 +226,29 @@
  * flags: NETRXF_*
  * status: -ve: NETIF_RSP_*; +ve: Rx'ed pkt size.
  *
+ * NOTE: Historically, to support GSO on the frontend receive side, Linux
+ *       netfront does not make use of the rx response id (because, as
+ *       described below, extra info structures overlay the id field).
+ *       Instead it assumes that responses always appear in the same ring
+ *       slot as their corresponding request. Thus, to maintain
+ *       compatibilty, backends must make sure this is the case.
+ *
  * Extra Info
  * ==========
  *
- * Can be present if initial request has NET{T,R}XF_extra_info, or
- * previous extra request has XEN_NETIF_EXTRA_MORE.
+ * Can be present if initial request or response 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.
  *
+ * NOTE: Because extra info data overlays the usual request/response
+ *       structures, there is no id information in the opposite direction.
+ *       So, if an extra info overlays an rx response the frontend can
+ *       assume that it is in the same ring slot as the request that was
+ *       consumed to make the slot available, and the backend must ensure
+ *       this assumption is true.
+ *
  * extra info (netif_extra_info_t)
  * -------------------------------
  *
@@ -373,7 +387,7 @@ struct netif_tx_response {
 typedef struct netif_tx_response netif_tx_response_t;
 
 struct netif_rx_request {
-    uint16_t    id;        /* Echoed in response message.        */
+    uint16_t    id;
     uint16_t    pad;
     grant_ref_t gref;      /* Reference to incoming granted frame */
 };
-- 
2.1.4

[PATCH v5 2/5] 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 4d2a300..46b85b8 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -409,6 +409,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 v5 3/5] 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>
---

v4:
 - Fix spelling of 'capabilities'
---
 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 46b85b8..b60e86b 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 capabilities 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.
+ *
+ * (Capabilities 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 capabilities.
+ *        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

Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash negotiation and mapping

[Jan Beulich]

>>> On 20.10.15 at 14:35, <paul.durrant@citrix.com> wrote:
> +/*
> + * 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"

Wouldn't comma separated be the more usual form?

> + * Additionally, in supporting a particular algorithm, it may be necessary
> + * for the backend to specify the capabilities 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.
> + *
> + * (Capabilities 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.

Same question regarding space uses as separator. Also I think the
separator(s) permitted should be mentioned above, where the types
key gets introduced.

> + * 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 capabilities.
> + *        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.

While I see no way around this proliferation of keys, have you
considered the resource consumption effect? Guests have a limit on
how much space they may consume in xenstore, and with additions
like these it seems increasingly likely for the defaults to no longer be
sufficient.

> +/*
> + * 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.

Again due to space constraints, is decimal really a good choice here?
Hex would be more dense, and one might consider an even higher
radix.

Jan

Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash negotiation and mapping

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 22 October 2015 09:48
> To: Paul Durrant
> Cc: Ian Campbell; Ian Jackson; xen-devel@lists.xenproject.org; Keir
> (Xen.org); Tim (Xen.org)
> Subject: Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash
> negotiation and mapping
> 
> >>> On 20.10.15 at 14:35, <paul.durrant@citrix.com> wrote:
> > +/*
> > + * 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"
> 
> Wouldn't comma separated be the more usual form?
> 

I was referring back to the original proposal made by Andrew Bennieston a couple of years ago (see http://lists.xen.org/archives/html/xen-devel/2013-06/msg02654.html) which proposed a space separated list and I don't think anyone disagreed). I'm happy with comma separated if everyone else is though.

> > + * Additionally, in supporting a particular algorithm, it may be necessary
> > + * for the backend to specify the capabilities 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.
> > + *
> > + * (Capabilities 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.
> 
> Same question regarding space uses as separator. Also I think the
> separator(s) permitted should be mentioned above, where the types
> key gets introduced.

Ok.

> 
> > + * 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 capabilities.
> > + *        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.
> 
> While I see no way around this proliferation of keys, have you
> considered the resource consumption effect? Guests have a limit on
> how much space they may consume in xenstore, and with additions
> like these it seems increasingly likely for the defaults to no longer be
> sufficient.
> 

I have considered it and I think it will probably mean adjustments when we pull this into XenServer. Do you think it's worth making a change in the default oxenstored.conf as part of this series?

> > +/*
> > + * 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.
> 
> Again due to space constraints, is decimal really a good choice here?
> Hex would be more dense, and one might consider an even higher
> radix.
> 

I'm ok with hex. Using an alternative higher radix may cause confusion I suspect.

  Paul

> Jan

Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash negotiation and mapping

[Jan Beulich]

>>> On 22.10.15 at 12:44, <Paul.Durrant@citrix.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 22 October 2015 09:48
>> >>> On 20.10.15 at 14:35, <paul.durrant@citrix.com> wrote:
>> > + * 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 capabilities.
>> > + *        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.
>> 
>> While I see no way around this proliferation of keys, have you
>> considered the resource consumption effect? Guests have a limit on
>> how much space they may consume in xenstore, and with additions
>> like these it seems increasingly likely for the defaults to no longer be
>> sufficient.
> 
> I have considered it and I think it will probably mean adjustments when we 
> pull this into XenServer. Do you think it's worth making a change in the 
> default oxenstored.conf as part of this series?

Well, I've actually been looking a the C variant when replying. And
whether increasing the defaults is an acceptable thing I'm not sure
- after all there is a point for these limits to be there (I suppose).

Jan

Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash negotiation and mapping

[Ian Campbell]

On Thu, 2015-10-22 at 05:00 -0600, Jan Beulich wrote:
> > > > On 22.10.15 at 12:44, <Paul.Durrant@citrix.com> wrote:
> > > From: Jan Beulich [mailto:JBeulich@suse.com]
> > > Sent: 22 October 2015 09:48
> > > > > > On 20.10.15 at 14:35, <paul.durrant@citrix.com> wrote:
> > > > + * 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 capabilities.
> > > > + *        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.
> > > 
> > > While I see no way around this proliferation of keys, have you
> > > considered the resource consumption effect? Guests have a limit on
> > > how much space they may consume in xenstore, and with additions
> > > like these it seems increasingly likely for the defaults to no longer
> > > be
> > > sufficient.
> > 
> > I have considered it and I think it will probably mean adjustments when
> > we 
> > pull this into XenServer. Do you think it's worth making a change in
> > the 
> > default oxenstored.conf as part of this series?
> 
> Well, I've actually been looking a the C variant when replying. And
> whether increasing the defaults is an acceptable thing I'm not sure
> - after all there is a point for these limits to be there (I suppose).

It's to prevent the guest consuming an arbitrary amount of resource in the
xenstored domain.

Is there not some other way this sort of "bulkish" (-ish because I'm not
sure how large these things can be) can be communicated, either via
add/remove ring operations or by a dedicated granted page or ... ?

IIRC there is also a limit on the maximum size of a key inherent in the XS
wire protocol. Maybe 2 or 4K?


Ian.

Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash negotiation and mapping

[Paul Durrant]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: 22 October 2015 12:32
> To: Jan Beulich; Paul Durrant
> Cc: Ian Jackson; xen-devel@lists.xenproject.org; Keir (Xen.org); Tim
> (Xen.org)
> Subject: Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash
> negotiation and mapping
> 
> On Thu, 2015-10-22 at 05:00 -0600, Jan Beulich wrote:
> > > > > On 22.10.15 at 12:44, <Paul.Durrant@citrix.com> wrote:
> > > > From: Jan Beulich [mailto:JBeulich@suse.com]
> > > > Sent: 22 October 2015 09:48
> > > > > > > On 20.10.15 at 14:35, <paul.durrant@citrix.com> wrote:
> > > > > + * 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 capabilities.
> > > > > + *        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.
> > > >
> > > > While I see no way around this proliferation of keys, have you
> > > > considered the resource consumption effect? Guests have a limit on
> > > > how much space they may consume in xenstore, and with additions
> > > > like these it seems increasingly likely for the defaults to no longer
> > > > be
> > > > sufficient.
> > >
> > > I have considered it and I think it will probably mean adjustments when
> > > we
> > > pull this into XenServer. Do you think it's worth making a change in
> > > the
> > > default oxenstored.conf as part of this series?
> >
> > Well, I've actually been looking a the C variant when replying. And
> > whether increasing the defaults is an acceptable thing I'm not sure
> > - after all there is a point for these limits to be there (I suppose).
> 
> It's to prevent the guest consuming an arbitrary amount of resource in the
> xenstored domain.
> 
> Is there not some other way this sort of "bulkish" (-ish because I'm not
> sure how large these things can be) can be communicated, either via
> add/remove ring operations or by a dedicated granted page or ... ?
> 
> IIRC there is also a limit on the maximum size of a key inherent in the XS
> wire protocol. Maybe 2 or 4K?
> 

The other precedent for this kind of thing would be the dummy tx + extra seg used for multicast add/remove. I could use the gref in such a beast to point at a guest page and then the extra seg type say whether it’s table or key data. Or, I could simply replace the lists here with grefs pointing at pages containing the data and state that the xenstore keys will simply be re-written (even if there is no change in the gref) to indicate table or key updates. Any preferences?

  Paul

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

Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash negotiation and mapping

[Paul Durrant]

> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> bounces@lists.xen.org] On Behalf Of Paul Durrant
> Sent: 22 October 2015 12:43
> To: Ian Campbell; Jan Beulich
> Cc: Ian Jackson; Tim (Xen.org); Keir (Xen.org); xen-
> devel@lists.xenproject.org
> Subject: Re: [Xen-devel] [PATCH v5 3/5] public/io/netif.h: add
> documentation for hash negotiation and mapping
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: 22 October 2015 12:32
> > To: Jan Beulich; Paul Durrant
> > Cc: Ian Jackson; xen-devel@lists.xenproject.org; Keir (Xen.org); Tim
> > (Xen.org)
> > Subject: Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash
> > negotiation and mapping
> >
> > On Thu, 2015-10-22 at 05:00 -0600, Jan Beulich wrote:
> > > > > > On 22.10.15 at 12:44, <Paul.Durrant@citrix.com> wrote:
> > > > > From: Jan Beulich [mailto:JBeulich@suse.com]
> > > > > Sent: 22 October 2015 09:48
> > > > > > > > On 20.10.15 at 14:35, <paul.durrant@citrix.com> wrote:
> > > > > > + * 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 capabilities.
> > > > > > + *        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.
> > > > >
> > > > > While I see no way around this proliferation of keys, have you
> > > > > considered the resource consumption effect? Guests have a limit on
> > > > > how much space they may consume in xenstore, and with additions
> > > > > like these it seems increasingly likely for the defaults to no longer
> > > > > be
> > > > > sufficient.
> > > >
> > > > I have considered it and I think it will probably mean adjustments when
> > > > we
> > > > pull this into XenServer. Do you think it's worth making a change in
> > > > the
> > > > default oxenstored.conf as part of this series?
> > >
> > > Well, I've actually been looking a the C variant when replying. And
> > > whether increasing the defaults is an acceptable thing I'm not sure
> > > - after all there is a point for these limits to be there (I suppose).
> >
> > It's to prevent the guest consuming an arbitrary amount of resource in the
> > xenstored domain.
> >
> > Is there not some other way this sort of "bulkish" (-ish because I'm not
> > sure how large these things can be) can be communicated, either via
> > add/remove ring operations or by a dedicated granted page or ... ?
> >
> > IIRC there is also a limit on the maximum size of a key inherent in the XS
> > wire protocol. Maybe 2 or 4K?
> >
> 
> The other precedent for this kind of thing would be the dummy tx + extra
> seg used for multicast add/remove. I could use the gref in such a beast to
> point at a guest page and then the extra seg type say whether it’s table or
> key data. Or, I could simply replace the lists here with grefs pointing at pages
> containing the data and state that the xenstore keys will simply be re-written
> (even if there is no change in the gref) to indicate table or key updates. Any
> preferences?
> 

Actually, one more slightly radical thought... Maybe add a new ring type... A command ring, as well as RX and TX rings? This could be used to issue key and table update commands to the backend and would be there for other uses in future.

  Paul

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

Re: [PATCH v5 3/5] public/io/netif.h: add documentation for hash negotiation and mapping

[Jan Beulich]

>>> On 22.10.15 at 14:10, <Paul.Durrant@citrix.com> wrote:
> Actually, one more slightly radical thought... Maybe add a new ring type... 
> A command ring, as well as RX and TX rings? This could be used to issue key 
> and table update commands to the backend and would be there for other uses in 
> future.

That sounds like a good idea to me.

Jan

[PATCH v5 4/5] 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>
---

v4:
 - Fix typo
---
 xen/include/public/io/netif.h | 42 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index b60e86b..8a6cf3d 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)
@@ -390,6 +396,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)? */
@@ -419,11 +437,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_HASH       (4) /* u.hash */
+#define XEN_NETIF_EXTRA_TYPE_MAX        (5)
 
 /* netif_extra_info_t flags. */
 #define _XEN_NETIF_EXTRA_FLAG_MORE (0)
@@ -434,6 +453,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.
@@ -476,6 +502,12 @@ struct netif_extra_info {
             uint8_t addr[6]; /* Address to add/remove. */
         } mcast;
 
+        struct {
+            uint8_t type;
+            uint8_t pad;
+            uint8_t value[4];
+        } hash;
+
         uint16_t pad[3];
     } u;
 };
-- 
2.1.4

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

[Jan Beulich]

>>> On 20.10.15 at 14:35, <paul.durrant@citrix.com> wrote:
> @@ -390,6 +396,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
>   */

For future extensibility, require the pad field to be zero?

Jan

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

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 22 October 2015 09:51
> To: Paul Durrant
> Cc: Ian Campbell; Ian Jackson; xen-devel@lists.xenproject.org; Keir
> (Xen.org); Tim (Xen.org)
> Subject: Re: [PATCH v5 4/5] public/io/netif.h: add extra info slots for passing
> hash values
> 
> >>> On 20.10.15 at 14:35, <paul.durrant@citrix.com> wrote:
> > @@ -390,6 +396,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
> >   */
> 
> For future extensibility, require the pad field to be zero?
> 

Absolutely. That was my intention but you're right that it needs to be stated.

  Paul

> Jan

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

[Jan Beulich]

>>> On 22.10.15 at 12:45, <Paul.Durrant@citrix.com> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 22 October 2015 09:51
>> To: Paul Durrant
>> Cc: Ian Campbell; Ian Jackson; xen-devel@lists.xenproject.org; Keir
>> (Xen.org); Tim (Xen.org)
>> Subject: Re: [PATCH v5 4/5] public/io/netif.h: add extra info slots for 
> passing
>> hash values
>> 
>> >>> On 20.10.15 at 14:35, <paul.durrant@citrix.com> wrote:
>> > @@ -390,6 +396,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
>> >   */
>> 
>> For future extensibility, require the pad field to be zero?
> 
> Absolutely. That was my intention but you're right that it needs to be 
> stated.

And (without having looked at the patches) also enforced.

Jan

[PATCH v5 5/5] public/io/netif.h: tidy up and remove duplicate comments

[Paul Durrant]

Now that requests and response types and extra info segments are
documented in block comments, we can get rid of the inline comments
in the structures. This has the happy side-effect of making the Linux
checkpatch.pl script make fewer complaints after import.

This patch also fixes a small whitespace issue in the initial boiler-
plate comment.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 xen/include/public/io/netif.h | 81 +++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 49 deletions(-)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index 8a6cf3d..ce2aa53 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -1,8 +1,8 @@
 /******************************************************************************
  * netif.h
- * 
+ *
  * Unified network-device I/O interface for Xen guest OSes.
- * 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to
  * deal in the Software without restriction, including without limitation the
@@ -260,8 +260,10 @@
 /*
  * 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)
- * [Request 3: netif_extra_info_t] (only if request 2 has XEN_NETIF_EXTRA_MORE)
+ * [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
  *  ...
@@ -363,7 +365,7 @@
  *
  *    0     1     2     3     4     5     6     7  octet
  * +-----+-----+-----+-----+-----+-----+-----+-----+
- * |type |flags| type specfic data                 |
+ * |type |flags| type specific data                |
  * +-----+-----+-----+-----+-----+-----+-----+-----+
  * | padding for tx        |
  * +-----+-----+-----+-----+
@@ -371,7 +373,8 @@
  * 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.
+ *                 from rx case. Not shown in type specific entries
+ *                 below.
  *
  * XEN_NETIF_EXTRA_TYPE_GSO:
  *
@@ -382,9 +385,14 @@
  *
  * 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_*
+ * size: Maximum payload size of each segment. For example,
+ *       for TCP this is just the path MSS.
+ * type: XEN_NETIF_GSO_TYPE_*: This determines the protocol of
+ *       the packet and any extra features required to segment the
+ *       packet properly.
+ * features: EN_NETIF_GSO_FEAT_*: This specifies any extra GSO
+ *           features required to process this packet, such as ECN
+ *           support for TCPv4.
  *
  * XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}:
  *
@@ -428,11 +436,11 @@
 
 #define XEN_NETIF_MAX_TX_SIZE 0xFFFF
 struct netif_tx_request {
-    grant_ref_t gref;      /* Reference to buffer page */
-    uint16_t offset;       /* Offset within buffer page */
-    uint16_t flags;        /* NETTXF_* */
-    uint16_t id;           /* Echoed in response message. */
-    uint16_t size;         /* Packet size in bytes.       */
+    grant_ref_t gref;
+    uint16_t offset;
+    uint16_t flags;
+    uint16_t id;
+    uint16_t size;
 };
 typedef struct netif_tx_request netif_tx_request_t;
 
@@ -465,43 +473,18 @@ typedef struct netif_tx_request netif_tx_request_t;
  * netif_rx_response_t for compatibility.
  */
 struct netif_extra_info {
-    uint8_t type;  /* XEN_NETIF_EXTRA_TYPE_* */
-    uint8_t flags; /* XEN_NETIF_EXTRA_FLAG_* */
-
+    uint8_t type;
+    uint8_t flags;
     union {
-        /*
-         * XEN_NETIF_EXTRA_TYPE_GSO:
-         */
         struct {
-            /*
-             * Maximum payload size of each segment. For example, for TCP this
-             * is just the path MSS.
-             */
             uint16_t size;
-
-            /*
-             * GSO type. This determines the protocol of the packet and any
-             * extra features required to segment the packet properly.
-             */
-            uint8_t type; /* XEN_NETIF_GSO_TYPE_* */
-
-            /* Future expansion. */
+            uint8_t type;
             uint8_t pad;
-
-            /*
-             * GSO features. This specifies any extra GSO features required
-             * to process this packet, such as ECN support for TCPv4.
-             */
-            uint16_t features; /* XEN_NETIF_GSO_FEAT_* */
+            uint16_t features;
         } gso;
-
-        /*
-         * XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}:
-         */
         struct {
-            uint8_t addr[6]; /* Address to add/remove. */
+            uint8_t addr[6];
         } mcast;
-
         struct {
             uint8_t type;
             uint8_t pad;
@@ -515,14 +498,14 @@ typedef struct netif_extra_info netif_extra_info_t;
 
 struct netif_tx_response {
     uint16_t id;
-    int16_t  status;       /* NETIF_RSP_* */
+    int16_t  status;
 };
 typedef struct netif_tx_response netif_tx_response_t;
 
 struct netif_rx_request {
     uint16_t    id;
     uint16_t    pad;
-    grant_ref_t gref;      /* Reference to incoming granted frame */
+    grant_ref_t gref;
 };
 typedef struct netif_rx_request netif_rx_request_t;
 
@@ -548,9 +531,9 @@ typedef struct netif_rx_request netif_rx_request_t;
 
 struct netif_rx_response {
     uint16_t id;
-    uint16_t offset;       /* Offset in page of start of received packet  */
-    uint16_t flags;        /* NETRXF_* */
-    int16_t  status;       /* -ve: NETIF_RSP_* ; +ve: Rx'ed pkt size. */
+    uint16_t offset;
+    uint16_t flags;
+    int16_t  status;
 };
 typedef struct netif_rx_response netif_rx_response_t;
 
-- 
2.1.4