On Sun, Feb 12, 2023 at 04:35:37AM -0500, Michael S. Tsirkin wrote: > On Sat, Feb 11, 2023 at 01:47:16PM +0000, Parav Pandit wrote: > > > > > > > From: Alvaro Karsz <alvaro.ka...@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. > > Since there's apparently some room for misunderstanding, I think adding these > examples can't hurt.
Ok, I'll handle this. > I would also be more specific and just use specific numbers in the > example, to avoid any ambiguity. Agree. Thanks. > > -- > MST --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
