Re: VIRTIO_NET_F_MTU not negotiated

[Eugenio Perez Martin]

On Thu, Jul 28, 2022 at 7:51 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Jul 28, 2022 at 1:39 PM Eli Cohen <elic@nvidia.com> wrote:
> >
> > > From: Jason Wang <jasowang@redhat.com>
> > > Sent: Thursday, July 28, 2022 5:09 AM
> > > To: Eli Cohen <elic@nvidia.com>
> > > Cc: Eugenio Perez Martin <eperezma@redhat.com>; qemu-devel@nongnu.org; 
> > > Michael S. Tsirkin <mst@redhat.com>;
> > > virtualization@lists.linux-foundation.org
> > > Subject: Re: VIRTIO_NET_F_MTU not negotiated
> > >
> > > On Wed, Jul 27, 2022 at 2:52 PM Eli Cohen <elic@nvidia.com> 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.
>

Maybe we can be less strict? Since config space mtu is the maximum
value accepted by the device.

So the case of setting mtu=9000 by cli while the device mtu is 1500
should fail, but not the reverse one. To set a cli mtu <= device mtu
should work in my opinion.

If libvirt has the knowledge to set max mtu properly, it can do so. If
not (or not using libvirt), it should be possible to pass the device's
one.

Other features may need to do the same.

Thanks!