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

[Denis Kirjanov <kda@xxxxxxxxxxxxxxxxx>]

On 5/18/20, Paul Durrant <xadimgnik@xxxxxxxxx> wrote:
>> -----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.
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
>
>
>