qemu-devel
[Top][All Lists]
Advanced

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

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


From: Yang Hongyang
Subject: Re: [Qemu-devel] [PATCH v10 02/10] init/cleanup of netfilter object
Date: Mon, 14 Sep 2015 17:05:59 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0



On 09/14/2015 04:54 PM, Daniel P. Berrange wrote:
On Wed, Sep 09, 2015 at 03:24:33PM +0800, Yang Hongyang wrote:
Add a netfilter object based on QOM.

A netfilter is attached to a netdev, captures all network packets
that pass through the netdev. When we delete the netdev, we also
delete the netfilter object attached to it, because if the netdev is
removed, the filter which attached to it is useless.

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.

Also init delayed object after net_init_clients, because netfilters need
to be initialized after net clients initialized.

Signed-off-by: Yang Hongyang <address@hidden>

diff --git a/net/filter.c b/net/filter.c
new file mode 100644
index 0000000..5192c6d
--- /dev/null
+++ b/net/filter.c

+static void netfilter_init(Object *obj)
+{
+    QTAILQ_INIT(&net_filters);
+    object_property_add_str(obj, "netdev",
+                            netfilter_get_netdev_id, netfilter_set_netdev_id,
+                            NULL);
+    object_property_add_enum(obj, "chain", "NetFilterChain",
+                             NetFilterChain_lookup,
+                             netfilter_get_chain, netfilter_set_chain,
+                             NULL);
+}
+
+static void netfilter_cleanup(Object *obj)

Nit-pick, it is preferable to name methods to match the class callback
they're registered against, so if you have to re-spin a v11 plesae
rename this to netfilter_finalize.

Sure, thanks!


+{
+    NetFilterState *nf = NETFILTER(obj);
+    NetFilterClass *nfc = NETFILTER_GET_CLASS(obj);
+
+    if (nfc->cleanup) {
+        nfc->cleanup(nf);
+    }
+
+    if (nf->netdev && !QTAILQ_EMPTY(&nf->netdev->filters)) {
+        QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
+    }
+    if (!QTAILQ_EMPTY(&net_filters)) {
+        QTAILQ_REMOVE(&net_filters, nf, global_list);
+    }
+
+    g_free(nf->netdev_id);
+}
+
+static void netfilter_complete(UserCreatable *uc, Error **errp)
+{
+    NetFilterState *nf = NETFILTER(uc);
+    NetClientState *ncs[MAX_QUEUE_NUM];
+    NetFilterClass *nfc = NETFILTER_GET_CLASS(uc);
+    int queues;
+    Error *local_err = NULL;
+
+    if (!nf->netdev_id) {
+        error_setg(errp, "Parameter 'netdev' is required");
+        return;
+    }
+
+    queues = qemu_find_net_clients_except(nf->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;
+    } else if (queues > 1) {
+        error_setg(errp, "Multi queue is not supported");
+        return;
+    }
+
+    if (get_vhost_net(ncs[0])) {
+        error_setg(errp, "Vhost is not supported");
+        return;
+    }
+
+    nf->netdev = ncs[0];
+
+    if (nfc->setup) {
+        nfc->setup(nf, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+    QTAILQ_INSERT_TAIL(&net_filters, nf, global_list);
+    QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
+}
+
+static bool netfilter_can_be_deleted(UserCreatable *uc, Error **errp)
+{
+    return true;
+}

You can just leave out the netfilter_can_be_deleted() method if you are
only going to return "true", since that's the default semantics.

Ok, thanks.



+static void netfilter_class_init(ObjectClass *oc, void *data)
+{
+    UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+
+    ucc->complete = netfilter_complete;
+    ucc->can_be_deleted = netfilter_can_be_deleted;
+}
+
+static const TypeInfo netfilter_info = {
+    .name = TYPE_NETFILTER,
+    .parent = TYPE_OBJECT,
+    .abstract = true,
+    .class_size = sizeof(NetFilterClass),
+    .class_init = netfilter_class_init,
+    .instance_size = sizeof(NetFilterState),
+    .instance_init = netfilter_init,
+    .instance_finalize = netfilter_cleanup,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_USER_CREATABLE },
+        { }
+    }
+};
+
+static void register_types(void)
+{
+    type_register_static(&netfilter_info);
+}
+
+type_init(register_types);
diff --git a/vl.c b/vl.c
index 584ca88..672f8b2 100644
--- a/vl.c
+++ b/vl.c
@@ -2783,6 +2783,7 @@ static bool object_create_initial(const char *type)
      if (g_str_equal(type, "rng-egd")) {
          return false;
      }
+    /* TODO: reture false for concrete netfilters */
      return true;
  }

Don't you want to be adding your object type here, alongside rng-egd
since IIRC, you said it needed to be created later

Yes, when there's concrete filters, we need to add them here. that's
why I leave a TODO here.

Thank you for the review!


@@ -4302,12 +4303,6 @@ int main(int argc, char **argv, char **envp)
          exit(0);
      }

-    if (qemu_opts_foreach(qemu_find_opts("object"),
-                          object_create,
-                          object_create_delayed, NULL)) {
-        exit(1);
-    }
-
      machine_opts = qemu_get_machine_opts();
      if (qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
                           NULL)) {
@@ -4413,6 +4408,12 @@ int main(int argc, char **argv, char **envp)
          exit(1);
      }

+    if (qemu_opts_foreach(qemu_find_opts("object"),
+                          object_create,
+                          object_create_delayed, NULL)) {
+        exit(1);
+    }
+
  #ifdef CONFIG_TPM

Regards,
Daniel


--
Thanks,
Yang.



reply via email to

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