diff mbox

[v8,02/11] init/cleanup of netfilter object

Message ID 1440583182-5828-3-git-send-email-yanghy@cn.fujitsu.com
State New
Headers show

Commit Message

Yang Hongyang Aug. 26, 2015, 9:59 a.m. UTC
QTAILQ_ENTRY global_list but used by filter layer, so that we can
manage all filters together.
QTAILQ_ENTRY next used by netdev, filter belongs to the specific netdev is
in this queue.
This is mostly the same with init/cleanup of netdev object.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
v8: include vhost_net header
v7: add check for vhost
    fix error propagate bug
v6: add multiqueue support (net_filter_init1)
v5: remove model from NetFilterState
    add a sent_cb param to receive_iov API
---
 include/net/filter.h    |  42 ++++++++++++++
 include/net/net.h       |   1 +
 include/qemu/typedefs.h |   1 +
 net/filter.c            | 149 ++++++++++++++++++++++++++++++++++++++++++++++++
 net/net.c               |   1 +
 qapi-schema.json        |  37 ++++++++++++
 6 files changed, 231 insertions(+)

Comments

Thomas Huth Aug. 26, 2015, 1:13 p.m. UTC | #1
On 26/08/15 11:59, Yang Hongyang wrote:
> QTAILQ_ENTRY global_list but used by filter layer, so that we can
> manage all filters together.
> QTAILQ_ENTRY next used by netdev, filter belongs to the specific netdev is
> in this queue.
> This is mostly the same with init/cleanup of netdev object.
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
> v8: include vhost_net header
> v7: add check for vhost
>     fix error propagate bug
> v6: add multiqueue support (net_filter_init1)
> v5: remove model from NetFilterState
>     add a sent_cb param to receive_iov API
> ---
>  include/net/filter.h    |  42 ++++++++++++++
>  include/net/net.h       |   1 +
>  include/qemu/typedefs.h |   1 +
>  net/filter.c            | 149 ++++++++++++++++++++++++++++++++++++++++++++++++
>  net/net.c               |   1 +
>  qapi-schema.json        |  37 ++++++++++++
>  6 files changed, 231 insertions(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>
Markus Armbruster Aug. 26, 2015, 2:41 p.m. UTC | #2
Only reviewing the QAPI part.

Yang Hongyang <yanghy@cn.fujitsu.com> writes:

> QTAILQ_ENTRY global_list but used by filter layer, so that we can
> manage all filters together.
> QTAILQ_ENTRY next used by netdev, filter belongs to the specific netdev is
> in this queue.
> This is mostly the same with init/cleanup of netdev object.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
[...]
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 4342a08..d7fb578 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2537,6 +2537,43 @@
>      'opts': 'NetClientOptions' } }
>  
>  ##
> +# @NetFilterOptions
> +#
> +# A discriminated record of network filters.
> +#
> +# Since 2.5
> +#
> +##
> +{ 'union': 'NetFilterOptions',
> +  'data': { } }
> +
> +##
> +# @NetFilter
> +#
> +# Captures the packets of a network backend.
> +#
> +# @id: identifier for monitor commands
> +#
> +# @netdev: the network backend it attached to
> +#
> +# @chain: #optional accept "in","out","all", if not specified, default is "all"
> +#         "in" means this filter will receive packets sent to the @netdev
> +#         "out" means this filter will receive packets sent from the @netdev
> +#         "all" means this filter will receive packets both sent to/from
> +#               the @netdev
> +#
> +# @opts: filter type specific properties
> +#
> +# Since 2.5
> +##
> +{ 'struct': 'NetFilter',
> +  'data': {
> +    'id':   'str',
> +    'netdev': 'str',
> +    '*chain': 'str',
> +    'opts': 'NetFilterOptions' } }
> +
> +##
>  # @InetSocketAddress
>  #
>  # Captures a socket address or address range in the Internet namespace.

Let's make this a flat union instead, to reduce nesting, and therefore
memory allocations and indirections.  Something like

{ 'enum': 'NetFilterType',
  'data': [] }

