Re: [virtio-comment] virtio spec change proposal

[Yuri Benditovich <ybendito@redhat.com>]

---

From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Yuri Benditovich" <ybendito@redhat.com>
Cc: virtio-comment@lists.oasis-open.org
Sent: Tuesday, October 9, 2018 6:08:00 PM
Subject: Re: [virtio-comment] virtio spec change proposal

On Tue, Oct 02, 2018 at 09:43:25AM -0400, Yuri Benditovich wrote:
> Hello all,
>
> We'd like to propose an extension of VIRTIO specification 1.0
> Please let us know if there are some objections.

I like the proposal. Some comments below, hope this helps with
preparing the spec patch.

> In case there is no objections we will prepare and send respective patch.
>
> Feature: RSC (receive side coalescing) compatibility with Windows HCK (hardware
> compatibility kit) requiremenets.
>
> Description:
>
> 1. Current state:
> VIRTIO device supports RSC, i.e. able to coalesce TCP/UDP v4 and v6 fragments
> and build complete packet from them.
> This feature is controlled by VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
> VIRTIO_NET_F_GUEST_UFO feature bits.
> When it is enabled and received packet was built from fragments, the device
> indicates that by setting field 'gso_type' field to one of
> VIRTIO_NET_HDR_GSO_TCPV4, VIRTIO_NET_HDR_GSO_TCPV6 or VIRTIO_NET_HDR_GSO_UDP.
> Functionally this is good enough, but such an implementation is not compatible
> with Windows certification requirements, resulting failures in Windows
> certification tests.
>
> 2. Expectations of Windows HCK testing software.
> Rules for coalescing are described in
> https://docs.microsoft.com/en-us/windows-hardware/drivers/network/
> rules-for-coalescing-tcp-ip-packets
> Guidelines for indication of coalesced segments
> https://docs.microsoft.com/en-us/windows-hardware/drivers/network/
> indicating-coalesced-segments
> These guidelines are applicable to the network driver, but the network driver
> shall be able to extract
> the required information from the received packet. The information required for
> the driver defined in
> https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/ndis/
> ns-ndis-_ndis_rsc_nbl_info
> and includes following per-packet information:
> - number of coalesced segments that were combined in the packet
> - number of duplicated ACKs that were encountered while received packet was
> combined
>
> Current implementation of RSC does not provide such information in the packet.
>
> 3. Existing workaround
> There is a workaround in the field for the problem described above.
> It uses feature bits 41 and 42 for control of TCP v4 and TCP v6 RSC and
> provides required information in the virtio header.
> For that it extends existing structure of virtio header from regular 12 bytes
> to 16 bytes in case one or both of these feature bits are acknowledged.
> The disadvantage of this solution is that such an extension of packet header
> may complicate further extensions of the specification.

So do we need to reserve bits 41 and 42 for now, as well?
Can be a separate patch.

Yes, this would be good to reserve them (if this change

is acceptable) or define them for the same RSC info

purpose with extended header (as the production driver expects

 from 2016). If the change will be accepted, we'll change

the driver and this will take several years until we

consider only obsolete drivers use it).

> 4. Proposal of specification change.
>
> a) Taking in account the workaround (3) declare feature bits 41 and 42 as
> reserved.
>
> b) Define feature bits 43 for extended RSC information in coalesced packets
> (VIRTIO_NET_F_GUEST_RSC)

Maybe VIRTIO_NET_F_GUEST_RSC_INFO? This does not control whether to do RSC.
Or maybe VIRTIO_NET_F_GUEST_RSC_EXTENDED? This allows coalescing
of duplicate acks and variable sized packets ...

VIRTIO_NET_F_GUEST_RSC_INFO seems

> c) Define additional flag bit 2 (value = 4) that the device can set in 'flags'
> field of existing virtio header with meaning of VIRTIO_NET_HDR_F_RSC_INFO. The
> device sets this bit only in coalesced packets and only if feature bits 43 was
> acknowledged.

And not valid for driver, right?

Can you please clarify, what is not valid for driver?

I mean is case the feature (43) is reported by host/device

and acked by the driver (for example, the driver of the day

never acks this feature being unaware of it). But current

drivers never look at mask 0x04 in flags.

> If VIRTIO_NET_HDR_F_RSC_INFO is set in the packet, the device:
>
> c.1) populates 16-bit value of 'number of coalesced segments' in 'csum_start'
> field of virtio header
>
> c.2) populates 16-bits value of 'number of duplicated ACKs' in 'csum_offset'
> field of virtio header

Maybe its a good time to document behaviour when the bit
is not set:  
- duplicate acks not coalesced (number = 0)
- only full MTU segments coalesced (number = round_up(len / mtu))

If duplicate acks even coalesced this is not reported and

we can't guess what exactly happens.

> d) The device should populate proper checksum in coalesced packets
> (VIRTIO_NET_HDR_F_DATA_VALID should be set). If due to some reason the checksum
> is not populated, VIRTIO_NET_HDR_F_DATA_VALID must be not set. In case
> VIRTIO_NET_HDR_F_DATA_VALID is not set and VIRTIO_NET_HDR_F_RSC_INFO is set in
> the packet, the driver shall compute packet checksum without rely on checksum
> information in 'csum_start' and 'csum_offset'.

More specifically, please formulate exactly what should device do.

As in existing spec, the device should populate the checksum

and report it is valid. But not must, IMO.

> e) new feature VIRTIO_NET_F_GUEST_RSC depends on VIRTIO_NET_F_CTRL_VQ,
> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS and (VIRTIO_NET_F_GUEST_TSO4 or
> VIRTIO_NET_F_GUEST_TSO6).
>
> Thanks,
> Yuri Benditovich,
> Red Hat, Inc.
>

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/