* [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter
@ 2015-08-26 9:59 Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter Yang Hongyang
` (13 more replies)
0 siblings, 14 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
stefanha, Yang Hongyang
This patch add a new object netfilter, capture all network packets.
Also implement a netbuffer based on this object.
the "buffer" netfilter could be used by VM FT solutions like
MicroCheckpointing, to buffer/release packets. Or to simulate
packet delay.
You can also get the series from:
https://github.com/macrosheep/qemu/tree/netfilter-v8
Usage:
-netdev tap,id=bn0
-netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
-device e1000,netdev=bn0
dynamically add/remove netfilters:
netfilter_add buffer,id=f0,netdev=bn0,chain=in,interval=1000
netfilter_del f0
NOTE:
interval's scale is microsecond.
chain is optional, and is one of in|out|all, 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
TODO:
- dump
v8:
- some minor fixes according to Thomas's comments
- rebased to the latest master branch
v7:
- print filter info when execute 'info network'
- addressed Jason's comments
v6:
- add multiqueue support, please see individual patch for detail
v5:
- add a sent_cb param to filter receive_iov api
- squash the 4th patch into patch 3
- remove dummy sent_cb (buffer filter)
- addressed Jason's other comments, see individual patches for detail
v4:
- get rid of struct Filter
- squash the 4th patch into patch 2
- fix qemu_netfilter_pass_to_next_iov
- get rid of bh (buffer filter)
- release the packet to next filter instead of to receiver (buffer filter)
v3:
- add an api to pass the packet to next filter
- remove netfilters when delete netdev
- add qtest testcases for netfilter
- addressed comments from Jason
v2:
- add a chain option to netfilter object
- move the hook place earlier, before net_queue_send
- drop the unused api in buffer filter
- squash buffer filter patches into one
- remove receive() api from netfilter, only receive_iov() is enough
- addressed comments from Jason&Thomas
v1:
initial patch.
Yang Hongyang (11):
net: add a new object netfilter
init/cleanup of netfilter object
netfilter: add netfilter_{add|del} commands
netfilter: hook packets before net queue send
move out net queue structs define
netfilter: add an API to pass the packet to next filter
netfilter: print filter info associate with the netdev
net/queue: export qemu_net_queue_append_iov
netfilter: add a netbuffer filter
filter/buffer: update command description and help
tests: add test cases for netfilter object
hmp-commands.hx | 30 +++++
hmp.c | 29 +++++
hmp.h | 4 +
include/net/filter.h | 64 ++++++++++
include/net/net.h | 1 +
include/net/queue.h | 26 ++++
include/qemu/typedefs.h | 1 +
include/sysemu/sysemu.h | 1 +
monitor.c | 33 +++++
net/Makefile.objs | 2 +
net/filter-buffer.c | 125 ++++++++++++++++++
net/filter.c | 332 ++++++++++++++++++++++++++++++++++++++++++++++++
net/filters.h | 17 +++
net/net.c | 85 +++++++++++++
net/queue.c | 31 +----
qapi-schema.json | 100 +++++++++++++++
qemu-options.hx | 17 +++
qmp-commands.hx | 57 +++++++++
tests/.gitignore | 1 +
tests/Makefile | 2 +
tests/test-netfilter.c | 194 ++++++++++++++++++++++++++++
vl.c | 13 ++
22 files changed, 1140 insertions(+), 25 deletions(-)
create mode 100644 include/net/filter.h
create mode 100644 net/filter-buffer.c
create mode 100644 net/filter.c
create mode 100644 net/filters.h
create mode 100644 tests/test-netfilter.c
--
1.9.1
^ permalink raw reply [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-26 14:04 ` Markus Armbruster
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object Yang Hongyang
` (12 subsequent siblings)
13 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
stefanha, Paolo Bonzini, Yang Hongyang
Add the framework for a new netfilter object and a new
-netfilter CLI option as a basis for the following patches.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
include/net/filter.h | 15 +++++++++++++++
include/sysemu/sysemu.h | 1 +
net/Makefile.objs | 1 +
net/filter.c | 27 +++++++++++++++++++++++++++
qemu-options.hx | 1 +
vl.c | 13 +++++++++++++
6 files changed, 58 insertions(+)
create mode 100644 include/net/filter.h
create mode 100644 net/filter.c
diff --git a/include/net/filter.h b/include/net/filter.h
new file mode 100644
index 0000000..4242ded
--- /dev/null
+++ b/include/net/filter.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_NET_FILTER_H
+#define QEMU_NET_FILTER_H
+
+#include "qemu-common.h"
+
+int net_init_filters(void);
+
+#endif /* QEMU_NET_FILTER_H */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 44570d1..15d6d00 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -212,6 +212,7 @@ extern QemuOptsList qemu_chardev_opts;
extern QemuOptsList qemu_device_opts;
extern QemuOptsList qemu_netdev_opts;
extern QemuOptsList qemu_net_opts;
+extern QemuOptsList qemu_netfilter_opts;
extern QemuOptsList qemu_global_opts;
extern QemuOptsList qemu_mon_opts;
diff --git a/net/Makefile.objs b/net/Makefile.objs
index ec19cb3..914aec0 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -13,3 +13,4 @@ common-obj-$(CONFIG_HAIKU) += tap-haiku.o
common-obj-$(CONFIG_SLIRP) += slirp.o
common-obj-$(CONFIG_VDE) += vde.o
common-obj-$(CONFIG_NETMAP) += netmap.o
+common-obj-y += filter.o
diff --git a/net/filter.c b/net/filter.c
new file mode 100644
index 0000000..4e40f08
--- /dev/null
+++ b/net/filter.c
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu-common.h"
+#include "net/filter.h"
+
+int net_init_filters(void)
+{
+ return 0;
+}
+
+QemuOptsList qemu_netfilter_opts = {
+ .name = "netfilter",
+ .implied_opt_name = "type",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_netfilter_opts.head),
+ .desc = {
+ /*
+ * no elements => accept any params
+ * validation will happen later
+ */
+ { /* end of list */ }
+ },
+};
diff --git a/qemu-options.hx b/qemu-options.hx
index 77f5853..0d52d02 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
"socket][,vlan=n][,option][,option][,...]\n"
" old way to initialize a host network interface\n"
" (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
+DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
STEXI
@item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
@findex -net
diff --git a/vl.c b/vl.c
index 584ca88..aee931a 100644
--- a/vl.c
+++ b/vl.c
@@ -75,6 +75,7 @@ int main(int argc, char **argv)
#include "monitor/qdev.h"
#include "sysemu/bt.h"
#include "net/net.h"
+#include "net/filter.h"
#include "net/slirp.h"
#include "monitor/monitor.h"
#include "ui/console.h"
@@ -2998,6 +2999,7 @@ int main(int argc, char **argv, char **envp)
qemu_add_opts(&qemu_device_opts);
qemu_add_opts(&qemu_netdev_opts);
qemu_add_opts(&qemu_net_opts);
+ qemu_add_opts(&qemu_netfilter_opts);
qemu_add_opts(&qemu_rtc_opts);
qemu_add_opts(&qemu_global_opts);
qemu_add_opts(&qemu_mon_opts);
@@ -3284,6 +3286,13 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
break;
+ case QEMU_OPTION_netfilter:
+ opts = qemu_opts_parse_noisily(qemu_find_opts("netfilter"),
+ optarg, true);
+ if (!opts) {
+ exit(1);
+ }
+ break;
#ifdef CONFIG_LIBISCSI
case QEMU_OPTION_iscsi:
opts = qemu_opts_parse_noisily(qemu_find_opts("iscsi"),
@@ -4413,6 +4422,10 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
+ if (net_init_filters() < 0) {
+ exit(1);
+ }
+
#ifdef CONFIG_TPM
if (tpm_init() < 0) {
exit(1);
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-26 13:13 ` Thomas Huth
2015-08-26 14:41 ` Markus Armbruster
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands Yang Hongyang
` (11 subsequent siblings)
13 siblings, 2 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
stefanha, Yang Hongyang
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(+)
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.
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-26 15:17 ` Markus Armbruster
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 04/11] netfilter: hook packets before net queue send Yang Hongyang
` (10 subsequent siblings)
13 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang,
Markus Armbruster, mrhines, Luiz Capitulino, stefanha,
Yang Hongyang
add netfilter_{add|del} commands
This is mostly the same with netdev_{add|del} commands.
When we delete the netdev, we also delete the netfilter object
attached to it, because if the netdev is removed, the filters
which attached to it is useless.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Luiz Capitulino <lcapitulino@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
v7: error msg fix
move qmp_opts_del() into qemu_del_net_filter()
v6: add multiqueue support (qemu_del_net_filter)
v5: squash "net: delete netfilter object when delete netdev"
---
hmp-commands.hx | 30 +++++++++++++++
hmp.c | 29 +++++++++++++++
hmp.h | 4 ++
include/net/filter.h | 3 ++
monitor.c | 33 +++++++++++++++++
net/filter.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++-
net/net.c | 7 ++++
qapi-schema.json | 47 ++++++++++++++++++++++++
qmp-commands.hx | 57 +++++++++++++++++++++++++++++
9 files changed, 310 insertions(+), 1 deletion(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d3b7932..902e2d1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1253,6 +1253,36 @@ Remove host network device.
ETEXI
{
+ .name = "netfilter_add",
+ .args_type = "netfilter:O",
+ .params = "[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
+ .help = "add netfilter",
+ .mhandler.cmd = hmp_netfilter_add,
+ .command_completion = netfilter_add_completion,
+ },
+
+STEXI
+@item netfilter_add
+@findex netfilter_add
+Add netfilter.
+ETEXI
+
+ {
+ .name = "netfilter_del",
+ .args_type = "id:s",
+ .params = "id",
+ .help = "remove netfilter",
+ .mhandler.cmd = hmp_netfilter_del,
+ .command_completion = netfilter_del_completion,
+ },
+
+STEXI
+@item netfilter_del
+@findex netfilter_del
+Remove netfilter.
+ETEXI
+
+ {
.name = "object_add",
.args_type = "object:O",
.params = "[qom-type=]type,id=str[,prop=value][,...]",
diff --git a/hmp.c b/hmp.c
index dcc66f1..09e3cda 100644
--- a/hmp.c
+++ b/hmp.c
@@ -15,6 +15,7 @@
#include "hmp.h"
#include "net/net.h"
+#include "net/filter.h"
#include "net/eth.h"
#include "sysemu/char.h"
#include "sysemu/block-backend.h"
@@ -1599,6 +1600,34 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+void hmp_netfilter_add(Monitor *mon, const QDict *qdict)
+{
+ Error *err = NULL;
+ QemuOpts *opts;
+
+ opts = qemu_opts_from_qdict(qemu_find_opts("netfilter"), qdict, &err);
+ if (err) {
+ goto out;
+ }
+
+ netfilter_add(opts, &err);
+ if (err) {
+ qemu_opts_del(opts);
+ }
+
+out:
+ hmp_handle_error(mon, &err);
+}
+
+void hmp_netfilter_del(Monitor *mon, const QDict *qdict)
+{
+ const char *id = qdict_get_str(qdict, "id");
+ Error *err = NULL;
+
+ qmp_netfilter_del(id, &err);
+ hmp_handle_error(mon, &err);
+}
+
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 0cf4f2a..a21dbbb 100644
--- a/hmp.h
+++ b/hmp.h
@@ -85,6 +85,8 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
void hmp_netdev_add(Monitor *mon, const QDict *qdict);
void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_netfilter_add(Monitor *mon, const QDict *qdict);
+void hmp_netfilter_del(Monitor *mon, const QDict *qdict);
void hmp_getfd(Monitor *mon, const QDict *qdict);
void hmp_closefd(Monitor *mon, const QDict *qdict);
void hmp_sendkey(Monitor *mon, const QDict *qdict);
@@ -112,6 +114,8 @@ void chardev_add_completion(ReadLineState *rs, int nb_args, const char *str);
void set_link_completion(ReadLineState *rs, int nb_args, const char *str);
void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str);
void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str);
+void netfilter_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void netfilter_del_completion(ReadLineState *rs, int nb_args, const char *str);
void ringbuf_write_completion(ReadLineState *rs, int nb_args, const char *str);
void watchdog_action_completion(ReadLineState *rs, int nb_args,
const char *str);
diff --git a/include/net/filter.h b/include/net/filter.h
index 7a858d8..f15d83d 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -53,5 +53,8 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
NetClientState *netdev,
const char *name,
int chain);
+void qemu_del_net_filter(NetFilterState *nf);
+void netfilter_add(QemuOpts *opts, Error **errp);
+void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
#endif /* QEMU_NET_FILTER_H */
diff --git a/monitor.c b/monitor.c
index fc32f12..58b43af 100644
--- a/monitor.c
+++ b/monitor.c
@@ -31,6 +31,7 @@
#include "hw/loader.h"
#include "exec/gdbstub.h"
#include "net/net.h"
+#include "net/filter.h"
#include "net/slirp.h"
#include "sysemu/char.h"
#include "ui/qemu-spice.h"
@@ -4193,6 +4194,21 @@ void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str)
}
}
+void netfilter_add_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+ size_t len;
+ int i;
+
+ if (nb_args != 2) {
+ return;
+ }
+ len = strlen(str);
+ readline_set_completion_index(rs, len);
+ for (i = 0; NetFilterOptionsKind_lookup[i]; i++) {
+ add_completion_option(rs, str, NetFilterOptionsKind_lookup[i]);
+ }
+}
+
void device_add_completion(ReadLineState *rs, int nb_args, const char *str)
{
GSList *list, *elt;
@@ -4429,6 +4445,23 @@ void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str)
}
}
+void netfilter_del_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+ int len;
+ QemuOpts *opts;
+
+ if (nb_args != 2) {
+ return;
+ }
+
+ len = strlen(str);
+ readline_set_completion_index(rs, len);
+ opts = qemu_opts_find(qemu_find_opts_err("netfilter", NULL), str);
+ if (opts) {
+ readline_add_completion(rs, str);
+ }
+}
+
void watchdog_action_completion(ReadLineState *rs, int nb_args, const char *str)
{
int i;
diff --git a/net/filter.c b/net/filter.c
index cb23384..dcb1891 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -13,6 +13,7 @@
#include "qapi/opts-visitor.h"
#include "qapi/dealloc-visitor.h"
#include "qemu/config-file.h"
+#include "qmp-commands.h"
#include "net/filter.h"
#include "net/net.h"
@@ -41,7 +42,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
return nf;
}
-static inline void qemu_cleanup_net_filter(NetFilterState *nf)
+static void qemu_cleanup_net_filter(NetFilterState *nf)
{
QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
QTAILQ_REMOVE(&net_filters, nf, global_list);
@@ -54,6 +55,104 @@ static inline void qemu_cleanup_net_filter(NetFilterState *nf)
g_free(nf);
}
+static int qemu_find_netfilters_by_name(const char *id, NetFilterState **nfs,
+ int max)
+{
+ NetFilterState *nf;
+ int ret = 0;
+
+ QTAILQ_FOREACH(nf, &net_filters, global_list) {
+ if (!strcmp(nf->name, id)) {
+ if (ret < max) {
+ nfs[ret] = nf;
+ }
+ ret++;
+ }
+ }
+
+ return ret;
+}
+
+void qemu_del_net_filter(NetFilterState *nf)
+{
+ NetFilterState *nfs[MAX_QUEUE_NUM];
+ int queues, i;
+ QemuOpts *opts;
+
+ opts = qemu_opts_find(qemu_find_opts_err("netfilter", NULL), nf->name);
+ if (!opts) {
+ error_report("Options of '%s' can not be found", nf->name);
+ }
+
+ queues = qemu_find_netfilters_by_name(nf->name, nfs, MAX_QUEUE_NUM);
+ assert(queues != 0);
+
+ for (i = 0; i < queues; i++) {
+ qemu_cleanup_net_filter(nfs[i]);
+ }
+
+ qemu_opts_del(opts);
+}
+
+static NetFilterState *qemu_find_netfilter(const char *id)
+{
+ NetFilterState *nf;
+
+ QTAILQ_FOREACH(nf, &net_filters, global_list) {
+ if (!strcmp(nf->name, id)) {
+ return nf;
+ }
+ }
+
+ return NULL;
+}
+
+static int net_init_filter(void *dummy, QemuOpts *opts, Error **errp);
+void netfilter_add(QemuOpts *opts, Error **errp)
+{
+ net_init_filter(NULL, opts, errp);
+}
+
+void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp)
+{
+ Error *local_err = NULL;
+ QemuOptsList *opts_list;
+ QemuOpts *opts;
+
+ opts_list = qemu_find_opts_err("netfilter", &local_err);
+ if (local_err) {
+ goto out;
+ }
+
+ opts = qemu_opts_from_qdict(opts_list, qdict, &local_err);
+ if (local_err) {
+ goto out;
+ }
+
+ netfilter_add(opts, &local_err);
+ if (local_err) {
+ qemu_opts_del(opts);
+ goto out;
+ }
+
+out:
+ error_propagate(errp, local_err);
+}
+
+void qmp_netfilter_del(const char *id, Error **errp)
+{
+ NetFilterState *nf;
+
+ nf = qemu_find_netfilter(id);
+ if (!nf) {
+ error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+ "Filter '%s' not found", id);
+ return;
+ }
+
+ qemu_del_net_filter(nf);
+}
+
typedef int (NetFilterInit)(const NetFilterOptions *opts,
const char *name, int chain,
NetClientState *netdev, Error **errp);
diff --git a/net/net.c b/net/net.c
index d9b70cd..74f3592 100644
--- a/net/net.c
+++ b/net/net.c
@@ -28,6 +28,7 @@
#include "hub.h"
#include "net/slirp.h"
#include "net/eth.h"
+#include "net/filter.h"
#include "util.h"
#include "monitor/monitor.h"
@@ -385,6 +386,7 @@ void qemu_del_net_client(NetClientState *nc)
{
NetClientState *ncs[MAX_QUEUE_NUM];
int queues, i;
+ NetFilterState *nf, *next;
assert(nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC);
@@ -396,6 +398,11 @@ void qemu_del_net_client(NetClientState *nc)
MAX_QUEUE_NUM);
assert(queues != 0);
+ /* qemu_del_net_filter() will handle the multiqueue case */
+ QTAILQ_FOREACH_SAFE(nf, &nc->filters, next, next) {
+ qemu_del_net_filter(nf);
+ }
+
/* If there is a peer NIC, delete and cleanup client, but do not free. */
if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
NICState *nic = qemu_get_nic(nc->peer);
diff --git a/qapi-schema.json b/qapi-schema.json
index d7fb578..9d97c21 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2537,6 +2537,53 @@
'opts': 'NetClientOptions' } }
##
+# @netfilter_add:
+#
+# Add a netfilter.
+#
+# @type: the type of netfilter.
+#
+# @id: the name of the new netfilter.
+#
+# @netdev: the name of the netdev which this filter will be 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
+#
+# @props: #optional a list of properties to be passed to the netfilter in
+# the format of 'name=value'
+#
+# Since: 2.5
+#
+# Returns: Nothing on success
+# If @type is not a valid netfilter, DeviceNotFound
+##
+{ 'command': 'netfilter_add',
+ 'data': {
+ 'type': 'str',
+ 'id': 'str',
+ 'netdev': 'str',
+ '*chain': 'str',
+ '*props': '**'}, 'gen': false }
+
+##
+# @netfilter_del:
+#
+# Remove a netfilter.
+#
+# @id: the name of the netfilter to remove
+#
+# Returns: Nothing on success
+# If @id is not a valid netfilter, DeviceNotFound
+#
+# Since: 2.5
+##
+{ 'command': 'netfilter_del', 'data': {'id': 'str'} }
+
+##
# @NetFilterOptions
#
# A discriminated record of network filters.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ba630b1..4f0dc98 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -926,6 +926,63 @@ Example:
EQMP
{
+ .name = "netfilter_add",
+ .args_type = "netfilter:O",
+ .mhandler.cmd_new = qmp_netfilter_add,
+ },
+
+SQMP
+netfilter_add
+----------
+
+Add netfilter.
+
+Arguments:
+
+- "type": the filter type (json-string)
+- "id": the netfilter's ID, must be unique (json-string)
+- "netdev": the netdev's ID which this filter will be attached to(json-string)
+- filter options
+
+Example:
+
+-> { "execute": "netfilter_add",
+ "arguments": { "type": "type", "id": "nf0",
+ "netdev": "bn",
+ "chain": "in" } }
+<- { "return": {} }
+
+Note: The supported filter options are the same ones supported by the
+ '-netfilter' command-line argument, which are listed in the '-help'
+ output or QEMU's manual
+
+EQMP
+
+ {
+ .name = "netfilter_del",
+ .args_type = "id:s",
+ .mhandler.cmd_new = qmp_marshal_input_netfilter_del,
+ },
+
+SQMP
+netfilter_del
+----------
+
+Remove netfilter.
+
+Arguments:
+
+- "id": the netfilter's ID, must be unique (json-string)
+
+Example:
+
+-> { "execute": "netfilter_del", "arguments": { "id": "nf0" } }
+<- { "return": {} }
+
+
+EQMP
+
+ {
.name = "object-add",
.args_type = "qom-type:s,id:s,props:q?",
.mhandler.cmd_new = qmp_object_add,
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 04/11] netfilter: hook packets before net queue send
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (2 preceding siblings ...)
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-27 14:35 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 05/11] move out net queue structs define Yang Hongyang
` (9 subsequent siblings)
13 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
stefanha, Yang Hongyang
Capture packets that will be sent.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
v5: do not check ret against iov_size
pass sent_cb to filters
---
net/net.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/net/net.c b/net/net.c
index 74f3592..00cca83 100644
--- a/net/net.c
+++ b/net/net.c
@@ -562,6 +562,44 @@ int qemu_can_send_packet(NetClientState *sender)
return 1;
}
+static ssize_t filter_receive_iov(NetClientState *nc, int chain,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
+{
+ ssize_t ret = 0;
+ NetFilterState *nf = NULL;
+
+ QTAILQ_FOREACH(nf, &nc->filters, next) {
+ if (nf->chain == chain || nf->chain == NET_FILTER_ALL) {
+ ret = nf->info->receive_iov(nf, sender, flags,
+ iov, iovcnt, sent_cb);
+ if (ret) {
+ return ret;
+ }
+ }
+ }
+
+ return ret;
+}
+
+static ssize_t filter_receive(NetClientState *nc, int chain,
+ NetClientState *sender,
+ unsigned flags,
+ const uint8_t *data,
+ size_t size,
+ NetPacketSent *sent_cb)
+{
+ struct iovec iov = {
+ .iov_base = (void *)data,
+ .iov_len = size
+ };
+
+ return filter_receive_iov(nc, chain, sender, flags, &iov, 1, sent_cb);
+}
+
ssize_t qemu_deliver_packet(NetClientState *sender,
unsigned flags,
const uint8_t *data,
@@ -633,6 +671,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
NetPacketSent *sent_cb)
{
NetQueue *queue;
+ int ret;
#ifdef DEBUG_NET
printf("qemu_send_packet_async:\n");
@@ -643,6 +682,19 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
return size;
}
+ /* Let filters handle the packet first */
+ ret = filter_receive(sender, NET_FILTER_OUT,
+ sender, flags, buf, size, sent_cb);
+ if (ret) {
+ return ret;
+ }
+
+ ret = filter_receive(sender->peer, NET_FILTER_IN,
+ sender, flags, buf, size, sent_cb);
+ if (ret) {
+ return ret;
+ }
+
queue = sender->peer->incoming_queue;
return qemu_net_queue_send(queue, sender, flags, buf, size, sent_cb);
@@ -713,11 +765,25 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
NetPacketSent *sent_cb)
{
NetQueue *queue;
+ int ret;
if (sender->link_down || !sender->peer) {
return iov_size(iov, iovcnt);
}
+ /* Let filters handle the packet first */
+ ret = filter_receive_iov(sender, NET_FILTER_OUT, sender,
+ QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
+ if (ret) {
+ return ret;
+ }
+
+ ret = filter_receive_iov(sender->peer, NET_FILTER_IN, sender,
+ QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
+ if (ret) {
+ return ret;
+ }
+
queue = sender->peer->incoming_queue;
return qemu_net_queue_send_iov(queue, sender,
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 05/11] move out net queue structs define
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (3 preceding siblings ...)
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 04/11] netfilter: hook packets before net queue send Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-27 14:38 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 06/11] netfilter: add an API to pass the packet to next filter Yang Hongyang
` (8 subsequent siblings)
13 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
stefanha, Yang Hongyang
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
include/net/queue.h | 19 +++++++++++++++++++
net/queue.c | 19 -------------------
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/include/net/queue.h b/include/net/queue.h
index fc02b33..1d65e47 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
+struct NetPacket {
+ QTAILQ_ENTRY(NetPacket) entry;
+ NetClientState *sender;
+ unsigned flags;
+ int size;
+ NetPacketSent *sent_cb;
+ uint8_t data[0];
+};
+
+struct NetQueue {
+ void *opaque;
+ uint32_t nq_maxlen;
+ uint32_t nq_count;
+
+ QTAILQ_HEAD(packets, NetPacket) packets;
+
+ unsigned delivering:1;
+};
+
#define QEMU_NET_PACKET_FLAG_NONE 0
#define QEMU_NET_PACKET_FLAG_RAW (1<<0)
diff --git a/net/queue.c b/net/queue.c
index ebbe2bb..1499479 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -39,25 +39,6 @@
* unbounded queueing.
*/
-struct NetPacket {
- QTAILQ_ENTRY(NetPacket) entry;
- NetClientState *sender;
- unsigned flags;
- int size;
- NetPacketSent *sent_cb;
- uint8_t data[0];
-};
-
-struct NetQueue {
- void *opaque;
- uint32_t nq_maxlen;
- uint32_t nq_count;
-
- QTAILQ_HEAD(packets, NetPacket) packets;
-
- unsigned delivering : 1;
-};
-
NetQueue *qemu_new_net_queue(void *opaque)
{
NetQueue *queue;
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 06/11] netfilter: add an API to pass the packet to next filter
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (4 preceding siblings ...)
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 05/11] move out net queue structs define Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-27 15:11 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 07/11] netfilter: print filter info associate with the netdev Yang Hongyang
` (7 subsequent siblings)
13 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
stefanha, Yang Hongyang
add an API qemu_netfilter_pass_to_next() to pass the packet
to next filter.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
v5: fold params to NetPacket struct
---
include/net/filter.h | 3 +++
net/filter.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/include/net/filter.h b/include/net/filter.h
index f15d83d..9278d40 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -57,4 +57,7 @@ void qemu_del_net_filter(NetFilterState *nf);
void netfilter_add(QemuOpts *opts, Error **errp);
void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
+/* pass the packet to the next filter */
+ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
+
#endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index dcb1891..44c17b0 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -14,10 +14,12 @@
#include "qapi/dealloc-visitor.h"
#include "qemu/config-file.h"
#include "qmp-commands.h"
+#include "qemu/iov.h"
#include "net/filter.h"
#include "net/net.h"
#include "net/vhost_net.h"
+#include "net/queue.h"
static QTAILQ_HEAD(, NetFilterState) net_filters;
@@ -153,6 +155,37 @@ void qmp_netfilter_del(const char *id, Error **errp)
qemu_del_net_filter(nf);
}
+ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet)
+{
+ int ret = 0;
+ NetFilterState *next = QTAILQ_NEXT(nf, next);
+ struct iovec iov = {
+ .iov_base = (void *)packet->data,
+ .iov_len = packet->size
+ };
+
+ while (next) {
+ if (next->chain == nf->chain || next->chain == NET_FILTER_ALL) {
+ ret = next->info->receive_iov(next, packet->sender, packet->flags,
+ &iov, 1, packet->sent_cb);
+ if (ret) {
+ return ret;
+ }
+ }
+ next = QTAILQ_NEXT(next, next);
+ }
+
+ /* we have gone through all filters, pass it to receiver */
+ if (packet->sender && packet->sender->peer) {
+ return qemu_net_queue_send_iov(packet->sender->peer->incoming_queue,
+ packet->sender, packet->flags,
+ &iov, 1, packet->sent_cb);
+ }
+
+ /* no receiver, or sender been deleted */
+ return packet->size;
+}
+
typedef int (NetFilterInit)(const NetFilterOptions *opts,
const char *name, int chain,
NetClientState *netdev, Error **errp);
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 07/11] netfilter: print filter info associate with the netdev
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (5 preceding siblings ...)
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 06/11] netfilter: add an API to pass the packet to next filter Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-27 14:46 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 08/11] net/queue: export qemu_net_queue_append_iov Yang Hongyang
` (6 subsequent siblings)
13 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
Yang Hongyang, stefanha, Yang Hongyang
From: Yang Hongyang <burnef@gmail.com>
When execute "info network", print filter info also.
current info printed is simple, can add more info later.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
v7: initial patch
---
include/net/filter.h | 1 +
net/filter.c | 22 ++++++++++++++++++++++
net/net.c | 11 +++++++++++
3 files changed, 34 insertions(+)
diff --git a/include/net/filter.h b/include/net/filter.h
index 9278d40..188ecb1 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -56,6 +56,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
void qemu_del_net_filter(NetFilterState *nf);
void netfilter_add(QemuOpts *opts, Error **errp);
void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
+const char *qemu_netfilter_get_chain_str(int chain);
/* pass the packet to the next filter */
ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
diff --git a/net/filter.c b/net/filter.c
index 44c17b0..76e12ea 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -23,6 +23,28 @@
static QTAILQ_HEAD(, NetFilterState) net_filters;
+const char *qemu_netfilter_get_chain_str(int chain)
+{
+ const char *str = NULL;
+
+ switch (chain) {
+ case NET_FILTER_IN:
+ str = "in";
+ break;
+ case NET_FILTER_OUT:
+ str = "out";
+ break;
+ case NET_FILTER_ALL:
+ str = "all";
+ break;
+ default:
+ str = "unknown";
+ break;
+ }
+
+ return str;
+}
+
NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
NetClientState *netdev,
const char *name,
diff --git a/net/net.c b/net/net.c
index 00cca83..99f0e87 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1199,10 +1199,21 @@ void qmp_netdev_del(const char *id, Error **errp)
void print_net_client(Monitor *mon, NetClientState *nc)
{
+ NetFilterState *nf;
+
monitor_printf(mon, "%s: index=%d,type=%s,%s\n", nc->name,
nc->queue_index,
NetClientOptionsKind_lookup[nc->info->type],
nc->info_str);
+ if (!QTAILQ_EMPTY(&nc->filters)) {
+ monitor_printf(mon, "filters:\n");
+ }
+ QTAILQ_FOREACH(nf, &nc->filters, next) {
+ monitor_printf(mon, " - %s: type=%s,netdev=%s,chain=%s\n", nf->name,
+ NetFilterOptionsKind_lookup[nf->info->type],
+ nf->netdev->name,
+ qemu_netfilter_get_chain_str(nf->chain));
+ }
}
RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 08/11] net/queue: export qemu_net_queue_append_iov
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (6 preceding siblings ...)
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 07/11] netfilter: print filter info associate with the netdev Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-27 15:05 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 09/11] netfilter: add a netbuffer filter Yang Hongyang
` (5 subsequent siblings)
13 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
stefanha, Yang Hongyang
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
include/net/queue.h | 7 +++++++
net/queue.c | 12 ++++++------
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/net/queue.h b/include/net/queue.h
index 1d65e47..e139cc7 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -55,6 +55,13 @@ struct NetQueue {
NetQueue *qemu_new_net_queue(void *opaque);
+void qemu_net_queue_append_iov(NetQueue *queue,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb);
+
void qemu_del_net_queue(NetQueue *queue);
ssize_t qemu_net_queue_send(NetQueue *queue,
diff --git a/net/queue.c b/net/queue.c
index 1499479..428fdd6 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -91,12 +91,12 @@ static void qemu_net_queue_append(NetQueue *queue,
QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
}
-static void qemu_net_queue_append_iov(NetQueue *queue,
- NetClientState *sender,
- unsigned flags,
- const struct iovec *iov,
- int iovcnt,
- NetPacketSent *sent_cb)
+void qemu_net_queue_append_iov(NetQueue *queue,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
{
NetPacket *packet;
size_t max_len = 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 09/11] netfilter: add a netbuffer filter
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (7 preceding siblings ...)
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 08/11] net/queue: export qemu_net_queue_append_iov Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help Yang Hongyang
` (4 subsequent siblings)
13 siblings, 0 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
stefanha, Yang Hongyang
This filter is to buffer/release packets, this feature can be used
when using MicroCheckpointing, or other Remus like VM FT solutions, you
can also use it to simulate the network delay.
It has an interval option, if supplied, this filter will release
packets by interval.
Usage:
-netdev tap,id=bn0
-netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
NOTE:
the scale of interval is microsecond.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
v7: use QTAILQ_FOREACH_SAFE() when flush packets
v6: move the interval check earlier and some comment adjust
v5: remove dummy sent_cb
change interval type from int64 to uint32
check interval!=0 when initialise
rename FILTERBUFFERState to FilterBufferState
v4: remove bh
pass the packet to next filter instead of receiver
v3: check packet's sender and sender->peer when flush it
fix for netbuffer
---
net/Makefile.objs | 1 +
net/filter-buffer.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++
net/filter.c | 2 +
net/filters.h | 17 +++++++
qapi-schema.json | 18 +++++++-
5 files changed, 162 insertions(+), 1 deletion(-)
create mode 100644 net/filter-buffer.c
create mode 100644 net/filters.h
diff --git a/net/Makefile.objs b/net/Makefile.objs
index 914aec0..5fa2f97 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -14,3 +14,4 @@ common-obj-$(CONFIG_SLIRP) += slirp.o
common-obj-$(CONFIG_VDE) += vde.o
common-obj-$(CONFIG_NETMAP) += netmap.o
common-obj-y += filter.o
+common-obj-y += filter-buffer.o
diff --git a/net/filter-buffer.c b/net/filter-buffer.c
new file mode 100644
index 0000000..622ac54
--- /dev/null
+++ b/net/filter-buffer.c
@@ -0,0 +1,125 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ * Author: Yang Hongyang <yanghy@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#include "net/filter.h"
+#include "net/queue.h"
+#include "filters.h"
+#include "qemu-common.h"
+#include "qemu/timer.h"
+#include "qemu/iov.h"
+#include "qapi/qmp/qerror.h"
+
+typedef struct FilterBufferState {
+ NetFilterState nf;
+ NetQueue *incoming_queue;
+ uint32_t interval;
+ QEMUTimer release_timer;
+} FilterBufferState;
+
+static void filter_buffer_flush(NetFilterState *nf)
+{
+ FilterBufferState *s = DO_UPCAST(FilterBufferState, nf, nf);
+ NetQueue *queue = s->incoming_queue;
+ NetPacket *packet, *next;
+
+ QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) {
+ QTAILQ_REMOVE(&queue->packets, packet, entry);
+ queue->nq_count--;
+
+ if (packet->sender && packet->sender->peer) {
+ qemu_netfilter_pass_to_next(nf, packet);
+ }
+
+ /*
+ * now that we have passed the packet to next filter (or there's
+ * no receiver). If it's queued by receiver's incoming_queue, there
+ * will be a copy of the packet->data, so simply free this packet
+ * now.
+ */
+ g_free(packet);
+ }
+}
+
+static void filter_buffer_release_timer(void *opaque)
+{
+ FilterBufferState *s = opaque;
+ filter_buffer_flush(&s->nf);
+ timer_mod(&s->release_timer,
+ qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval);
+}
+
+/* filter APIs */
+static ssize_t filter_buffer_receive_iov(NetFilterState *nf,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
+{
+ FilterBufferState *s = DO_UPCAST(FilterBufferState, nf, nf);
+ NetQueue *queue = s->incoming_queue;
+
+ qemu_net_queue_append_iov(queue, sender, flags, iov, iovcnt, sent_cb);
+ return iov_size(iov, iovcnt);
+}
+
+static void filter_buffer_cleanup(NetFilterState *nf)
+{
+ FilterBufferState *s = DO_UPCAST(FilterBufferState, nf, nf);
+
+ if (s->interval) {
+ timer_del(&s->release_timer);
+ }
+
+ /* flush packets */
+ filter_buffer_flush(nf);
+ g_free(s->incoming_queue);
+ return;
+}
+
+static NetFilterInfo net_filter_buffer_info = {
+ .type = NET_FILTER_OPTIONS_KIND_BUFFER,
+ .size = sizeof(FilterBufferState),
+ .receive_iov = filter_buffer_receive_iov,
+ .cleanup = filter_buffer_cleanup,
+};
+
+int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
+ int chain, NetClientState *netdev, Error **errp)
+{
+ NetFilterState *nf;
+ FilterBufferState *s;
+ const NetFilterBufferOptions *bufferopt;
+ int interval;
+
+ assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
+ bufferopt = opts->buffer;
+ /*
+ * this check will be dropped when there're VM FT solutions like MC
+ * or COLO use this filter to release packets on demand.
+ */
+ interval = bufferopt->has_interval ? bufferopt->interval : 0;
+ if (!interval) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "interval",
+ "a non-zero interval");
+ return -1;
+ }
+
+ nf = qemu_new_net_filter(&net_filter_buffer_info, netdev, name, chain);
+ s = DO_UPCAST(FilterBufferState, nf, nf);
+ s->incoming_queue = qemu_new_net_queue(nf);
+ s->interval = interval;
+ if (s->interval) {
+ timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL,
+ filter_buffer_release_timer, s);
+ timer_mod(&s->release_timer,
+ qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval);
+ }
+
+ return 0;
+}
diff --git a/net/filter.c b/net/filter.c
index 76e12ea..f23fc7f 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -20,6 +20,7 @@
#include "net/net.h"
#include "net/vhost_net.h"
#include "net/queue.h"
+#include "filters.h"
static QTAILQ_HEAD(, NetFilterState) net_filters;
@@ -214,6 +215,7 @@ typedef int (NetFilterInit)(const NetFilterOptions *opts,
static
NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
+ [NET_FILTER_OPTIONS_KIND_BUFFER] = net_init_filter_buffer,
};
static int net_filter_init1(const NetFilter *netfilter, Error **errp)
diff --git a/net/filters.h b/net/filters.h
new file mode 100644
index 0000000..3b546db
--- /dev/null
+++ b/net/filters.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_NET_FILTERS_H
+#define QEMU_NET_FILTERS_H
+
+#include "net/net.h"
+#include "net/filter.h"
+
+int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
+ int chain, NetClientState *netdev, Error **errp);
+
+#endif /* QEMU_NET_FILTERS_H */
diff --git a/qapi-schema.json b/qapi-schema.json
index 9d97c21..7882641 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2584,6 +2584,21 @@
{ 'command': 'netfilter_del', 'data': {'id': 'str'} }
##
+# @NetFilterBufferOptions
+#
+# a netbuffer filter for network backend.
+#
+# @interval: #optional release packets by interval, if no interval supplied,
+# will release packets when filter_buffer_release_all been called.
+# scale: microsecond
+#
+# Since 2.5
+##
+{ 'struct': 'NetFilterBufferOptions',
+ 'data': {
+ '*interval': 'uint32' } }
+
+##
# @NetFilterOptions
#
# A discriminated record of network filters.
@@ -2592,7 +2607,8 @@
#
##
{ 'union': 'NetFilterOptions',
- 'data': { } }
+ 'data': {
+ 'buffer': 'NetFilterBufferOptions'} }
##
# @NetFilter
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (8 preceding siblings ...)
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 09/11] netfilter: add a netbuffer filter Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-26 15:55 ` Markus Armbruster
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 11/11] tests: add test cases for netfilter object Yang Hongyang
` (3 subsequent siblings)
13 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang,
Markus Armbruster, mrhines, Luiz Capitulino, stefanha,
Yang Hongyang
now that we have a buffer netfilter, update the command
description and help.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Luiz Capitulino <lcapitulino@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
---
v8: add more description for the filter to the TEXI section
---
hmp-commands.hx | 2 +-
qemu-options.hx | 18 +++++++++++++++++-
qmp-commands.hx | 2 +-
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 902e2d1..63177a8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1255,7 +1255,7 @@ ETEXI
{
.name = "netfilter_add",
.args_type = "netfilter:O",
- .params = "[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
+ .params = "[buffer],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
.help = "add netfilter",
.mhandler.cmd = hmp_netfilter_add,
.command_completion = netfilter_add_completion,
diff --git a/qemu-options.hx b/qemu-options.hx
index 0d52d02..390e055 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1575,7 +1575,10 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
"socket][,vlan=n][,option][,option][,...]\n"
" old way to initialize a host network interface\n"
" (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
-DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
+DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter,
+ "-netfilter buffer,id=str,netdev=str[,chain=in|out|all,interval=n]\n"
+ " buffer netdev in/out packets. if interval provided, will release\n"
+ " packets by interval. interval scale: microsecond\n", QEMU_ARCH_ALL)
STEXI
@item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
@findex -net
@@ -1990,6 +1993,19 @@ libpcap, so it can be analyzed with tools such as tcpdump or Wireshark.
Indicate that no network devices should be configured. It is used to
override the default configuration (@option{-net nic -net user}) which
is activated if no @option{-net} options are provided.
+
+@item -netfilter buffer,id=@var{id},netdev=@var{netdevid}[,chain=@var{in/out/all}][,interval=@var{n}]
+Buffer netdev @var{netdevid} input or output packets. if interval @var{n}
+provided, will release packets by interval. interval scale: microsecond.
+
+chain @var{in/out/all} is an option that can be applied to any netfilters, if
+not provided, default is @option{all}.
+
+@option{in} means this filter will receive packets sent to the netdev
+
+@option{out} means this filter will receive packets sent from the netdev
+
+@option{all} means this filter will receive packets both sent to/from the netdev
ETEXI
STEXI
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4f0dc98..9419a6f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -947,7 +947,7 @@ Arguments:
Example:
-> { "execute": "netfilter_add",
- "arguments": { "type": "type", "id": "nf0",
+ "arguments": { "type": "buffer", "id": "nf0",
"netdev": "bn",
"chain": "in" } }
<- { "return": {} }
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Qemu-devel] [PATCH v8 11/11] tests: add test cases for netfilter object
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (9 preceding siblings ...)
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help Yang Hongyang
@ 2015-08-26 9:59 ` Yang Hongyang
2015-08-26 15:58 ` [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Markus Armbruster
` (2 subsequent siblings)
13 siblings, 0 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-08-26 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, mrhines,
stefanha, Yang Hongyang
Using qtest qmp interface to implement following cases:
1) add/remove netfilter
2) add a netfilter then delete the netdev
3) add/remove more than one netfilters
4) add more than one netfilters and then delete the netdev
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
tests/.gitignore | 1 +
tests/Makefile | 2 +
tests/test-netfilter.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 197 insertions(+)
create mode 100644 tests/test-netfilter.c
diff --git a/tests/.gitignore b/tests/.gitignore
index ccc92e4..395962b 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -41,5 +41,6 @@ test-vmstate
test-write-threshold
test-x86-cpuid
test-xbzrle
+test-netfilter
*-test
qapi-schema/*.test.*
diff --git a/tests/Makefile b/tests/Makefile
index 5271123..bc59ad8 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -185,6 +185,7 @@ check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
check-qtest-i386-y += tests/q35-test$(EXESUF)
gcov-files-i386-y += hw/pci-host/q35.c
check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF)
+check-qtest-i386-y += tests/test-netfilter$(EXESUF)
check-qtest-x86_64-y = $(check-qtest-i386-y)
gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
@@ -409,6 +410,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o
tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a
tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(block-obj-y) libqemuutil.a libqemustub.a
+tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
ifeq ($(CONFIG_POSIX),y)
LIBS += -lutil
diff --git a/tests/test-netfilter.c b/tests/test-netfilter.c
new file mode 100644
index 0000000..acbea4c
--- /dev/null
+++ b/tests/test-netfilter.c
@@ -0,0 +1,194 @@
+/*
+ * QTest testcase for netfilter
+ *
+ * Copyright (c) 2015 FUJITSU LIMITED
+ * Author: Yang Hongyang <yanghy@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+#include "libqtest.h"
+
+/* add a netfilter to a netdev and then remove it */
+static void add_one_netfilter(void)
+{
+ QDict *response;
+
+ response = qmp("{\"execute\": \"netfilter_add\","
+ " \"arguments\": {"
+ " \"type\": \"buffer\","
+ " \"id\": \"qtest-f0\","
+ " \"netdev\": \"qtest-bn0\","
+ " \"chain\": \"in\","
+ " \"interval\": \"1000\""
+ "}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{\"execute\": \"netfilter_del\","
+ " \"arguments\": {"
+ " \"id\": \"qtest-f0\""
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+}
+
+/* add a netfilter to a netdev and then remove the netdev */
+static void remove_netdev_with_one_netfilter(void)
+{
+ QDict *response;
+
+ response = qmp("{\"execute\": \"netfilter_add\","
+ " \"arguments\": {"
+ " \"type\": \"buffer\","
+ " \"id\": \"qtest-f0\","
+ " \"netdev\": \"qtest-bn0\","
+ " \"chain\": \"in\","
+ " \"interval\": \"1000\""
+ "}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{\"execute\": \"netdev_del\","
+ " \"arguments\": {"
+ " \"id\": \"qtest-bn0\""
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ /* add back the netdev */
+ response = qmp("{\"execute\": \"netdev_add\","
+ " \"arguments\": {"
+ " \"type\": \"user\","
+ " \"id\": \"qtest-bn0\""
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+}
+
+/* add multi(2) netfilters to a netdev and then remove them */
+static void add_multi_netfilter(void)
+{
+ QDict *response;
+
+ response = qmp("{\"execute\": \"netfilter_add\","
+ " \"arguments\": {"
+ " \"type\": \"buffer\","
+ " \"id\": \"qtest-f0\","
+ " \"netdev\": \"qtest-bn0\","
+ " \"chain\": \"in\","
+ " \"interval\": \"1000\""
+ "}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{\"execute\": \"netfilter_add\","
+ " \"arguments\": {"
+ " \"type\": \"buffer\","
+ " \"id\": \"qtest-f1\","
+ " \"netdev\": \"qtest-bn0\","
+ " \"chain\": \"in\","
+ " \"interval\": \"1000\""
+ "}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{\"execute\": \"netfilter_del\","
+ " \"arguments\": {"
+ " \"id\": \"qtest-f0\""
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{\"execute\": \"netfilter_del\","
+ " \"arguments\": {"
+ " \"id\": \"qtest-f1\""
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+}
+
+/* add multi(2) netfilters to a netdev and then remove the netdev */
+static void remove_netdev_with_multi_netfilter(void)
+{
+ QDict *response;
+
+ response = qmp("{\"execute\": \"netfilter_add\","
+ " \"arguments\": {"
+ " \"type\": \"buffer\","
+ " \"id\": \"qtest-f0\","
+ " \"netdev\": \"qtest-bn0\","
+ " \"chain\": \"in\","
+ " \"interval\": \"1000\""
+ "}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{\"execute\": \"netfilter_add\","
+ " \"arguments\": {"
+ " \"type\": \"buffer\","
+ " \"id\": \"qtest-f1\","
+ " \"netdev\": \"qtest-bn0\","
+ " \"chain\": \"in\","
+ " \"interval\": \"1000\""
+ "}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{\"execute\": \"netdev_del\","
+ " \"arguments\": {"
+ " \"id\": \"qtest-bn0\""
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ /* add back the netdev */
+ response = qmp("{\"execute\": \"netdev_add\","
+ " \"arguments\": {"
+ " \"type\": \"user\","
+ " \"id\": \"qtest-bn0\""
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+ g_test_init(&argc, &argv, NULL);
+ qtest_add_func("/netfilter/addremove_one", add_one_netfilter);
+ qtest_add_func("/netfilter/remove_netdev_one",
+ remove_netdev_with_one_netfilter);
+ qtest_add_func("/netfilter/addremove_multi", add_multi_netfilter);
+ qtest_add_func("/netfilter/remove_netdev_multi",
+ remove_netdev_with_multi_netfilter);
+
+ qtest_start("-netdev user,id=qtest-bn0 -device e1000,netdev=qtest-bn0");
+ ret = g_test_run();
+
+ qtest_end();
+
+ return ret;
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object Yang Hongyang
@ 2015-08-26 13:13 ` Thomas Huth
2015-08-26 14:41 ` Markus Armbruster
1 sibling, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2015-08-26 13:13 UTC (permalink / raw)
To: Yang Hongyang, qemu-devel
Cc: zhang.zhanghailiang, lizhijian, jasowang, mrhines, stefanha
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>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter Yang Hongyang
@ 2015-08-26 14:04 ` Markus Armbruster
2015-08-27 2:34 ` Yang Hongyang
0 siblings, 1 reply; 42+ messages in thread
From: Markus Armbruster @ 2015-08-26 14:04 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, stefanha, Paolo Bonzini
Missed a bunch of revisions of this series, please excuse gaps in my
understanding.
Yang Hongyang <yanghy@cn.fujitsu.com> writes:
> Add the framework for a new netfilter object and a new
> -netfilter CLI option as a basis for the following patches.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
> include/net/filter.h | 15 +++++++++++++++
> include/sysemu/sysemu.h | 1 +
> net/Makefile.objs | 1 +
> net/filter.c | 27 +++++++++++++++++++++++++++
> qemu-options.hx | 1 +
> vl.c | 13 +++++++++++++
> 6 files changed, 58 insertions(+)
> create mode 100644 include/net/filter.h
> create mode 100644 net/filter.c
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> new file mode 100644
> index 0000000..4242ded
> --- /dev/null
> +++ b/include/net/filter.h
> @@ -0,0 +1,15 @@
> +/*
> + * Copyright (c) 2015 FUJITSU LIMITED
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later. See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_NET_FILTER_H
> +#define QEMU_NET_FILTER_H
> +
> +#include "qemu-common.h"
> +
> +int net_init_filters(void);
> +
> +#endif /* QEMU_NET_FILTER_H */
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 44570d1..15d6d00 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -212,6 +212,7 @@ extern QemuOptsList qemu_chardev_opts;
> extern QemuOptsList qemu_device_opts;
> extern QemuOptsList qemu_netdev_opts;
> extern QemuOptsList qemu_net_opts;
> +extern QemuOptsList qemu_netfilter_opts;
> extern QemuOptsList qemu_global_opts;
> extern QemuOptsList qemu_mon_opts;
>
> diff --git a/net/Makefile.objs b/net/Makefile.objs
> index ec19cb3..914aec0 100644
> --- a/net/Makefile.objs
> +++ b/net/Makefile.objs
> @@ -13,3 +13,4 @@ common-obj-$(CONFIG_HAIKU) += tap-haiku.o
> common-obj-$(CONFIG_SLIRP) += slirp.o
> common-obj-$(CONFIG_VDE) += vde.o
> common-obj-$(CONFIG_NETMAP) += netmap.o
> +common-obj-y += filter.o
> diff --git a/net/filter.c b/net/filter.c
> new file mode 100644
> index 0000000..4e40f08
> --- /dev/null
> +++ b/net/filter.c
> @@ -0,0 +1,27 @@
> +/*
> + * Copyright (c) 2015 FUJITSU LIMITED
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later. See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu-common.h"
> +#include "net/filter.h"
> +
> +int net_init_filters(void)
> +{
> + return 0;
> +}
> +
> +QemuOptsList qemu_netfilter_opts = {
> + .name = "netfilter",
> + .implied_opt_name = "type",
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_netfilter_opts.head),
> + .desc = {
> + /*
> + * no elements => accept any params
> + * validation will happen later
> + */
> + { /* end of list */ }
> + },
> +};
Ignorant question: why dynamic validation (empty .desc) instead of
statically defined parameters? The documentation you add in PATCH 10
suggests they're not actually dynamic.
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 77f5853..0d52d02 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
> "socket][,vlan=n][,option][,option][,...]\n"
> " old way to initialize a host network interface\n"
> " (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
> STEXI
> @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
> @findex -net
Wrong spot: this is between DEF(net, ...) and its STEXI..ETEXI stanza.
Suggest to move it behind the ETEXI.
Missing help string. You add the help in PATCH 10. What about adding
it here already? Would serve as a hint of the things to come later in
your series.
Missing STEXI..ETEXI stanza for the user manual.
> diff --git a/vl.c b/vl.c
> index 584ca88..aee931a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -75,6 +75,7 @@ int main(int argc, char **argv)
> #include "monitor/qdev.h"
> #include "sysemu/bt.h"
> #include "net/net.h"
> +#include "net/filter.h"
> #include "net/slirp.h"
> #include "monitor/monitor.h"
> #include "ui/console.h"
> @@ -2998,6 +2999,7 @@ int main(int argc, char **argv, char **envp)
> qemu_add_opts(&qemu_device_opts);
> qemu_add_opts(&qemu_netdev_opts);
> qemu_add_opts(&qemu_net_opts);
> + qemu_add_opts(&qemu_netfilter_opts);
> qemu_add_opts(&qemu_rtc_opts);
> qemu_add_opts(&qemu_global_opts);
> qemu_add_opts(&qemu_mon_opts);
> @@ -3284,6 +3286,13 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> }
> break;
> + case QEMU_OPTION_netfilter:
> + opts = qemu_opts_parse_noisily(qemu_find_opts("netfilter"),
> + optarg, true);
> + if (!opts) {
> + exit(1);
> + }
> + break;
> #ifdef CONFIG_LIBISCSI
> case QEMU_OPTION_iscsi:
> opts = qemu_opts_parse_noisily(qemu_find_opts("iscsi"),
> @@ -4413,6 +4422,10 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> }
>
> + if (net_init_filters() < 0) {
> + exit(1);
> + }
> +
> #ifdef CONFIG_TPM
> if (tpm_init() < 0) {
> exit(1);
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object Yang Hongyang
2015-08-26 13:13 ` Thomas Huth
@ 2015-08-26 14:41 ` Markus Armbruster
2015-08-26 15:31 ` Eric Blake
1 sibling, 1 reply; 42+ messages in thread
From: Markus Armbruster @ 2015-08-26 14:41 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, stefanha
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.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands Yang Hongyang
@ 2015-08-26 15:17 ` Markus Armbruster
2015-08-26 15:37 ` Eric Blake
0 siblings, 1 reply; 42+ messages in thread
From: Markus Armbruster @ 2015-08-26 15:17 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
Only reviewing QAPI/QMP and HMP interface parts for now.
I apologize for not having reviewed this series earlier. v8 is awfully
late for the kind of review comments I have.
Yang Hongyang <yanghy@cn.fujitsu.com> writes:
> add netfilter_{add|del} commands
> This is mostly the same with netdev_{add|del} commands.
>
> When we delete the netdev, we also delete the netfilter object
> attached to it, because if the netdev is removed, the filters
> which attached to it is useless.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Luiz Capitulino <lcapitulino@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
> v7: error msg fix
> move qmp_opts_del() into qemu_del_net_filter()
> v6: add multiqueue support (qemu_del_net_filter)
> v5: squash "net: delete netfilter object when delete netdev"
> ---
> hmp-commands.hx | 30 +++++++++++++++
> hmp.c | 29 +++++++++++++++
> hmp.h | 4 ++
> include/net/filter.h | 3 ++
> monitor.c | 33 +++++++++++++++++
> net/filter.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> net/net.c | 7 ++++
> qapi-schema.json | 47 ++++++++++++++++++++++++
> qmp-commands.hx | 57 +++++++++++++++++++++++++++++
> 9 files changed, 310 insertions(+), 1 deletion(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d3b7932..902e2d1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1253,6 +1253,36 @@ Remove host network device.
> ETEXI
>
> {
> + .name = "netfilter_add",
> + .args_type = "netfilter:O",
> + .params = "[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
> + .help = "add netfilter",
> + .mhandler.cmd = hmp_netfilter_add,
Supporting completion from the start is a nice touch.
> + .command_completion = netfilter_add_completion,
> + },
> +
> +STEXI
> +@item netfilter_add
> +@findex netfilter_add
> +Add netfilter.
> +ETEXI
Awfully terse for a user manual. Please try to follow the good examples
instead of the bad examples in this file :)
> +
> + {
> + .name = "netfilter_del",
> + .args_type = "id:s",
> + .params = "id",
> + .help = "remove netfilter",
> + .mhandler.cmd = hmp_netfilter_del,
> + .command_completion = netfilter_del_completion,
> + },
> +
> +STEXI
> +@item netfilter_del
> +@findex netfilter_del
> +Remove netfilter.
> +ETEXI
> +
> + {
Likewise.
> .name = "object_add",
> .args_type = "object:O",
> .params = "[qom-type=]type,id=str[,prop=value][,...]",
[...]
> diff --git a/qapi-schema.json b/qapi-schema.json
> index d7fb578..9d97c21 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2537,6 +2537,53 @@
> 'opts': 'NetClientOptions' } }
>
> ##
> +# @netfilter_add:
> +#
> +# Add a netfilter.
> +#
> +# @type: the type of netfilter.
> +#
> +# @id: the name of the new netfilter.
> +#
> +# @netdev: the name of the netdev which this filter will be 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
> +#
> +# @props: #optional a list of properties to be passed to the netfilter in
> +# the format of 'name=value'
> +#
> +# Since: 2.5
> +#
> +# Returns: Nothing on success
> +# If @type is not a valid netfilter, DeviceNotFound
> +##
> +{ 'command': 'netfilter_add',
> + 'data': {
> + 'type': 'str',
> + 'id': 'str',
> + 'netdev': 'str',
> + '*chain': 'str',
> + '*props': '**'}, 'gen': false }
I figure you're merely following netdev_add precedence here (can't fault
you for that), but netdev_add cheats, and we shouldn't add more cheats.
First, 'gen': false is best avoided. It suppresses the generated
marshaller, and that lets you cheat. There are cases where we can't do
without, but I don't think this is one.
When we suppress the generated marshaller, 'data' is at best a
declaration of intent; the code can do something else entirely. For
instance, netdev_add declares
{ 'command': 'netdev_add',
'data': {'type': 'str', 'id': 'str', '*props': '**'},
'gen': false }
but the '*props' part is a bald-faced lie: it doesn't take a 'props'
argument. See
http://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00460.html
and maybe even slides 37-38 of
https://events.linuxfoundation.org/sites/events/files/slides/armbru-qemu-introspection.pdf
I didn't check your code, but I suspect yours is a lie, too.
I intend to clean up netdev_add, hopefully soon.
You should use a proper QAPI data type here. I guess the flat union I
sketched in my reply to PATCH 2 would do nicely, except we don't support
commands with union type data, yet. I expect to add support to clean up
netdev_del.
If you don't want to wait for that (understandable!), then I suggest you
keep 'NetFilter' a struct type for now, use it as command data here, and
we convert it to a flat union later. Must be done before the release,
to avoid backward incompatibility.
Then this becomes something like:
{ 'command': 'netfilter-add', 'data': 'NetFilter' }
If you need the command to take arguments you don't want to put into
NetFilter for some reason, I can help you achieve that cleanly.
> +
> +##
> +# @netfilter_del:
> +#
> +# Remove a netfilter.
> +#
> +# @id: the name of the netfilter to remove
> +#
> +# Returns: Nothing on success
> +# If @id is not a valid netfilter, DeviceNotFound
> +#
> +# Since: 2.5
> +##
> +{ 'command': 'netfilter_del', 'data': {'id': 'str'} }
> +
> +##
> # @NetFilterOptions
> #
> # A discriminated record of network filters.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index ba630b1..4f0dc98 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -926,6 +926,63 @@ Example:
> EQMP
>
> {
> + .name = "netfilter_add",
'-' instead of '_' in new QMP commands, please.
> + .args_type = "netfilter:O",
Again, you're merely following netdev_add precedence here, but args_type
'O' is problematic, and should not be used in new code. I hope to get
rid of it entirely. Easiest for now is probably something like
"options:q". Details depend on how exactly you do the schema.
> + .mhandler.cmd_new = qmp_netfilter_add,
> + },
> +
> +SQMP
> +netfilter_add
> +----------
> +
> +Add netfilter.
> +
> +Arguments:
> +
> +- "type": the filter type (json-string)
> +- "id": the netfilter's ID, must be unique (json-string)
> +- "netdev": the netdev's ID which this filter will be attached to(json-string)
> +- filter options
> +
> +Example:
> +
> +-> { "execute": "netfilter_add",
> + "arguments": { "type": "type", "id": "nf0",
> + "netdev": "bn",
> + "chain": "in" } }
> +<- { "return": {} }
> +
> +Note: The supported filter options are the same ones supported by the
> + '-netfilter' command-line argument, which are listed in the '-help'
> + output or QEMU's manual
> +
> +EQMP
> +
> + {
> + .name = "netfilter_del",
> + .args_type = "id:s",
> + .mhandler.cmd_new = qmp_marshal_input_netfilter_del,
> + },
> +
> +SQMP
> +netfilter_del
> +----------
> +
> +Remove netfilter.
> +
> +Arguments:
> +
> +- "id": the netfilter's ID, must be unique (json-string)
> +
> +Example:
> +
> +-> { "execute": "netfilter_del", "arguments": { "id": "nf0" } }
> +<- { "return": {} }
> +
> +
> +EQMP
> +
> + {
> .name = "object-add",
> .args_type = "qom-type:s,id:s,props:q?",
> .mhandler.cmd_new = qmp_object_add,
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object
2015-08-26 14:41 ` Markus Armbruster
@ 2015-08-26 15:31 ` Eric Blake
0 siblings, 0 replies; 42+ messages in thread
From: Eric Blake @ 2015-08-26 15:31 UTC (permalink / raw)
To: Markus Armbruster, Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, stefanha
[-- Attachment #1: Type: text/plain, Size: 1540 bytes --]
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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands
2015-08-26 15:17 ` Markus Armbruster
@ 2015-08-26 15:37 ` Eric Blake
2015-08-28 11:37 ` Markus Armbruster
0 siblings, 1 reply; 42+ messages in thread
From: Eric Blake @ 2015-08-26 15:37 UTC (permalink / raw)
To: Markus Armbruster, Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
[-- Attachment #1: Type: text/plain, Size: 3430 bytes --]
On 08/26/2015 09:17 AM, Markus Armbruster wrote:
> Only reviewing QAPI/QMP and HMP interface parts for now.
>
> I apologize for not having reviewed this series earlier. v8 is awfully
> late for the kind of review comments I have.
And I've also been behind the curve, intending to review this since it
touches API, but not getting there yet.
>> +##
>> +{ 'command': 'netfilter_add',
>> + 'data': {
>> + 'type': 'str',
>> + 'id': 'str',
>> + 'netdev': 'str',
>> + '*chain': 'str',
>> + '*props': '**'}, 'gen': false }
>
> I figure you're merely following netdev_add precedence here (can't fault
> you for that), but netdev_add cheats, and we shouldn't add more cheats.
>
> First, 'gen': false is best avoided. It suppresses the generated
> marshaller, and that lets you cheat. There are cases where we can't do
> without, but I don't think this is one.
>
> When we suppress the generated marshaller, 'data' is at best a
> declaration of intent; the code can do something else entirely. For
> instance, netdev_add declares
>
> { 'command': 'netdev_add',
> 'data': {'type': 'str', 'id': 'str', '*props': '**'},
> 'gen': false }
>
> but the '*props' part is a bald-faced lie: it doesn't take a 'props'
> argument. See
> http://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00460.html
> and maybe even slides 37-38 of
> https://events.linuxfoundation.org/sites/events/files/slides/armbru-qemu-introspection.pdf
>
> I didn't check your code, but I suspect yours is a lie, too.
>
> I intend to clean up netdev_add, hopefully soon.
>
> You should use a proper QAPI data type here. I guess the flat union I
> sketched in my reply to PATCH 2 would do nicely, except we don't support
> commands with union type data, yet. I expect to add support to clean up
> netdev_del.
In fact, I've already proposed adding such support:
http://thread.gmane.org/gmane.comp.emulators.qemu/356265/focus=356291
>
> If you don't want to wait for that (understandable!), then I suggest you
> keep 'NetFilter' a struct type for now, use it as command data here, and
> we convert it to a flat union later. Must be done before the release,
> to avoid backward incompatibility.
>
> Then this becomes something like:
>
> { 'command': 'netfilter-add', 'data': 'NetFilter' }
or use NetFilter as a union, but have the command be:
{ 'command': 'netfilter-add',
'data': { 'data': 'NetFilter' } }
where you have to pass an extra layer of nesting at the QMP layer.
>
> If you need the command to take arguments you don't want to put into
> NetFilter for some reason, I can help you achieve that cleanly.
In fact, having the 'NetFilter' union be a single argument of a larger
struct makes that larger struct the nice place to stick any additional
arguments that don't need to be part of the union.
>
>> +
>> +##
>> +# @netfilter_del:
>> +#
>> +# Remove a netfilter.
>> +#
>> +# @id: the name of the netfilter to remove
>> +#
>> +# Returns: Nothing on success
>> +# If @id is not a valid netfilter, DeviceNotFound
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'command': 'netfilter_del', 'data': {'id': 'str'} }
Please name new QMP commands with '-' not '_'; this should be
'netfilter-del'.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help Yang Hongyang
@ 2015-08-26 15:55 ` Markus Armbruster
2015-08-27 2:42 ` Yang Hongyang
0 siblings, 1 reply; 42+ messages in thread
From: Markus Armbruster @ 2015-08-26 15:55 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
Yang Hongyang <yanghy@cn.fujitsu.com> writes:
> now that we have a buffer netfilter, update the command
> description and help.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Luiz Capitulino <lcapitulino@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> ---
> v8: add more description for the filter to the TEXI section
> ---
> hmp-commands.hx | 2 +-
> qemu-options.hx | 18 +++++++++++++++++-
> qmp-commands.hx | 2 +-
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 902e2d1..63177a8 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1255,7 +1255,7 @@ ETEXI
> {
> .name = "netfilter_add",
> .args_type = "netfilter:O",
> - .params = "[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
> + .params = "[buffer],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
> .help = "add netfilter",
> .mhandler.cmd = hmp_netfilter_add,
> .command_completion = netfilter_add_completion,
This change looks odd. Actually, params look odd before and after the
change :)
I suspect you're trying to follow netdev_add precedence. Its params:
.params = "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
Neglects to mention the long form type=, but that's pretty common.
Uses square brackets both for grouping and for optionalness. We suck.
Users can probably figure out that the first [ ] is just grouping, and
the others are optionalness.
Your params are even more confusing, because the first [ ] is again
grouping, but there's just one alternative: buffer.
What about not simply writing
.params = "buffer,id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
for now?
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0d52d02..390e055 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1575,7 +1575,10 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
> "socket][,vlan=n][,option][,option][,...]\n"
> " old way to initialize a host network interface\n"
> " (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
> -DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter,
> + "-netfilter buffer,id=str,netdev=str[,chain=in|out|all,interval=n]\n"
> + " buffer netdev in/out packets. if interval provided, will release\n"
> + " packets by interval. interval scale: microsecond\n", QEMU_ARCH_ALL)
If I understand your intent correctly, "interval" is the amount of time
to delay packets. What about calling it "delay"?
Suggest something like
buffer network packets, delaying them for 'delay' microseconds
(default 0us)
> STEXI
> @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
> @findex -net
> @@ -1990,6 +1993,19 @@ libpcap, so it can be analyzed with tools such as tcpdump or Wireshark.
> Indicate that no network devices should be configured. It is used to
> override the default configuration (@option{-net nic -net user}) which
> is activated if no @option{-net} options are provided.
> +
> +@item -netfilter buffer,id=@var{id},netdev=@var{netdevid}[,chain=@var{in/out/all}][,interval=@var{n}]
'n' is an unusual choice for a time delay. What about 't', 'dt', or
'delay'?
> +Buffer netdev @var{netdevid} input or output packets.
Are there any other kinds of packets?
> if interval @var{n}
> +provided, will release packets by interval. interval scale: microsecond.
Please start sentences with a capital letter.
Suggest something like
Packets are delayed by @var{n} microseconds. Defaults to 0us,
i.e. no delay.
> +
> +chain @var{in/out/all} is an option that can be applied to any netfilters, if
> +not provided, default is @option{all}.
"to any netfilter" (singular)
Drop "if not provided,"
> +
> +@option{in} means this filter will receive packets sent to the netdev
> +
> +@option{out} means this filter will receive packets sent from the netdev
> +
> +@option{all} means this filter will receive packets both sent to/from the netdev
> ETEXI
>
> STEXI
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4f0dc98..9419a6f 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -947,7 +947,7 @@ Arguments:
> Example:
>
> -> { "execute": "netfilter_add",
> - "arguments": { "type": "type", "id": "nf0",
> + "arguments": { "type": "buffer", "id": "nf0",
> "netdev": "bn",
> "chain": "in" } }
> <- { "return": {} }
Does the example before this patch actually work?
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (10 preceding siblings ...)
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 11/11] tests: add test cases for netfilter object Yang Hongyang
@ 2015-08-26 15:58 ` Markus Armbruster
2015-08-27 2:25 ` Yang Hongyang
2015-08-27 1:05 ` Thomas Huth
2015-08-27 3:15 ` Jason Wang
13 siblings, 1 reply; 42+ messages in thread
From: Markus Armbruster @ 2015-08-26 15:58 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, stefanha
Yang Hongyang <yanghy@cn.fujitsu.com> writes:
> This patch add a new object netfilter, capture all network packets.
> Also implement a netbuffer based on this object.
> the "buffer" netfilter could be used by VM FT solutions like
> MicroCheckpointing, to buffer/release packets. Or to simulate
> packet delay.
I like the idea of having a generic netfilter infrastructure. The
external interfaces need a bit more work. I'm sure you can get them
right with a little help.
Thanks for tackling this!
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (11 preceding siblings ...)
2015-08-26 15:58 ` [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Markus Armbruster
@ 2015-08-27 1:05 ` Thomas Huth
2015-08-27 2:24 ` Yang Hongyang
2015-08-27 3:15 ` Jason Wang
13 siblings, 1 reply; 42+ messages in thread
From: Thomas Huth @ 2015-08-27 1:05 UTC (permalink / raw)
To: Yang Hongyang, qemu-devel
Cc: zhang.zhanghailiang, lizhijian, jasowang, mrhines, stefanha
On 26/08/15 11:59, Yang Hongyang wrote:
> This patch add a new object netfilter, capture all network packets.
> Also implement a netbuffer based on this object.
> the "buffer" netfilter could be used by VM FT solutions like
> MicroCheckpointing, to buffer/release packets. Or to simulate
> packet delay.
>
> You can also get the series from:
> https://github.com/macrosheep/qemu/tree/netfilter-v8
>
> Usage:
> -netdev tap,id=bn0
> -netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
> -device e1000,netdev=bn0
>
> dynamically add/remove netfilters:
> netfilter_add buffer,id=f0,netdev=bn0,chain=in,interval=1000
> netfilter_del f0
>
> NOTE:
> interval's scale is microsecond.
> chain is optional, and is one of in|out|all, 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
>
> TODO:
> - dump
FYI, I've now reworked my dump patch series to use your netfilter
infrastructure - worked out fine and it was pretty easy since your
netfilter infrastructure is very usable! I'll polish my patches a little
bit more, then I'll send them out, too. So I am looking forward to see
your netfilter infrastructure included in upstream soon :-)
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter
2015-08-27 1:05 ` Thomas Huth
@ 2015-08-27 2:24 ` Yang Hongyang
0 siblings, 0 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-08-27 2:24 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: zhang.zhanghailiang, lizhijian, jasowang, mrhines, stefanha
On 08/27/2015 09:05 AM, Thomas Huth wrote:
> On 26/08/15 11:59, Yang Hongyang wrote:
>> This patch add a new object netfilter, capture all network packets.
>> Also implement a netbuffer based on this object.
>> the "buffer" netfilter could be used by VM FT solutions like
>> MicroCheckpointing, to buffer/release packets. Or to simulate
>> packet delay.
>>
>> You can also get the series from:
>> https://github.com/macrosheep/qemu/tree/netfilter-v8
>>
>> Usage:
>> -netdev tap,id=bn0
>> -netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
>> -device e1000,netdev=bn0
>>
>> dynamically add/remove netfilters:
>> netfilter_add buffer,id=f0,netdev=bn0,chain=in,interval=1000
>> netfilter_del f0
>>
>> NOTE:
>> interval's scale is microsecond.
>> chain is optional, and is one of in|out|all, 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
>>
>> TODO:
>> - dump
>
> FYI, I've now reworked my dump patch series to use your netfilter
> infrastructure - worked out fine and it was pretty easy since your
> netfilter infrastructure is very usable! I'll polish my patches a little
> bit more, then I'll send them out, too. So I am looking forward to see
> your netfilter infrastructure included in upstream soon :-)
That's great! Thank you! Seems the patchset still needs some work on the QAPI
part, I will address it as soon as possiable.
>
> Thomas
>
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter
2015-08-26 15:58 ` [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Markus Armbruster
@ 2015-08-27 2:25 ` Yang Hongyang
0 siblings, 0 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-08-27 2:25 UTC (permalink / raw)
To: Markus Armbruster
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, stefanha
On 08/26/2015 11:58 PM, Markus Armbruster wrote:
> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>
>> This patch add a new object netfilter, capture all network packets.
>> Also implement a netbuffer based on this object.
>> the "buffer" netfilter could be used by VM FT solutions like
>> MicroCheckpointing, to buffer/release packets. Or to simulate
>> packet delay.
>
> I like the idea of having a generic netfilter infrastructure. The
> external interfaces need a bit more work. I'm sure you can get them
> right with a little help.
Thanks a lot for the review and help!
>
> Thanks for tackling this!
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter
2015-08-26 14:04 ` Markus Armbruster
@ 2015-08-27 2:34 ` Yang Hongyang
2015-08-28 11:29 ` Markus Armbruster
0 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-27 2:34 UTC (permalink / raw)
To: Markus Armbruster
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, stefanha, Paolo Bonzini
On 08/26/2015 10:04 PM, Markus Armbruster wrote:
> Missed a bunch of revisions of this series, please excuse gaps in my
> understanding.
Thank you for the review.
>
> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>
>> Add the framework for a new netfilter object and a new
>> -netfilter CLI option as a basis for the following patches.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> ---
>> include/net/filter.h | 15 +++++++++++++++
>> include/sysemu/sysemu.h | 1 +
>> net/Makefile.objs | 1 +
>> net/filter.c | 27 +++++++++++++++++++++++++++
>> qemu-options.hx | 1 +
>> vl.c | 13 +++++++++++++
>> 6 files changed, 58 insertions(+)
>> create mode 100644 include/net/filter.h
>> create mode 100644 net/filter.c
>>
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> new file mode 100644
>> index 0000000..4242ded
>> --- /dev/null
>> +++ b/include/net/filter.h
>> @@ -0,0 +1,15 @@
>> +/*
>> + * Copyright (c) 2015 FUJITSU LIMITED
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>> + * later. See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef QEMU_NET_FILTER_H
>> +#define QEMU_NET_FILTER_H
>> +
>> +#include "qemu-common.h"
>> +
>> +int net_init_filters(void);
>> +
>> +#endif /* QEMU_NET_FILTER_H */
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 44570d1..15d6d00 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -212,6 +212,7 @@ extern QemuOptsList qemu_chardev_opts;
>> extern QemuOptsList qemu_device_opts;
>> extern QemuOptsList qemu_netdev_opts;
>> extern QemuOptsList qemu_net_opts;
>> +extern QemuOptsList qemu_netfilter_opts;
>> extern QemuOptsList qemu_global_opts;
>> extern QemuOptsList qemu_mon_opts;
>>
>> diff --git a/net/Makefile.objs b/net/Makefile.objs
>> index ec19cb3..914aec0 100644
>> --- a/net/Makefile.objs
>> +++ b/net/Makefile.objs
>> @@ -13,3 +13,4 @@ common-obj-$(CONFIG_HAIKU) += tap-haiku.o
>> common-obj-$(CONFIG_SLIRP) += slirp.o
>> common-obj-$(CONFIG_VDE) += vde.o
>> common-obj-$(CONFIG_NETMAP) += netmap.o
>> +common-obj-y += filter.o
>> diff --git a/net/filter.c b/net/filter.c
>> new file mode 100644
>> index 0000000..4e40f08
>> --- /dev/null
>> +++ b/net/filter.c
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Copyright (c) 2015 FUJITSU LIMITED
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>> + * later. See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu-common.h"
>> +#include "net/filter.h"
>> +
>> +int net_init_filters(void)
>> +{
>> + return 0;
>> +}
>> +
>> +QemuOptsList qemu_netfilter_opts = {
>> + .name = "netfilter",
>> + .implied_opt_name = "type",
>> + .head = QTAILQ_HEAD_INITIALIZER(qemu_netfilter_opts.head),
>> + .desc = {
>> + /*
>> + * no elements => accept any params
>> + * validation will happen later
>> + */
>> + { /* end of list */ }
>> + },
>> +};
>
> Ignorant question: why dynamic validation (empty .desc) instead of
> statically defined parameters? The documentation you add in PATCH 10
> suggests they're not actually dynamic.
Actually I was following the netdev stuff. There might be bunch of filters,
and I don't know the params of each filter until it is realized.
>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 77f5853..0d52d02 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>> "socket][,vlan=n][,option][,option][,...]\n"
>> " old way to initialize a host network interface\n"
>> " (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
>> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>> STEXI
>> @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
>> @findex -net
>
> Wrong spot: this is between DEF(net, ...) and its STEXI..ETEXI stanza.
> Suggest to move it behind the ETEXI.
>
> Missing help string. You add the help in PATCH 10. What about adding
> it here already? Would serve as a hint of the things to come later in
> your series.
>
> Missing STEXI..ETEXI stanza for the user manual.
If I understand correctly, you are suggesting separate the netfilter from
net, seems more reasonable, so I should add something like:
DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
STEXI..ETEXI
after the DEF(net, ...) and its STEXI..ETEXI stanza, am I right?
>
>> diff --git a/vl.c b/vl.c
>> index 584ca88..aee931a 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -75,6 +75,7 @@ int main(int argc, char **argv)
>> #include "monitor/qdev.h"
>> #include "sysemu/bt.h"
>> #include "net/net.h"
>> +#include "net/filter.h"
>> #include "net/slirp.h"
>> #include "monitor/monitor.h"
>> #include "ui/console.h"
>> @@ -2998,6 +2999,7 @@ int main(int argc, char **argv, char **envp)
>> qemu_add_opts(&qemu_device_opts);
>> qemu_add_opts(&qemu_netdev_opts);
>> qemu_add_opts(&qemu_net_opts);
>> + qemu_add_opts(&qemu_netfilter_opts);
>> qemu_add_opts(&qemu_rtc_opts);
>> qemu_add_opts(&qemu_global_opts);
>> qemu_add_opts(&qemu_mon_opts);
>> @@ -3284,6 +3286,13 @@ int main(int argc, char **argv, char **envp)
>> exit(1);
>> }
>> break;
>> + case QEMU_OPTION_netfilter:
>> + opts = qemu_opts_parse_noisily(qemu_find_opts("netfilter"),
>> + optarg, true);
>> + if (!opts) {
>> + exit(1);
>> + }
>> + break;
>> #ifdef CONFIG_LIBISCSI
>> case QEMU_OPTION_iscsi:
>> opts = qemu_opts_parse_noisily(qemu_find_opts("iscsi"),
>> @@ -4413,6 +4422,10 @@ int main(int argc, char **argv, char **envp)
>> exit(1);
>> }
>>
>> + if (net_init_filters() < 0) {
>> + exit(1);
>> + }
>> +
>> #ifdef CONFIG_TPM
>> if (tpm_init() < 0) {
>> exit(1);
>
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help
2015-08-26 15:55 ` Markus Armbruster
@ 2015-08-27 2:42 ` Yang Hongyang
2015-08-28 11:42 ` Markus Armbruster
0 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-27 2:42 UTC (permalink / raw)
To: Markus Armbruster
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
On 08/26/2015 11:55 PM, Markus Armbruster wrote:
> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>
>> now that we have a buffer netfilter, update the command
>> description and help.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> CC: Luiz Capitulino <lcapitulino@redhat.com>
>> CC: Markus Armbruster <armbru@redhat.com>
>> ---
>> v8: add more description for the filter to the TEXI section
>> ---
>> hmp-commands.hx | 2 +-
>> qemu-options.hx | 18 +++++++++++++++++-
>> qmp-commands.hx | 2 +-
>> 3 files changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 902e2d1..63177a8 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1255,7 +1255,7 @@ ETEXI
>> {
>> .name = "netfilter_add",
>> .args_type = "netfilter:O",
>> - .params = "[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
>> + .params = "[buffer],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
>> .help = "add netfilter",
>> .mhandler.cmd = hmp_netfilter_add,
>> .command_completion = netfilter_add_completion,
>
> This change looks odd. Actually, params look odd before and after the
> change :)
>
> I suspect you're trying to follow netdev_add precedence. Its params:
>
> .params = "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
>
> Neglects to mention the long form type=, but that's pretty common.
>
> Uses square brackets both for grouping and for optionalness. We suck.
>
> Users can probably figure out that the first [ ] is just grouping, and
> the others are optionalness.
>
> Your params are even more confusing, because the first [ ] is again
> grouping, but there's just one alternative: buffer.
>
> What about not simply writing
>
> .params = "buffer,id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
>
> for now?
Sure, I agree it is confusing when it has only one alternative now...When we
added more filters, we can change it later.
>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 0d52d02..390e055 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1575,7 +1575,10 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>> "socket][,vlan=n][,option][,option][,...]\n"
>> " old way to initialize a host network interface\n"
>> " (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
>> -DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter,
>> + "-netfilter buffer,id=str,netdev=str[,chain=in|out|all,interval=n]\n"
>> + " buffer netdev in/out packets. if interval provided, will release\n"
>> + " packets by interval. interval scale: microsecond\n", QEMU_ARCH_ALL)
>
> If I understand your intent correctly, "interval" is the amount of time
> to delay packets. What about calling it "delay"?
You can take it as the amount of time to delay packets, but for other usecase
like MC, you can also take it as an interval to release packets, that is to
say, to release packets every {interval} time.
>
> Suggest something like
>
> buffer network packets, delaying them for 'delay' microseconds
> (default 0us)
>
>> STEXI
>> @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
>> @findex -net
>> @@ -1990,6 +1993,19 @@ libpcap, so it can be analyzed with tools such as tcpdump or Wireshark.
>> Indicate that no network devices should be configured. It is used to
>> override the default configuration (@option{-net nic -net user}) which
>> is activated if no @option{-net} options are provided.
>> +
>> +@item -netfilter buffer,id=@var{id},netdev=@var{netdevid}[,chain=@var{in/out/all}][,interval=@var{n}]
>
> 'n' is an unusual choice for a time delay. What about 't', 'dt', or
> 'delay'?
't' is better, thanks!
>
>> +Buffer netdev @var{netdevid} input or output packets.
>
> Are there any other kinds of packets?
No, only input/output packets now.
>
>> if interval @var{n}
>> +provided, will release packets by interval. interval scale: microsecond.
>
> Please start sentences with a capital letter.
>
> Suggest something like
>
> Packets are delayed by @var{n} microseconds. Defaults to 0us,
> i.e. no delay.
>
>> +
>> +chain @var{in/out/all} is an option that can be applied to any netfilters, if
>> +not provided, default is @option{all}.
>
> "to any netfilter" (singular)
>
> Drop "if not provided,"
Ok, thanks!
>
>> +
>> +@option{in} means this filter will receive packets sent to the netdev
>> +
>> +@option{out} means this filter will receive packets sent from the netdev
>> +
>> +@option{all} means this filter will receive packets both sent to/from the netdev
>> ETEXI
>>
>> STEXI
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 4f0dc98..9419a6f 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -947,7 +947,7 @@ Arguments:
>> Example:
>>
>> -> { "execute": "netfilter_add",
>> - "arguments": { "type": "type", "id": "nf0",
>> + "arguments": { "type": "buffer", "id": "nf0",
>> "netdev": "bn",
>> "chain": "in" } }
>> <- { "return": {} }
>
> Does the example before this patch actually work?
No, there's only type=buffer now.
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
` (12 preceding siblings ...)
2015-08-27 1:05 ` Thomas Huth
@ 2015-08-27 3:15 ` Jason Wang
2015-08-31 1:43 ` Yang Hongyang
13 siblings, 1 reply; 42+ messages in thread
From: Jason Wang @ 2015-08-27 3:15 UTC (permalink / raw)
To: Yang Hongyang, qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, mrhines, stefanha
On 08/26/2015 05:59 PM, Yang Hongyang wrote:
> This patch add a new object netfilter, capture all network packets.
> Also implement a netbuffer based on this object.
> the "buffer" netfilter could be used by VM FT solutions like
> MicroCheckpointing, to buffer/release packets. Or to simulate
> packet delay.
>
> You can also get the series from:
> https://github.com/macrosheep/qemu/tree/netfilter-v8
>
> Usage:
> -netdev tap,id=bn0
> -netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
> -device e1000,netdev=bn0
>
> dynamically add/remove netfilters:
> netfilter_add buffer,id=f0,netdev=bn0,chain=in,interval=1000
> netfilter_del f0
>
> NOTE:
> interval's scale is microsecond.
> chain is optional, and is one of in|out|all, 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
>
> TODO:
> - dump
>
> v8:
> - some minor fixes according to Thomas's comments
> - rebased to the latest master branch
>
> v7:
> - print filter info when execute 'info network'
> - addressed Jason's comments
>
> v6:
> - add multiqueue support, please see individual patch for detail
>
> v5:
> - add a sent_cb param to filter receive_iov api
> - squash the 4th patch into patch 3
> - remove dummy sent_cb (buffer filter)
> - addressed Jason's other comments, see individual patches for detail
>
> v4:
> - get rid of struct Filter
> - squash the 4th patch into patch 2
> - fix qemu_netfilter_pass_to_next_iov
> - get rid of bh (buffer filter)
> - release the packet to next filter instead of to receiver (buffer filter)
>
> v3:
> - add an api to pass the packet to next filter
> - remove netfilters when delete netdev
> - add qtest testcases for netfilter
> - addressed comments from Jason
>
> v2:
> - add a chain option to netfilter object
> - move the hook place earlier, before net_queue_send
> - drop the unused api in buffer filter
> - squash buffer filter patches into one
> - remove receive() api from netfilter, only receive_iov() is enough
> - addressed comments from Jason&Thomas
>
> v1:
> initial patch.
>
> Yang Hongyang (11):
> net: add a new object netfilter
> init/cleanup of netfilter object
> netfilter: add netfilter_{add|del} commands
> netfilter: hook packets before net queue send
> move out net queue structs define
> netfilter: add an API to pass the packet to next filter
> netfilter: print filter info associate with the netdev
> net/queue: export qemu_net_queue_append_iov
> netfilter: add a netbuffer filter
> filter/buffer: update command description and help
> tests: add test cases for netfilter object
>
> hmp-commands.hx | 30 +++++
> hmp.c | 29 +++++
> hmp.h | 4 +
> include/net/filter.h | 64 ++++++++++
> include/net/net.h | 1 +
> include/net/queue.h | 26 ++++
> include/qemu/typedefs.h | 1 +
> include/sysemu/sysemu.h | 1 +
> monitor.c | 33 +++++
> net/Makefile.objs | 2 +
> net/filter-buffer.c | 125 ++++++++++++++++++
> net/filter.c | 332 ++++++++++++++++++++++++++++++++++++++++++++++++
> net/filters.h | 17 +++
> net/net.c | 85 +++++++++++++
> net/queue.c | 31 +----
> qapi-schema.json | 100 +++++++++++++++
> qemu-options.hx | 17 +++
> qmp-commands.hx | 57 +++++++++
> tests/.gitignore | 1 +
> tests/Makefile | 2 +
> tests/test-netfilter.c | 194 ++++++++++++++++++++++++++++
> vl.c | 13 ++
> 22 files changed, 1140 insertions(+), 25 deletions(-)
> create mode 100644 include/net/filter.h
> create mode 100644 net/filter-buffer.c
> create mode 100644 net/filter.c
> create mode 100644 net/filters.h
> create mode 100644 tests/test-netfilter.c
>
Looks good to me. After addressing comments of interfaces, I think it
was pretty ready to be merged.
Thanks
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 04/11] netfilter: hook packets before net queue send
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 04/11] netfilter: hook packets before net queue send Yang Hongyang
@ 2015-08-27 14:35 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2015-08-27 14:35 UTC (permalink / raw)
To: Yang Hongyang, qemu-devel
Cc: zhang.zhanghailiang, lizhijian, jasowang, mrhines, stefanha
On 26/08/15 11:59, Yang Hongyang wrote:
> Capture packets that will be sent.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
> v5: do not check ret against iov_size
> pass sent_cb to filters
> ---
> net/net.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/net/net.c b/net/net.c
> index 74f3592..00cca83 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -562,6 +562,44 @@ int qemu_can_send_packet(NetClientState *sender)
> return 1;
> }
>
> +static ssize_t filter_receive_iov(NetClientState *nc, int chain,
> + NetClientState *sender,
> + unsigned flags,
> + const struct iovec *iov,
> + int iovcnt,
> + NetPacketSent *sent_cb)
> +{
> + ssize_t ret = 0;
> + NetFilterState *nf = NULL;
> +
> + QTAILQ_FOREACH(nf, &nc->filters, next) {
> + if (nf->chain == chain || nf->chain == NET_FILTER_ALL) {
> + ret = nf->info->receive_iov(nf, sender, flags,
> + iov, iovcnt, sent_cb);
> + if (ret) {
> + return ret;
> + }
> + }
> + }
> +
> + return ret;
I think that could also be "return 0" since all other return values are
handled by the return within the loop already. Then you could also
remove the pre-init of "ret = 0" at the beginning of the function.
> +}
> +
> +static ssize_t filter_receive(NetClientState *nc, int chain,
> + NetClientState *sender,
> + unsigned flags,
> + const uint8_t *data,
> + size_t size,
> + NetPacketSent *sent_cb)
> +{
> + struct iovec iov = {
> + .iov_base = (void *)data,
> + .iov_len = size
> + };
> +
> + return filter_receive_iov(nc, chain, sender, flags, &iov, 1, sent_cb);
> +}
> +
> ssize_t qemu_deliver_packet(NetClientState *sender,
> unsigned flags,
> const uint8_t *data,
> @@ -633,6 +671,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
> NetPacketSent *sent_cb)
> {
> NetQueue *queue;
> + int ret;
>
> #ifdef DEBUG_NET
> printf("qemu_send_packet_async:\n");
> @@ -643,6 +682,19 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
> return size;
> }
>
> + /* Let filters handle the packet first */
> + ret = filter_receive(sender, NET_FILTER_OUT,
> + sender, flags, buf, size, sent_cb);
> + if (ret) {
> + return ret;
> + }
> +
> + ret = filter_receive(sender->peer, NET_FILTER_IN,
> + sender, flags, buf, size, sent_cb);
> + if (ret) {
> + return ret;
> + }
> +
> queue = sender->peer->incoming_queue;
>
> return qemu_net_queue_send(queue, sender, flags, buf, size, sent_cb);
> @@ -713,11 +765,25 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
> NetPacketSent *sent_cb)
> {
> NetQueue *queue;
> + int ret;
>
> if (sender->link_down || !sender->peer) {
> return iov_size(iov, iovcnt);
> }
>
> + /* Let filters handle the packet first */
> + ret = filter_receive_iov(sender, NET_FILTER_OUT, sender,
> + QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
> + if (ret) {
> + return ret;
> + }
> +
> + ret = filter_receive_iov(sender->peer, NET_FILTER_IN, sender,
> + QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
> + if (ret) {
> + return ret;
> + }
> +
> queue = sender->peer->incoming_queue;
>
> return qemu_net_queue_send_iov(queue, sender,
My comment above was just cosmetics, patch looks already fine to me as
it is, so anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 05/11] move out net queue structs define
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 05/11] move out net queue structs define Yang Hongyang
@ 2015-08-27 14:38 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2015-08-27 14:38 UTC (permalink / raw)
To: Yang Hongyang, qemu-devel
Cc: zhang.zhanghailiang, lizhijian, jasowang, mrhines, stefanha
Maybe add a short comment why/where this is needed later?
On 26/08/15 11:59, Yang Hongyang wrote:
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
> include/net/queue.h | 19 +++++++++++++++++++
> net/queue.c | 19 -------------------
> 2 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/include/net/queue.h b/include/net/queue.h
> index fc02b33..1d65e47 100644
> --- a/include/net/queue.h
> +++ b/include/net/queue.h
> @@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
>
> typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
>
> +struct NetPacket {
> + QTAILQ_ENTRY(NetPacket) entry;
> + NetClientState *sender;
> + unsigned flags;
> + int size;
> + NetPacketSent *sent_cb;
> + uint8_t data[0];
> +};
> +
> +struct NetQueue {
> + void *opaque;
> + uint32_t nq_maxlen;
> + uint32_t nq_count;
> +
> + QTAILQ_HEAD(packets, NetPacket) packets;
> +
> + unsigned delivering:1;
> +};
> +
> #define QEMU_NET_PACKET_FLAG_NONE 0
> #define QEMU_NET_PACKET_FLAG_RAW (1<<0)
>
> diff --git a/net/queue.c b/net/queue.c
> index ebbe2bb..1499479 100644
> --- a/net/queue.c
> +++ b/net/queue.c
> @@ -39,25 +39,6 @@
> * unbounded queueing.
> */
>
> -struct NetPacket {
> - QTAILQ_ENTRY(NetPacket) entry;
> - NetClientState *sender;
> - unsigned flags;
> - int size;
> - NetPacketSent *sent_cb;
> - uint8_t data[0];
> -};
> -
> -struct NetQueue {
> - void *opaque;
> - uint32_t nq_maxlen;
> - uint32_t nq_count;
> -
> - QTAILQ_HEAD(packets, NetPacket) packets;
> -
> - unsigned delivering : 1;
> -};
> -
> NetQueue *qemu_new_net_queue(void *opaque)
> {
> NetQueue *queue;
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 07/11] netfilter: print filter info associate with the netdev
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 07/11] netfilter: print filter info associate with the netdev Yang Hongyang
@ 2015-08-27 14:46 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2015-08-27 14:46 UTC (permalink / raw)
To: Yang Hongyang, qemu-devel
Cc: zhang.zhanghailiang, lizhijian, jasowang, mrhines, Yang Hongyang,
stefanha
On 26/08/15 11:59, Yang Hongyang wrote:
> From: Yang Hongyang <burnef@gmail.com>
>
> When execute "info network", print filter info also.
> current info printed is simple, can add more info later.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
> v7: initial patch
> ---
> include/net/filter.h | 1 +
> net/filter.c | 22 ++++++++++++++++++++++
> net/net.c | 11 +++++++++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 9278d40..188ecb1 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -56,6 +56,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
> void qemu_del_net_filter(NetFilterState *nf);
> void netfilter_add(QemuOpts *opts, Error **errp);
> void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
> +const char *qemu_netfilter_get_chain_str(int chain);
>
> /* pass the packet to the next filter */
> ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
> diff --git a/net/filter.c b/net/filter.c
> index 44c17b0..76e12ea 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -23,6 +23,28 @@
>
> static QTAILQ_HEAD(, NetFilterState) net_filters;
>
> +const char *qemu_netfilter_get_chain_str(int chain)
> +{
> + const char *str = NULL;
The "= NULL" is not necessary here - the default case below should
handle this.
> + switch (chain) {
> + case NET_FILTER_IN:
> + str = "in";
> + break;
> + case NET_FILTER_OUT:
> + str = "out";
> + break;
> + case NET_FILTER_ALL:
> + str = "all";
> + break;
> + default:
> + str = "unknown";
> + break;
> + }
> +
> + return str;
> +}
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 08/11] net/queue: export qemu_net_queue_append_iov
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 08/11] net/queue: export qemu_net_queue_append_iov Yang Hongyang
@ 2015-08-27 15:05 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2015-08-27 15:05 UTC (permalink / raw)
To: Yang Hongyang, qemu-devel
Cc: zhang.zhanghailiang, lizhijian, jasowang, mrhines, stefanha
Maybe add a short description why/where this will be needed?
On 26/08/15 11:59, Yang Hongyang wrote:
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
> include/net/queue.h | 7 +++++++
> net/queue.c | 12 ++++++------
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/queue.h b/include/net/queue.h
> index 1d65e47..e139cc7 100644
> --- a/include/net/queue.h
> +++ b/include/net/queue.h
> @@ -55,6 +55,13 @@ struct NetQueue {
>
> NetQueue *qemu_new_net_queue(void *opaque);
>
> +void qemu_net_queue_append_iov(NetQueue *queue,
> + NetClientState *sender,
> + unsigned flags,
> + const struct iovec *iov,
> + int iovcnt,
> + NetPacketSent *sent_cb);
> +
> void qemu_del_net_queue(NetQueue *queue);
>
> ssize_t qemu_net_queue_send(NetQueue *queue,
> diff --git a/net/queue.c b/net/queue.c
> index 1499479..428fdd6 100644
> --- a/net/queue.c
> +++ b/net/queue.c
> @@ -91,12 +91,12 @@ static void qemu_net_queue_append(NetQueue *queue,
> QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
> }
>
> -static void qemu_net_queue_append_iov(NetQueue *queue,
> - NetClientState *sender,
> - unsigned flags,
> - const struct iovec *iov,
> - int iovcnt,
> - NetPacketSent *sent_cb)
> +void qemu_net_queue_append_iov(NetQueue *queue,
> + NetClientState *sender,
> + unsigned flags,
> + const struct iovec *iov,
> + int iovcnt,
> + NetPacketSent *sent_cb)
> {
> NetPacket *packet;
> size_t max_len = 0;
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 06/11] netfilter: add an API to pass the packet to next filter
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 06/11] netfilter: add an API to pass the packet to next filter Yang Hongyang
@ 2015-08-27 15:11 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2015-08-27 15:11 UTC (permalink / raw)
To: Yang Hongyang, qemu-devel
Cc: zhang.zhanghailiang, lizhijian, jasowang, mrhines, stefanha
On 26/08/15 11:59, Yang Hongyang wrote:
> add an API qemu_netfilter_pass_to_next() to pass the packet
> to next filter.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>
> ---
> v5: fold params to NetPacket struct
> ---
> include/net/filter.h | 3 +++
> net/filter.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter
2015-08-27 2:34 ` Yang Hongyang
@ 2015-08-28 11:29 ` Markus Armbruster
2015-08-31 1:31 ` Yang Hongyang
0 siblings, 1 reply; 42+ messages in thread
From: Markus Armbruster @ 2015-08-28 11:29 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, stefanha, Paolo Bonzini
Yang Hongyang <yanghy@cn.fujitsu.com> writes:
> On 08/26/2015 10:04 PM, Markus Armbruster wrote:
>> Missed a bunch of revisions of this series, please excuse gaps in my
>> understanding.
>
> Thank you for the review.
>
>>
>> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>>
>>> Add the framework for a new netfilter object and a new
>>> -netfilter CLI option as a basis for the following patches.
>>>
>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>> CC: Paolo Bonzini <pbonzini@redhat.com>
>>> CC: Eric Blake <eblake@redhat.com>
>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> include/net/filter.h | 15 +++++++++++++++
>>> include/sysemu/sysemu.h | 1 +
>>> net/Makefile.objs | 1 +
>>> net/filter.c | 27 +++++++++++++++++++++++++++
>>> qemu-options.hx | 1 +
>>> vl.c | 13 +++++++++++++
>>> 6 files changed, 58 insertions(+)
>>> create mode 100644 include/net/filter.h
>>> create mode 100644 net/filter.c
>>>
>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>> new file mode 100644
>>> index 0000000..4242ded
>>> --- /dev/null
>>> +++ b/include/net/filter.h
>>> @@ -0,0 +1,15 @@
>>> +/*
>>> + * Copyright (c) 2015 FUJITSU LIMITED
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>>> + * later. See the COPYING file in the top-level directory.
>>> + */
>>> +
>>> +#ifndef QEMU_NET_FILTER_H
>>> +#define QEMU_NET_FILTER_H
>>> +
>>> +#include "qemu-common.h"
>>> +
>>> +int net_init_filters(void);
>>> +
>>> +#endif /* QEMU_NET_FILTER_H */
>>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>>> index 44570d1..15d6d00 100644
>>> --- a/include/sysemu/sysemu.h
>>> +++ b/include/sysemu/sysemu.h
>>> @@ -212,6 +212,7 @@ extern QemuOptsList qemu_chardev_opts;
>>> extern QemuOptsList qemu_device_opts;
>>> extern QemuOptsList qemu_netdev_opts;
>>> extern QemuOptsList qemu_net_opts;
>>> +extern QemuOptsList qemu_netfilter_opts;
>>> extern QemuOptsList qemu_global_opts;
>>> extern QemuOptsList qemu_mon_opts;
>>>
>>> diff --git a/net/Makefile.objs b/net/Makefile.objs
>>> index ec19cb3..914aec0 100644
>>> --- a/net/Makefile.objs
>>> +++ b/net/Makefile.objs
>>> @@ -13,3 +13,4 @@ common-obj-$(CONFIG_HAIKU) += tap-haiku.o
>>> common-obj-$(CONFIG_SLIRP) += slirp.o
>>> common-obj-$(CONFIG_VDE) += vde.o
>>> common-obj-$(CONFIG_NETMAP) += netmap.o
>>> +common-obj-y += filter.o
>>> diff --git a/net/filter.c b/net/filter.c
>>> new file mode 100644
>>> index 0000000..4e40f08
>>> --- /dev/null
>>> +++ b/net/filter.c
>>> @@ -0,0 +1,27 @@
>>> +/*
>>> + * Copyright (c) 2015 FUJITSU LIMITED
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>>> + * later. See the COPYING file in the top-level directory.
>>> + */
>>> +
>>> +#include "qemu-common.h"
>>> +#include "net/filter.h"
>>> +
>>> +int net_init_filters(void)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +QemuOptsList qemu_netfilter_opts = {
>>> + .name = "netfilter",
>>> + .implied_opt_name = "type",
>>> + .head = QTAILQ_HEAD_INITIALIZER(qemu_netfilter_opts.head),
>>> + .desc = {
>>> + /*
>>> + * no elements => accept any params
>>> + * validation will happen later
>>> + */
>>> + { /* end of list */ }
>>> + },
>>> +};
>>
>> Ignorant question: why dynamic validation (empty .desc) instead of
>> statically defined parameters? The documentation you add in PATCH 10
>> suggests they're not actually dynamic.
>
> Actually I was following the netdev stuff. There might be bunch of filters,
> and I don't know the params of each filter until it is realized.
I realized that later on in your series.
>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index 77f5853..0d52d02 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>>> "socket][,vlan=n][,option][,option][,...]\n"
>>> " old way to initialize a host network interface\n"
>>> " (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
>>> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>>> STEXI
>>> @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
>>> @findex -net
>>
>> Wrong spot: this is between DEF(net, ...) and its STEXI..ETEXI stanza.
>> Suggest to move it behind the ETEXI.
>>
>> Missing help string. You add the help in PATCH 10. What about adding
>> it here already? Would serve as a hint of the things to come later in
>> your series.
>>
>> Missing STEXI..ETEXI stanza for the user manual.
>
> If I understand correctly, you are suggesting separate the netfilter from
> net, seems more reasonable, so I should add something like:
> DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
> STEXI..ETEXI
>
> after the DEF(net, ...) and its STEXI..ETEXI stanza, am I right?
Yes. Always keep DEF(whatever, ...) and its STEXI..ETEXI together.
Inserting netfilter after net seems the most logical place.
[...]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands
2015-08-26 15:37 ` Eric Blake
@ 2015-08-28 11:37 ` Markus Armbruster
2015-08-31 1:36 ` Yang Hongyang
0 siblings, 1 reply; 42+ messages in thread
From: Markus Armbruster @ 2015-08-28 11:37 UTC (permalink / raw)
To: Eric Blake
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha, Yang Hongyang
Eric Blake <eblake@redhat.com> writes:
> On 08/26/2015 09:17 AM, Markus Armbruster wrote:
>> Only reviewing QAPI/QMP and HMP interface parts for now.
>>
>> I apologize for not having reviewed this series earlier. v8 is awfully
>> late for the kind of review comments I have.
>
> And I've also been behind the curve, intending to review this since it
> touches API, but not getting there yet.
>
>
>>> +##
>>> +{ 'command': 'netfilter_add',
>>> + 'data': {
>>> + 'type': 'str',
>>> + 'id': 'str',
>>> + 'netdev': 'str',
>>> + '*chain': 'str',
>>> + '*props': '**'}, 'gen': false }
>>
>> I figure you're merely following netdev_add precedence here (can't fault
>> you for that), but netdev_add cheats, and we shouldn't add more cheats.
>>
>> First, 'gen': false is best avoided. It suppresses the generated
>> marshaller, and that lets you cheat. There are cases where we can't do
>> without, but I don't think this is one.
>>
>> When we suppress the generated marshaller, 'data' is at best a
>> declaration of intent; the code can do something else entirely. For
>> instance, netdev_add declares
>>
>> { 'command': 'netdev_add',
>> 'data': {'type': 'str', 'id': 'str', '*props': '**'},
>> 'gen': false }
>>
>> but the '*props' part is a bald-faced lie: it doesn't take a 'props'
>> argument. See
>> http://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00460.html
>> and maybe even slides 37-38 of
>> https://events.linuxfoundation.org/sites/events/files/slides/armbru-qemu-introspection.pdf
>>
>> I didn't check your code, but I suspect yours is a lie, too.
>>
>> I intend to clean up netdev_add, hopefully soon.
>>
>> You should use a proper QAPI data type here. I guess the flat union I
>> sketched in my reply to PATCH 2 would do nicely, except we don't support
>> commands with union type data, yet. I expect to add support to clean up
>> netdev_del.
>
> In fact, I've already proposed adding such support:
>
> http://thread.gmane.org/gmane.comp.emulators.qemu/356265/focus=356291
In my review queue. Which has become sickeningly long again...
>>
>> If you don't want to wait for that (understandable!), then I suggest you
>> keep 'NetFilter' a struct type for now, use it as command data here, and
>> we convert it to a flat union later. Must be done before the release,
>> to avoid backward incompatibility.
>>
>> Then this becomes something like:
>>
>> { 'command': 'netfilter-add', 'data': 'NetFilter' }
>
> or use NetFilter as a union, but have the command be:
>
> { 'command': 'netfilter-add',
> 'data': { 'data': 'NetFilter' } }
>
> where you have to pass an extra layer of nesting at the QMP layer.
>
>>
>> If you need the command to take arguments you don't want to put into
>> NetFilter for some reason, I can help you achieve that cleanly.
>
> In fact, having the 'NetFilter' union be a single argument of a larger
> struct makes that larger struct the nice place to stick any additional
> arguments that don't need to be part of the union.
To make progress, I suggest you try the following:
1. Make NetFilter a flat union, as I suggested in my review of PATCH 2.
2. Use Eric's idea above, because it avoids the dependency on code
that's still under review.
Drawback: extra layer of nesting. Ugly, but not the end of the world,
and we still have a chance to peel it off before the next release.
[...]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help
2015-08-27 2:42 ` Yang Hongyang
@ 2015-08-28 11:42 ` Markus Armbruster
2015-08-31 1:30 ` Yang Hongyang
0 siblings, 1 reply; 42+ messages in thread
From: Markus Armbruster @ 2015-08-28 11:42 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
Yang Hongyang <yanghy@cn.fujitsu.com> writes:
> On 08/26/2015 11:55 PM, Markus Armbruster wrote:
>> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>>
>>> now that we have a buffer netfilter, update the command
>>> description and help.
>>>
>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>> CC: Luiz Capitulino <lcapitulino@redhat.com>
>>> CC: Markus Armbruster <armbru@redhat.com>
>>> ---
>>> v8: add more description for the filter to the TEXI section
>>> ---
>>> hmp-commands.hx | 2 +-
>>> qemu-options.hx | 18 +++++++++++++++++-
>>> qmp-commands.hx | 2 +-
>>> 3 files changed, 19 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>> index 902e2d1..63177a8 100644
>>> --- a/hmp-commands.hx
>>> +++ b/hmp-commands.hx
>>> @@ -1255,7 +1255,7 @@ ETEXI
>>> {
>>> .name = "netfilter_add",
>>> .args_type = "netfilter:O",
>>> - .params = "[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
>>> + .params = "[buffer],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
>>> .help = "add netfilter",
>>> .mhandler.cmd = hmp_netfilter_add,
>>> .command_completion = netfilter_add_completion,
>>
>> This change looks odd. Actually, params look odd before and after the
>> change :)
>>
>> I suspect you're trying to follow netdev_add precedence. Its params:
>>
>> .params =
>> "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
>>
>> Neglects to mention the long form type=, but that's pretty common.
>>
>> Uses square brackets both for grouping and for optionalness. We suck.
>>
>> Users can probably figure out that the first [ ] is just grouping, and
>> the others are optionalness.
>>
>> Your params are even more confusing, because the first [ ] is again
>> grouping, but there's just one alternative: buffer.
>>
>> What about not simply writing
>>
>> .params =
>> "buffer,id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
>>
>> for now?
>
> Sure, I agree it is confusing when it has only one alternative now...When we
> added more filters, we can change it later.
>
>>
>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index 0d52d02..390e055 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -1575,7 +1575,10 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>>> "socket][,vlan=n][,option][,option][,...]\n"
>>> " old way to initialize a host network interface\n"
>>> " (use the -netdev option if possible instead)\n",
>>> QEMU_ARCH_ALL)
>>> -DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>>> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter,
>>> + "-netfilter buffer,id=str,netdev=str[,chain=in|out|all,interval=n]\n"
>>> + " buffer netdev in/out packets. if interval provided, will
>>> release\n"
>>> + " packets by interval. interval scale: microsecond\n",
>>> QEMU_ARCH_ALL)
>>
>> If I understand your intent correctly, "interval" is the amount of time
>> to delay packets. What about calling it "delay"?
>
> You can take it as the amount of time to delay packets, but for other usecase
> like MC, you can also take it as an interval to release packets, that is to
> say, to release packets every {interval} time.
Perhaps a less terse explanation would make that easier to understand.
Try to do it for STEXI..ETEXI, and then we can see whether we still want
to make the help text less terse, too.
>>
>> Suggest something like
>>
>> buffer network packets, delaying them for 'delay' microseconds
>> (default 0us)
>>
>>> STEXI
>>> @item -net
>>> nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}]
>>> [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
>>> @findex -net
>>> @@ -1990,6 +1993,19 @@ libpcap, so it can be analyzed with tools
>>> such as tcpdump or Wireshark.
>>> Indicate that no network devices should be configured. It is used to
>>> override the default configuration (@option{-net nic -net user}) which
>>> is activated if no @option{-net} options are provided.
>>> +
>>> +@item -netfilter
>>> buffer,id=@var{id},netdev=@var{netdevid}[,chain=@var{in/out/all}][,interval=@var{n}]
>>
>> 'n' is an unusual choice for a time delay. What about 't', 'dt', or
>> 'delay'?
>
> 't' is better, thanks!
>
>>
>>> +Buffer netdev @var{netdevid} input or output packets.
>>
>> Are there any other kinds of packets?
>
> No, only input/output packets now.
Recommend to say just "network packets" then.
>>> if interval @var{n}
>>> +provided, will release packets by interval. interval scale: microsecond.
>>
>> Please start sentences with a capital letter.
>>
>> Suggest something like
>>
>> Packets are delayed by @var{n} microseconds. Defaults to 0us,
>> i.e. no delay.
>>
>>> +
>>> +chain @var{in/out/all} is an option that can be applied to any
>>> netfilters, if
>>> +not provided, default is @option{all}.
>>
>> "to any netfilter" (singular)
>>
>> Drop "if not provided,"
>
> Ok, thanks!
>
>>
>>> +
>>> +@option{in} means this filter will receive packets sent to the netdev
>>> +
>>> +@option{out} means this filter will receive packets sent from the netdev
>>> +
>>> +@option{all} means this filter will receive packets both sent to/from the netdev
>>> ETEXI
>>>
>>> STEXI
>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>>> index 4f0dc98..9419a6f 100644
>>> --- a/qmp-commands.hx
>>> +++ b/qmp-commands.hx
>>> @@ -947,7 +947,7 @@ Arguments:
>>> Example:
>>>
>>> -> { "execute": "netfilter_add",
>>> - "arguments": { "type": "type", "id": "nf0",
>>> + "arguments": { "type": "buffer", "id": "nf0",
>>> "netdev": "bn",
>>> "chain": "in" } }
>>> <- { "return": {} }
>>
>> Does the example before this patch actually work?
>
> No, there's only type=buffer now.
The commit message of the patch that adds the still non-functional
command should explain that it doesn't yet work, because it's still
merely infrastructure without an actual user, and that you'll add the
first user shortly.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help
2015-08-28 11:42 ` Markus Armbruster
@ 2015-08-31 1:30 ` Yang Hongyang
0 siblings, 0 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-08-31 1:30 UTC (permalink / raw)
To: Markus Armbruster
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
On 08/28/2015 07:42 PM, Markus Armbruster wrote:
> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>
>> On 08/26/2015 11:55 PM, Markus Armbruster wrote:
>>> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>>>
>>>> now that we have a buffer netfilter, update the command
>>>> description and help.
>>>>
>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>> CC: Luiz Capitulino <lcapitulino@redhat.com>
>>>> CC: Markus Armbruster <armbru@redhat.com>
>>>> ---
>>>> v8: add more description for the filter to the TEXI section
>>>> ---
>>>> hmp-commands.hx | 2 +-
>>>> qemu-options.hx | 18 +++++++++++++++++-
>>>> qmp-commands.hx | 2 +-
>>>> 3 files changed, 19 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>>> index 902e2d1..63177a8 100644
>>>> --- a/hmp-commands.hx
>>>> +++ b/hmp-commands.hx
>>>> @@ -1255,7 +1255,7 @@ ETEXI
>>>> {
>>>> .name = "netfilter_add",
>>>> .args_type = "netfilter:O",
>>>> - .params = "[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
>>>> + .params = "[buffer],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
>>>> .help = "add netfilter",
>>>> .mhandler.cmd = hmp_netfilter_add,
>>>> .command_completion = netfilter_add_completion,
>>>
>>> This change looks odd. Actually, params look odd before and after the
>>> change :)
>>>
>>> I suspect you're trying to follow netdev_add precedence. Its params:
>>>
>>> .params =
>>> "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
>>>
>>> Neglects to mention the long form type=, but that's pretty common.
>>>
>>> Uses square brackets both for grouping and for optionalness. We suck.
>>>
>>> Users can probably figure out that the first [ ] is just grouping, and
>>> the others are optionalness.
>>>
>>> Your params are even more confusing, because the first [ ] is again
>>> grouping, but there's just one alternative: buffer.
>>>
>>> What about not simply writing
>>>
>>> .params =
>>> "buffer,id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
>>>
>>> for now?
>>
>> Sure, I agree it is confusing when it has only one alternative now...When we
>> added more filters, we can change it later.
>>
>>>
>>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>>> index 0d52d02..390e055 100644
>>>> --- a/qemu-options.hx
>>>> +++ b/qemu-options.hx
>>>> @@ -1575,7 +1575,10 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>>>> "socket][,vlan=n][,option][,option][,...]\n"
>>>> " old way to initialize a host network interface\n"
>>>> " (use the -netdev option if possible instead)\n",
>>>> QEMU_ARCH_ALL)
>>>> -DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>>>> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter,
>>>> + "-netfilter buffer,id=str,netdev=str[,chain=in|out|all,interval=n]\n"
>>>> + " buffer netdev in/out packets. if interval provided, will
>>>> release\n"
>>>> + " packets by interval. interval scale: microsecond\n",
>>>> QEMU_ARCH_ALL)
>>>
>>> If I understand your intent correctly, "interval" is the amount of time
>>> to delay packets. What about calling it "delay"?
>>
>> You can take it as the amount of time to delay packets, but for other usecase
>> like MC, you can also take it as an interval to release packets, that is to
>> say, to release packets every {interval} time.
>
> Perhaps a less terse explanation would make that easier to understand.
> Try to do it for STEXI..ETEXI, and then we can see whether we still want
> to make the help text less terse, too.
Ok, thanks!
>
>>>
>>> Suggest something like
>>>
>>> buffer network packets, delaying them for 'delay' microseconds
>>> (default 0us)
>>>
>>>> STEXI
>>>> @item -net
>>>> nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}]
>>>> [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
>>>> @findex -net
>>>> @@ -1990,6 +1993,19 @@ libpcap, so it can be analyzed with tools
>>>> such as tcpdump or Wireshark.
>>>> Indicate that no network devices should be configured. It is used to
>>>> override the default configuration (@option{-net nic -net user}) which
>>>> is activated if no @option{-net} options are provided.
>>>> +
>>>> +@item -netfilter
>>>> buffer,id=@var{id},netdev=@var{netdevid}[,chain=@var{in/out/all}][,interval=@var{n}]
>>>
>>> 'n' is an unusual choice for a time delay. What about 't', 'dt', or
>>> 'delay'?
>>
>> 't' is better, thanks!
>>
>>>
>>>> +Buffer netdev @var{netdevid} input or output packets.
>>>
>>> Are there any other kinds of packets?
>>
>> No, only input/output packets now.
>
> Recommend to say just "network packets" then.
Ok, thanks.
>
>>>> if interval @var{n}
>>>> +provided, will release packets by interval. interval scale: microsecond.
>>>
>>> Please start sentences with a capital letter.
>>>
>>> Suggest something like
>>>
>>> Packets are delayed by @var{n} microseconds. Defaults to 0us,
>>> i.e. no delay.
>>>
>>>> +
>>>> +chain @var{in/out/all} is an option that can be applied to any
>>>> netfilters, if
>>>> +not provided, default is @option{all}.
>>>
>>> "to any netfilter" (singular)
>>>
>>> Drop "if not provided,"
>>
>> Ok, thanks!
>>
>>>
>>>> +
>>>> +@option{in} means this filter will receive packets sent to the netdev
>>>> +
>>>> +@option{out} means this filter will receive packets sent from the netdev
>>>> +
>>>> +@option{all} means this filter will receive packets both sent to/from the netdev
>>>> ETEXI
>>>>
>>>> STEXI
>>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>>>> index 4f0dc98..9419a6f 100644
>>>> --- a/qmp-commands.hx
>>>> +++ b/qmp-commands.hx
>>>> @@ -947,7 +947,7 @@ Arguments:
>>>> Example:
>>>>
>>>> -> { "execute": "netfilter_add",
>>>> - "arguments": { "type": "type", "id": "nf0",
>>>> + "arguments": { "type": "buffer", "id": "nf0",
>>>> "netdev": "bn",
>>>> "chain": "in" } }
>>>> <- { "return": {} }
>>>
>>> Does the example before this patch actually work?
>>
>> No, there's only type=buffer now.
>
> The commit message of the patch that adds the still non-functional
> command should explain that it doesn't yet work, because it's still
> merely infrastructure without an actual user, and that you'll add the
> first user shortly.
Will do, thanks!
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter
2015-08-28 11:29 ` Markus Armbruster
@ 2015-08-31 1:31 ` Yang Hongyang
0 siblings, 0 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-08-31 1:31 UTC (permalink / raw)
To: Markus Armbruster
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, stefanha, Paolo Bonzini
On 08/28/2015 07:29 PM, Markus Armbruster wrote:
> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>
>> On 08/26/2015 10:04 PM, Markus Armbruster wrote:
>>> Missed a bunch of revisions of this series, please excuse gaps in my
>>> understanding.
>>
>> Thank you for the review.
>>
>>>
>>> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>>>
>>>> Add the framework for a new netfilter object and a new
>>>> -netfilter CLI option as a basis for the following patches.
>>>>
>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>> CC: Paolo Bonzini <pbonzini@redhat.com>
>>>> CC: Eric Blake <eblake@redhat.com>
>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>> include/net/filter.h | 15 +++++++++++++++
>>>> include/sysemu/sysemu.h | 1 +
>>>> net/Makefile.objs | 1 +
>>>> net/filter.c | 27 +++++++++++++++++++++++++++
>>>> qemu-options.hx | 1 +
>>>> vl.c | 13 +++++++++++++
>>>> 6 files changed, 58 insertions(+)
>>>> create mode 100644 include/net/filter.h
>>>> create mode 100644 net/filter.c
>>>>
>>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>>> new file mode 100644
>>>> index 0000000..4242ded
>>>> --- /dev/null
>>>> +++ b/include/net/filter.h
>>>> @@ -0,0 +1,15 @@
>>>> +/*
>>>> + * Copyright (c) 2015 FUJITSU LIMITED
>>>> + *
>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>>>> + * later. See the COPYING file in the top-level directory.
>>>> + */
>>>> +
>>>> +#ifndef QEMU_NET_FILTER_H
>>>> +#define QEMU_NET_FILTER_H
>>>> +
>>>> +#include "qemu-common.h"
>>>> +
>>>> +int net_init_filters(void);
>>>> +
>>>> +#endif /* QEMU_NET_FILTER_H */
>>>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>>>> index 44570d1..15d6d00 100644
>>>> --- a/include/sysemu/sysemu.h
>>>> +++ b/include/sysemu/sysemu.h
>>>> @@ -212,6 +212,7 @@ extern QemuOptsList qemu_chardev_opts;
>>>> extern QemuOptsList qemu_device_opts;
>>>> extern QemuOptsList qemu_netdev_opts;
>>>> extern QemuOptsList qemu_net_opts;
>>>> +extern QemuOptsList qemu_netfilter_opts;
>>>> extern QemuOptsList qemu_global_opts;
>>>> extern QemuOptsList qemu_mon_opts;
>>>>
>>>> diff --git a/net/Makefile.objs b/net/Makefile.objs
>>>> index ec19cb3..914aec0 100644
>>>> --- a/net/Makefile.objs
>>>> +++ b/net/Makefile.objs
>>>> @@ -13,3 +13,4 @@ common-obj-$(CONFIG_HAIKU) += tap-haiku.o
>>>> common-obj-$(CONFIG_SLIRP) += slirp.o
>>>> common-obj-$(CONFIG_VDE) += vde.o
>>>> common-obj-$(CONFIG_NETMAP) += netmap.o
>>>> +common-obj-y += filter.o
>>>> diff --git a/net/filter.c b/net/filter.c
>>>> new file mode 100644
>>>> index 0000000..4e40f08
>>>> --- /dev/null
>>>> +++ b/net/filter.c
>>>> @@ -0,0 +1,27 @@
>>>> +/*
>>>> + * Copyright (c) 2015 FUJITSU LIMITED
>>>> + *
>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>>>> + * later. See the COPYING file in the top-level directory.
>>>> + */
>>>> +
>>>> +#include "qemu-common.h"
>>>> +#include "net/filter.h"
>>>> +
>>>> +int net_init_filters(void)
>>>> +{
>>>> + return 0;
>>>> +}
>>>> +
>>>> +QemuOptsList qemu_netfilter_opts = {
>>>> + .name = "netfilter",
>>>> + .implied_opt_name = "type",
>>>> + .head = QTAILQ_HEAD_INITIALIZER(qemu_netfilter_opts.head),
>>>> + .desc = {
>>>> + /*
>>>> + * no elements => accept any params
>>>> + * validation will happen later
>>>> + */
>>>> + { /* end of list */ }
>>>> + },
>>>> +};
>>>
>>> Ignorant question: why dynamic validation (empty .desc) instead of
>>> statically defined parameters? The documentation you add in PATCH 10
>>> suggests they're not actually dynamic.
>>
>> Actually I was following the netdev stuff. There might be bunch of filters,
>> and I don't know the params of each filter until it is realized.
>
> I realized that later on in your series.
>
>>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>>> index 77f5853..0d52d02 100644
>>>> --- a/qemu-options.hx
>>>> +++ b/qemu-options.hx
>>>> @@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>>>> "socket][,vlan=n][,option][,option][,...]\n"
>>>> " old way to initialize a host network interface\n"
>>>> " (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
>>>> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>>>> STEXI
>>>> @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
>>>> @findex -net
>>>
>>> Wrong spot: this is between DEF(net, ...) and its STEXI..ETEXI stanza.
>>> Suggest to move it behind the ETEXI.
>>>
>>> Missing help string. You add the help in PATCH 10. What about adding
>>> it here already? Would serve as a hint of the things to come later in
>>> your series.
>>>
>>> Missing STEXI..ETEXI stanza for the user manual.
>>
>> If I understand correctly, you are suggesting separate the netfilter from
>> net, seems more reasonable, so I should add something like:
>> DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>> STEXI..ETEXI
>>
>> after the DEF(net, ...) and its STEXI..ETEXI stanza, am I right?
>
> Yes. Always keep DEF(whatever, ...) and its STEXI..ETEXI together.
>
> Inserting netfilter after net seems the most logical place.
Yes, thank you.
>
> [...]
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands
2015-08-28 11:37 ` Markus Armbruster
@ 2015-08-31 1:36 ` Yang Hongyang
2015-08-31 7:08 ` Markus Armbruster
0 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-31 1:36 UTC (permalink / raw)
To: Markus Armbruster, Eric Blake
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
On 08/28/2015 07:37 PM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> On 08/26/2015 09:17 AM, Markus Armbruster wrote:
>>> Only reviewing QAPI/QMP and HMP interface parts for now.
>>>
>>> I apologize for not having reviewed this series earlier. v8 is awfully
>>> late for the kind of review comments I have.
>>
>> And I've also been behind the curve, intending to review this since it
>> touches API, but not getting there yet.
>>
>>
>>>> +##
>>>> +{ 'command': 'netfilter_add',
>>>> + 'data': {
>>>> + 'type': 'str',
>>>> + 'id': 'str',
>>>> + 'netdev': 'str',
>>>> + '*chain': 'str',
>>>> + '*props': '**'}, 'gen': false }
>>>
>>> I figure you're merely following netdev_add precedence here (can't fault
>>> you for that), but netdev_add cheats, and we shouldn't add more cheats.
>>>
>>> First, 'gen': false is best avoided. It suppresses the generated
>>> marshaller, and that lets you cheat. There are cases where we can't do
>>> without, but I don't think this is one.
>>>
>>> When we suppress the generated marshaller, 'data' is at best a
>>> declaration of intent; the code can do something else entirely. For
>>> instance, netdev_add declares
>>>
>>> { 'command': 'netdev_add',
>>> 'data': {'type': 'str', 'id': 'str', '*props': '**'},
>>> 'gen': false }
>>>
>>> but the '*props' part is a bald-faced lie: it doesn't take a 'props'
>>> argument. See
>>> http://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00460.html
>>> and maybe even slides 37-38 of
>>> https://events.linuxfoundation.org/sites/events/files/slides/armbru-qemu-introspection.pdf
>>>
>>> I didn't check your code, but I suspect yours is a lie, too.
>>>
>>> I intend to clean up netdev_add, hopefully soon.
>>>
>>> You should use a proper QAPI data type here. I guess the flat union I
>>> sketched in my reply to PATCH 2 would do nicely, except we don't support
>>> commands with union type data, yet. I expect to add support to clean up
>>> netdev_del.
>>
>> In fact, I've already proposed adding such support:
>>
>> http://thread.gmane.org/gmane.comp.emulators.qemu/356265/focus=356291
>
> In my review queue. Which has become sickeningly long again...
>
>>>
>>> If you don't want to wait for that (understandable!), then I suggest you
>>> keep 'NetFilter' a struct type for now, use it as command data here, and
>>> we convert it to a flat union later. Must be done before the release,
>>> to avoid backward incompatibility.
>>>
>>> Then this becomes something like:
>>>
>>> { 'command': 'netfilter-add', 'data': 'NetFilter' }
>>
>> or use NetFilter as a union, but have the command be:
>>
>> { 'command': 'netfilter-add',
>> 'data': { 'data': 'NetFilter' } }
>>
>> where you have to pass an extra layer of nesting at the QMP layer.
>>
>>>
>>> If you need the command to take arguments you don't want to put into
>>> NetFilter for some reason, I can help you achieve that cleanly.
>>
>> In fact, having the 'NetFilter' union be a single argument of a larger
>> struct makes that larger struct the nice place to stick any additional
>> arguments that don't need to be part of the union.
>
> To make progress, I suggest you try the following:
>
> 1. Make NetFilter a flat union, as I suggested in my review of PATCH 2.
>
> 2. Use Eric's idea above, because it avoids the dependency on code
> that's still under review.
>
> Drawback: extra layer of nesting. Ugly, but not the end of the world,
> and we still have a chance to peel it off before the next release.
Thanks for the explanation, I will try to see if I can fully understand
your point.
>
> [...]
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter
2015-08-27 3:15 ` Jason Wang
@ 2015-08-31 1:43 ` Yang Hongyang
0 siblings, 0 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-08-31 1:43 UTC (permalink / raw)
To: Jason Wang, qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, mrhines, stefanha
On 08/27/2015 11:15 AM, Jason Wang wrote:
>
>
> On 08/26/2015 05:59 PM, Yang Hongyang wrote:
>> This patch add a new object netfilter, capture all network packets.
>> Also implement a netbuffer based on this object.
>> the "buffer" netfilter could be used by VM FT solutions like
>> MicroCheckpointing, to buffer/release packets. Or to simulate
>> packet delay.
>>
>> You can also get the series from:
>> https://github.com/macrosheep/qemu/tree/netfilter-v8
>>
>> Usage:
>> -netdev tap,id=bn0
>> -netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
>> -device e1000,netdev=bn0
>>
>> dynamically add/remove netfilters:
>> netfilter_add buffer,id=f0,netdev=bn0,chain=in,interval=1000
>> netfilter_del f0
>>
>> NOTE:
>> interval's scale is microsecond.
>> chain is optional, and is one of in|out|all, 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
>>
>> TODO:
>> - dump
>>
>> v8:
>> - some minor fixes according to Thomas's comments
>> - rebased to the latest master branch
>>
>> v7:
>> - print filter info when execute 'info network'
>> - addressed Jason's comments
>>
>> v6:
>> - add multiqueue support, please see individual patch for detail
>>
>> v5:
>> - add a sent_cb param to filter receive_iov api
>> - squash the 4th patch into patch 3
>> - remove dummy sent_cb (buffer filter)
>> - addressed Jason's other comments, see individual patches for detail
>>
>> v4:
>> - get rid of struct Filter
>> - squash the 4th patch into patch 2
>> - fix qemu_netfilter_pass_to_next_iov
>> - get rid of bh (buffer filter)
>> - release the packet to next filter instead of to receiver (buffer filter)
>>
>> v3:
>> - add an api to pass the packet to next filter
>> - remove netfilters when delete netdev
>> - add qtest testcases for netfilter
>> - addressed comments from Jason
>>
>> v2:
>> - add a chain option to netfilter object
>> - move the hook place earlier, before net_queue_send
>> - drop the unused api in buffer filter
>> - squash buffer filter patches into one
>> - remove receive() api from netfilter, only receive_iov() is enough
>> - addressed comments from Jason&Thomas
>>
>> v1:
>> initial patch.
>>
>> Yang Hongyang (11):
>> net: add a new object netfilter
>> init/cleanup of netfilter object
>> netfilter: add netfilter_{add|del} commands
>> netfilter: hook packets before net queue send
>> move out net queue structs define
>> netfilter: add an API to pass the packet to next filter
>> netfilter: print filter info associate with the netdev
>> net/queue: export qemu_net_queue_append_iov
>> netfilter: add a netbuffer filter
>> filter/buffer: update command description and help
>> tests: add test cases for netfilter object
>>
>> hmp-commands.hx | 30 +++++
>> hmp.c | 29 +++++
>> hmp.h | 4 +
>> include/net/filter.h | 64 ++++++++++
>> include/net/net.h | 1 +
>> include/net/queue.h | 26 ++++
>> include/qemu/typedefs.h | 1 +
>> include/sysemu/sysemu.h | 1 +
>> monitor.c | 33 +++++
>> net/Makefile.objs | 2 +
>> net/filter-buffer.c | 125 ++++++++++++++++++
>> net/filter.c | 332 ++++++++++++++++++++++++++++++++++++++++++++++++
>> net/filters.h | 17 +++
>> net/net.c | 85 +++++++++++++
>> net/queue.c | 31 +----
>> qapi-schema.json | 100 +++++++++++++++
>> qemu-options.hx | 17 +++
>> qmp-commands.hx | 57 +++++++++
>> tests/.gitignore | 1 +
>> tests/Makefile | 2 +
>> tests/test-netfilter.c | 194 ++++++++++++++++++++++++++++
>> vl.c | 13 ++
>> 22 files changed, 1140 insertions(+), 25 deletions(-)
>> create mode 100644 include/net/filter.h
>> create mode 100644 net/filter-buffer.c
>> create mode 100644 net/filter.c
>> create mode 100644 net/filters.h
>> create mode 100644 tests/test-netfilter.c
>>
>
> Looks good to me. After addressing comments of interfaces, I think it
> was pretty ready to be merged.
Thank you, I will address them asap.
>
> Thanks
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands
2015-08-31 1:36 ` Yang Hongyang
@ 2015-08-31 7:08 ` Markus Armbruster
2015-08-31 9:01 ` Yang Hongyang
0 siblings, 1 reply; 42+ messages in thread
From: Markus Armbruster @ 2015-08-31 7:08 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
Yang Hongyang <yanghy@cn.fujitsu.com> writes:
> On 08/28/2015 07:37 PM, Markus Armbruster wrote:
[...]
>> To make progress, I suggest you try the following:
>>
>> 1. Make NetFilter a flat union, as I suggested in my review of PATCH 2.
>>
>> 2. Use Eric's idea above, because it avoids the dependency on code
>> that's still under review.
>>
>> Drawback: extra layer of nesting. Ugly, but not the end of the world,
>> and we still have a chance to peel it off before the next release.
>
> Thanks for the explanation, I will try to see if I can fully understand
> your point.
If you have questions on the QAPI part, Eric and I will be happy to
answer them.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands
2015-08-31 7:08 ` Markus Armbruster
@ 2015-08-31 9:01 ` Yang Hongyang
2015-08-31 14:53 ` Eric Blake
0 siblings, 1 reply; 42+ messages in thread
From: Yang Hongyang @ 2015-08-31 9:01 UTC (permalink / raw)
To: Markus Armbruster
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
On 08/31/2015 03:08 PM, Markus Armbruster wrote:
> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>
>> On 08/28/2015 07:37 PM, Markus Armbruster wrote:
> [...]
>>> To make progress, I suggest you try the following:
>>>
>>> 1. Make NetFilter a flat union, as I suggested in my review of PATCH 2.
>>>
>>> 2. Use Eric's idea above, because it avoids the dependency on code
>>> that's still under review.
>>>
>>> Drawback: extra layer of nesting. Ugly, but not the end of the world,
>>> and we still have a chance to peel it off before the next release.
>>
>> Thanks for the explanation, I will try to see if I can fully understand
>> your point.
>
> If you have questions on the QAPI part, Eric and I will be happy to
> answer them.
Thanks a lot for the help!
Sorry that I don't know much about the QAPI part, I have a question, in
previous reply, Eric suggested:
>
> Then this becomes something like:
>
> { 'command': 'netfilter-add', 'data': 'NetFilter' }
or use NetFilter as a union, but have the command be:
{ 'command': 'netfilter-add',
'data': { 'data': 'NetFilter' } }
where you have to pass an extra layer of nesting at the QMP layer.
What do you mean by pass an extra layer of nesting?
I've already switched to flat union as you suggested:
{ 'struct': 'NetFilterDummyOptions',
'data': { } }
{ 'enum': 'NetFilterType',
'data': ['dummy'] }
{ 'struct': 'NetFilterBase',
'data': {
'id': 'str',
'netdev': 'str',
'*chain': 'str',
'type': 'NetFilterType' } }
{ 'union': 'NetFilter',
'base': 'NetFilterBase',
'discriminator': 'type',
'data': {
'dummy': 'NetFilterDummyOptions' } }
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands
2015-08-31 9:01 ` Yang Hongyang
@ 2015-08-31 14:53 ` Eric Blake
2015-09-01 1:24 ` Yang Hongyang
0 siblings, 1 reply; 42+ messages in thread
From: Eric Blake @ 2015-08-31 14:53 UTC (permalink / raw)
To: Yang Hongyang, Markus Armbruster
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]
On 08/31/2015 03:01 AM, Yang Hongyang wrote:
>
> Sorry that I don't know much about the QAPI part, I have a question, in
> previous reply, Eric suggested:
> >
> > Then this becomes something like:
> >
> > { 'command': 'netfilter-add', 'data': 'NetFilter' }
>
If we do this (which requires pending patches to be flushed), then the
user specifies the following QMP:
{ "execute":"netfilter-add",
"arguments":{ "id":"abc", "netdev":"def", "type":"dummy" }}
> or use NetFilter as a union, but have the command be:
>
> { 'command': 'netfilter-add',
> 'data': { 'data': 'NetFilter' } }
This approach would work right now without waiting for pending qapi
commits, but the QMP command would look like:
{ "execute":"netfilter-add",
"arguments":{ "data":{ "id":"abc", "netdev":"def", "type":"dummy" }}}
>
> where you have to pass an extra layer of nesting at the QMP layer.
>
> What do you mean by pass an extra layer of nesting?
The fact that I had to pass "arguments":{"data":{...}}, thereby nesting
the real options inside another relatively pointless data wrapper.
>
> I've already switched to flat union as you suggested:
>
> { 'struct': 'NetFilterDummyOptions',
> 'data': { } }
>
> { 'enum': 'NetFilterType',
> 'data': ['dummy'] }
>
> { 'struct': 'NetFilterBase',
> 'data': {
> 'id': 'str',
> 'netdev': 'str',
> '*chain': 'str',
> 'type': 'NetFilterType' } }
>
> { 'union': 'NetFilter',
> 'base': 'NetFilterBase',
> 'discriminator': 'type',
> 'data': {
> 'dummy': 'NetFilterDummyOptions' } }
Looks reasonable for a first round. Some of the pending qapi commits
may allow us to further simplify things to not be quite so verbose, but
that doesn't stop us from using this now.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands
2015-08-31 14:53 ` Eric Blake
@ 2015-09-01 1:24 ` Yang Hongyang
0 siblings, 0 replies; 42+ messages in thread
From: Yang Hongyang @ 2015-09-01 1:24 UTC (permalink / raw)
To: Eric Blake, Markus Armbruster
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Luiz Capitulino, stefanha
On 08/31/2015 10:53 PM, Eric Blake wrote:
> On 08/31/2015 03:01 AM, Yang Hongyang wrote:
>
>>
>> Sorry that I don't know much about the QAPI part, I have a question, in
>> previous reply, Eric suggested:
>> >
>> > Then this becomes something like:
>> >
>> > { 'command': 'netfilter-add', 'data': 'NetFilter' }
>>
>
> If we do this (which requires pending patches to be flushed), then the
> user specifies the following QMP:
>
> { "execute":"netfilter-add",
> "arguments":{ "id":"abc", "netdev":"def", "type":"dummy" }}
>
>> or use NetFilter as a union, but have the command be:
>>
>> { 'command': 'netfilter-add',
>> 'data': { 'data': 'NetFilter' } }
>
> This approach would work right now without waiting for pending qapi
> commits, but the QMP command would look like:
>
> { "execute":"netfilter-add",
> "arguments":{ "data":{ "id":"abc", "netdev":"def", "type":"dummy" }}}
>
>>
>> where you have to pass an extra layer of nesting at the QMP layer.
>>
>> What do you mean by pass an extra layer of nesting?
>
> The fact that I had to pass "arguments":{"data":{...}}, thereby nesting
> the real options inside another relatively pointless data wrapper.
Now I understand, with the flat union, all I need to do now is to specify the
command schema like:
{ 'command': 'netfilter-add',
'data': { 'data': 'NetFilter' } }
and use the qmp command like you noted above.
Thanks a lot Eric!
>
>>
>> I've already switched to flat union as you suggested:
>>
>> { 'struct': 'NetFilterDummyOptions',
>> 'data': { } }
>>
>> { 'enum': 'NetFilterType',
>> 'data': ['dummy'] }
>>
>> { 'struct': 'NetFilterBase',
>> 'data': {
>> 'id': 'str',
>> 'netdev': 'str',
>> '*chain': 'str',
>> 'type': 'NetFilterType' } }
>>
>> { 'union': 'NetFilter',
>> 'base': 'NetFilterBase',
>> 'discriminator': 'type',
>> 'data': {
>> 'dummy': 'NetFilterDummyOptions' } }
>
> Looks reasonable for a first round. Some of the pending qapi commits
> may allow us to further simplify things to not be quite so verbose, but
> that doesn't stop us from using this now.
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2015-09-01 1:25 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-26 9:59 [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter Yang Hongyang
2015-08-26 14:04 ` Markus Armbruster
2015-08-27 2:34 ` Yang Hongyang
2015-08-28 11:29 ` Markus Armbruster
2015-08-31 1:31 ` Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object Yang Hongyang
2015-08-26 13:13 ` Thomas Huth
2015-08-26 14:41 ` Markus Armbruster
2015-08-26 15:31 ` Eric Blake
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands Yang Hongyang
2015-08-26 15:17 ` Markus Armbruster
2015-08-26 15:37 ` Eric Blake
2015-08-28 11:37 ` Markus Armbruster
2015-08-31 1:36 ` Yang Hongyang
2015-08-31 7:08 ` Markus Armbruster
2015-08-31 9:01 ` Yang Hongyang
2015-08-31 14:53 ` Eric Blake
2015-09-01 1:24 ` Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 04/11] netfilter: hook packets before net queue send Yang Hongyang
2015-08-27 14:35 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 05/11] move out net queue structs define Yang Hongyang
2015-08-27 14:38 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 06/11] netfilter: add an API to pass the packet to next filter Yang Hongyang
2015-08-27 15:11 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 07/11] netfilter: print filter info associate with the netdev Yang Hongyang
2015-08-27 14:46 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 08/11] net/queue: export qemu_net_queue_append_iov Yang Hongyang
2015-08-27 15:05 ` Thomas Huth
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 09/11] netfilter: add a netbuffer filter Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help Yang Hongyang
2015-08-26 15:55 ` Markus Armbruster
2015-08-27 2:42 ` Yang Hongyang
2015-08-28 11:42 ` Markus Armbruster
2015-08-31 1:30 ` Yang Hongyang
2015-08-26 9:59 ` [Qemu-devel] [PATCH v8 11/11] tests: add test cases for netfilter object Yang Hongyang
2015-08-26 15:58 ` [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter Markus Armbruster
2015-08-27 2:25 ` Yang Hongyang
2015-08-27 1:05 ` Thomas Huth
2015-08-27 2:24 ` Yang Hongyang
2015-08-27 3:15 ` Jason Wang
2015-08-31 1:43 ` Yang Hongyang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).