Re: [Qemu-devel] [PATCH v6 02/10] init/cleanup of netfilter object

[Yang Hongyang]

On 08/10/2015 05:18 PM, Jason Wang wrote:

On 08/07/2015 10:46 AM, 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 <yan...@cn.fujitsu.com>
---
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            | 141 ++++++++++++++++++++++++++++++++++++++++++++++++
  net/net.c               |   1 +
  qapi-schema.json        |  37 +++++++++++++
  6 files changed, 223 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index 4242ded..7a858d8 100644
--- a/include/net/filter.h

[...]

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

Let's fail when vhost is used here.

I think you mean vhost-user here?

+
+    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);
+    }
+
+    error_propagate(errp, err);

should print out the error here instead of propagate it,otherwise the error msg
is lost.

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

Then errors will be lost here?

Yes, good catch, thank you !

.

--
Thanks,
Yang.