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

[Denis Kirjanov <kda@xxxxxxxxxxxxxxxxx>]

On 5/22/20, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx> wrote:
> On 5/18/20 6:04 PM, Denis Kirjanov wrote:
>> 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>
>> ---
>> v4:
>> - updated the commit and documenation
>>
>> v3:
>> - updated the commit message
>>
>> v2:
>> - added documentation
>> - fixed padding for netif_extra_info
>> ---
>> ---
>>   xen/include/public/io/netif.h | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/include/public/io/netif.h
>> b/xen/include/public/io/netif.h
>> index 9fcf91a..a92bf04 100644
>> --- a/xen/include/public/io/netif.h
>> +++ b/xen/include/public/io/netif.h
>> @@ -161,6 +161,17 @@
>>    */
>>
>>   /*
>> + * "xdp-headroom" is used to request that extra space is added
>> + * for XDP processing.  The value is measured in bytes and passed by
>
> not sure that we should use word "bytes" here as the rest of the
> protocol (mostly)
>
> talks about octets. It is somewhat mixed here, no strong opinion

sure, but since the public header mixes it I've decided to use that word.


>
>> + * 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.
>> + *
>> + * "feature-xdp-headroom" is set to "1" by the netback side like other
>> features
>> + * so a guest can check if an XDP program can be processed.
>> + */
>> +
>> +/*
>>    * Control ring
>>    * ============
>>    *
>> @@ -985,7 +996,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 +1030,10 @@ struct netif_extra_info {
>>               uint8_t algorithm;
>>               uint8_t value[4];
>>           } hash;
>> +        struct {
>> +            uint16_t headroom;
> why do you need "pad" field here?

To state that we have a fixed size available.

>> +            uint16_t pad[2]
>> +        } xdp;
>>           uint16_t pad[3];
>>       } u;
>>   };