Re: [Qemu-devel] [PATCH v1 0/1] Allow TAP to unnegotiate virtio-net header

[Jason Wang]

On 2019/2/19 下午4:28, Vincenzo Maffione wrote:
> Il giorno mar 19 feb 2019 alle ore 07:48 Jason Wang <address@hidden>
> ha scritto:
>
> > On 2019/2/18 下午7:53, Vincenzo Maffione wrote:
> > > Hi Jason,
> > >     Thanks for the quick reply.
> > >
> > > My PV device (to be open sourced soon) uses the QEMU net backend
> > interface,
> > > in a way similar to virtio-net.
> > > For example it uses qemu_set_offload(), qemu_has_vnet_hdr_len(),
> > > qemu_using_vnet_hdr(), qemu_send_packet(), etc.
> > > This means that the device itself does not know which net backend is
> > using,
> > > of course.
> > > In addition to TAP, also the netmap backend supports the virtio-net
> > header,
> > > and so the PV device will work with both.
> > > Regarding the virtio-net header, the netmap backend is more flexible
> > > because you can change the virtio-net header length as you wish (choosing
> > > between 0, 10 and 12 bytes).
> >
> > Interesting, actually, there's case that vnet header is not used even
> > for virtio-net, e.g for the case of XDP withouth csum support. If we
> > don't pass vnet header in this case, we may get even higher PPS.
> I guess in this case we simply pass a zeroed header. According to my
> experience I think it's unlikely that PPS are affected by having 12
> additional bytes to copy around, as long as no processing is done on the
> header. I may be wrong of course.


The problem is device doesn't know whether it was zeroed, so it need to 
examine some fields.

Thanks


> > > My problem cannot be solved by making sure that TAPs are opened without
> > > IFF_VNET_HDR. What I want instead
> > > is to dynamically change the length of the virtio-net header that the
> > > backend accepts, switching between 12 bytes (virtio 1.0 header) and 0 (no
> > > header). By "dynamically" I mean while the guest (and the device) is
> > > running. This is possible and safe if we get rid of that assert().
> > > The virtio-net device does not need that flexibility (once you enable the
> > > vtnet header, you never disable that again), so I guess that's why you
> > have
> > > that assert().
> > > My PV device can be reconfigured on the fly in such a way it may or not
> > may
> > > be aware of the virtio-net header, and as a consequence it may need to
> > call
> > > qemu_using_vnet_hdr() with true or false (multiple times).
> > >
> > > Does it make sense?
> >
> > Yes, it is. But I think maybe it's better to send this patch with your
> > new PV device implementation.
> >
> > Sounds good, thanks.
> Cheers,
>    Vincenzo
>
>
> > Thanks
> >
> >
> > > Cheers,
> > >     Vincenzo
> > >
> > >
> > > Il giorno lun 18 feb 2019 alle ore 05:19 Jason Wang <address@hidden
> > >
> > > ha scritto:
> > >
> > > > On 2019/2/16 上午1:46, Vincenzo Maffione wrote:
> > > > > Hi,
> > > > >      I was doing experiments with a custom paravirtualized net device,
> > > > > and I ran into a limitation of the TAP backend. I see from the kernel
> > > > > code that it is not possible to set the TAP virtio-net header
> > > > > length to something different from 10 or 12, which means that it
> > > > > is not possible to set it to 0. That's fine.
> > > > > The QEMU implementation of the TAP backend already supports
> > > > > the case where virtio-net header is set to 10 or 12 in kernel,
> > > > > but the emulated/paravirtualized net device is not using that:
> > > > > the TAP backend will just strip/prepend a zeroed header in this case
> > > > > (e.g., see the if statement in tap_receive_iov()).
> > > > > However, the tap_using_vnet_hdr() has an assert() that prevents
> > > > > this situation to happen, and I don't understand why. Maybe it
> > > > > is a leftover? I tried to remove the assert and by doing that
> > > > > my paravirtualized device was able to stop using the virtio-net
> > > > > header.
> > > > Hi:
> > > >
> > > > If  I understand this correctly, your PV device is API compatible with
> > > > TAP? Then you may just adding code to call TUNSETIFF without
> > IFF_VNET_HDR?
> > > > Thanks
> > > >
> > > >
> > > > > Vincenzo Maffione (1):
> > > > >      net: tap: allow net frontends to un-negotiate virtio-net header
> > > > >
> > > > >     net/tap.c | 1 -
> > > > >     1 file changed, 1 deletion(-)