> From: Jason Wang <jasow...@redhat.com> > Sent: Wednesday, January 11, 2023 12:51 AM > > On Wed, Jan 11, 2023 at 12:40 PM Parav Pandit <pa...@nvidia.com> wrote: > > > > > > > From: Jason Wang <jasow...@redhat.com> > > > Sent: Tuesday, January 10, 2023 11:35 PM > > > > > > On Tue, Jan 10, 2023 at 11:02 AM Parav Pandit <pa...@nvidia.com> wrote: > > > > > > > > Hi Jason, > > > > > > > > > From: Jason Wang <jasow...@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 This isn’t explicitly called out. It may be worth to add to the spec.
> - it only helps for debugging > - may cause issues for migration compatibility We have this missing for long time regardless of this feature. So let's not mix here. The mlx5 vdpa device can do eventual in-order completion before/at time of suspend, so I think we can wait for now to until more advance hw arrives. > - requires new infrastructure to be invented > > Thanks > > [1] spec said > This doesn’t say that its zero-length completion. Len is a mandatory field to tell how many bytes device wrote. > " > 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. > "