Re: [virtio-comment] [PATCH v2] Introduction of Virtio Network device notifications coalescing feature

[Stefan Hajnoczi <stefanha@redhat.com>]

On Tue, May 17, 2022 at 01:01:54PM +0300, Alvaro Karsz wrote:
> I forgot to address this point in your previous  email:
> 
> > My guess is that VIRTIO_F_EVENT_IDX and
> > *-max-frames have similar effects in suppressing notifications.
> 
> One big difference is that if you want to ask the device to send an
> interrupt every 20 packets, you should set VIRTIO_F_EVENT_IDX to 20.
> Then when an interrupt is received, change it to 40, then to 60..
> 
> Using the notifications coalescing feature, you just need to set the
> max-frames values once, and that's it.

That's a good point. It makes me wonder whether there should be a
virtqueue-level coalescing feature. The device would not need to do a
DMA read like it does with VIRTIO_F_EVENT_IDX because the coalescing
value is set once by the driver and rarely updated. I don't want to
distract you though, I think the virtio-net-level coalescing makes sense
since it counts Ethernet frames rather than virtqueue buffers.

> > I'm thinking about the scenario where EVENT_IDX and max-frames are not
> > multiples and the coalesce timer is disabled (0). Can the driver be in a
> > situation where is waits forever?
> 
> The timer can't be disabled.
> Without the timer, you'll have problems even without  EVENT_IDX/
> notification suppressions.
> Assume a scenario when max-frames is 10, and the device receives only
> 5 packets, and then the network is silent.
> In this case, these packets will never reach the driver.
> 
> So, the timer must always work if a device supports this feature.

This reasoning applies to rx but not tx. It would be fine to disable the
timer since the driver controls tx buffer submission.

> timer=0 should mean that an interrupt is sent if 0 usecs elapsed since
> the last interrupt, which always will be "yes".
> 
> 
> > I think the max-frames state should only be reset when the Used Buffer
> > Notification is sent. That way a notification is sent after 3 tx buffers
> > have been used and the driver will be able to make progress.
> 
> It's possible to ask the device to reset the coalescing counters only
> when an interrupt is sent, but I think that it is not required because
> of the coalescing timer.

I agree with you. Please add a paragraph to the spec that summarizes
what we've discussed regarding EVENT_IDX and timer vs max-frames. That
way it will be clear exactly how this should be implemented in drivers
and devices.

Thanks,
Stefan

Attachment: signature.asc
Description: PGP signature