qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jamie Lokier <jamie@shareable.org>
To: Paul Brook <paul@codesourcery.com>
Cc: qemu-devel@nongnu.org, Alex Williamson <alex.williamson@hp.com>,
	kvm@vger.kernel.org
Subject: Re: [Qemu-devel] [PATCH 3/4] qemu:virtio-net: Add support for qemu_vlan_rxfilter
Date: Fri, 13 Feb 2009 16:46:01 +0000	[thread overview]
Message-ID: <20090213164601.GK18471@shareable.org> (raw)
In-Reply-To: <200902131617.54616.paul@codesourcery.com>

Paul Brook wrote:
> > Well, you do need some way to notify a client that the filter which
> > used to work has been removed because it's no longer available, for
> > example when migrating between host kernels.
> 
> This is actually more evidence that the "add" and "remove" callbacks are 
> bogus.

I agree, "add" and "remove" are misnamed among other bogosities.

> My point is that we're allowing the filter to fail for arbitrary
> reasons. Once you assume this, trying to be clever is just going to
> catch you out when we encounter a case you didn't think of.

Yes.

> A simple "Something changed, please try your filter again" callback 
> automatically covers all these cases.

It doesn't, if tap has no memory of how many clients require a filter.

If tap just answers "YES and installs the kernel filter" or "NO and
doesn't install the kernel filter" and doesn't remember how many
clients need a filter, then:

    1. Client A requests filter A
         -> tap says YES, installs kernel filter.
    2. Client B requests filter B
         -> tap can't do both, so deinstalls kernel filter.
         -> tap says NO to client B.
         -> tap notifies client A "there's been a change".
    3. Client A requests filter A
         -> tap doesn't remember that client B wants a filter, so
         -> tap says YES.
         -> tap notifies client B "there's been a change".
    4. Goto 2
         -> endless loop.

In other words, tap needs to distinguish three states:

     "1 filter requested and installed in the kernel"
     ">1 filter requested, none installed in the kernel"
     "0 filters requested, none installed in the kernel"

The interface to clients needs to keep that state updated in tap
somehow.  Just requesting a filter and reverting to software filtering
when the request failed doesn't do that.

Note this has nothing to do with the software filtering fallback.
Even if qemu internal filtering is always done, you still need the
above to keep the kernel filter correct.

Imho, since there needs to be software filter code for fallback
_anyway_, a better place to put that fallback is in the
client-independent plumbing which relays packets to each client.  And
if it's there, you can have a different software fallback filter for
each client anyway, so clients can assume filters are always installed
and working perfectly.

The interface between the plumbing and the tap driver still needs to
handle the issues described above.

-- Jamie

  reply	other threads:[~2009-02-13 16:46 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
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 [this message]
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=20090213164601.GK18471@shareable.org \
    --to=jamie@shareable.org \
    --cc=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).