Re: [Qemu-devel] [PATCH 0/9] For QEMU 2.5: Add a net filter and a netbuffer plugin based on the filter

[Daniel P. Berrange]

On Sun, Jul 26, 2015 at 10:13:55PM +0800, Yang Hongyang wrote:
> Thank you, and sorry to Daniel that I forgot to CC you...
> 
> On 07/25/2015 01:06 PM, zhanghailiang wrote:
> [...]
> >>
> >>                 +--------------+       +-------------+
> >>   +----------+  |    filter    |       |frontend(NIC)|
> >>   |      peer+-->              |       |             |
> >>   | network  <--+backend       <-------+ peer        |
> >>   | backend  |  |         peer +------->             |
> >>   +----------+  +--------------+       +-------------+
> >>
> >>Usage:
> >>   -netdev tap,id=bn0  # you can use whatever backend as needed
> >>   -netdev filter,id=f0,backend=bn0
> >>   -netdev filter-<plugin>,id=p0,filter=f0
> >>   -device e1000,netdev=f0
> >
> >Have you considered Daniel's suggestion ? Using the bellow style:
> 
> Yes, but by dig into the implementation, I think the current way is better,
> here is the reason:
> 
> 1. The flexibility to easily dynamically add/remove/change filters on the fly.
>    This is what Daniel was worrying about (please correct me if I didn't
>    get your point) I think I addressed his main concern on this series.
>    And you can specify any param to the filter plugin under existing netdev
>    design.
> 
> 2. Reuse as many existing code as we can. think about doing a standalone 
> object,
>    add/remove filters will duplicate the existing netdev_add/netdev_del code.
>    the filter plugin need to implement at lease 3 functions, init/cleanup and
>    receive_filter, this is also duplicate the existing netdev design, we 
> already
>    have the architecture to init/cleanup/receive_filter, why not use it?
> 
> 3. A filter is a backend in my design, so it is absolutely reasonable to
>    implement it as a netdev and it is easy to implement it as a netdev, if
>    you implement it as a standalone object, how do you integrate it to the
>    backend? A filter plugin might be able to be a standalone object, but just
>    as I said on #2, as long as we can archive our goal under the existing
>    design, why duplicate it?
> 
> 4. Current implementation don't affact the existing code too much, it is a
>    bolt-on feature.
> 
> Overall, we reuse the existing design, implemented a flexible and extensible
> net filter feature, so I think the current way is better.
> 
> 
> >   -netfilter id=f0,plugin=dump
> >   -netdev tap,id=bn0,filter=f0
> >   -device e1000,netdev=bn0
> >
> >Considering the filter as a new 'netdev' seems to be unreasonable,
> >Whenever we add a new plugin, we have to add a new member to
> >'NetClientOptions', there will be lots of 'filter' objects in 
> >NetClientOptions
> >area.
> 
> Why can't we extend this struct? I don't see any problem with this. Doing the
> other way is just to extend another struct.
> 
>  Besides when we want to describe a net device with several filter plugin
> >for VM,
> >it will become like:
> >    -netdev tap,id=bn0
> >    -netdev filter,id=f0,backend=bn0
> >    -netdev filter-<plugin-0>,id=p0,filter=f0
> >    -netdev filter-<plugin-1>,id=p1,filter=f1
> >    ... ...
> >    -device e1000,netdev=f0
> >Which is a little verbose for 'netdev' option.
> 
> It's just the name diffrence, using netfilter will be
> -netfilter ... -netfilter ...
> 
> using plugin=xxx will make us hard to extend the plugin params under existing
> netdev design thus will needs lots of extra effort to archive our goal, but we
> already have a simple way, do we? and do note that Daniel's concern was based
> on my initial RFC patch, which has a usage about "plugin=xxx", this series
> is totally different.

The current -netdev / netdev_add/netdev_del interfaces have a fairly
static view of the world. If you just want to setup filters at the
time you setup the guest NIC that's fine, but if you want to be able
to dynamically change the filters that are used, without altering
the guest device or the real host backend, I think you're going to
run into problems using -netdev. eg consider you have a pre-exisiting
guest running and you want to add in a 'dump' filter to temporarily
record traffic to a file, without having any impact on guest
connectivity. I'm not seeing how you could achieve that with the
proposed netdev approach, because you'd basically have to delete the
existing NIC and add a new one from scratch.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|