On Wed, Jan 11, 2023 at 01:51:06PM +0800, Jason Wang wrote: > On Wed, Jan 11, 2023 at 12:40 PM Parav Pandit <[email protected]> wrote: > > > > > > > From: Jason Wang <[email protected]> > > > Sent: Tuesday, January 10, 2023 11:35 PM > > > > > > On Tue, Jan 10, 2023 at 11:02 AM Parav Pandit <[email protected]> wrote: > > > > > > > > Hi Jason, > > > > > > > > > From: Jason Wang <[email protected]> > > > > > 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.
And drop them all? You end up with multisecond delays for things like DHCP. Yes theoretically packets can be dropped at any time, but practically people expect this to happen on busy systems, not randomly out of the blue. > > > > > > > > > > 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. It's just one example. And it's enough in my eyes to prove we really can't start sending zero length RX buffers to drivers and expect all to be well. If we want to we need to negotiate a new feature bit. > > 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. > " I don't think this talks about zero length at all. Let me try to explain what this talk about in my opinion. There are cases where device does not know exactly how much data it wrote into buffer. Should it over-estimate such that driver can be sure that buffer after the reported length is unchanged? Or should it instead under-estimate such that driver can be sure that the reported length has been initialized by device? What this text in the spec says is that it must always under-estimate and not over-estimate. And it attempts to explain why this is useful: imagine driver that trusts the device and wants to make sure buffer is initialized. With the definition in the spec, it only needs to initialize data after the reported length. Initialize how? It's up to the driver but for example it can zero this buffer. In short, all the text says is "do not over-report length, only set it to part of buffer you wrote". Besides that, the text itself is from the original spec and it did not age well: 1)- no one actually relies on this 2)- rather than untrusted "buffers" what we commonly have is untrusted devices so length can't be trusted either 3)- writes on PCI are posted and if your security model depends on buffer being initialized and you want to recover from errors you really can't expect device to give you this info. Luckily no one cares see 1) above. -- MST
