Re: [Qemu-devel] [PATCH COLO-Frame v11 34/39] net/filter-buffer: Add default filter-buffer for each netdev

[Hailiang Zhang <zhang.zhanghailiang@huawei.com>]

On 2015/12/3 15:21, Yang Hongyang wrote:
>
>
> On 2015年12月03日 14:48, Hailiang Zhang wrote:
>> On 2015/12/3 14:25, Wen Congyang wrote:
>>> On 12/03/2015 11:53 AM, Hailiang Zhang wrote:
>>>> On 2015/12/3 9:17, Wen Congyang wrote:
>>>>> On 11/24/2015 05:25 PM, zhanghailiang wrote:
>>>>>> We add each netdev a default filter-buffer, which will be used for
>>>>>> COLO
>>>>>> or Micro-checkpoint to buffer VM's packets. The name of default
>>>>>> filter-buffer
>>>>>> is 'nop'.
>>>>>> For the default filter-buffer, it will not buffer any packets in
>>>>>> default.
>>>>>> So it has no side effect for the netdev.
>>>>>
>>>>> No, filter-buffer doesn't support vhost, so if you add default
>>>>> filter-buffer
>>>>> for each netdev, you can't use vhost.
>>>>>
>>>>
>>>> Have you tested it ? Did the default filter-buffer break vhost ?
>>>> It's not supposed to break vhost, I will look into it. Thanks.
>>>
>>> Yes, I have tested it. When I want to start a normal vm with vhost, I get
>>> the following error messages:
>>>
>>> qemu-system-x86_64: -netdev tap,id=hn0,queues=1,vhost=on: Vhost is not
>>> supported
>>>
>>
>> Hmm, that is reported by filter in nextfilter_complete(), i will
>> investigate it.
>> We hope the default buffer filter has no side effect on netdev.
>
> I think you'd better add an option to netdev to turn on/off default
> filter, that will solve the problem.
>

OK, i will try to fix it like that. Thanks.

