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

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

[Denis Kirjanov]

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

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

[Paul Durrant]

> -----Original Message-----
> From: Denis Kirjanov <kda@linux-powerpc.org>
> Sent: 18 May 2020 14:26
> To: xen-devel@lists.xenproject.org
> Cc: paul@xen.org; jgross@suse.com
> 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@suse.com>
> ---
> 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

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

[Denis Kirjanov]

On 5/18/20, Paul Durrant <xadimgnik@gmail.com> wrote:
>> -----Original Message-----
>> From: Denis Kirjanov <kda@linux-powerpc.org>
>> Sent: 18 May 2020 14:26
>> To: xen-devel@lists.xenproject.org
>> Cc: paul@xen.org; jgross@suse.com
>> 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@suse.com>
>> ---
>> 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.
Ok.

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?

Looks like it has also be stated here since I've added another value
"feature-xdp-headroom" which is set by  the netback side.

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

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

[Paul Durrant]

> -----Original Message-----
> From: Denis Kirjanov <kda@linux-powerpc.org>
> Sent: 18 May 2020 14:49
> To: paul@xen.org
> Cc: xen-devel@lists.xenproject.org; jgross@suse.com
> Subject: Re: [PATCH v3] public/io/netif.h: add a new extra type for XDP
> 
> On 5/18/20, Paul Durrant <xadimgnik@gmail.com> wrote:
> >> -----Original Message-----
> >> From: Denis Kirjanov <kda@linux-powerpc.org>
> >> Sent: 18 May 2020 14:26
> >> To: xen-devel@lists.xenproject.org
> >> Cc: paul@xen.org; jgross@suse.com
> >> 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@suse.com>
> >> ---
> >> 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.
> Ok.
> 
> 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?
> 
> Looks like it has also be stated here since I've added another value
> "feature-xdp-headroom" which is set by  the netback side.
> 

Yeah, that needs to be mentioned. I also think you should drop the 'netfront' out of the name. I think simply 'xdp-headroom' would be fine.

  Paul

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