Re: [RFC PATCH for 8.0 10/13] virtio-net: Migrate vhost inflight descriptors

[Jason Wang <jasowang@redhat.com>]

On Wed, Jan 11, 2023 at 12:40 PM Parav Pandit <parav@nvidia.com> wrote:
>
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Tuesday, January 10, 2023 11:35 PM
> >
> > On Tue, Jan 10, 2023 at 11:02 AM Parav Pandit <parav@nvidia.com> wrote:
> > >
> > > Hi Jason,
> > >
> > > > From: Jason Wang <jasowang@redhat.com>
> > > > Sent: Monday, December 5, 2022 10:25 PM
> > >
> > > >
> > > > A dumb question, any reason we need bother with virtio-net? It looks
> > > > to me it's not a must and would complicate migration compatibility.
> > >
> > > Virtio net vdpa device is processing the descriptors out of order.
> > > This vdpa device doesn’t offer IN_ORDER flag.
> > >
> > > And when a VQ is suspended it cannot complete these descriptors as some
> > dummy zero length completions.
> > > The guest VM is flooded with [1].
> >
> > Yes, but any reason for the device to do out-of-order for RX?
> >
> For some devices it is more optimal to process them out of order.
> And its not limited to RX.

TX should be fine, since the device can anyhow pretend to send all
packets, so we won't have any in-flight descriptors.

>
> > >
> > > So it is needed for the devices that doesn’t offer IN_ORDER feature.
> > >
> > > [1]
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tre
> > > e/drivers/net/virtio_net.c?h=v6.2-rc3#n1252
> >
> > It is only enabled in a debug kernel which should be harmless?
> it is KERN_DEBUG log level. Its is not debug kernel, just the debug log level.

Ok, but the production environment should not use that level anyhow.

> And regardless, generating zero length packets for debug kernel is even more confusing.

Note that it is allowed in the virtio-spec[1] (we probably can fix
that in the driver) and we have pr_debug() all over this drivers and
other places. It doesn't cause any side effects except for the
debugging purpose.

So I think having inflight tracking is useful, but I'm not sure it's
worth bothering with virtio-net (or worth to bothering now):

- zero length is allowed
- it only helps for debugging
- may cause issues for migration compatibility
- requires new infrastructure to be invented

Thanks

[1] spec said

"
Note: len is particularly useful for drivers using untrusted buffers:
if a driver does not know exactly how much has been written by the
device, the driver would have to zero the buffer in advance to ensure
no data leakage occurs.
"