On Fri, Feb 17, 2023 at 01:17:50PM +0200, Alvaro Karsz wrote: > > > Yes, max_packets and max_usecs SHOULD be reset to 0. > > > When the virtqueue is re-enabled, it means that notification conditions > > > are met after each packet is sent/received. > > > > > > As described by Alvaro in "[PATCH v5] virtio-net: Fix and update > > > VIRTIO_NET_F_NOTF_COAL feature": > > > "+When the device has \field{max_usecs} = 0 or \field{max_packets} = 0, > > > the notification conditions are met after every packet received/sent." > > > > Oh. Hmm. > > > > What Alvaro's patch does not describe is what happens when VQ is reset. > > > > Alvaro you said you have hardware implementing this right? > > How does the command interact with vq reset? > > > > The device doesn't offer VIRTIO_F_RING_RESET at the moment. > > > > > > > What about VIRTIO_NET_CTRL_NOTF_COAL? > > > > > > I think it should be handled in the same way as the above VQ_SET, that > > > is, reset the corresponding virtqeuue parameters to 0. > > > > sounds consistent. but the problem is, I don't think this is > > how we previously behaved. and RING_RESET is out in 1.2. > > So we need something compatible. I am sorry. > > > > I suspect that instead we can say that existing hardware has a default set > > of > > parameters for tx and rx. And global commands change that > > besides changing all enabled VQs. > > > > That is a side effect beyond just changing all VQs. > > > > Hmm. > > Alvaro? > > > > This is indeed a good point. > We mention the device reset case, but nothing about VQ reset. > > I feel that no matter how we handle this, we break something. > > Having default coalescing values may collide with "Upon reset, a > device MUST initialize all coalescing parameters to 0."
No this is after device reset. > We can say that VQ reset doesn't affect the "global parameters" and a > device reset does, but this collides with "Device Requirements: > Virtqueue Reset". > > In fact, resetting coalescing values after vq reset may be derived > from "Upon reset, a device MUST initialize all coalescing parameters > to 0". > This is consistent with "Device Requirements: Virtqueue Reset". > > I can add in my patch some clarifications. > > This will break the linux virtio_net ethtool implementation a little, > we'll need to fix it. Not good. I feel we must come up with spec that is backwards compatible. Hmm, maybe this is why Parav kept talking about modes. I did not realize at the time, sorry Parav. I still feel modes are not the best way to describe things so I propose this: - in addition to per vq parameters, device that supports global TX/RX commands and ring reset maintains two sets of default parameters: for RX and TX - existing commands change default and change all enabled vqs of the correct type (RX/TX) to the same value - vq reset changes a vq to the default - device reset changes defaults to 0 and changes all vqs to 0 note how defaults are only used for ring reset. is "vq reset parameter" a better name? I feel we will repeat "reset" too many times in a sentence if we call it that though. So fundamentally the only change is with RING_RESET, then default is not always 0, it can be set by the global command. -- MST --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org