[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v2] virtio-net: support the virtqueue coalescing moderation
On Sat, Feb 11, 2023 at 01:47:16PM +0000, Parav Pandit wrote: > > > > 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. Adding examples of the two command sequences is due, I will do so in the next release. :) Thanks.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]