[PATCH] virtio_net: Introduce extended RSC feature

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [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

[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
  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