* [PATCH] virtio_net: Introduce extended RSC feature @ 2019-01-29 12:52 Yuri Benditovich 2019-01-29 16:07 ` Michael S. Tsirkin 2019-01-29 16:07 ` Michael S. Tsirkin 0 siblings, 2 replies; 5+ messages in thread From: Yuri Benditovich @ 2019-01-29 12:52 UTC (permalink / raw) To: mst, jasowang, virtualization, linux-kernel; +Cc: yan VIRTIO_NET_F_RSC_EXT feature bit indicates that the device is able to provide extended RSC information. When the feature is active and 'gso_type' field in received packet is not GSO_NONE, the device reports number of coalesced packets in 'csum_start' field and number of duplicated acks in 'csum_offset' field and sets VIRTIO_NET_HDR_F_RSC_INFO in 'flags' field. Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> --- include/uapi/linux/virtio_net.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h index a3715a3224c1..93c71d714475 100644 --- a/include/uapi/linux/virtio_net.h +++ b/include/uapi/linux/virtio_net.h @@ -56,7 +56,7 @@ #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow * Steering */ #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ - +#define VIRTIO_NET_F_RSC_EXT 61 /* Provides extended RSC info */ #define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device * with the same MAC. */ @@ -104,6 +104,7 @@ struct virtio_net_config { struct virtio_net_hdr_v1 { #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ +#define VIRTIO_NET_HDR_F_RSC_INFO 4 /* rsc_ext data in csum_ fields */ __u8 flags; #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ @@ -140,6 +141,18 @@ struct virtio_net_hdr_mrg_rxbuf { struct virtio_net_hdr hdr; __virtio16 num_buffers; /* Number of merged rx buffers */ }; + +static inline __virtio16 *virtio_net_rsc_ext_num_packets( + struct virtio_net_hdr *hdr) +{ + return &hdr->csum_start; +} + +static inline __virtio16 *virtio_net_rsc_ext_num_dupacks( + struct virtio_net_hdr *hdr) +{ + return &hdr->csum_offset; +} #endif /* ...VIRTIO_NET_NO_LEGACY */ /* -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio_net: Introduce extended RSC feature 2019-01-29 12:52 [PATCH] virtio_net: Introduce extended RSC feature Yuri Benditovich @ 2019-01-29 16:07 ` Michael S. Tsirkin [not found] ` <CAOEp5OfBWVcSnVEm2S3VhOYH3uv+vdUvT5z2yss9v8wvriiKtA@mail.gmail.com> 2019-01-29 16:07 ` Michael S. Tsirkin 1 sibling, 1 reply; 5+ messages in thread From: Michael S. Tsirkin @ 2019-01-29 16:07 UTC (permalink / raw) To: Yuri Benditovich; +Cc: jasowang, virtualization, linux-kernel, yan On Tue, Jan 29, 2019 at 02:52:36PM +0200, Yuri Benditovich wrote: > VIRTIO_NET_F_RSC_EXT feature bit indicates that the device > is able to provide extended RSC information. When the feature > is active and 'gso_type' field in received packet is not GSO_NONE, > the device reports number of coalesced packets in 'csum_start' > field and number of duplicated acks in 'csum_offset' field > and sets VIRTIO_NET_HDR_F_RSC_INFO in 'flags' field. > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > --- > include/uapi/linux/virtio_net.h | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index a3715a3224c1..93c71d714475 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -56,7 +56,7 @@ > #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow > * Steering */ > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > - > +#define VIRTIO_NET_F_RSC_EXT 61 /* Provides extended RSC info */ > #define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device > * with the same MAC. > */ > @@ -104,6 +104,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_RSC_INFO 4 /* rsc_ext data in csum_ fields */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > @@ -140,6 +141,18 @@ struct virtio_net_hdr_mrg_rxbuf { > struct virtio_net_hdr hdr; > __virtio16 num_buffers; /* Number of merged rx buffers */ > }; > + > +static inline __virtio16 *virtio_net_rsc_ext_num_packets( > + struct virtio_net_hdr *hdr) > +{ > + return &hdr->csum_start; > +} > + > +static inline __virtio16 *virtio_net_rsc_ext_num_dupacks( > + struct virtio_net_hdr *hdr) > +{ > + return &hdr->csum_offset; > +} > #endif /* ...VIRTIO_NET_NO_LEGACY */ Coding style is off here. But really I don't think these inlines are needed here. Put them in qemu or something. > /* > -- > 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <CAOEp5OfBWVcSnVEm2S3VhOYH3uv+vdUvT5z2yss9v8wvriiKtA@mail.gmail.com>]
* Re: [PATCH] virtio_net: Introduce extended RSC feature [not found] ` <CAOEp5OfBWVcSnVEm2S3VhOYH3uv+vdUvT5z2yss9v8wvriiKtA@mail.gmail.com> @ 2019-01-30 16:28 ` Michael S. Tsirkin 2019-01-30 16:28 ` Michael S. Tsirkin 1 sibling, 0 replies; 5+ messages in thread From: Michael S. Tsirkin @ 2019-01-30 16:28 UTC (permalink / raw) To: Yuri Benditovich; +Cc: Jason Wang, virtualization, linux-kernel, Yan Vugenfirer On Wed, Jan 30, 2019 at 09:42:07AM +0200, Yuri Benditovich wrote: > > > On Tue, Jan 29, 2019 at 6:07 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Jan 29, 2019 at 02:52:36PM +0200, Yuri Benditovich wrote: > > VIRTIO_NET_F_RSC_EXT feature bit indicates that the device > > is able to provide extended RSC information. When the feature > > is active and 'gso_type' field in received packet is not GSO_NONE, > > the device reports number of coalesced packets in 'csum_start' > > field and number of duplicated acks in 'csum_offset' field > > and sets VIRTIO_NET_HDR_F_RSC_INFO in 'flags' field. > > > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > > --- > > include/uapi/linux/virtio_net.h | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/ > virtio_net.h > > index a3715a3224c1..93c71d714475 100644 > > --- a/include/uapi/linux/virtio_net.h > > +++ b/include/uapi/linux/virtio_net.h > > @@ -56,7 +56,7 @@ > > #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow > > * Steering */ > > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > - > > +#define VIRTIO_NET_F_RSC_EXT 61 /* Provides extended RSC info */ > > #define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another > device > > * with the same MAC. > > */ > > @@ -104,6 +104,7 @@ struct virtio_net_config { > > struct virtio_net_hdr_v1 { > > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, > csum_offset */ > > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > > +#define VIRTIO_NET_HDR_F_RSC_INFO 4 /* rsc_ext data in csum_ fields > */ > > __u8 flags; > > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame > */ > > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP > (TSO) */ > > @@ -140,6 +141,18 @@ struct virtio_net_hdr_mrg_rxbuf { > > struct virtio_net_hdr hdr; > > __virtio16 num_buffers; /* Number of merged rx buffers */ > > }; > > + > > +static inline __virtio16 *virtio_net_rsc_ext_num_packets( > > + struct virtio_net_hdr *hdr) > > +{ > > + return &hdr->csum_start; > > +} > > + > > +static inline __virtio16 *virtio_net_rsc_ext_num_dupacks( > > + struct virtio_net_hdr *hdr) > > +{ > > + return &hdr->csum_offset; > > +} > > #endif /* ...VIRTIO_NET_NO_LEGACY */ I also wonder why do we want to put this code in the legacy section and use the legacy virtio_net_hdr as opposed to the new virtio_net_hdr_v1. > > Coding style is off here. But really I don't think these inlines are > needed here. Put them in qemu or something. > > > > The are copied from qemu as is (what exactly is wrong?). coding style says: Descendants are always substantially shorter than the parent and are placed substantially to the right. placing a line to the left of ( doesn't count as substantially to the right :) Maybe start a new line at virtio_net_rsc_ext_num_dupacks. Lack of documentation is also a problem. > The reason I place these inlines here is following: > We pull this include sometimes to virtio-win repo exactly as qemu do. > If I place them into qemu, then we'll have these inlines in virtio-win and in > qemu and they are not synchronized. > > So, I suggest to keep them in one common header and fix style problems, if they > present. > Please respond if you still disagree. Okay but this assumes specific usage. E.g. someone might want an offset and not a pointer. Or have a struct instance on stack. Given all above issues (and also header version issues described above) I'm inclined to say macros are better: #define virtio_net_rsc_ext_num_packets csum_start #define virtio_net_rsc_ext_num_dupacks csum_offset But please in any case also add documentation same as we have for fields. > > > /* > > -- > > 2.17.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio_net: Introduce extended RSC feature [not found] ` <CAOEp5OfBWVcSnVEm2S3VhOYH3uv+vdUvT5z2yss9v8wvriiKtA@mail.gmail.com> 2019-01-30 16:28 ` Michael S. Tsirkin @ 2019-01-30 16:28 ` Michael S. Tsirkin 1 sibling, 0 replies; 5+ messages in thread From: Michael S. Tsirkin @ 2019-01-30 16:28 UTC (permalink / raw) To: Yuri Benditovich; +Cc: Yan Vugenfirer, linux-kernel, virtualization On Wed, Jan 30, 2019 at 09:42:07AM +0200, Yuri Benditovich wrote: > > > On Tue, Jan 29, 2019 at 6:07 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Jan 29, 2019 at 02:52:36PM +0200, Yuri Benditovich wrote: > > VIRTIO_NET_F_RSC_EXT feature bit indicates that the device > > is able to provide extended RSC information. When the feature > > is active and 'gso_type' field in received packet is not GSO_NONE, > > the device reports number of coalesced packets in 'csum_start' > > field and number of duplicated acks in 'csum_offset' field > > and sets VIRTIO_NET_HDR_F_RSC_INFO in 'flags' field. > > > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > > --- > > include/uapi/linux/virtio_net.h | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/ > virtio_net.h > > index a3715a3224c1..93c71d714475 100644 > > --- a/include/uapi/linux/virtio_net.h > > +++ b/include/uapi/linux/virtio_net.h > > @@ -56,7 +56,7 @@ > > #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow > > * Steering */ > > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > - > > +#define VIRTIO_NET_F_RSC_EXT 61 /* Provides extended RSC info */ > > #define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another > device > > * with the same MAC. > > */ > > @@ -104,6 +104,7 @@ struct virtio_net_config { > > struct virtio_net_hdr_v1 { > > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, > csum_offset */ > > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > > +#define VIRTIO_NET_HDR_F_RSC_INFO 4 /* rsc_ext data in csum_ fields > */ > > __u8 flags; > > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame > */ > > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP > (TSO) */ > > @@ -140,6 +141,18 @@ struct virtio_net_hdr_mrg_rxbuf { > > struct virtio_net_hdr hdr; > > __virtio16 num_buffers; /* Number of merged rx buffers */ > > }; > > + > > +static inline __virtio16 *virtio_net_rsc_ext_num_packets( > > + struct virtio_net_hdr *hdr) > > +{ > > + return &hdr->csum_start; > > +} > > + > > +static inline __virtio16 *virtio_net_rsc_ext_num_dupacks( > > + struct virtio_net_hdr *hdr) > > +{ > > + return &hdr->csum_offset; > > +} > > #endif /* ...VIRTIO_NET_NO_LEGACY */ I also wonder why do we want to put this code in the legacy section and use the legacy virtio_net_hdr as opposed to the new virtio_net_hdr_v1. > > Coding style is off here. But really I don't think these inlines are > needed here. Put them in qemu or something. > > > > The are copied from qemu as is (what exactly is wrong?). coding style says: Descendants are always substantially shorter than the parent and are placed substantially to the right. placing a line to the left of ( doesn't count as substantially to the right :) Maybe start a new line at virtio_net_rsc_ext_num_dupacks. Lack of documentation is also a problem. > The reason I place these inlines here is following: > We pull this include sometimes to virtio-win repo exactly as qemu do. > If I place them into qemu, then we'll have these inlines in virtio-win and in > qemu and they are not synchronized. > > So, I suggest to keep them in one common header and fix style problems, if they > present. > Please respond if you still disagree. Okay but this assumes specific usage. E.g. someone might want an offset and not a pointer. Or have a struct instance on stack. Given all above issues (and also header version issues described above) I'm inclined to say macros are better: #define virtio_net_rsc_ext_num_packets csum_start #define virtio_net_rsc_ext_num_dupacks csum_offset But please in any case also add documentation same as we have for fields. > > > /* > > -- > > 2.17.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio_net: Introduce extended RSC feature 2019-01-29 12:52 [PATCH] virtio_net: Introduce extended RSC feature Yuri Benditovich 2019-01-29 16:07 ` Michael S. Tsirkin @ 2019-01-29 16:07 ` Michael S. Tsirkin 1 sibling, 0 replies; 5+ messages in thread From: Michael S. Tsirkin @ 2019-01-29 16:07 UTC (permalink / raw) To: Yuri Benditovich; +Cc: yan, linux-kernel, virtualization On Tue, Jan 29, 2019 at 02:52:36PM +0200, Yuri Benditovich wrote: > VIRTIO_NET_F_RSC_EXT feature bit indicates that the device > is able to provide extended RSC information. When the feature > is active and 'gso_type' field in received packet is not GSO_NONE, > the device reports number of coalesced packets in 'csum_start' > field and number of duplicated acks in 'csum_offset' field > and sets VIRTIO_NET_HDR_F_RSC_INFO in 'flags' field. > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > --- > include/uapi/linux/virtio_net.h | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index a3715a3224c1..93c71d714475 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -56,7 +56,7 @@ > #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow > * Steering */ > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > - > +#define VIRTIO_NET_F_RSC_EXT 61 /* Provides extended RSC info */ > #define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device > * with the same MAC. > */ > @@ -104,6 +104,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_RSC_INFO 4 /* rsc_ext data in csum_ fields */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > @@ -140,6 +141,18 @@ struct virtio_net_hdr_mrg_rxbuf { > struct virtio_net_hdr hdr; > __virtio16 num_buffers; /* Number of merged rx buffers */ > }; > + > +static inline __virtio16 *virtio_net_rsc_ext_num_packets( > + struct virtio_net_hdr *hdr) > +{ > + return &hdr->csum_start; > +} > + > +static inline __virtio16 *virtio_net_rsc_ext_num_dupacks( > + struct virtio_net_hdr *hdr) > +{ > + return &hdr->csum_offset; > +} > #endif /* ...VIRTIO_NET_NO_LEGACY */ Coding style is off here. But really I don't think these inlines are needed here. Put them in qemu or something. > /* > -- > 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-30 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-29 12:52 [PATCH] virtio_net: Introduce extended RSC feature Yuri Benditovich
2019-01-29 16:07 ` Michael S. Tsirkin
[not found] ` <CAOEp5OfBWVcSnVEm2S3VhOYH3uv+vdUvT5z2yss9v8wvriiKtA@mail.gmail.com>
2019-01-30 16:28 ` Michael S. Tsirkin
2019-01-30 16:28 ` Michael S. Tsirkin
2019-01-29 16:07 ` Michael S. Tsirkin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.