qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: Vincenzo Maffione <v.maffione@gmail.com>
Cc: marcel.a@redhat.com, Stefan Hajnoczi <stefanha@gmail.com>,
	Jason Wang <jasowang@redhat.com>,
	qemu-devel@nongnu.org, lcapitulino@redhat.com,
	Anthony Liguori <aliguori@amazon.com>,
	Dmitry Fleytman <dmitry@daynix.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Giuseppe Lettieri <g.lettieri@iet.unipi.it>,
	Luigi Rizzo <rizzo@iet.unipi.it>
Subject: Re: [Qemu-devel] [PATCH v2 0/6] Add netmap backend offloadings support
Date: Fri, 17 Jan 2014 11:28:53 +0800	[thread overview]
Message-ID: <20140117032853.GC16061@stefanha-thinkpad.redhat.com> (raw)
In-Reply-To: <CA+_eA9hj3O-y2vbKvWamr4nnmjpQTBoK0Aizeri4NkhGZEYjAg@mail.gmail.com>

On Thu, Jan 16, 2014 at 04:00:36PM +0100, Vincenzo Maffione wrote:
> 2014/1/16 Stefan Hajnoczi <stefanha@gmail.com>
> 
> > On Tue, Jan 14, 2014 at 11:59:44AM +0100, Vincenzo Maffione wrote:
> > > (3) There is actually an important problem. In the previous patch
> > version, TCP/UDP traffic was
> > >     supported between two guests attached to a VALE switch if and only
> > if both guests use (or
> > >     don't) the same offloadings: e.g. two virtio-net guests or two e1000
> > guests. This is why
> > >     in this version I put the commit which adds the support for netmap
> > as the last one and
> > >     I temporarily disable the offloading feature (e.g.
> > netmap_has_vnet_hdr() returns false).
> > >     We are working at adding proper support also in the general case in
> > which there is
> > >     a mismatch between the NIC offloadings capabilities. As soon as we
> > have proper support,
> > >     we can enable it in net/netmap.c.
> > >     What do you think about that? What's the best way to manage this
> > temporary lack of
> > >     support?
> >
> > What you described is known problem in QEMU.  You cannot unplug a
> > vnet_hdr-enabled tap netdev and plug in a non-vnet_hdr socket netdev
> > since the socket netdev and its remote side don't understand vnet_hdr.
> > This has stopped us from supporting changing NetClient peers at runtime.
> >
> > When this issue was raised we figured we'll have to add code to QEMU to
> > emulate the offload in software (i.e. TSO, checksums, etc).  But no one
> > has implemented that yet (although vmxnet3 has VMware offload software
> > emulation IIRC).
> >
> > So maybe the answer is to finally implement vnet_hdr offload emulation
> > inside QEMU?  Then netmap can negotiate with its remote side and fall
> > back to offload emulation if the remote side doesn't understand
> > vnet_hdr.
> >
> > Keep in mind that virtio-net does not allow the host to disable an
> > offload feature that was already enabled, except after the device has
> > been reset.  This precludes a simple solution where we just tell the
> > guest to stop using vnet_hdr.
> >
> 
> So what you are saying is that once you turn on an offload feature, it's
> not possible to turn it off (apart from device reset).

Yes.

> By the way, let me ask a "sidechannel" question.
> It's not clear to me how the virtio-net device decides which features to
> enable and which features not. I guess there is a negotiation between the
> guest virtio-net driver and the QEMU virtio-net frontend, and this is good.
> But if I am not mistaken there is not a negotiation between the virtio-net
> frontend and the netdev (the backend), apart from the UFO feature (I see a
> tap_has_ufo() function). This means that the virtio-frontend will enable
> the feature negotiated with the guest driver regardless of what the backend
> is able to do. This is ok as long as only tap provides those offloadings,
> but don't you see the need for a more general negotiation also between
> netdev and the frontend?e.g. extend tap_has_ufo() to all the other
> offloadings?

Here is the code:

  if (!peer_has_vnet_hdr(n)) {
      features &= ~(0x1 << VIRTIO_NET_F_CSUM);
      features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4);
      features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO6);
      features &= ~(0x1 << VIRTIO_NET_F_HOST_ECN);

      features &= ~(0x1 << VIRTIO_NET_F_GUEST_CSUM);
      features &= ~(0x1 << VIRTIO_NET_F_GUEST_TSO4);
      features &= ~(0x1 << VIRTIO_NET_F_GUEST_TSO6);
      features &= ~(0x1 << VIRTIO_NET_F_GUEST_ECN);
  }

  if (!peer_has_vnet_hdr(n) || !peer_has_ufo(n)) {
      features &= ~(0x1 << VIRTIO_NET_F_GUEST_UFO);
      features &= ~(0x1 << VIRTIO_NET_F_HOST_UFO);
  }

