| Message ID | 1589814292-1789-1-git-send-email-kda@linux-powerpc.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] public/io/netif.h: add a new extra type for XDP | expand |
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@suse.com> > --- > 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 > + * 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? > + uint16_t pad[2] > + } xdp; > uint16_t pad[3]; > } u; > };
On 5/22/20, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com> 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@suse.com> >> --- >> 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; >> };
On 5/18/20, Denis Kirjanov <kda@linux-powerpc.org> 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. I'm going to send a new version for Linux with the above changes applied. > > Signed-off-by: Denis Kirjanov <denis.kirjanov@suse.com> > --- > 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 > + * 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; > + uint16_t pad[2] > + } xdp; > uint16_t pad[3]; > } u; > }; > -- > 1.8.3.1 > >
On 5/22/20 12:17 PM, Denis Kirjanov wrote: > On 5/22/20, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com> 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@suse.com> >>> --- >>> 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. Well, I would expect "reserved" or something in that case and "pad" in case there are other fields following (see gso above). But here I think "pad" is not required, just like mcast doesn't add any > >>> + uint16_t pad[2] >>> + } xdp; >>> uint16_t pad[3]; >>> } u; >>> };
On 5/22/20, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com> wrote: > On 5/22/20 12:17 PM, Denis Kirjanov wrote: >> On 5/22/20, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com> >> 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@suse.com> >>>> --- >>>> 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. > > Well, I would expect "reserved" or something in that case and "pad" in case > > there are other fields following (see gso above). it can be consistent with other names like pad at then end of the structure. If it really matters I can change it, no problem. > > But here I think "pad" is not required, just like mcast doesn't add any because it's already 6-bytes long > >> >>>> + uint16_t pad[2] >>>> + } xdp; >>>> uint16_t pad[3]; >>>> } u; >>>> };
On 5/22/20 12:33 PM, Denis Kirjanov wrote: > On 5/22/20, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com> wrote: >> On 5/22/20 12:17 PM, Denis Kirjanov wrote: >>> On 5/22/20, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com> >>> 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@suse.com> >>>>> --- >>>>> 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. >> Well, I would expect "reserved" or something in that case and "pad" in case >> >> there are other fields following (see gso above). > it can be consistent with other names like pad at then end of the structure. > > If it really matters I can change it, no problem. My point is that IMO it is not required at all, but this is up to maintainers to decide > >> But here I think "pad" is not required, just like mcast doesn't add any > because it's already 6-bytes long you are right > >>>>> + uint16_t pad[2] >>>>> + } xdp; >>>>> uint16_t pad[3]; >>>>> } u; >>>>> };
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 + * 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; + uint16_t pad[2] + } xdp; uint16_t pad[3]; } u; };
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> --- 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(-)