* [PATCH] virtio_net: Introduce extended RSC feature
@ 2019-01-29 12:52 Yuri Benditovich
2019-01-29 16:07 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ 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] 3+ 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>
0 siblings, 1 reply; 3+ 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] 3+ 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
0 siblings, 0 replies; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2019-01-30 16:28 UTC | newest]
Thread overview: 3+ 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).