RE: [PATCH v2] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit <parav@nvidia.com>]

> From: Alvaro Karsz <alvaro.karsz@solid-run.com>
> Sent: Saturday, February 11, 2023 3:45 AM
> 
> > Please add short description something like,
> >
> > When the driver prefers to use per virtqueue notifications coalescing, and if
> queue group (transmit or receive) level notification coalescing is enabled, driver
> SHOULD first disable device level notification coalescing.
> > Or it should be,
> >
> 
> I disagree here.
> IMO "queue group level notification coalescing" is not something to enable or
> disable, but a shortcut to set all TX/RX queues at once.
That short cut is the enable/disablement.

> Why should the spec force a driver to "disable device level notification
> coalescing" (I assume you mean send a
> VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET command with zeros)?
Yes. Because to have well defined behavior when sw configured both one after the another.

> What if the driver sends a VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET
> command, and then a single queue traffic increases? why should it zero the
> parameters to all other queues?
That is short transition when driver is switching over to per queue mode.
This is fine to have short glitch.

> I think that this should be discussed in the driver implementation stage, not in
> the spec.
> 
There should be a clear guidance on how device should behave when both per q and per device are configured.

> > Virtqueue level notifications coalescing, and device level notifications can be
> enabled together.
> > When both of them are enabled, per virtqueue notifications coalescing take
> priority over queue group level.
> 
> How do you enable  Virtqueue level notifications coalescing? Why are they
> different entities?
Using the new command that has vqn in it.

> I don't think that we should have priorities, but the last command should be the
> one that dictates the coalescing parameters.
> 
Priority is applicable when driver has issued both the commands. Per tx/rx, and per vqn.

> For example, let's look at vq0 (RX):
> Device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 should change
> the parameters accordingly (all RX vqs should do the same).
> Then device receives VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with vqn = 0,
> vq0 changes the parameters accordingly (all RX vqs are still using the "old"
> parameters) Then device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0
> changes the parameters accordingly (all RX vqs should do the same).
In this example, per VQ were overridden with per device.
Yes, so the last one is applicable, so priority of last one applies.

We continue to refuse to add the mode, and hence need to supply these description of both the sequence on how device should behave.

Sequence_1:
1. tx/rx group level
2. per vqn level
When #2 is done, VQ's whose per vq level is configured, follows vqn, rest of the VQs follow #1.

Sequence_2:
1. per vqn
2. tx/rx group level
When #2 is done, group level overrides the per vqn parameters.