[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/9] For QEMU 2.5: Add a net filter and a netbuf

From: Yang Hongyang
Subject: Re: [Qemu-devel] [PATCH 0/9] For QEMU 2.5: Add a net filter and a netbuffer plugin based on the filter
Date: Sun, 26 Jul 2015 22:13:55 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

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 +------->             |
   +----------+  +--------------+       +-------------+

   -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

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

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.

We'd better come to an agreement on the command line style for net filter :)

This is my opinion, Daniel, what do you think?

Cc: Daniel P. Berrange <address@hidden>


   You can attach multiple plugins to the filter, dynamically add/remove
filter and filter-<plugin>.

The netbuffer plugin:
   -netdev tap,id=bn0  # you can use whatever backend as needed
   -netdev filter,id=f0,backend=bn0
   -netdev filter-buffer,id=p0,filter=f0
   -device e1000,netdev=f0

Will supply a public API to release the buffer. But there's no
callers currently.
To test this feature, it's quite simple, just use
netdev_add filter-buffer,id=p0,filter=f0
to buffer packets,
netdev_del p0
will release packets.

You can also implement whatever plugin you needed based on this filter.

Yang Hongyang (9):
   netdev: Add a net filter
   virtio-net: add filter support
   filter: remove plugins when remove filter
   filter: remove filter before remove network backend
   filter: add netbuffer plugin
   introduce qemu_find_net_clients_by_model
   net/queue: export qemu_net_queue_append
   move out net queue structs define
   add a public api to release buffer

  hw/net/virtio-net.c  |  17 ++-
  include/net/filter.h |  21 ++++
  include/net/net.h    |   5 +
  include/net/queue.h  |  26 ++++
  net/Makefile.objs    |   2 +
  net/clients.h        |   6 +
  net/filter-buffer.c  | 185 ++++++++++++++++++++++++++++
  net/filter.c         | 331 +++++++++++++++++++++++++++++++++++++++++++++++++++
  net/net.c            |  51 +++++++-
  net/queue.c          |  31 +----
  qapi-schema.json     |  40 ++++++-
  11 files changed, 679 insertions(+), 36 deletions(-)
  create mode 100644 include/net/filter.h
  create mode 100644 net/filter-buffer.c
  create mode 100644 net/filter.c



reply via email to

[Prev in Thread] Current Thread [Next in Thread]