Re: [RFC PATCH] virtio-spec: Introduce new Receive-Segment-Coalescing feature for guest

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jun 06, 2016 at 05:27:12PM +0800, Wei Xu wrote:
> Hi TCers,
> Recently i'm working on a feature named as Receive-Segment-Coalescing for
> userspace virtio-net, which is mainly aimed to support WHQL test case for
> windows guest, the below link is the detailed information from microsoft.
> 
> https://msdn.microsoft.com/en-us/library/windows/hardware/jj853323%28v=vs.85%29.aspx
> 
> A new feature bit and a few new fields in virtio network header are
> introduced to achieve this goal, please consider the request.
> 
> Index: content.tex
> ===================================================================
> --- content.tex	(revision 569)
> +++ content.tex	(working copy)
> @@ -3086,6 +3086,11 @@
> 
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>      channel.
> +
> +\item[VIRTIO_NET_F_GUEST_RSC4 (24)] Driver can receive coalesced ipv4 tcp
> packets.
> +
> +\item[VIRTIO_NET_F_GUEST_RSC6 (25)] Driver can receive coalesced ipv6 tcp
> packets.
> +
>  \end{description}
> 
>  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network

25 should not be used by devices.


I suggest you use a high feature bit for this.
How about bits 41 and 42?


> Device / Feature bits / Feature bit requirements}
> @@ -3247,8 +3252,11 @@
>          u8 flags;
>  #define VIRTIO_NET_HDR_GSO_NONE        0
>  #define VIRTIO_NET_HDR_GSO_TCPV4       1
> -#define VIRTIO_NET_HDR_GSO_UDP         3
> -#define VIRTIO_NET_HDR_GSO_TCPV6       4
> +#define VIRTIO_NET_HDR_GSO_UDP         2
> +#define VIRTIO_NET_HDR_GSO_TCPV6       3
> +#define VIRTIO_NET_HDR_RSC_NONE        4
> +#define VIRTIO_NET_HDR_RSC_TCPV4       5
> +#define VIRTIO_NET_HDR_RSC_TCPV6       6
>  #define VIRTIO_NET_HDR_GSO_ECN      0x80
>          u8 gso_type;
>          le16 hdr_len;
> @@ -3256,6 +3264,9 @@
>          le16 csum_start;
>          le16 csum_offset;
>          le16 num_buffers;
> +        le16 coalesced_pkts;
> +        le16 coalesced_size;
> +        le16 dup_acks;
>  };
>  \end{lstlisting}
> 
> @@ -3272,6 +3283,12 @@
>  when VIRTIO_NET_F_MRG_RXBUF was negotiated; without that feature the
>  structure was 2 bytes shorter.
> 
> +The legacy driver only presented \field{coalesced_pkts},
> \field{coalesced_size}
> +and \field{dup_ack} in the struct virtio_net_hdr when
> VIRTIO_NET_F_GUEST_RSC4
> +or VIRTIO_NET_F_GUEST_RSC6 was negotiated and a packet really has coalesced
> +additional packet(s), without satisfying the 2 conditions the structure was
> +6 bytes shorter.
> +
>  When using the legacy interface, the driver SHOULD ignore the
>  \field{len} value in used ring entries for the transmit queues
>  and the controlq queue.
> @@ -3451,6 +3468,19 @@
>  1514 bytes.  The 12-byte struct virtio_net_hdr is prepended to this,
>  making for 65562 or 1526 bytes.
> 
> +If the VIRTIO_NET_F_GUEST_RSC4 feature is used, the maximum incoming
> +packet will be up to 65550 bytes long (the maximum size of a TCP packet,
> +plus the 14 byte ethernet header), otherwise 1514 bytes.
> +The 16-byte struct virtio_net_hdr is prepended to this,
> +making for 65566 or 1528 bytes.
> +
> +If the VIRTIO_NET_F_GUEST_RSC6 feature is used, the maximum incoming
> +packet will be up to 65590 bytes long (the maximum size of a TCP packet,
> +plus the 40-byte standard ipv6 header, plus the 14 byte ethernet header),
> +otherwise 1514 bytes.
> +The 16-byte struct virtio_net_hdr is prepended to this,
> +making for 65606 or 1528 bytes.
> +
>  \drivernormative{\paragraph}{Setting Up Receive Buffers}{Device Types /
> Network Device / Device Operation / Setting Up Receive Buffers}
> 
>  \begin{itemize}
> @@ -3514,6 +3544,24 @@
>    set: if so, device has validated the packet checksum.
>    In case of multiple encapsulated protocols, one level of checksums
>    has been validated.
> +
> +\item If one or both VIRTIO_NET_F_GUEST_RSC4, RSC6 were negotiated,
> +  but it is a original packet which didn't coalesce any packet,
> +  then the \field{flags} should be set to VIRTIO_NET_HDR_RSC_NONE.
> +
> +\item If the VIRTIO_NET_F_GUEST_RSC4, RSC6 options were negotiated,
> +  and \field{flags} notifies it's a coalesced packet, then
> +  \field{coalesced_pkts} indicates how many packets are coalesced into
> +  this packet.
> +
> +\item If the VIRTIO_NET_F_GUEST_RSC4, RSC6 options were negotiated,
> +  and \field{flags} notifies it's a coalesced packet, then
> +  \field{coalesced_size} indicates the real packet size after coalesced.
> +
> +\item If the VIRTIO_NET_F_GUEST_RSC4, RSC6 options were negotiated,
> +  and \field{flags} notifies it's a coalesced packet, then
> +  \field{dup_ack} indicates how many duplicated ack packets are
> +  coalesced into this packet.
>  \end{enumerate}
> 
>  Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP and ECN
> @@ -3626,6 +3674,15 @@
>  If neither VIRTIO_NET_HDR_F_NEEDS_CSUM nor
>  VIRTIO_NET_HDR_F_DATA_VALID is set, the driver MUST NOT
>  rely on the packet checksum being correct.
> +
> +If one of VIRTIO_NET_F_GUEST_RSC4, RSC6 options have
> +been negotiated, the driver MAY use \field{hdr_len} as a
> +hint about the real virtio header size, for example, if a
> +packets is an original packet, then the virtio header size
> +should not contain the \field{coalesced_pkts}, \field{coalesced_size}
> +and \field {dup_ack}, otherwise, these 3 fields should be included in
> +virito header size.
> +
>  \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device /
> Device Operation / Control Virtqueue}
> 
>  The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is