RE: [PATCH v3] public/io/netif.h: add a new extra type for XDP

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Denis Kirjanov <kda@xxxxxxxxxxxxxxxxx>
> Sent: 18 May 2020 14:26
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: paul@xxxxxxx; jgross@xxxxxxxx
> Subject: [PATCH v3] public/io/netif.h: add a new extra type for XDP
> 
> The patch adds a new extra type to be able to diffirentiate
> between RX responses on xen-netfront side with the adjusted offset
> required for XDP processing.
> 
> The offset value from a guest is passed via xenstore.
> 
> Signed-off-by: Denis Kirjanov <denis.kirjanov@xxxxxxxx>
> ---
> v3:
> - updated the commit message
> 
> v2:
> - added documentation
> - fixed padding for netif_extra_info
> ---
>  xen/include/public/io/netif.h | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
> index 9fcf91a..ec56a15 100644
> --- a/xen/include/public/io/netif.h
> +++ b/xen/include/public/io/netif.h
> @@ -161,6 +161,13 @@
>   */
> 
>  /*
> + * "netfront-xdp-headroom" is used to add an extra space before packet data
> + * for XDP processing. The value is passed by the frontend to be consistent
> + * between both ends. If the value is greater than zero that means that
> + * an RX response is going to be passed to an XDP program for processing.
> + */

I think 'used to add extra space' is probably the wrong phrase. How about 'is 
used to request that extra space is added'
It also does not state what unit the value is specified in so you need 
something to clarify that. I also don't understand what "The
value is passed by the frontend to be consistent between both ends" means. What 
happens if the backend is older and does not know
what this key means? 

  Paul

> +
> +/*
>   * Control ring
>   * ============
>   *
> @@ -985,7 +992,8 @@ typedef struct netif_tx_request netif_tx_request_t;
>  #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)
> +#define XEN_NETIF_EXTRA_TYPE_XDP       (5)  /* u.xdp */
> +#define XEN_NETIF_EXTRA_TYPE_MAX       (6)
> 
>  /* netif_extra_info_t flags. */
>  #define _XEN_NETIF_EXTRA_FLAG_MORE (0)
> @@ -1018,6 +1026,10 @@ struct netif_extra_info {
>              uint8_t algorithm;
>              uint8_t value[4];
>          } hash;
> +        struct {
> +            uint16_t headroom;
> +            uint16_t pad[2]
> +        } xdp;
>          uint16_t pad[3];
>      } u;
>  };
> --
> 1.8.3.1