Re: [Qemu-devel] [PATCH 1/2] tests: introduce basic pci test for virtio-net

[Jason Wang <jasowang@redhat.com>]

On 07/08/2015 11:19 AM, Fam Zheng wrote:
> On Tue, 07/07 18:58, Jason Wang wrote:
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  tests/Makefile          |   2 +-
>>  tests/virtio-net-test.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 178 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/Makefile b/tests/Makefile
>> index eff5e11..56179e5 100644
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -372,7 +372,7 @@ tests/ne2000-test$(EXESUF): tests/ne2000-test.o
>>  tests/wdt_ib700-test$(EXESUF): tests/wdt_ib700-test.o
>>  tests/virtio-balloon-test$(EXESUF): tests/virtio-balloon-test.o
>>  tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y)
>> -tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o $(libqos-pc-obj-y)
>> +tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o $(libqos-pc-obj-y) $(libqos-virtio-obj-y)
>>  tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o $(libqos-pc-obj-y)
>>  tests/virtio-scsi-test$(EXESUF): tests/virtio-scsi-test.o $(libqos-virtio-obj-y)
>>  tests/virtio-9p-test$(EXESUF): tests/virtio-9p-test.o
>> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
>> index ea7478c..97aa442 100644
>> --- a/tests/virtio-net-test.c
>> +++ b/tests/virtio-net-test.c
>> @@ -10,20 +10,191 @@
>>  #include <glib.h>
>>  #include <string.h>
>>  #include "libqtest.h"
>> +#include "qemu-common.h"
>> +#include "qemu/sockets.h"
>>  #include "qemu/osdep.h"
>> -#include "libqos/pci.h"
>> +#include "qemu/iov.h"
>> +#include "libqos/pci-pc.h"
>> +#include "libqos/virtio.h"
>> +#include "libqos/virtio-pci.h"
>> +#include "libqos/malloc.h"
>> +#include "libqos/malloc-pc.h"
>> +#include "libqos/malloc-generic.h"
>> +#include "qemu/bswap.h"
>>  
>>  #define PCI_SLOT_HP             0x06
>> +#define PCI_SLOT                0x04
>> +#define PCI_FN                  0x00
>>  
>> -/* Tests only initialization so far. TODO: Replace with functional tests */
>> -static void pci_nop(void)
>> +#define QVIRTIO_NET_TIMEOUT_US (30 * 1000 * 1000)
>> +
>> +static void test_end(void)
>> +{
>> +    qtest_end();
>> +}
>> +
>> +#ifndef _WIN32
>> +
>> +static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
>> +{
>> +    QVirtioPCIDevice *dev;
>> +
>> +    dev = qvirtio_pci_device_find(bus, QVIRTIO_NET_DEVICE_ID);
>> +    g_assert(dev != NULL);
>> +    g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_NET_DEVICE_ID);
>> +
>> +    qvirtio_pci_device_enable(dev);
>> +    qvirtio_reset(&qvirtio_pci, &dev->vdev);
>> +    qvirtio_set_acknowledge(&qvirtio_pci, &dev->vdev);
>> +    qvirtio_set_driver(&qvirtio_pci, &dev->vdev);
>> +
>> +    return dev;
>> +}
>> +
>> +static QPCIBus *pci_test_start(int socket)
>> +{
>> +    char *cmdline;
>> +
>> +    cmdline = g_strdup_printf("-netdev socket,fd=%d,id=hs0 -device "
>> +                              "virtio-net-pci,netdev=hs0", socket);
>> +    qtest_start(cmdline);
>> +    g_free(cmdline);
>> +
>> +    return qpci_init_pc();
>> +}
>> +
>> +static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
>> +{
>> +    uint32_t features;
>> +
>> +    features = qvirtio_get_features(bus, dev);
>> +    features = features & ~(QVIRTIO_F_BAD_FEATURE |
>> +                            QVIRTIO_F_RING_INDIRECT_DESC |
>> +                            QVIRTIO_F_RING_EVENT_IDX);
>> +    qvirtio_set_features(bus, dev, features);
>> +
>> +    qvirtio_set_driver_ok(bus, dev);
>> +}
>> +
>> +static void rx_test(const QVirtioBus *bus, QVirtioDevice *dev,
>> +                    QGuestAllocator *alloc, QVirtQueue *vq,
>> +                    int socket)
>>  {
>> +    uint64_t req_addr;
>> +    uint32_t free_head;
>> +    char test[] = "TEST";
>> +    char buffer[64];
>> +    int len = htonl(sizeof(test));
>> +    struct iovec iov[] = {
>> +        {
>> +            .iov_base = &len,
>> +            .iov_len = sizeof(len),
>> +        }, {
>> +            .iov_base = test,
>> +            .iov_len = sizeof(test),
>> +        },
>> +    };
>> +    int ret;
>> +
>> +    req_addr = guest_alloc(alloc, 64);
>> +
>> +    free_head = qvirtqueue_add(vq, req_addr, 64, true, false);
>> +    qvirtqueue_kick(bus, dev, vq, free_head);
>> +
>> +    ret = iov_send(socket, iov, 2, 0, sizeof(len) + sizeof(test));
>> +    g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len));
>> +
>> +    qvirtio_wait_queue_isr(bus, dev, vq, QVIRTIO_NET_TIMEOUT_US);
>> +    memread(req_addr + 12, buffer, sizeof(test));
> What does 12 mean?

