Re: [PATCH v3 1/1] public/io/netif.h: add gref mapping control messages

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Joao Martins [mailto:joao.m.martins@oracle.com]
> Sent: 13 September 2017 19:11
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>;
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Joao Martins
> <joao.m.martins@oracle.com>
> Subject: [PATCH v3 1/1] public/io/netif.h: add gref mapping control messages
> 
> Adds 3 messages to allow guest to let backend keep grants mapped,
> such that 1) guests allowing fast recycling of pages can avoid doing
> grant ops for those cases, or otherwise 2) preferring copies over
> grants and 3) always using a fixed set of pages for network I/O.
> 
> The three control ring messages added are:
>  - Add grefs to be mapped by backend
>  - Remove grefs mappings (If they are not in use)
>  - Get maximum amount of grefs kept mapped.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
> v3:
> * Use DEL for unmapping grefs instead of PUT
> * Rname from xen_netif_gref_alloc to xen_netif_gref
> * Add 'status' field on xen_netif_gref
> * Clarify what 'inflight' means
> * Use "beginning of the page" instead of "beginning of the grant"
> * Mention that page needs to be r/w (as it will have to modify \.status)
> * `data` on ADD|PUT returns number of entries mapped/unmapped.
> ---
>  xen/include/public/io/netif.h | 115
> ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 115 insertions(+)
> 
> diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
> index ca0061410d..0080a260fd 100644
> --- a/xen/include/public/io/netif.h
> +++ b/xen/include/public/io/netif.h
> @@ -353,6 +353,9 @@ struct xen_netif_ctrl_request {
>  #define XEN_NETIF_CTRL_TYPE_SET_HASH_MAPPING_SIZE 5
>  #define XEN_NETIF_CTRL_TYPE_SET_HASH_MAPPING      6
>  #define XEN_NETIF_CTRL_TYPE_SET_HASH_ALGORITHM    7
> +#define XEN_NETIF_CTRL_TYPE_GET_GREF_MAPPING_SIZE 8
> +#define XEN_NETIF_CTRL_TYPE_ADD_GREF_MAPPING      9
> +#define XEN_NETIF_CTRL_TYPE_DEL_GREF_MAPPING     10
> 
>      uint32_t data[3];
>  };
> @@ -391,6 +394,41 @@ struct xen_netif_ctrl_response {
>  };
> 
>  /*
> + * Static Grants (struct xen_netif_gref)
> + * =====================================
> + *
> + * A frontend may provide a fixed set of grant references to be mapped on
> + * the backend. The message of type
> XEN_NETIF_CTRL_TYPE_ADD_GREF_MAPPING
> + * prior its usage in the command ring allows for creation of these mappings.
> + * The backend will maintain a fixed amount of these mappings.
> + *
> + * XEN_NETIF_CTRL_TYPE_GET_GREF_MAPPING_SIZE lets a frontend
> query how many
> + * of these mappings can be kept.
> + *
> + * Each entry in the XEN_NETIF_CTRL_TYPE_{ADD,DEL}_GREF_MAPPING
> input table has
> + * the following format:
> + *
> + *    0     1     2     3     4     5     6     7  octet
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * | grant ref             |  flags    |  status   |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + *
> + * grant ref: grant reference
> + * flags: flags describing the control operation
> + * status: XEN_NETIF_CTRL_STATUS_*
> + */

You may want to add some words here pointing out that the status is an 'out' field, and also whether it should be initialized to zero or not.

> +
> +struct xen_netif_gref {
> +       grant_ref_t ref;
> +       uint16_t flags;
> +
> +#define _XEN_NETIF_CTRLF_GREF_readonly    0
> +#define XEN_NETIF_CTRLF_GREF_readonly
> (1U<<_XEN_NETIF_CTRLF_GREF_readonly)
> +
> +       uint16_t status;
> +};
> +
> +/*
>   * Control messages
>   * ================
>   *
> @@ -609,6 +647,83 @@ struct xen_netif_ctrl_response {
>   *       invalidate any table data outside that range.
>   *       The grant reference may be read-only and must remain valid until
>   *       the response has been processed.
> + *
> + * XEN_NETIF_CTRL_TYPE_GET_GREF_MAPPING_SIZE
> + * -----------------------------------------
> + *
> + * This is sent by the frontend to fetch the number of grefs that can be kept
> + * mapped in the backend.
> + *
> + * Request:
> + *
> + *  type    = XEN_NETIF_CTRL_TYPE_GET_GREF_MAPPING_SIZE
> + *  data[0] = queue index (assumed 0 for single queue)
> + *  data[1] = 0
> + *  data[2] = 0
> + *
> + * Response:
> + *
> + *  status = XEN_NETIF_CTRL_STATUS_NOT_SUPPORTED     - Operation not
> + *                                                     supported
> + *           XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER - The queue index
> is
> + *                                                     out of range
> + *           XEN_NETIF_CTRL_STATUS_SUCCESS           - Operation successful
> + *  data   = maximum number of entries allowed in the gref mapping table
> + *           (if operation was successful) or zero if it is not supported.
> + *
> + * XEN_NETIF_CTRL_TYPE_ADD_GREF_MAPPING
> + * ------------------------------------
> + *
> + * This is sent by the frontend for backend to map a list of grant
> + * references.
> + *
> + * Request:
> + *
> + *  type    = XEN_NETIF_CTRL_TYPE_ADD_GREF_MAPPING
> + *  data[0] = queue index
> + *  data[1] = grant reference of page containing the mapping list
> + *            (r/w and assumed to start at beginning of page)
> + *  data[2] = size of list in entries
> + *
> + * Response:
> + *
> + *  status = XEN_NETIF_CTRL_STATUS_NOT_SUPPORTED     - Operation not
> + *                                                     supported
> + *           XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER - Operation failed
> + *           XEN_NETIF_CTRL_STATUS_SUCCESS           - Operation successful
> + *  data   = number of entries that were mapped
> + *
> + * NOTE: Each entry in the input table has the format outlined
> + *       in struct xen_netif_gref.

You may want to put words here about the 'all or nothing' semantics of this operation vs. the semantics of the 'del' operation below.

> + *
> + * XEN_NETIF_CTRL_TYPE_DEL_GREF_MAPPING
> + * ------------------------------------
> + *
> + * This is sent by the frontend for backend to unmap a list of grant
> + * references.
> + *
> + * Request:
> + *
> + *  type    = XEN_NETIF_CTRL_TYPE_DEL_GREF_MAPPING
> + *  data[0] = queue index
> + *  data[1] = grant reference of page containing the mapping list
> + *            (r/w and assumed to start at beginning of page)
> + *  data[2] = size of list in entries
> + *
> + * Response:
> + *
> + *  status = XEN_NETIF_CTRL_STATUS_NOT_SUPPORTED     - Operation not
> + *                                                     supported
> + *           XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER - Operation failed
> + *           XEN_NETIF_CTRL_STATUS_SUCCESS           - Operation successful
> + *  data   = number of entries that were unmapped
> + *
> + * NOTE: Each entry in the input table has the format outlined in struct
> + *       xen_netif_gref. The only valid entries are those previously added with
> + *       message XEN_NETIF_CTRL_TYPE_ADD_GREF_MAPPING are valid,
> otherwise
> + *       entries status are set to
> XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER.
> + *       It will also have the same error for entries inflight i.e. used in
> + *       requests for which haven't been sent responses to the the frontend.

This last paragraph is a bit garbled.

  Paul

>   */
> 
>  DEFINE_RING_TYPES(xen_netif_ctrl,
> --
> 2.11.0


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