All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Amos Kong <akong@redhat.com>,
	qemu-devel@nongnu.org, stefanha@redhat.com,
	lcapitulino@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2 1/2] net: introduce MAC_TABLE_CHANGED event
Date: Thu, 16 May 2013 18:17:23 +0300	[thread overview]
Message-ID: <20130516151723.GA2726@redhat.com> (raw)
In-Reply-To: <5194F764.6010809@redhat.com>

On Thu, May 16, 2013 at 09:12:36AM -0600, Eric Blake wrote:
> On 05/16/2013 09:03 AM, Michael S. Tsirkin wrote:
> > On Thu, May 16, 2013 at 08:58:38AM -0600, Eric Blake wrote:
> >> On 05/16/2013 06:17 AM, Michael S. Tsirkin wrote:
> >>> On Thu, May 16, 2013 at 07:07:24PM +0800, Amos Kong wrote:
> >>>> Introduce this new QMP event to notify management after guest changes
> >>>> mac-table configuration.
> >>>>
> >>>
> >>> This makes it easy for guest to flood management with
> >>> spurious events.
> >>> How about we set a flag after this, and avoid sending any more
> >>> events until management queries the filter status?
> >>>  
> >>
> >> Or use rate-limiting, similar to what we have done for other
> >> guest-triggered events (such as BALLOON_CHANGE), where management can
> >> then tweak the maximum frequency at which it is willing to receive events.
> >>
> >> -- 
> >> Eric Blake   eblake redhat com    +1-919-301-3266
> >> Libvirt virtualization library http://libvirt.org
> >>
> > 
> > I'm not sure how would management set the rate though,
> > and any throttling here might hurt the guest,
> > unlike the balloon.
> 
> If I understand how memballoon throttling works, throttling is NOT
> guest-visible; it merely controls how frequently the guest can trigger
> an event to the host.  The host always gets the latest guest status, but
> only after a timeout has occurred since the last event sent (therefore,
> 2 back-to-back changes mean that the second event isn't sent until the
> timeout elapses; 3 back-to-back means that the 2nd is dropped and only
> the first and third changes get sent, with the 3rd waiting until after
> the timeout).  That is, not all changes reach the host, the first change
> always happens immediately, but subsequent changes may be deferred until
> the timeout elapses, but the host will eventually see the final change,
> and no slower than the frequency it configures for the throttling.
> 
> Or are you are arguing that if the guest makes a request, but the host
> waits until a second has elapsed before it even gets the event to act on
> the request, then the guest has suffered a performance loss?

Yes, that's what I'm saying.

> > 
> > OTOH what I proposed kind of moderates itself automatically.
> 
> Your approach (no more events until the host has acknowleged) has a
> potential problem if the host misses the event (because of a libvirtd
> restart, for example - but then again, on a libvirtd restart, libvirt
> should probably query current state to get itself back in sync);

exactly
> and
> also means that the host sees stale event data if subsequent events were
> squelched because the host hasn't reacted to the first event yet.

So, let's not send any data in the event. Amos's patch does exafctly
that.

> The
> existing throttling approach ensures that if the event includes latest
> guest information, then the host doesn't even have to do do a query, and
> is guaranteed that reacting to the final event will always see the most
> recent request.  But most importantly, if the existing throttling works,
> why do we have to invent a one-off approach for this event instead of
> reusing existing code?

Because of the 1st issue above. A large delay because we
exceed an arbitrary throttling rate would be bad
for the guest. Contrast with delay in e.g.
device delete event.
The throttling mechanism is good for events that host cares
about, not for events that guest cares about.

> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

  reply	other threads:[~2013-05-16 15:17 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-16 11:07 [Qemu-devel] [PATCH v2 0/2] mac programming over macvtap Amos Kong
2013-05-16 11:07 ` [Qemu-devel] [PATCH v2 1/2] net: introduce MAC_TABLE_CHANGED event Amos Kong
2013-05-16 12:12   ` Michael S. Tsirkin
2013-05-16 12:17   ` Michael S. Tsirkin
2013-05-16 12:24     ` Luiz Capitulino
2013-05-16 12:45       ` Michael S. Tsirkin
2013-05-16 12:52         ` Luiz Capitulino
2013-05-16 14:58     ` Eric Blake
2013-05-16 15:03       ` Michael S. Tsirkin
2013-05-16 15:06         ` Michael S. Tsirkin
2013-05-16 15:12         ` Eric Blake
2013-05-16 15:17           ` Michael S. Tsirkin [this message]
2013-05-16 15:24             ` Eric Blake
2013-05-23 15:54             ` Luiz Capitulino
2013-05-23 17:18               ` Michael S. Tsirkin
2013-05-23 17:26                 ` Luiz Capitulino
2013-05-24 12:10                   ` Michael S. Tsirkin
2013-05-24 12:51                     ` Luiz Capitulino
2013-05-27  9:34                       ` Amos Kong
2013-05-27 13:10                         ` Luiz Capitulino
2013-05-27 13:24                           ` Luiz Capitulino
2013-05-27 22:43                             ` Amos Kong
2013-05-28 12:25                               ` Luiz Capitulino
2013-05-30 13:50                                 ` Amos Kong
2013-05-30 13:50                                   ` [Qemu-devel] " Amos Kong
2013-05-30 13:57                                   ` Michael S. Tsirkin
2013-05-30 13:57                                     ` Michael S. Tsirkin
2013-05-30 13:54                                 ` Michael S. Tsirkin
2013-05-31  0:35                                   ` Amos Kong
2013-05-31  3:02                                     ` Amos Kong
2013-06-04  6:43                                       ` Amos Kong
2013-06-04  7:42                                         ` Amos Kong
2013-06-04 11:11                                           ` Michael S. Tsirkin
2013-05-21  5:04     ` Amos Kong
2013-05-21  8:51       ` Michael S. Tsirkin
2013-05-23  6:08         ` Amos Kong
2013-05-16 14:56   ` Eric Blake
2013-05-16 15:01     ` Michael S. Tsirkin
2013-05-16 11:07 ` [Qemu-devel] [PATCH v2 2/2] net: introduce command to query mac-table information Amos Kong
2013-05-16 12:19   ` Michael S. Tsirkin
2013-05-21  3:31     ` Amos Kong
2013-05-16 15:38   ` Eric Blake
2013-05-23  4:03     ` Amos Kong
2013-05-17  7:39   ` Stefan Hajnoczi
2013-05-21  4:46     ` Amos Kong
2013-05-21  7:38       ` Stefan Hajnoczi
2013-05-29  5:31   ` Jason Wang
2013-06-05  7:18     ` Amos Kong

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=20130516151723.GA2726@redhat.com \
    --to=mst@redhat.com \
    --cc=akong@redhat.com \
    --cc=eblake@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.