Re: [Qemu-devel] [PATCH] net: QEMU_NET_PACKET_FLAG_MORE introduced

[Vincenzo Maffione]

Ok,

   We will prepare the support for the vnet-header and offloadings.

However, this requires some API extensions because AFAIK only the TAP backend currently supports TSO/UFO/CSUM, and consequently virtio-net code directly calls TAP-specific functions like tap_set_offload(), tap_using_vnet_hdr(), tap_has_vnet_hdr(), .... These should become something like qemu_peer_set_offload(), qemu_peer_using_vnet_hdr(), ... adding the proper callbacks into the NetClientInfo struct.

Does this sound good to you?

Regards,

  Vincenzo

2013/12/9 Michael S. Tsirkin <address@hidden>

On Mon, Dec 09, 2013 at 02:25:46PM +0100, Vincenzo Maffione wrote:
>
>
>
> 2013/12/9 Stefan Hajnoczi <address@hidden>
>
>     On Mon, Dec 09, 2013 at 01:14:31PM +0200, Michael S. Tsirkin wrote:
>     > On Mon, Dec 09, 2013 at 11:55:57AM +0100, Vincenzo Maffione wrote:
>     > > If you don't think adding the new flag support for virtio-net is a good
>     idea
>     > > (though TAP performance is not affected in every case) we could also
>     make it
>     > > optional.
>     > >
>     > >
>     > > Cheers
>     > >   Vincenzo
>     > >
>     >
>     > I think it's too early to say whether this patch is benefitial for
>     > netmap, too.  It looks like something that trades off latency
>     > for throughput, and this is a decision the endpoint (VM) should
>     > make, not the network (host).
>     > So you should measure with offloads on before you make conclusions about
>     it.
>
>     Just to check my understanding, we're talking about the following kind
>     of batching:
>
>       int num_packets = peek_available_packets(device);
>       while (num_packets-- > 0) {
>           int flags = MORE;
>           if (num_packets == 0) {
>               flags = NONE;
>           }
>           qemu_net_send_packet(..., flags);
>       }
>
>     In other words, this only batches up a single burst of packets.  It
>     doesn't introduce timers or blocking calls.
>
>     So the effect of batching should be relatively small on latency.  In
>     fact, it's almost like sendmmsg(2)/recvmmsg(2) but using a
>     one-packet-at-a-time interface.
>
>     Does this sound right?
>
>     Stefan
>
>
> Totally correct.
>
> In reply to Michael:
>    - what you say is right with netmap used as a backend with typical TCP
> applications in the guests, and we have already an implementation that supports
> those offloadings
>
>    - however, consider that the main use of netmap is fast packet processing in
> middleboxes, where packet aggregation is not always possible. Applications that
> use netmap **in the guest** typically use "packet batching" (i.e. send multiple
> packets with one system call), so batches originate in the guest. Without the
> MORE flag, those batches are split at the frontend-backend interface. This is
> just a different workload.
>
>
> Regards,
> --
> Vincenzo Maffione

Considering that you have measured performance regression under
netperf, I don't understand why do we keep arguing
about theory. Increasing latency is a problem and if it can already be
seen with netperf it will only get worse with real life workloads.

So my advice is, start by merging offload support for netmap, then check
whether this optimization adds enough performance to be worth it, if yes
it needs more heuristics to avoid hurting latency.

--
MST

--
Vincenzo Maffione