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

[Wei Xu <wexu@redhat.com>]

On 2016年06月14日 15:59, Cornelia Huck wrote:
> On Mon, 13 Jun 2016 20:55:31 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Wed, Jun 08, 2016 at 08:43:28AM +0800, Wei Xu wrote:
>
> > > @@ -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
> >
> > This is not a good idea I think.
> > We have code out there using the existing flags.
> >
> > Also, pls look at 34a48579e4fb380604d06f0409db3851bd22d785 in linux -
> > some very old guests used
> > -#define VIRTIO_NET_HDR_GSO_TCPV4_ECN   2       // GSO frame, IPv4 TCP w/ ECN
> > so it's best not to touch it.
>
> Should we maybe add some "reserved" flag, so nobody tries to reuse them?
Yes, that's a good idea.
> > >   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.
> >
> >
> > I don't see why do you ever need to discuss legacy.
> > Legacy drivers never negotiate any RSC options.
> > Just drop this part?
>
> Agreed.
Thanks.
> > > +\item If one or both VIRTIO_NET_F_GUEST_RSC4, RSC6 were negotiated,
>
> "If one or both of..."
>
> > > +  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,
>
> Also one or both? (Also below.)
Thanks.
> No additional comments from me right now.