Re: [Qemu-devel] [PATCH V4 2/2] tests/test-filter-mirror:add filter-mirror unit test

[Zhang Chen]

On 02/23/2016 10:31 AM, Jason Wang wrote:
> On 02/18/2016 04:10 PM, Zhang Chen wrote:
> > From: ZhangChen <address@hidden>
> >
> > In this unit test we will test the mirror function.
> >
> > start qemu with:
> >      -netdev socket,id=qtest-bn0,fd=%d
> >      -device e1000,netdev=qtest-bn0,id=qtest-e0
> >      -chardev socket,id=mirror0,path=/tmp/filter-mirror-test.sock,server,nowait
> >      -object filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0
> >
> > We inject packet to netdev socket id = qtest-bn0,
> > filter-mirror will copy and mirror the packet to mirror0.
> > we read packet from mirror0 and then compare to what we inject.
> >
> > Signed-off-by: zhangchen <address@hidden>
> > Signed-off-by: Wen Congyang <address@hidden>
> > ---
> >   tests/.gitignore           |  1 +
> >   tests/Makefile             |  2 ++
> >   tests/test-filter-mirror.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 92 insertions(+)
> >   create mode 100644 tests/test-filter-mirror.c
> >
> > diff --git a/tests/.gitignore b/tests/.gitignore
> > index 787c95c..10df017 100644
> > --- a/tests/.gitignore
> > +++ b/tests/.gitignore
> > @@ -63,5 +63,6 @@ test-write-threshold
> >   test-x86-cpuid
> >   test-xbzrle
> >   test-netfilter
> > +test-filter-mirror
> >   *-test
> >   qapi-schema/*.test.*
> > diff --git a/tests/Makefile b/tests/Makefile
> > index 650e654..e56c514 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -212,6 +212,7 @@ ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
> >   check-qtest-x86_64-$(CONFIG_VHOST_NET_TEST_x86_64) += 
> > tests/vhost-user-test$(EXESUF)
> >   endif
> >   check-qtest-i386-y += tests/test-netfilter$(EXESUF)
> > +check-qtest-i386-y += tests/test-filter-mirror$(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))
> > @@ -563,6 +564,7 @@ tests/qemu-iotests/socket_scm_helper$(EXESUF): 
> > tests/qemu-iotests/socket_scm_hel
> >   tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o $(test-util-obj-y)
> >   tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o 
> > $(test-block-obj-y)
> >   tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
> > +tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
> >   tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o 
> > contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
> >   tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
> >   
> > diff --git a/tests/test-filter-mirror.c b/tests/test-filter-mirror.c
> > new file mode 100644
> > index 0000000..9c23135
> > --- /dev/null
> > +++ b/tests/test-filter-mirror.c
> > @@ -0,0 +1,89 @@
> > +/*
> > + * QTest testcase for filter-mirror
> > + *
> > + * Copyright (c) 2016 FUJITSU LIMITED
> > + * Author: Zhang chen <address@hidden>
> > + *
> > + * 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"
> > +#include "qemu/iov.h"
> > +#include "qemu/sockets.h"
> > +#include "qemu/error-report.h"
> > +#include "qemu/main-loop.h"
> > +
> > +static void test_mirror(void)
> > +{
> > +    int send_sock[2];
> > +    char *cmdline;
> > +    uint32_t ret = 0;
> > +    char send_buf[] = "Hello! filter-mirror~";
> > +    uint32_t size = sizeof(send_buf);
> > +    size = htonl(size);
> > +
> > +    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, send_sock);
> > +    g_assert_cmpint(ret, !=, -1);
> socketpair does not exist on w32, let's exclude this from windows.

OK, I will fix~~

> > +
> > +    cmdline = g_strdup_printf("-netdev socket,id=qtest-bn0,fd=%d "
> > +                 "-device e1000,netdev=qtest-bn0,id=qtest-e0 "
> > +                 "-chardev 
> > socket,id=mirror0,path=/tmp/filter-mirror-test.sock,server,nowait "
> > +                 "-object 
> > filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0 "
> > +                 , send_sock[1]);
> > +    qtest_start(cmdline);
> > +    g_free(cmdline);
> > +
> > +    if (fork() == 0) {
> Don't get the point of why need a fork() here. I think you can just do
> the test in one process, first inject the packet though sending socket
> and then try to receive it.

I will fix it in next version.

> > +        int recv_sock;
> > +        uint32_t len = 0;
> > +        char *recv_buf;
> > +
> > +        recv_sock = unix_connect("/tmp/filter-mirror-test.sock", NULL);
> Let's define /tmp/filter-mirror-test.sock as a macro.

OK~

> > +        if (recv_sock < 0) {
> > +            error_report("test_mirror connect filter-mirror-test.sock failed");
> I was think maybe we can just pass the test here? Since we don't even
> start the test.

yes, I will fix it in next version.

> > +            exit(1);
> > +        }
> > +        ret = qemu_recv(recv_sock, &len, sizeof(len), 0);
> > +        g_assert_cmpint(ret, ==, sizeof(len));
> > +        len = ntohl(len);
> > +
> > +        g_assert_cmpint(len, ==, sizeof(send_buf));
> > +        recv_buf = g_malloc0(len);
> > +        ret = qemu_recv(recv_sock, recv_buf, len, 0);
> > +        g_assert_cmpstr(recv_buf, ==, send_buf);
> > +
> > +        g_free(recv_buf);
> > +        close(recv_sock);
> > +        exit(0);
> > +    }
> > +
> > +    usleep(5000);
> > +    struct iovec iov[] = {
> > +        {
> > +            .iov_base = &size,
> > +            .iov_len = sizeof(size),
> > +        }, {
> > +            .iov_base = send_buf,
> > +            .iov_len = sizeof(send_buf),
> > +        },
> > +    };
> > +    ret = iov_send(send_sock[0], iov, 2, 0, sizeof(size) + sizeof(send_buf));
> > +    g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
> > +    close(send_sock[0]);
> > +
> Should we unlink the socket file?

OK, I will fix.

> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +    int ret;
> > +
> > +    g_test_init(&argc, &argv, NULL);
> > +
> > +    qtest_add_func("/netfilter/test-mirror", test_mirror);
> We've already in the namespace of test, so probably no need for "test"
> prefix here.

OK, I will fix it in next version.

Thanks
zhangchen

> > +    ret = g_test_run();
> > +    qtest_end();
> > +
> > +    return ret;
> > +}
>
>
> .

--
Thanks
zhangchen