{ 'struct': NetFilterBase',
  'data': {
      'id':   'str',
      'netdev': 'str',
      '*chain': 'str',
      'type': 'NetFilterType'

{ 'union': 'NetFilter',
  'base': 'NetFilterBase',
  'discriminator': 'type',
  'data': {
  }
}

I hope to reduce the notational overhead of such flat unions in the near
future.

Not sure empty unions actually work.  If they don't, you can either
squash adding members into this patch, or add a dummy member, and drop
it when you add the real ones.
Eric Blake Aug. 26, 2015, 3:31 p.m. UTC | #3
On 08/26/2015 08:41 AM, Markus Armbruster wrote:
> Only reviewing the QAPI part.
> 
> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
> 

> Let's make this a flat union instead, to reduce nesting, and therefore
> memory allocations and indirections.  Something like
> 
> { 'enum': 'NetFilterType',
>   'data': [] }
> 
> { 'struct': NetFilterBase',
>   'data': {
>       'id':   'str',
>       'netdev': 'str',
>       '*chain': 'str',
>       'type': 'NetFilterType'
> 
> { 'union': 'NetFilter',
>   'base': 'NetFilterBase',
>   'discriminator': 'type',
>   'data': {
>   }
> }
> 
> I hope to reduce the notational overhead of such flat unions in the near
> future.
> 
> Not sure empty unions actually work.  If they don't, you can either
> squash adding members into this patch, or add a dummy member, and drop
> it when you add the real ones.

The generator handles an empty union without compilation error, but
creates C code that will always abort() if the union is passed on the
wire.  I have pending patches on top of Markus' that outlaw an empty union:

[uggh: https://lists.gnu.org/archive/html/qemu-devel/ is hosed right
now, and it's harder to find messages on other archivers...]

http://thread.gmane.org/gmane.comp.emulators.qemu/356265/focus=356258
http://thread.gmane.org/gmane.comp.emulators.qemu/356265/focus=356294

so a dummy member for now is a reasonable compromise.
diff mbox

Patch

diff --git a/include/net/filter.h b/include/net/filter.h
index 4242ded..7a858d8 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -9,7 +9,49 @@ 
 #define QEMU_NET_FILTER_H
 
 #include "qemu-common.h"
+#include "qemu/typedefs.h"
+#include "net/queue.h"
+
+/* the netfilter chain */
+enum {
+    NET_FILTER_IN,
+    NET_FILTER_OUT,
+    NET_FILTER_ALL,
+};
+
+typedef void (FilterCleanup) (NetFilterState *);
+/*
+ * Return:
+ *   0: finished handling the packet, we should continue
+ *   size: filter stolen this packet, we stop pass this packet further
+ */
+typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
+                                   NetClientState *sender,
+                                   unsigned flags,
+                                   const struct iovec *iov,
+                                   int iovcnt,
+                                   NetPacketSent *sent_cb);
+
+typedef struct NetFilterInfo {
+    NetFilterOptionsKind type;
+    size_t size;
+    FilterCleanup *cleanup;
+    FilterReceiveIOV *receive_iov;
+} NetFilterInfo;
+
+struct NetFilterState {
+    NetFilterInfo *info;
+    char *name;
+    NetClientState *netdev;
+    int chain;
+    QTAILQ_ENTRY(NetFilterState) global_list;
+    QTAILQ_ENTRY(NetFilterState) next;
+};
 
 int net_init_filters(void);
+NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
+                                    NetClientState *netdev,
+                                    const char *name,
+                                    int chain);
 
 #endif /* QEMU_NET_FILTER_H */
diff --git a/include/net/net.h b/include/net/net.h
index 6a6cbef..36e5fab 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -92,6 +92,7 @@  struct NetClientState {
     NetClientDestructor *destructor;
     unsigned int queue_index;
     unsigned rxfilter_notify_enabled:1;
+    QTAILQ_HEAD(, NetFilterState) filters;
 };
 
 typedef struct NICState {
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index f8a9dd6..2c0648f 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -45,6 +45,7 @@  typedef struct Monitor Monitor;
 typedef struct MouseTransformInfo MouseTransformInfo;
 typedef struct MSIMessage MSIMessage;
 typedef struct NetClientState NetClientState;
+typedef struct NetFilterState NetFilterState;
 typedef struct NICInfo NICInfo;
 typedef struct PcGuestInfo PcGuestInfo;
 typedef struct PCIBridge PCIBridge;
diff --git a/net/filter.c b/net/filter.c
index 4e40f08..cb23384 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -6,10 +6,159 @@ 
  */
 
 #include "qemu-common.h"
+#include "qapi-visit.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
+#include "qapi-visit.h"
+#include "qapi/opts-visitor.h"
+#include "qapi/dealloc-visitor.h"
+#include "qemu/config-file.h"
+
 #include "net/filter.h"
+#include "net/net.h"
+#include "net/vhost_net.h"
+
+static QTAILQ_HEAD(, NetFilterState) net_filters;
+
+NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
+                                    NetClientState *netdev,
+                                    const char *name,
+                                    int chain)
+{
+    NetFilterState *nf;
+
+    assert(info->size >= sizeof(NetFilterState));
+    assert(info->receive_iov);
+
+    nf = g_malloc0(info->size);
+    nf->info = info;
+    nf->name = g_strdup(name);
+    nf->netdev = netdev;
+    nf->chain = chain;
+    QTAILQ_INSERT_TAIL(&net_filters, nf, global_list);
+    QTAILQ_INSERT_TAIL(&netdev->filters, nf, next);
+
+    return nf;
+}
+
+static inline void qemu_cleanup_net_filter(NetFilterState *nf)
+{
+    QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
+    QTAILQ_REMOVE(&net_filters, nf, global_list);
+
+    if (nf->info->cleanup) {
+        nf->info->cleanup(nf);
+    }
+
+    g_free(nf->name);
+    g_free(nf);
+}
+
+typedef int (NetFilterInit)(const NetFilterOptions *opts,
+                            const char *name, int chain,
+                            NetClientState *netdev, Error **errp);
+
+static
+NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
+};
+
+static int net_filter_init1(const NetFilter *netfilter, Error **errp)
+{
+    NetClientState *ncs[MAX_QUEUE_NUM];
+    const char *name = netfilter->id;
+    const char *netdev_id = netfilter->netdev;
+    const char *chain_str = NULL;
+    const NetFilterOptions *opts = netfilter->opts;
+    int chain, queues, i;
+
+    if (!net_filter_init_fun[opts->kind]) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+                   "a net filter type");
+        return -1;
+    }
+
+    if (netfilter->has_chain) {
+        chain_str = netfilter->chain;
+        if (!strcmp(chain_str, "in")) {
+            chain = NET_FILTER_IN;
+        } else if (!strcmp(chain_str, "out")) {
+            chain = NET_FILTER_OUT;
+        } else if (!strcmp(chain_str, "all")) {
+            chain = NET_FILTER_ALL;
+        } else {
+            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "chain",
+                       "netfilter chain (in/out/all)");
+            return -1;
+        }
+    } else {
+        /* default */
+        chain = NET_FILTER_ALL;
+    }
+
+    queues = qemu_find_net_clients_except(netdev_id, ncs,
+                                          NET_CLIENT_OPTIONS_KIND_NIC,
+                                          MAX_QUEUE_NUM);
+    if (queues < 1) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "netdev",
+                   "a network backend id");
+        return -1;
+    }
+
+    if (get_vhost_net(ncs[0])) {
+        error_setg(errp, "vhost is not supported");
+        return -1;
+    }
+
+    for (i = 0; i < queues; i++) {
+        if (net_filter_init_fun[opts->kind](opts, name,
+                                            chain, ncs[i], errp) < 0) {
+            if (errp && !*errp) {
+                error_setg(errp, QERR_DEVICE_INIT_FAILED,
+                           NetFilterOptionsKind_lookup[opts->kind]);
+            }
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+static int net_init_filter(void *dummy, QemuOpts *opts, Error **errp)
+{
+    NetFilter *object = NULL;
+    Error *err = NULL;
+    int ret = -1;
+    OptsVisitor *ov = opts_visitor_new(opts);
+
+    visit_type_NetFilter(opts_get_visitor(ov), &object, NULL, &err);
+    opts_visitor_cleanup(ov);
+
+    if (!err) {
+        ret = net_filter_init1(object, &err);
+    }
+
+    if (object) {
+        QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
+
+        visit_type_NetFilter(qapi_dealloc_get_visitor(dv), &object, NULL, NULL);
+        qapi_dealloc_visitor_cleanup(dv);
+    }
+
+    if (err) {
+        error_report_err(err);
+    }
+    return ret;
+}
 
 int net_init_filters(void)
 {
+    QTAILQ_INIT(&net_filters);
+
+    if (qemu_opts_foreach(qemu_find_opts("netfilter"),
+                          net_init_filter, NULL, NULL)) {
+        return -1;
+    }
+
     return 0;
 }
 
diff --git a/net/net.c b/net/net.c
index 28a5597..d9b70cd 100644
--- a/net/net.c
+++ b/net/net.c
@@ -287,6 +287,7 @@  static void qemu_net_client_setup(NetClientState *nc,
 
     nc->incoming_queue = qemu_new_net_queue(nc);
     nc->destructor = destructor;
+    QTAILQ_INIT(&nc->filters);
 }
 
 NetClientState *qemu_new_net_client(NetClientInfo *info,
diff --git a/qapi-schema.json b/qapi-schema.json
index 4342a08..d7fb578 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2537,6 +2537,43 @@ 
     'opts': 'NetClientOptions' } }
 
 ##
+# @NetFilterOptions
+#
+# A discriminated record of network filters.
+#
+# Since 2.5
+#
+##
+{ 'union': 'NetFilterOptions',
+  'data': { } }
+
+##
+# @NetFilter
+#
+# Captures the packets of a network backend.
+#
+# @id: identifier for monitor commands
+#
+# @netdev: the network backend it attached to
+#
+# @chain: #optional accept "in","out","all", if not specified, default is "all"
+#         "in" means this filter will receive packets sent to the @netdev
+#         "out" means this filter will receive packets sent from the @netdev
+#         "all" means this filter will receive packets both sent to/from
+#               the @netdev
+#
+# @opts: filter type specific properties
+#
+# Since 2.5
+##
+{ 'struct': 'NetFilter',
+  'data': {
+    'id':   'str',
+    'netdev': 'str',
+    '*chain': 'str',
+    'opts': 'NetFilterOptions' } }
+
+##
 # @InetSocketAddress
 #
 # Captures a socket address or address range in the Internet namespace.