qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@hp.com>
To: Paul Brook <paul@codesourcery.com>
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: Re: [Qemu-devel] [PATCH 3/4] qemu:virtio-net: Add support for qemu_vlan_rxfilter
Date: Thu, 12 Feb 2009 11:21:38 -0700	[thread overview]
Message-ID: <1234462898.25178.277.camel@bling> (raw)
In-Reply-To: <200902121705.54473.paul@codesourcery.com>

On Thu, 2009-02-12 at 17:05 +0000, Paul Brook wrote:
> On Thursday 12 February 2009, Alex Williamson wrote:
> > On Thu, 2009-02-12 at 16:26 +0000, Paul Brook wrote:
> > > > +static void virtio_net_vlan_client_added(void *opaque)
> > > >...
> > > > +static void virtio_net_vlan_client_removed(void *opaque)
> > >
> > > Why are these two different?
> > >
> > > It looks like what you really want is a callback for "Something changed,
> > > and you need to reset your MAC filter."
> >
> > I think we'd end up with a race if we only had one callback.  For
> > instance if "change" was the result of a vlan client being removed, the
> > tap would clear the filter and the nic would try to install a filter.
> > The results would be different based on the calling order.
> 
> In that case either your implementation or your abstraction is wrong. 
> Devices shouldn't need to know or care why a filter failed to apply.

They don't, but they do need to know whether a filter they previously
applied successfully is no longer in place.  If they don't know this,
they have to assume the filter on the other side of the vlan is
transient and always double check it with their own filtering, which
seems like a waste of cache and cycles.

Is your objection that the NIC needs to associate the change in filter
state with a vlan event?  If so, maybe we can re-architect the callbacks
to something more appropriate.  We would need both a filter_cleared()
and a filter_retry() to achieve the same mechanics.  filter_cleared()
feels like something the tap device (the "filterer") would call on the
NIC (the "filteree").  But then the "filterer" needs some callback to
know when to clear and notify, which gets us back to somebody needs to
associate a vlan event with a change in the filter.  Suggestions?

> I'm not sure what you mean by "the tap would clear the filter".

By clear, I mean disable the filter, bringing the vlan back to an
unfiltered state.

> You have three options:
> 
> - Devices request a filter, and that request may fail. qemu notifies the 
> device when it needs to retry the filter. It doesn't make any difference 
> whether we think we just broke the old filter, or we think a previously 
> failed filter may succeed now, the device response is the same: Try to set a 
> filter and see if it works. This is what I assume you're trying to implement.
> - Implement reliable filters. The device requests a filter once[1]. qemu makes 
> sure that filter is always honored.
> - Devices request a filter once. qemu remembers what that filter is, and 
> notifies the device if/when that filter becomes active/inactive.

None of these are complete solutions.  My code behaves like this:

- A device requests a filter and is told if the request is successful
  - On success the device may skip it's own filtering
- If another vlan client is added, the following _must_ occur:
  - The "filterer" must clear (remove) the filter
  - The "filteree" must revert to using their own filtering
- If a vlan client is removed, the following _may_ occur:
  - vlan clients are notified that they may retry their filter

The added()/removed() interface feels like the right solution to this.
We could use a changed() function, but it would need to know the
direction of the change, which leads back to the same mechanics.  If
there's a better way, please suggest it.  Thanks,

Alex

  reply	other threads:[~2009-02-12 18:22 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-10 21:28 [Qemu-devel] [PATCH 0/4] qemu: TAP filtering support Alex Williamson
2009-02-10 21:28 ` [Qemu-devel] [PATCH 1/4] qemu:net: Add infrastructure for setting an RX filter through the vlan Alex Williamson
2009-02-10 21:28 ` [Qemu-devel] [PATCH 2/4] qemu:net: Add TAP support for RX filtering on Linux Alex Williamson
2009-02-10 21:28 ` [Qemu-devel] [PATCH 3/4] qemu:virtio-net: Add support for qemu_vlan_rxfilter Alex Williamson
2009-02-12 16:26   ` Paul Brook
2009-02-12 16:36     ` Alex Williamson
2009-02-12 17:05       ` Paul Brook
2009-02-12 18:21         ` Alex Williamson [this message]
2009-02-12 20:26           ` Jamie Lokier
2009-02-13 12:40           ` Paul Brook
2009-02-13 16:00             ` Jamie Lokier
2009-02-13 16:17               ` Paul Brook
2009-02-13 16:46                 ` Jamie Lokier
2009-02-13 17:04                   ` Paul Brook
2009-02-13 20:38                     ` Jamie Lokier
2009-02-15 16:25                       ` Paul Brook
2009-02-10 21:29 ` [Qemu-devel] [PATCH 4/4] qemu:e1000: " Alex Williamson
2009-02-11 15:11   ` [Qemu-devel] " Alex Williamson
2009-02-11 17:11   ` [Qemu-devel] " Alex Williamson
2009-02-11 19:31 ` [Qemu-devel] Re: [PATCH 0/4] qemu: TAP filtering support Mark McLoughlin
2009-02-11 19:43   ` Anthony Liguori
2009-02-11 19:51   ` Alex Williamson
2009-02-11 20:19     ` Mark McLoughlin
2009-02-11 20:37       ` Alex Williamson
2009-02-12 19:57         ` Jamie Lokier

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=1234462898.25178.277.camel@bling \
    --to=alex.williamson@hp.com \
    --cc=kvm@vger.kernel.org \
    --cc=paul@codesourcery.com \
    --cc=qemu-devel@nongnu.org \
    /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).