It was the size of vnet header which needs to be put ahead of the real
packet.

>
>> +    g_assert_cmpstr(buffer, ==, "TEST");
>> +
>> +    guest_free(alloc, req_addr);
>>  }
>>  
>> +static void tx_test(const QVirtioBus *bus, QVirtioDevice *dev,
>> +                    QGuestAllocator *alloc, QVirtQueue *vq,
>> +                    int socket)
>> +{
>> +    uint64_t req_addr;
>> +    uint32_t free_head;
>> +    uint32_t len;
>> +    char buffer[64];
>> +    int ret;
>> +
>> +    req_addr = guest_alloc(alloc, 64);
>> +    memwrite(req_addr + 12, "TEST", 4);
>> +
>> +    free_head = qvirtqueue_add(vq, req_addr, 64, false, false);
>> +    qvirtqueue_kick(bus, dev, vq, free_head);
>> +
>> +    qvirtio_wait_queue_isr(bus, dev, vq, QVIRTIO_NET_TIMEOUT_US);
>> +    guest_free(alloc, req_addr);
>> +
>> +    ret = qemu_recv(socket, &len, sizeof(len), 0);
>> +    g_assert_cmpint(ret, ==, sizeof(len));
>> +    len = ntohl(len);
>> +
>> +    ret = qemu_recv(socket, buffer, len, 0);
>> +    g_assert_cmpstr(buffer, ==, "TEST");
>> +}
>> +
>> +static void send_recv_test(const QVirtioBus *bus, QVirtioDevice *dev,
>> +                           QGuestAllocator *alloc, QVirtQueue *rvq,
>> +                           QVirtQueue *tvq, int socket)
>> +{
>> +    rx_test(bus, dev, alloc, rvq, socket);
>> +    tx_test(bus, dev, alloc, tvq, socket);
>> +}
>> +
>> +static void pci_basic(gconstpointer data)
> I would just say "void *data", but it's bikeshedding.
>
>> +{
>> +    QVirtioPCIDevice *dev;
>> +    QPCIBus *bus;
>> +    QVirtQueuePCI *tx, *rx;
>> +    QGuestAllocator *alloc;
>> +    void (*func) (const QVirtioBus *bus,
>> +                  QVirtioDevice *dev,
>> +                  QGuestAllocator *alloc,
>> +                  QVirtQueue *rvq,
>> +                  QVirtQueue *tvq,
>> +                  int socket) = data;
>> +    int sv[2], ret;
>> +
>> +    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
>> +    g_assert_cmpint(ret, !=, -1);
>> +
>> +    bus = pci_test_start(sv[1]);
>> +    dev = virtio_net_pci_init(bus, PCI_SLOT);
>> +
>> +    alloc = pc_alloc_init();
>> +    rx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> +                                           alloc, 0);
>> +    tx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> +                                           alloc, 1);
>> +
>> +    driver_init(&qvirtio_pci, &dev->vdev);
>> +    func(&qvirtio_pci, &dev->vdev, alloc, &rx->vq, &tx->vq, sv[0]);
>> +
>> +    /* End test */
>> +    close(sv[0]);
>> +    guest_free(alloc, tx->vq.desc);
>> +    pc_alloc_uninit(alloc);
>> +    qvirtio_pci_device_disable(dev);
>> +    g_free(dev);
>> +    qpci_free_pc(bus);
>> +    test_end();
>> +}
>> +#endif
>> +
>>  static void hotplug(void)
>>  {
>> +    qtest_start("-device virtio-net-pci");
>> +
>>      qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL);
>>      qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
>> +    qtest_start("-device virtio-net-pci");
>> +
>> +    test_end();
>>  }
>>  
>>  int main(int argc, char **argv)
>> @@ -31,13 +202,12 @@ int main(int argc, char **argv)
>>      int ret;
>>  
>>      g_test_init(&argc, &argv, NULL);
>> -    qtest_add_func("/virtio/net/pci/nop", pci_nop);
>> +#ifndef _WIN32
>> +    qtest_add_data_func("/virtio/net/pci/basic", send_recv_test, pci_basic);
>> +#endif
>>      qtest_add_func("/virtio/net/pci/hotplug", hotplug);
>>  
>> -    qtest_start("-device virtio-net-pci");
>>      ret = g_test_run();
>>  
>> -    qtest_end();
>> -
>>      return ret;
>>  }
>> -- 
>> 2.1.4
>>
> It fails on current master:
>
> $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img \
>   MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose \
>   -m=quick tests/virtio-net-test
>
> TEST: tests/virtio-net-test... (pid=21185)
>   /x86_64/virtio/net/pci/basic:                                        **
> ERROR:/home/fam/qemu/tests/virtio-net-test.c:109:rx_test: assertion failed (buffer == "TEST"): ("" == "TEST")
> FAIL
> GTester: last random seed: R02S99f81f44f8f10319fa90f6110a54c5af
> (pid=21190)
>   /x86_64/virtio/net/pci/hotplug:                                      OK
> FAIL: tests/virtio-net-test
>
> Any idea?

I believe this is because of a bug in socket. My patch "socket: pass
correct size in net_socket_send()" should fix this.

Thanks

>
> Fam