Re: [Qemu-devel] [RFC v2 0/3] virtio-net: Add support to MTU feature

[Michael S. Tsirkin]

On Tue, Nov 22, 2016 at 06:56:43PM +0100, Maxime Coquelin wrote:
> 
> 
> On 11/22/2016 03:41 PM, Michael S. Tsirkin wrote:
> > On Tue, Nov 22, 2016 at 09:32:01AM -0500, Aaron Conole wrote:
> > > Maxime Coquelin <address@hidden> writes:
> > > 
> > > > On 11/22/2016 05:07 AM, Jason Wang wrote:
> > > > > 
> > > > > 
> > > > > On 2016年11月22日 00:23, Michael S. Tsirkin wrote:
> > > > > > On Fri, Nov 18, 2016 at 02:21:54PM -0500, Aaron Conole wrote:
> > > > > > > Maxime Coquelin <address@hidden> writes:
> > > > > > > 
> > > > > > > > On 11/18/2016 07:15 PM, Aaron Conole wrote:
> > > > > > > > > Maxime Coquelin <address@hidden> writes:
> > > > > > > > > 
> > > > > > > > > > This series implements Virtio spec update from Aaron Conole 
> > > > > > > > > > which
> > > > > > > > > > defines a way for the host to expose its max MTU to the 
> > > > > > > > > > guest.
> > > > > > > > > > 
> > > > > > > > > > Changes since RFC v1:
> > > > > > > > > > ---------------------
> > > > > > > > > >   - Rebased on top of v2.8.0-rc0 (2.7.90)
> > > > > > > > > >   - Write MTU unconditionnaly in netcfg to avoid memory 
> > > > > > > > > > leak (Paolo)
> > > > > > > > > >   - Add host_mtu property to be able to disable the feature 
> > > > > > > > > > from QEMU
> > > > > > > > > > 
> > > > > > > > > > Maxime Coquelin (3):
> > > > > > > > > >    vhost-user: Add new protocol feature MTU
> > > > > > > > > >    vhost-net: Add new MTU feature support
> > > > > > > > > >    virtio-net: Add MTU feature support
> > > > > > > > > > 
> > > > > > > > > >   hw/net/vhost_net.c             | 11 +++++++++++
> > > > > > > > > >   hw/net/virtio-net.c            | 14 ++++++++++++++
> > > > > > > > > >   hw/virtio/vhost-user.c         | 11 +++++++++++
> > > > > > > > > >   include/hw/virtio/vhost.h      |  1 +
> > > > > > > > > >   include/hw/virtio/virtio-net.h |  1 +
> > > > > > > > > >   include/net/vhost_net.h        |  2 ++
> > > > > > > > > >   6 files changed, 40 insertions(+)
> > > > > > > > > I ran this with a VM, but it seems the offered maximum MTU 
> > > > > > > > > was of
> > > > > > > > > value
> > > > > > > > > 0 - is this expected with this version?  How can I change the 
> > > > > > > > > offered
> > > > > > > > > value?  Sorry, I'm not as familiar with QEMU/libvirt side of 
> > > > > > > > > the
> > > > > > > > > world.
> > > > > > > > They way I implemented it, the MTU value is to be provided by
> > > > > > > > vhost-user process (e.g. OVS/DPDK). I added a Vhost protocol
> > > > > > > > feature for this. The sequence is:
> > > > > > > > 1. Qemu send VHOST_USER_GET_PROTOCOL_FEATURES request
> > > > > > > > 2. DPDK replies with providing supported features
> > > > > > > > 3. If DPDK supports VHOST_USER_PROTOCOL_F_MTU, Qemu send
> > > > > > > >     VHOST_USER_GET_MTU resuest
> > > > > > > > 4. DPDK replies with MTU value
> > > > > > > > 
> > > > > > > > Does that make sense?
> > > > > > > In the case of a vhost-user backed port, yes (so for instance, if 
> > > > > > > I use
> > > > > > > ovs+dpdk vhost-user in client or server mode).  However, what 
> > > > > > > about the
> > > > > > > non-dpdk case, where I still use a virtio-net driver in kernel 
> > > > > > > and want
> > > > > > > to have it backed with, say, a tap device in the host attached to
> > > > > > > virbr0 (or some other bridge).  It should still pull the mtu from 
> > > > > > > that
> > > > > > > device and offer it, I think.
> > > > > > > 
> > > > > > > > Another possibility would be that we could directly pass the 
> > > > > > > > MTU value
> > > > > > > > to Qemu. It may be easier to implement, and to handle migration.
> > > > > > > > Problem is that if we do this, this is not the vSwitch that 
> > > > > > > > decides the
> > > > > > > > MTU to set.
> > > > > > > Might be better to determined the mtu by looking at what actually
> > > > > > > provides the back-end for the networking.
> > > > > > > 
> > > > > > > > Regards,
> > > > > > > > Maxime
> > > > > > Right. So in case it's not vhost-user, I would say it has to
> > > > > > be specified from QEMU command line.
> > > > > > It's probably easier to do the same everywhere, and just send
> > > > > > the MTU from qemu to backend.
> > > > > 
> > > > > Or vice-versa? E.g qemu need to be notified if the MTU of tap or 
> > > > > macvtap
> > > > > were changed?
> > > 
> > > I think qemu should just query the MTU at start time and then
> > > provide that as the value to the VM.  Why specify with command line
> > > option?
> > 
> > Passing it on command line is an easy way to make sure we can migrate
> > the VM correctly.  Also, if we get it from the backend, this requires
> > backend changes.  In particular, tun/macvtap will need new ioctls.  So
> > generally that's more work.
> 
> I agree this is easier for migration.
> 
> > >  Seems to me like an easy way to get out of sync.
> > 
> > If we send it to the backend, that has a chance to check
> > mtu and disconnect on error.
> 
> For vhost-user backend, we can send it the MTU value with a
> vhost-user protocol feature.
> 
> For tun/macvtap, how do you do without adding a new ioctl ?

Have management configure same mtu on the backend and in qemu.


> > 
> > > > The spec says the MTU must not be modified by the device once it has
> > > > been set.
> > > 
> > > +1 - we don't have an exchange or negotiation mechanism for this.  That
> > > would require additional bits and communication, and it seems like it
> > > isn't worth it.
> > > 
> > > > I think it would require a device reset if MTU came to change.
> > > 
> > > It's just too much work for not enough gain.  Guest can change MTU all
> > > it wants.  Host should just know what it will limit to the guest from
> > > the beginning.  Maybe I'm too simple, though.
> > > 
> > > -Aaron
> > 
> > I agree it's a good start.
> 
> Thanks,
> Maxime