On Mon, Mar 23, 2026 at 9:39 AM Koushik Dutta <[email protected]> wrote: > > Hi Eugenio, > Regarding the point about not enabling VIRTIO_NET_F_NOTF_COAL > unconditionally: since feature negotiation happens during VM bootup and > coalescing is configured via ethtool after the VM has already booted, how > would we handle this under a conditional property? > > If the feature isn't negotiated at the start, we wouldn't be able to enable > it dynamically later. Could you clarify how you'd like the command-line > regulation to interact with the guest's ability to enable it later?
Replying at the position of the comment, marked with [1]. Please interleave your reply for the next time, as it makes revision easier :) [2]. > ------------------------------------------------------------ > > > @@ -37,6 +37,7 @@ bool vhost_svq_valid_features(uint64_t features, Error > > **errp) > > case VIRTIO_RING_F_INDIRECT_DESC: > > continue; > > > > + case VIRTIO_F_RING_RESET: > > This patch does not handle VIRTIO_F_RING_RESET, so this should not be enabled. > > => I included VIRTIO_F_RING_RESET in the valid features list because the vDPA > host device offers it by default; without it, SVQ fails the feature > negotiation check during initialization. > qemu-system-x86_64: -netdev > type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhost-vdpa0,x-svq=on: SVQ > Invalid device feature flags, offer: 0x1e0010330bfffa7, ok: 0x1e0000330bfffa7 > Since you're testing the feature with nested virtualization, you can disable the features in the L0 QEMU with: -device virtio-net-pci,...,queue_reset=off,iommu_platform=off,guest_uso4=off,guest_uso6=off,host_uso=off,guest_announce=off > Regards, > Koushik Dutta > > On Thu, Mar 12, 2026 at 3:47 PM Eugenio Perez Martin <[email protected]> > wrote: >> >> On Wed, Mar 11, 2026 at 2:28 PM Koushik Dutta <[email protected]> wrote: >> > [...] >> > + >> > +static void virtio_net_add_queue(VirtIONet *n, int index) >> > +{ >> > + VirtIODevice *vdev = VIRTIO_DEVICE(n); >> > + >> > + n->vqs[index].rx_vq = >> > + virtio_add_queue(vdev, >> > + n->net_conf.rx_queue_size, >> > + virtio_net_handle_rx); >> > + >> > + n->vqs[index].tx_vq = >> > + virtio_add_queue(vdev, >> > + n->net_conf.tx_queue_size, >> > + virtio_net_handle_tx_dispatch); >> > + >> > + n->vqs[index].tx_timer = >> > + timer_new_ns(QEMU_CLOCK_VIRTUAL, >> > + virtio_net_tx_timer, >> > + &n->vqs[index]); >> >> We should avoid creating a new timer if it will not be used by >> default. Create it only if cmdline enables it or the guest enable it >> via control vq. >> [1] I'm proposing we switch this to something like: if (!strcmp(n->net_conf.tx, "timer")) { n->vqs[index].tx_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, virtio_net_tx_timer, &n->vqs[index]); } And, in the CVQ code that you created, if (n->tx_coal_usecs) { n->vqs[index].tx_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, virtio_net_tx_timer, &n->vqs[index]); } And the same applies to RX timers. [2] https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying
