On Thu, Jul 28, 2022 at 2:48 PM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > > On Thu, Jul 28, 2022 at 01:51:37PM +0800, Jason Wang wrote: > > On Thu, Jul 28, 2022 at 1:39 PM Eli Cohen <elic@xxxxxxxxxx> wrote: > > > > > > > From: Jason Wang <jasowang@xxxxxxxxxx> > > > > Sent: Thursday, July 28, 2022 5:09 AM > > > > To: Eli Cohen <elic@xxxxxxxxxx> > > > > Cc: Eugenio Perez Martin <eperezma@xxxxxxxxxx>; qemu-devel@xxxxxxxxxx; Michael S. Tsirkin <mst@xxxxxxxxxx>; > > > > virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx > > > > Subject: Re: VIRTIO_NET_F_MTU not negotiated > > > > > > > > On Wed, Jul 27, 2022 at 2:52 PM Eli Cohen <elic@xxxxxxxxxx> wrote: > > > > > > > > > > I found out that the reason why I could not enforce the mtu stems from the fact that I did not configure max mtu for the net device > > > > (e.g. through libvirt <mtu size="9000"/>). > > > > > Libvirt does not allow this configuration for vdpa devices and probably for a reason. The vdpa backend driver has the freedom to do > > > > it using its copy of virtio_net_config. > > > > > > > > > > The code in qemu that is responsible to allow to consider the device MTU restriction is here: > > > > > > > > > > static void virtio_net_device_realize(DeviceState *dev, Error **errp) > > > > > { > > > > > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > > > > > VirtIONet *n = VIRTIO_NET(dev); > > > > > NetClientState *nc; > > > > > int i; > > > > > > > > > > if (n->net_conf.mtu) { > > > > > n->host_features |= (1ULL << VIRTIO_NET_F_MTU); > > > > > } > > > > > > > > > > The above code can be interpreted as follows: > > > > > if the command line arguments of qemu indicates that mtu should be limited, then we would read this mtu limitation from the > > > > device (that actual value is ignored). > > > > > > > > > > I worked around this limitation by unconditionally setting VIRTIO_NET_F_MTU in the host features. As said, it only indicates that > > > > we should read the actual limitation for the device. > > > > > > > > > > If this makes sense I can send a patch to fix this. > > > > > > > > I wonder whether it's worth to bother: > > > > > > > > 1) mgmt (above libvirt) should have the knowledge to prepare the correct XML > > > > 2) it's not specific to MTU, we had other features work like, for > > > > example, the multiqueue? > > > > > > > > > > > > > Currently libvirt does not recognize setting the mtu through XML for vdpa device. So you mean the fix should go to libvirt? > > > > Probably. > > > > > Furthermore, even if libvirt supports MTU configuration for a vdpa device, the actual value provided will be ignored and the limitation will be taken from what the vdpa device published in its virtio_net_config structure. That makes the XML configuration binary. > > > > Yes, we suffer from a similar issue for "queues=". I think we should > > fix qemu by failing the initialization if the value provided by cli > > doesn't match what is read from config space. > > > > E.g when mtu=9000 was set by cli but the actual mtu is 1500. > > > > Thanks > > > Jason most features are passthrough now, no? > Why do you want to make this one special? I don't want to make anything special, but I couldn't find a better approach. MTU is not the only thing. It applies to all the other features whose default value is false (MQ, RSS, etc). Thanks > > > > > > > > > > Thanks > > > > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization