Re: [PATCH v7 2/2] public/io/netif.h: document control ring and toeplitz hashing

[Ian Campbell <ian.campbell@citrix.com>]

On Tue, 2016-01-12 at 09:58 +0000, Paul Durrant wrote:
> This patch documents a new shared ring between frontend and backend that
> can be used to pass bulk out-of-band data, such as that required to
> implement toeplitz hashing in the backend such that it is configurable by
> the frontend (which is needed to support NDIS RSS for Windows guests).
> 
> The patch then goes on to document the messages passed over the control
> ring that can be used to configure toeplitz hashing and a new extra info
> fragment that can be used to pass hash values between frontend and
> backend for both transmit and receive packets.
> 
> 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>
> ---
> v7:
>  - s/feature-control-ring/feature-ctrl-ring/g for consistency
> 
> v5:
>  - Clarify the control API for toeplitz hashing in many places.
>  - Add messages for getting and setting mapping table order to allow
>    for a table that is larger than can be mapped by a single grant
>    reference.
>  - Fold in the definition of the new extra info type for passing
>    hash values and make it toeplitz specific.
> 
> v4:
>  - Fix netif_ctrl_response_t definition to match specification.
> 
> v3:
>  - Fix commit comment.
> 
> v2:
>  - Use a balanced fix-sized message ring for the control ring
>    (bulk data now passed by grant reference).
> ---
>  xen/include/public/io/netif.h | 380
> +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 379 insertions(+), 1 deletion(-)

I noticed (after trimming the quotes unfortunately) that the request gained
a data[2] in v5 (as part of handling the table differently), so the req +
rsp are no longer the same size.

I'm not sure if that is worth worrying about. I don't think it would
simplify anything to include a padding bit, but it might be nice?

> 
> + * NETIF_CTRL_TYPE_SET_TOEPLITZ_MAPPING
> + * ------------------------------------
> + *
> + * This is sent by the frontend to set the content of the table mapping
> + * toeplitz hash value to queue number. The backend should calculate the
> + * hash from the packet header, use it as an index into the table (modulo
> + * the size of the table) and then steer the packet to the queue number
> + * found at that index.
> + *
> + * Request:
> + *
> + *  type    = NETIF_CTRL_TYPE_SET_TOEPLITZ_MAPPING
> + *  data[0] = grant reference of page containing the mapping (sub-)table
> + *            (assumed to start at beginning of grant)
> + *  data[1] = size of (sub-)table in entries
> + *  data[2] = offset, in entries, of sub-table within overall table

Adding data[2] seems reasonable to me, but if you wanted to avoid adding it
then saying data[1][8:0] == size and data[1][31:9] == offset would allow a
size of 512 (biggest possible in a single gref) and 8.3M for the offset.

Do the updates tend to come in large batches, or is setting single entries
or small runs of contiguous entries the norm? I suspect you are trying to
avoid copying 4K worth of data ofr each update when only a couple of
entries have changed, is that right?

All the above are just suggestions, which you are free to ignore, so if you
prefer things as they are that's fine by me:

Acked-by: Ian Campbell <ian.campbell@citrix.com>


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