It is assumed that most offloads are supported if vnet_hdr is available.
Only UFO is a separate feature on top of vnet_hdr.

> > I don't want to merge the tap -> net API changes and netmap offload
> > enablement until there is a solution to this.
> >
> >
> Ok, you pointed out a possible solution, but at the moment I don't think is
> easy/convenient to add negotiation support to netmap (for example, in the
> VALE switch there are more than two ports, so many possible pairs to keep
> track...).
> 
> I'm currently working on adding offloading support inside netmap (so, no
> QEMU code) and this will be enough to fix the problem for netmap, meaning
> that two arbitrary netmap clients will be able to communicate regardless of
> wether they understand or not the virtio-net-header, exactly how the tap
> (and the associated in-kernel bridge) is currently able to do. Will you
> accept the patches provided I complete the offloading support?

Yes.

> As a further step, I could try to convert the offloading code for QEMU, so
> that you can solve the socket problem you described, regardless of netmap.
> 
> Does this sound good to you?

Yes.

Stefan

      reply	other threads:[~2014-01-17  3:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-14 10:59 [Qemu-devel] [PATCH v2 0/6] Add netmap backend offloadings support Vincenzo Maffione
2014-01-14 10:59 ` [Qemu-devel] [PATCH v2 1/6] net: change vnet-hdr TAP prototypes Vincenzo Maffione
2014-01-14 10:59 ` [Qemu-devel] [PATCH v2 2/6] net: removing tap_using_vnet_hdr() function Vincenzo Maffione
2014-01-16  8:39   ` Stefan Hajnoczi
2014-01-16  9:29   ` Michael S. Tsirkin
2014-01-17  3:29     ` Stefan Hajnoczi
2014-01-14 10:59 ` [Qemu-devel] [PATCH v2 3/6] net: extend NetClientInfo for offloading manipulations Vincenzo Maffione
2014-01-14 10:59 ` [Qemu-devel] [PATCH v2 4/6] net: TAP uses NetClientInfo offloading callbacks Vincenzo Maffione
2014-01-14 10:59 ` [Qemu-devel] [PATCH v2 5/6] net: virtio-net and vmxnet3 use offloading API Vincenzo Maffione
2014-01-14 10:59 ` [Qemu-devel] [PATCH v2 6/6] net: add offloadings support to netmap backend Vincenzo Maffione
2014-01-15  6:49 ` [Qemu-devel] [PATCH v2 0/6] Add netmap backend offloadings support Barak Wasserstrom
2014-01-16  9:08 ` Stefan Hajnoczi
2014-01-16 15:00   ` Vincenzo Maffione
2014-01-17  3:28     ` Stefan Hajnoczi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140117032853.GC16061@stefanha-thinkpad.redhat.com \
    --to=stefanha@redhat.com \
    --cc=aliguori@amazon.com \
    --cc=dmitry@daynix.com \
    --cc=g.lettieri@iet.unipi.it \
    --cc=jasowang@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rizzo@iet.unipi.it \
    --cc=stefanha@gmail.com \
    --cc=v.maffione@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).