>>
>> Thanks,
>> Hailiang
>>
>>>>
>>>>> Thanks
>>>>> Wen Congyang
>>>>>
>>>>>>
>>>>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>>>>> Cc: Jason Wang <jasowang@redhat.com>
>>>>>> Cc: Yang Hongyang <hongyang.yang@easystack.cn>
>>>>>> ---
>>>>>> v11:
>>>>>> - New patch
>>>>>> ---
>>>>>>    include/net/filter.h |  3 +++
>>>>>>    net/filter-buffer.c  | 74
>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>    net/net.c            |  8 ++++++
>>>>>>    3 files changed, 85 insertions(+)
>>>>>>
>>>>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>>>>> index 2deda36..01a7e90 100644
>>>>>> --- a/include/net/filter.h
>>>>>> +++ b/include/net/filter.h
>>>>>> @@ -74,4 +74,7 @@ ssize_t
>>>>>> qemu_netfilter_pass_to_next(NetClientState *sender,
>>>>>>                                        int iovcnt,
>>>>>>                                        void *opaque);
>>>>>>
>>>>>> +void netdev_add_default_filter_buffer(const char *netdev_id,
>>>>>> +                                      NetFilterDirection direction,
>>>>>> +                                      Error **errp);
>>>>>>    #endif /* QEMU_NET_FILTER_H */
>>>>>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
>>>>>> index 57be149..195af68 100644
>>>>>> --- a/net/filter-buffer.c
>>>>>> +++ b/net/filter-buffer.c
>>>>>> @@ -14,6 +14,12 @@
>>>>>>    #include "qapi/qmp/qerror.h"
>>>>>>    #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"
>>>>>> +#include "qmp-commands.h"
>>>>>>
>>>>>>    #define TYPE_FILTER_BUFFER "filter-buffer"
>>>>>>
>>>>>> @@ -26,6 +32,8 @@ typedef struct FilterBufferState {
>>>>>>        NetQueue *incoming_queue;
>>>>>>        uint32_t interval;
>>>>>>        QEMUTimer release_timer;
>>>>>> +    bool is_default;
>>>>>> +    bool enable_buffer;
>>>>>>    } FilterBufferState;
>>>>>>
>>>>>>    static void filter_buffer_flush(NetFilterState *nf)
>>>>>> @@ -65,6 +73,10 @@ static ssize_t
>>>>>> filter_buffer_receive_iov(NetFilterState *nf,
>>>>>>    {
>>>>>>        FilterBufferState *s = FILTER_BUFFER(nf);
>>>>>>
>>>>>> +    /* Don't buffer any packets if the filter is not enabled */
>>>>>> +    if (!s->enable_buffer) {
>>>>>> +        return 0;
>>>>>> +    }
>>>>>>        /*
>>>>>>         * We return size when buffer a packet, the sender will take
>>>>>> it as
>>>>>>         * a already sent packet, so sent_cb should not be called
>>>>>> later.
>>>>>> @@ -102,6 +114,7 @@ static void
>>>>>> filter_buffer_cleanup(NetFilterState *nf)
>>>>>>    static void filter_buffer_setup(NetFilterState *nf, Error **errp)
>>>>>>    {
>>>>>>        FilterBufferState *s = FILTER_BUFFER(nf);
>>>>>> +    char *path = object_get_canonical_path_component(OBJECT(nf));
>>>>>>
>>>>>>        /*
>>>>>>         * We may want to accept zero interval when VM FT solutions
>>>>>> like MC
>>>>>> @@ -114,6 +127,7 @@ static void filter_buffer_setup(NetFilterState
>>>>>> *nf, Error **errp)
>>>>>>        }
>>>>>>
>>>>>>        s->incoming_queue =
>>>>>> qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
>>>>>> +    s->is_default = !strcmp(path, "nop");
>>>>>>        if (s->interval) {
>>>>>>            timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL,
>>>>>>                          filter_buffer_release_timer, nf);
>>>>>> @@ -163,6 +177,66 @@ out:
>>>>>>        error_propagate(errp, local_err);
>>>>>>    }
>>>>>>
>>>>>> +/*
>>>>>> +* This will be used by COLO or MC FT, for which they will need
>>>>>> +* to buffer the packets of VM's net devices, Here we add a default
>>>>>> +* buffer filter for each netdev. The name of default buffer filter is
>>>>>> +* 'nop'
>>>>>> +*/
>>>>>> +void netdev_add_default_filter_buffer(const char *netdev_id,
>>>>>> +                                      NetFilterDirection direction,
>>>>>> +                                      Error **errp)
>>>>>> +{
>>>>>> +    QmpOutputVisitor *qov;
>>>>>> +    QmpInputVisitor *qiv;
>>>>>> +    Visitor *ov, *iv;
>>>>>> +    QObject *obj = NULL;
>>>>>> +    QDict *qdict;
>>>>>> +    void *dummy = NULL;
>>>>>> +    const char *id = "nop";
>>>>>> +    char *queue = g_strdup(NetFilterDirection_lookup[direction]);
>>>>>> +    NetClientState *nc = qemu_find_netdev(netdev_id);
>>>>>> +    Error *err = NULL;
>>>>>> +
>>>>>> +    /* FIXME: Not support multiple queues */
>>>>>> +    if (!nc || nc->queue_index > 1) {
>>>>>> +        return;
>>>>>> +    }
>>>>>> +    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_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(queue);
>>>>>> +    if (err) {
>>>>>> +        error_propagate(errp, err);
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>>    static void filter_buffer_init(Object *obj)
>>>>>>    {
>>>>>>        object_property_add(obj, "interval", "int",
>>>>>> diff --git a/net/net.c b/net/net.c
>>>>>> index ade6051..b36d49f 100644
>>>>>> --- a/net/net.c
>>>>>> +++ b/net/net.c
>>>>>> @@ -1028,6 +1028,14 @@ static int net_client_init1(const void
>>>>>> *object, int is_netdev, Error **errp)
>>>>>>            }
>>>>>>            return -1;
>>>>>>        }
>>>>>> +
>>>>>> +    if (is_netdev) {
>>>>>> +        const Netdev *netdev = object;
>>>>>> +
>>>>>> +        netdev_add_default_filter_buffer(netdev->id,
>>>>>> +                                         NET_FILTER_DIRECTION_RX,
>>>>>> +                                         errp);
>>>>>> +    }
>>>>>>        return 0;
>>>>>>    }
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>>>
>>>
>>> .
>>>
>>
>>
>>
>