From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59014) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuGiJ-0001zS-Nd for qemu-devel@nongnu.org; Thu, 05 Nov 2015 04:20:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZuGiE-0003qs-UD for qemu-devel@nongnu.org; Thu, 05 Nov 2015 04:20:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46537) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuGiE-0003pv-KN for qemu-devel@nongnu.org; Thu, 05 Nov 2015 04:20:06 -0500 References: <1446551816-15768-1-git-send-email-zhang.zhanghailiang@huawei.com> <1446551816-15768-36-git-send-email-zhang.zhanghailiang@huawei.com> <563973C9.8000607@redhat.com> <563B08A0.7030802@huawei.com> From: Jason Wang Message-ID: <563B1F3C.7050903@redhat.com> Date: Thu, 5 Nov 2015 17:19:56 +0800 MIME-Version: 1.0 In-Reply-To: <563B08A0.7030802@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v10 35/38] netfilter: Introduce a API to automatically add filter-buffer for each netdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: zhanghailiang , qemu-devel@nongnu.org Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, dgilbert@redhat.com, arei.gonglei@huawei.com, stefanha@redhat.com, amit.shah@redhat.com On 11/05/2015 03:43 PM, zhanghailiang wrote: > Hi Jason, > > On 2015/11/4 10:56, Jason Wang wrote: >> >> >> On 11/03/2015 07:56 PM, zhanghailiang wrote: >>> Signed-off-by: zhanghailiang >>> Cc: Jason Wang >> >> Commit log please. >> >>> --- >>> v10: new patch >>> --- >>> include/net/filter.h | 1 + >>> include/net/net.h | 3 ++ >>> net/filter-buffer.c | 84 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> net/net.c | 20 +++++++++++++ >>> 4 files changed, 108 insertions(+) >>> >>> diff --git a/include/net/filter.h b/include/net/filter.h >>> index 4499d60..b0954ba 100644 >>> --- a/include/net/filter.h >>> +++ b/include/net/filter.h >>> @@ -75,5 +75,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState >>> *sender, >>> void *opaque); >>> void filter_buffer_release_all(void); >>> void filter_buffer_del_all_timers(void); >>> +void qemu_auto_add_filter_buffer(NetFilterDirection direction, >>> Error **errp); >>> >>> #endif /* QEMU_NET_FILTER_H */ >>> diff --git a/include/net/net.h b/include/net/net.h >>> index 5c65c45..e32bd90 100644 >>> --- a/include/net/net.h >>> +++ b/include/net/net.h >>> @@ -129,6 +129,9 @@ typedef void >>> (*qemu_netfilter_foreach)(NetFilterState *nf, void *opaque, >>> Error **errp); >>> void qemu_foreach_netfilter(qemu_netfilter_foreach func, void >>> *opaque, >>> Error **errp); >>> +typedef void (*qemu_netdev_foreach)(NetClientState *nc, void *opaque, >>> + Error **errp); >>> +void qemu_foreach_netdev(qemu_netdev_foreach func, void *opaque, >>> Error **errp); >>> int qemu_can_send_packet(NetClientState *nc); >>> ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec >>> *iov, >>> int iovcnt); >>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c >>> index 05313de..0dc1efb 100644 >>> --- a/net/filter-buffer.c >>> +++ b/net/filter-buffer.c >>> @@ -15,6 +15,11 @@ >>> #include "qapi-visit.h" >>> #include "qom/object.h" >>> #include "net/net.h" >>> +#include "qapi/qmp/qdict.h" >>> +#include "qapi/qmp-output-visitor.h" >>> +#include "qapi/qmp-input-visitor.h" >>> +#include "monitor/monitor.h" >>> + >>> >>> #define TYPE_FILTER_BUFFER "filter-buffer" >>> >>> @@ -185,6 +190,85 @@ void filter_buffer_del_all_timers(void) >>> qemu_foreach_netfilter(filter_buffer_del_timer, NULL, NULL); >>> } >>> >>> +static void netdev_add_filter_buffer(NetClientState *nc, void *opaque, >>> + Error **errp) >>> +{ >>> + NetFilterState *nf; >>> + bool found = false; >>> + >>> + QTAILQ_FOREACH(nf, &nc->filters, next) { >>> + if (!strcmp(object_get_typename(OBJECT(nf)), >>> TYPE_FILTER_BUFFER)) { >>> + found = true; >>> + break; >>> + } >>> + } >>> + >>> + if (!found) { >>> + QmpOutputVisitor *qov; >>> + QmpInputVisitor *qiv; >>> + Visitor *ov, *iv; >>> + QObject *obj = NULL; >>> + QDict *qdict; >>> + void *dummy = NULL; >>> + char *id = g_strdup_printf("%s-%s.0", nc->name, >>> TYPE_FILTER_BUFFER); >>> + char *queue = (char *) opaque; >>> + bool auto_add = true; >>> + Error *err = NULL; >>> + >>> + qov = qmp_output_visitor_new(); >>> + ov = qmp_output_get_visitor(qov); >>> + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); >>> + if (err) { >>> + goto out; >>> + } >>> + visit_type_str(ov, &nc->name, "netdev", &err); >>> + if (err) { >>> + goto out; >>> + } >>> + visit_type_str(ov, &queue, "queue", &err); >>> + if (err) { >>> + goto out; >>> + } >>> + visit_type_bool(ov, &auto_add, "auto", &err); >>> + if (err) { >>> + goto out; >>> + } >>> + visit_end_struct(ov, &err); >>> + if (err) { >>> + goto out; >>> + } >>> + obj = qmp_output_get_qobject(qov); >>> + g_assert(obj != NULL); >>> + qdict = qobject_to_qdict(obj); >>> + qmp_output_visitor_cleanup(qov); >>> + >>> + qiv = qmp_input_visitor_new(obj); >>> + iv = qmp_input_get_visitor(qiv); >>> + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); >>> + qmp_input_visitor_cleanup(qiv); >>> + qobject_decref(obj); >>> +out: >>> + g_free(id); >>> + if (err) { >>> + error_propagate(errp, err); >>> + } >>> + } >>> +} >>> +/* >>> +* This will be used by COLO or MC FT, for which they will need >>> +* to buffer all the packets of all VM's net devices, Here we check >>> +* and automatically add netfilter for netdev that doesn't attach >>> any buffer >>> +* netfilter. >>> +*/ >>> +void qemu_auto_add_filter_buffer(NetFilterDirection direction, >>> Error **errp) >>> +{ >>> + char *queue = g_strdup(NetFilterDirection_lookup[direction]); >>> + >>> + qemu_foreach_netdev(netdev_add_filter_buffer, queue, >>> + errp); >>> + g_free(queue); >>> +} >>> + >> >> This make me think for following questions: >> >> - What if a nic is hot added after this "automatically" filter add? >> - Maybe a better way is to have a default filter? It could be specified >> through qemu cli or other (And default filter could be 'nop' which means >> no filter) ? >> > > I have thought about this. I'd like to add this default buffer filter > quietly, > not through qemu cli. In this way, we can still keep the buffer filter > that configured by users, Actually, this does not break the ones that added by user. We support attach more than one filters to be attached to a single netdev. If I understand the case correctly (I was only partially cced in this series). Before each synchronization, you need: 1) add a buffer filter to each netdev 2) release all buffers on demand 3) delete all buffer filters You can just remove step 1 if you know all device has a default buffer filter. And step 3 could be also removed if you can let buffer filter won't buffer any packet through a new command or other. > and keep its delay release packets capability. Though the delay time > is not what > users suppose. (This is only happened in COLO's periodic mode, in > normal colo mode, the delay time > is almost same with user's configure.) This is not good unless you want to limit the buffer filter only for COLO. And I want also know the role of management: technically it can do all the above 3 steps ( And looks like management was a better place to do this). Thanks > > What about call netdev_add_filter_buffer() in each netdev's init() ? > I didn't found a common code path for every netdev in their init path. > > Thanks, > zhanghailiang > >>> static void filter_buffer_init(Object *obj) >>> { >>> object_property_add(obj, "interval", "int", >>> diff --git a/net/net.c b/net/net.c >>> index a333b01..4fbe0af 100644 >>> --- a/net/net.c >>> +++ b/net/net.c >>> @@ -283,6 +283,26 @@ void >>> qemu_foreach_netfilter(qemu_netfilter_foreach func, void *opaque, >>> } >>> } >>> >>> +void qemu_foreach_netdev(qemu_netdev_foreach func, void *opaque, >>> Error **errp) >>> +{ >>> + NetClientState *nc; >>> + >>> + QTAILQ_FOREACH(nc, &net_clients, next) { >>> + if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) { >>> + continue; >>> + } >>> + if (func) { >>> + Error *local_err = NULL; >>> + >>> + func(nc, opaque, &local_err); >>> + if (local_err) { >>> + error_propagate(errp, local_err); >>> + return; >>> + } >>> + } >>> + } >>> +} >>> + >>> static void qemu_net_client_destructor(NetClientState *nc) >>> { >>> g_free(nc); >> >> >> . >> > > >