introduce vhost_net_test basing on virtio_test to test
vhost_net changing in the kernel.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
tools/virtio/.gitignore | 1 +
tools/virtio/Makefile | 8 +-
tools/virtio/vhost_net_test.c | 576 ++++++++++++++++++++++++++++++++++
3 files changed, 582 insertions(+), 3 deletions(-)
create mode 100644 tools/virtio/vhost_net_test.c
diff --git a/tools/virtio/.gitignore b/tools/virtio/.gitignore
index 9934d48d9a55..7e47b281c442 100644
--- a/tools/virtio/.gitignore
+++ b/tools/virtio/.gitignore
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
*.d
virtio_test
+vhost_net_test
vringh_test
virtio-trace/trace-agent
diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
index d128925980e0..e25e99c1c3b7 100644
--- a/tools/virtio/Makefile
+++ b/tools/virtio/Makefile
@@ -1,8 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
all: test mod
-test: virtio_test vringh_test
+test: virtio_test vringh_test vhost_net_test
virtio_test: virtio_ring.o virtio_test.o
vringh_test: vringh_test.o vringh.o virtio_ring.o
+vhost_net_test: virtio_ring.o vhost_net_test.o
try-run = $(shell set -e; \
if ($(1)) >/dev/null 2>&1; \
@@ -49,6 +50,7 @@ oot-clean: OOT_BUILD+=clean
.PHONY: all test mod clean vhost oot oot-clean oot-build
clean:
- ${RM} *.o vringh_test virtio_test vhost_test/*.o vhost_test/.*.cmd \
- vhost_test/Module.symvers vhost_test/modules.order *.d
+ ${RM} *.o vringh_test virtio_test vhost_net_test vhost_test/*.o \
+ vhost_test/.*.cmd vhost_test/Module.symvers \
+ vhost_test/modules.order *.d
-include *.d
diff --git a/tools/virtio/vhost_net_test.c b/tools/virtio/vhost_net_test.c
new file mode 100644
index 000000000000..e336792a0d77
--- /dev/null
+++ b/tools/virtio/vhost_net_test.c
@@ -0,0 +1,576 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <getopt.h>
+#include <limits.h>
+#include <string.h>
+#include <poll.h>
+#include <sys/eventfd.h>
+#include <stdlib.h>
+#include <assert.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <linux/virtio_types.h>
+#include <linux/vhost.h>
+#include <linux/virtio.h>
+#include <linux/virtio_ring.h>
+#include <linux/if.h>
+#include <linux/if_tun.h>
+#include <linux/in.h>
+#include <linux/if_packet.h>
+#include <netinet/ether.h>
+
+#define RANDOM_BATCH -1
+#define HDR_LEN 12
+#define TEST_BUF_LEN 256
+#define TEST_PTYPE ETH_P_LOOPBACK
+
+/* Used by implementation of kmalloc() in tools/virtio/linux/kernel.h */
+void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;
+
+struct vq_info {
+ int kick;
+ int call;
+ int idx;
+ long started;
+ long completed;
+ struct pollfd fds;
+ void *ring;
+ /* copy used for control */
+ struct vring vring;
+ struct virtqueue *vq;
+};
+
+struct vdev_info {
+ struct virtio_device vdev;
+ int control;
+ struct vq_info vqs[2];
+ int nvqs;
+ void *buf;
+ size_t buf_size;
+ char *test_buf;
+ char *res_buf;
+ struct vhost_memory *mem;
+ int sock;
+ int ifindex;
+ unsigned char mac[ETHER_ADDR_LEN];
+};
+
+static int tun_alloc(struct vdev_info *dev)
+{
+ struct ifreq ifr;
+ int len = HDR_LEN;
+ int fd, e;
+
+ fd = open("/dev/net/tun", O_RDWR);
+ if (fd < 0) {
+ perror("Cannot open /dev/net/tun");
+ return fd;
+ }
+
+ memset(&ifr, 0, sizeof(ifr));
+
+ ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
+ snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
+
+ e = ioctl(fd, TUNSETIFF, &ifr);
+ if (e < 0) {
+ perror("ioctl[TUNSETIFF]");
+ close(fd);
+ return e;
+ }
+
+ e = ioctl(fd, TUNSETVNETHDRSZ, &len);
+ if (e < 0) {
+ perror("ioctl[TUNSETVNETHDRSZ]");
+ close(fd);
+ return e;
+ }
+
+ e = ioctl(fd, SIOCGIFHWADDR, &ifr);
+ if (e < 0) {
+ perror("ioctl[SIOCGIFHWADDR]");
+ close(fd);
+ return e;
+ }
+
+ memcpy(dev->mac, &ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN);
+ return fd;
+}
+
+static void vdev_create_socket(struct vdev_info *dev)
+{
+ struct ifreq ifr;
+
+ dev->sock = socket(AF_PACKET, SOCK_RAW, htons(TEST_PTYPE));
+ assert(dev->sock != -1);
+
+ snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
+ assert(ioctl(dev->sock, SIOCGIFINDEX, &ifr) >= 0);
+
+ dev->ifindex = ifr.ifr_ifindex;
+
+ /* Set the flags that bring the device up */
+ assert(ioctl(dev->sock, SIOCGIFFLAGS, &ifr) >= 0);
+ ifr.ifr_flags |= (IFF_UP | IFF_RUNNING);
+ assert(ioctl(dev->sock, SIOCSIFFLAGS, &ifr) >= 0);
+}
+
+static void vdev_send_packet(struct vdev_info *dev)
+{
+ char *sendbuf = dev->test_buf + HDR_LEN;
+ struct sockaddr_ll saddrll = {0};
+ int sockfd = dev->sock;
+ int ret;
+
+ saddrll.sll_family = PF_PACKET;
+ saddrll.sll_ifindex = dev->ifindex;
+ saddrll.sll_halen = ETH_ALEN;
+ saddrll.sll_protocol = htons(TEST_PTYPE);
+
+ ret = sendto(sockfd, sendbuf, TEST_BUF_LEN, 0,
+ (struct sockaddr *)&saddrll,
+ sizeof(struct sockaddr_ll));
+ assert(ret >= 0);
+}
+
+static bool vq_notify(struct virtqueue *vq)
+{
+ struct vq_info *info = vq->priv;
+ unsigned long long v = 1;
+ int r;
+
+ r = write(info->kick, &v, sizeof(v));
+ assert(r == sizeof(v));
+
+ return true;
+}
+
+static void vq_callback(struct virtqueue *vq)
+{
+}
+
+static void vhost_vq_setup(struct vdev_info *dev, struct vq_info *info)
+{
+ struct vhost_vring_addr addr = {
+ .index = info->idx,
+ .desc_user_addr = (uint64_t)(unsigned long)info->vring.desc,
+ .avail_user_addr = (uint64_t)(unsigned long)info->vring.avail,
+ .used_user_addr = (uint64_t)(unsigned long)info->vring.used,
+ };
+ struct vhost_vring_state state = { .index = info->idx };
+ struct vhost_vring_file file = { .index = info->idx };
+ int r;
+
+ state.num = info->vring.num;
+ r = ioctl(dev->control, VHOST_SET_VRING_NUM, &state);
+ assert(r >= 0);
+
+ state.num = 0;
+ r = ioctl(dev->control, VHOST_SET_VRING_BASE, &state);
+ assert(r >= 0);
+
+ r = ioctl(dev->control, VHOST_SET_VRING_ADDR, &addr);
+ assert(r >= 0);
+
+ file.fd = info->kick;
+ r = ioctl(dev->control, VHOST_SET_VRING_KICK, &file);
+ assert(r >= 0);
+
+ file.fd = info->call;
+ r = ioctl(dev->control, VHOST_SET_VRING_CALL, &file);
+ assert(r >= 0);
+}
+
+static void vq_reset(struct vq_info *info, int num, struct virtio_device *vdev)
+{
+ if (info->vq)
+ vring_del_virtqueue(info->vq);
+
+ memset(info->ring, 0, vring_size(num, 4096));
+ vring_init(&info->vring, num, info->ring, 4096);
+ info->vq = vring_new_virtqueue(info->idx, num, 4096, vdev, true, false,
+ info->ring, vq_notify, vq_callback, "test");
+ assert(info->vq);
+ info->vq->priv = info;
+}
+
+static void vq_info_add(struct vdev_info *dev, int idx, int num, int fd)
+{
+ struct vhost_vring_file backend = { .index = idx, .fd = fd };
+ struct vq_info *info = &dev->vqs[idx];
+ int r;
+
+ info->idx = idx;
+ info->kick = eventfd(0, EFD_NONBLOCK);
+ info->call = eventfd(0, EFD_NONBLOCK);
+ r = posix_memalign(&info->ring, 4096, vring_size(num, 4096));
+ assert(r >= 0);
+ vq_reset(info, num, &dev->vdev);
+ vhost_vq_setup(dev, info);
+ info->fds.fd = info->call;
+ info->fds.events = POLLIN;
+
+ r = ioctl(dev->control, VHOST_NET_SET_BACKEND, &backend);
+ assert(!r);
+}
+
+static void vdev_info_init(struct vdev_info *dev, unsigned long long features)
+{
+ struct ether_header *eh;
+ int i, r;
+
+ dev->vdev.features = features;
+ INIT_LIST_HEAD(&dev->vdev.vqs);
+ spin_lock_init(&dev->vdev.vqs_list_lock);
+
+ dev->buf_size = (HDR_LEN + TEST_BUF_LEN) * 2;
+ dev->buf = malloc(dev->buf_size);
+ assert(dev->buf);
+ dev->test_buf = dev->buf;
+ dev->res_buf = dev->test_buf + HDR_LEN + TEST_BUF_LEN;
+
+ memset(dev->test_buf, 0, HDR_LEN + TEST_BUF_LEN);
+ eh = (struct ether_header *)(dev->test_buf + HDR_LEN);
+ eh->ether_type = htons(TEST_PTYPE);
+ memcpy(eh->ether_dhost, dev->mac, ETHER_ADDR_LEN);
+ memcpy(eh->ether_shost, dev->mac, ETHER_ADDR_LEN);
+
+ for (i = sizeof(*eh); i < TEST_BUF_LEN; i++)
+ dev->test_buf[i + HDR_LEN] = (char)i;
+
+ dev->control = open("/dev/vhost-net", O_RDWR);
+ assert(dev->control >= 0);
+
+ r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
+ assert(r >= 0);
+
+ dev->mem = malloc(offsetof(struct vhost_memory, regions) +
+ sizeof(dev->mem->regions[0]));
+ assert(dev->mem);
+ memset(dev->mem, 0, offsetof(struct vhost_memory, regions) +
+ sizeof(dev->mem->regions[0]));
+ dev->mem->nregions = 1;
+ dev->mem->regions[0].guest_phys_addr = (long)dev->buf;
+ dev->mem->regions[0].userspace_addr = (long)dev->buf;
+ dev->mem->regions[0].memory_size = dev->buf_size;
+
+ r = ioctl(dev->control, VHOST_SET_MEM_TABLE, dev->mem);
+ assert(r >= 0);
+
+ r = ioctl(dev->control, VHOST_SET_FEATURES, &features);
+ assert(r >= 0);
+
+ dev->nvqs = 2;
+}
+
+static void wait_for_interrupt(struct vq_info *vq)
+{
+ unsigned long long val;
+
+ poll(&vq->fds, 1, -1);
+
+ if (vq->fds.revents & POLLIN)
+ read(vq->fds.fd, &val, sizeof(val));
+}
+
+static void verify_res_buf(char *res_buf)
+{
+ int i;
+
+ for (i = ETHER_HDR_LEN; i < TEST_BUF_LEN; i++)
+ assert(res_buf[i] == (char)i);
+}
+
+static void run_tx_test(struct vdev_info *dev, struct vq_info *vq,
+ bool delayed, int batch, int bufs)
+{
+ const bool random_batch = batch == RANDOM_BATCH;
+ long long spurious = 0;
+ struct scatterlist sl;
+ unsigned int len;
+ int r;
+
+ for (;;) {
+ long started_before = vq->started;
+ long completed_before = vq->completed;
+
+ virtqueue_disable_cb(vq->vq);
+ do {
+ if (random_batch)
+ batch = (random() % vq->vring.num) + 1;
+
+ while (vq->started < bufs &&
+ (vq->started - vq->completed) < batch) {
+ sg_init_one(&sl, dev->test_buf, HDR_LEN + TEST_BUF_LEN);
+ r = virtqueue_add_outbuf(vq->vq, &sl, 1,
+ dev->test_buf + vq->started,
+ GFP_ATOMIC);
+ if (unlikely(r != 0)) {
+ if (r == -ENOSPC &&
+ vq->started > started_before)
+ r = 0;
+ else
+ r = -1;
+ break;
+ }
+
+ ++vq->started;
+
+ if (unlikely(!virtqueue_kick(vq->vq))) {
+ r = -1;
+ break;
+ }
+ }
+
+ if (vq->started >= bufs)
+ r = -1;
+
+ /* Flush out completed bufs if any */
+ while (virtqueue_get_buf(vq->vq, &len)) {
+ int n;
+
+ n = recvfrom(dev->sock, dev->res_buf, TEST_BUF_LEN, 0, NULL, NULL);
+ assert(n == TEST_BUF_LEN);
+ verify_res_buf(dev->res_buf);
+
+ ++vq->completed;
+ r = 0;
+ }
+ } while (r == 0);
+
+ if (vq->completed == completed_before && vq->started == started_before)
+ ++spurious;
+
+ assert(vq->completed <= bufs);
+ assert(vq->started <= bufs);
+ if (vq->completed == bufs)
+ break;
+
+ if (delayed) {
+ if (virtqueue_enable_cb_delayed(vq->vq))
+ wait_for_interrupt(vq);
+ } else {
+ if (virtqueue_enable_cb(vq->vq))
+ wait_for_interrupt(vq);
+ }
+ }
+ printf("TX spurious wakeups: 0x%llx started=0x%lx completed=0x%lx\n",
+ spurious, vq->started, vq->completed);
+}
+
+static void run_rx_test(struct vdev_info *dev, struct vq_info *vq,
+ bool delayed, int batch, int bufs)
+{
+ const bool random_batch = batch == RANDOM_BATCH;
+ long long spurious = 0;
+ struct scatterlist sl;
+ unsigned int len;
+ int r;
+
+ for (;;) {
+ long started_before = vq->started;
+ long completed_before = vq->completed;
+
+ do {
+ if (random_batch)
+ batch = (random() % vq->vring.num) + 1;
+
+ while (vq->started < bufs &&
+ (vq->started - vq->completed) < batch) {
+ sg_init_one(&sl, dev->res_buf, HDR_LEN + TEST_BUF_LEN);
+
+ r = virtqueue_add_inbuf(vq->vq, &sl, 1,
+ dev->res_buf + vq->started,
+ GFP_ATOMIC);
+ if (unlikely(r != 0)) {
+ if (r == -ENOSPC &&
+ vq->started > started_before)
+ r = 0;
+ else
+ r = -1;
+ break;
+ }
+
+ ++vq->started;
+
+ vdev_send_packet(dev);
+
+ if (unlikely(!virtqueue_kick(vq->vq))) {
+ r = -1;
+ break;
+ }
+ }
+
+ if (vq->started >= bufs)
+ r = -1;
+
+ /* Flush out completed bufs if any */
+ while (virtqueue_get_buf(vq->vq, &len)) {
+ struct ether_header *eh;
+
+ eh = (struct ether_header *)(dev->res_buf + HDR_LEN);
+
+ /* tun netdev is up and running, ignore the
+ * non-TEST_PTYPE packet.
+ */
+ if (eh->ether_type != htons(TEST_PTYPE)) {
+ ++vq->completed;
+ r = 0;
+ continue;
+ }
+
+ assert(len == TEST_BUF_LEN + HDR_LEN);
+ verify_res_buf(dev->res_buf + HDR_LEN);
+
+ ++vq->completed;
+ r = 0;
+ }
+ } while (r == 0);
+ if (vq->completed == completed_before && vq->started == started_before)
+ ++spurious;
+
+ assert(vq->completed <= bufs);
+ assert(vq->started <= bufs);
+ if (vq->completed == bufs)
+ break;
+ }
+
+ printf("RX spurious wakeups: 0x%llx started=0x%lx completed=0x%lx\n",
+ spurious, vq->started, vq->completed);
+}
+
+static const char optstring[] = "h";
+static const struct option longopts[] = {
+ {
+ .name = "help",
+ .val = 'h',
+ },
+ {
+ .name = "event-idx",
+ .val = 'E',
+ },
+ {
+ .name = "no-event-idx",
+ .val = 'e',
+ },
+ {
+ .name = "indirect",
+ .val = 'I',
+ },
+ {
+ .name = "no-indirect",
+ .val = 'i',
+ },
+ {
+ .name = "virtio-1",
+ .val = '1',
+ },
+ {
+ .name = "no-virtio-1",
+ .val = '0',
+ },
+ {
+ .name = "delayed-interrupt",
+ .val = 'D',
+ },
+ {
+ .name = "no-delayed-interrupt",
+ .val = 'd',
+ },
+ {
+ .name = "buf-num",
+ .val = 'n',
+ .has_arg = required_argument,
+ },
+ {
+ .name = "batch",
+ .val = 'b',
+ .has_arg = required_argument,
+ },
+ {
+ }
+};
+
+static void help(int status)
+{
+ fprintf(stderr, "Usage: vhost_net_test [--help]"
+ " [--no-indirect]"
+ " [--no-event-idx]"
+ " [--no-virtio-1]"
+ " [--delayed-interrupt]"
+ " [--buf-num]"
+ " [--batch=random/N]"
+ "\n");
+
+ exit(status);
+}
+
+int main(int argc, char **argv)
+{
+ unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1ULL << VIRTIO_RING_F_EVENT_IDX) | (1ULL << VIRTIO_F_VERSION_1);
+ long batch = 1, nbufs = 0x100000;
+ struct vdev_info dev;
+ bool delayed = false;
+ int o, fd;
+
+ for (;;) {
+ o = getopt_long(argc, argv, optstring, longopts, NULL);
+ switch (o) {
+ case -1:
+ goto done;
+ case '?':
+ help(2);
+ case 'e':
+ features &= ~(1ULL << VIRTIO_RING_F_EVENT_IDX);
+ break;
+ case 'h':
+ help(0);
+ case 'i':
+ features &= ~(1ULL << VIRTIO_RING_F_INDIRECT_DESC);
+ break;
+ case '0':
+ features &= ~(1ULL << VIRTIO_F_VERSION_1);
+ break;
+ case 'D':
+ delayed = true;
+ break;
+ case 'b':
+ if (!strcmp(optarg, "random")) {
+ batch = RANDOM_BATCH;
+ } else {
+ batch = strtol(optarg, NULL, 10);
+ assert(batch > 0);
+ assert(batch < (long)INT_MAX + 1);
+ }
+ break;
+ case 'n':
+ nbufs = strtol(optarg, NULL, 10);
+ assert(nbufs > 0);
+ break;
+ default:
+ assert(0);
+ break;
+ }
+ }
+
+done:
+ memset(&dev, 0, sizeof(dev));
+
+ fd = tun_alloc(&dev);
+ assert(fd >= 0);
+
+ vdev_info_init(&dev, features);
+ vq_info_add(&dev, 0, 256, fd);
+ vq_info_add(&dev, 1, 256, fd);
+ vdev_create_socket(&dev);
+
+ run_rx_test(&dev, &dev.vqs[0], delayed, batch, nbufs);
+ run_tx_test(&dev, &dev.vqs[1], delayed, batch, nbufs);
+
+ return 0;
+}
--
2.33.0On Tue, Jan 30, 2024 at 7:38 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> introduce vhost_net_test basing on virtio_test to test
> vhost_net changing in the kernel.
Let's describe what kind of test is being done and how it is done here.
>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> ---
> tools/virtio/.gitignore | 1 +
> tools/virtio/Makefile | 8 +-
> tools/virtio/vhost_net_test.c | 576 ++++++++++++++++++++++++++++++++++
> 3 files changed, 582 insertions(+), 3 deletions(-)
> create mode 100644 tools/virtio/vhost_net_test.c
>
> diff --git a/tools/virtio/.gitignore b/tools/virtio/.gitignore
> index 9934d48d9a55..7e47b281c442 100644
> --- a/tools/virtio/.gitignore
> +++ b/tools/virtio/.gitignore
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> *.d
> virtio_test
> +vhost_net_test
> vringh_test
> virtio-trace/trace-agent
> diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
> index d128925980e0..e25e99c1c3b7 100644
> --- a/tools/virtio/Makefile
> +++ b/tools/virtio/Makefile
> @@ -1,8 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> all: test mod
> -test: virtio_test vringh_test
> +test: virtio_test vringh_test vhost_net_test
> virtio_test: virtio_ring.o virtio_test.o
> vringh_test: vringh_test.o vringh.o virtio_ring.o
> +vhost_net_test: virtio_ring.o vhost_net_test.o
>
> try-run = $(shell set -e; \
> if ($(1)) >/dev/null 2>&1; \
> @@ -49,6 +50,7 @@ oot-clean: OOT_BUILD+=clean
>
> .PHONY: all test mod clean vhost oot oot-clean oot-build
> clean:
> - ${RM} *.o vringh_test virtio_test vhost_test/*.o vhost_test/.*.cmd \
> - vhost_test/Module.symvers vhost_test/modules.order *.d
> + ${RM} *.o vringh_test virtio_test vhost_net_test vhost_test/*.o \
> + vhost_test/.*.cmd vhost_test/Module.symvers \
> + vhost_test/modules.order *.d
> -include *.d
> diff --git a/tools/virtio/vhost_net_test.c b/tools/virtio/vhost_net_test.c
> new file mode 100644
> index 000000000000..e336792a0d77
> --- /dev/null
> +++ b/tools/virtio/vhost_net_test.c
> @@ -0,0 +1,576 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <getopt.h>
> +#include <limits.h>
> +#include <string.h>
> +#include <poll.h>
> +#include <sys/eventfd.h>
> +#include <stdlib.h>
> +#include <assert.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <fcntl.h>
> +#include <stdbool.h>
> +#include <linux/virtio_types.h>
> +#include <linux/vhost.h>
> +#include <linux/virtio.h>
> +#include <linux/virtio_ring.h>
> +#include <linux/if.h>
> +#include <linux/if_tun.h>
> +#include <linux/in.h>
> +#include <linux/if_packet.h>
> +#include <netinet/ether.h>
> +
> +#define RANDOM_BATCH -1
> +#define HDR_LEN 12
> +#define TEST_BUF_LEN 256
> +#define TEST_PTYPE ETH_P_LOOPBACK
> +
> +/* Used by implementation of kmalloc() in tools/virtio/linux/kernel.h */
> +void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;
> +
> +struct vq_info {
> + int kick;
> + int call;
> + int idx;
> + long started;
> + long completed;
> + struct pollfd fds;
> + void *ring;
> + /* copy used for control */
> + struct vring vring;
> + struct virtqueue *vq;
> +};
> +
> +struct vdev_info {
> + struct virtio_device vdev;
> + int control;
> + struct vq_info vqs[2];
> + int nvqs;
> + void *buf;
> + size_t buf_size;
> + char *test_buf;
> + char *res_buf;
> + struct vhost_memory *mem;
> + int sock;
> + int ifindex;
> + unsigned char mac[ETHER_ADDR_LEN];
> +};
> +
> +static int tun_alloc(struct vdev_info *dev)
> +{
> + struct ifreq ifr;
> + int len = HDR_LEN;
Any reason you can't just use the virtio_net uapi?
> + int fd, e;
> +
> + fd = open("/dev/net/tun", O_RDWR);
> + if (fd < 0) {
> + perror("Cannot open /dev/net/tun");
> + return fd;
> + }
> +
> + memset(&ifr, 0, sizeof(ifr));
> +
> + ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
> + snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
> +
> + e = ioctl(fd, TUNSETIFF, &ifr);
> + if (e < 0) {
> + perror("ioctl[TUNSETIFF]");
> + close(fd);
> + return e;
> + }
> +
> + e = ioctl(fd, TUNSETVNETHDRSZ, &len);
> + if (e < 0) {
> + perror("ioctl[TUNSETVNETHDRSZ]");
> + close(fd);
> + return e;
> + }
> +
> + e = ioctl(fd, SIOCGIFHWADDR, &ifr);
> + if (e < 0) {
> + perror("ioctl[SIOCGIFHWADDR]");
> + close(fd);
> + return e;
> + }
> +
> + memcpy(dev->mac, &ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN);
> + return fd;
> +}
> +
> +static void vdev_create_socket(struct vdev_info *dev)
> +{
> + struct ifreq ifr;
> +
> + dev->sock = socket(AF_PACKET, SOCK_RAW, htons(TEST_PTYPE));
> + assert(dev->sock != -1);
> +
> + snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
Nit: it might be better to accept the device name instead of repeating
the snprintf trick here, this would facilitate the future changes.
> + assert(ioctl(dev->sock, SIOCGIFINDEX, &ifr) >= 0);
> +
> + dev->ifindex = ifr.ifr_ifindex;
> +
> + /* Set the flags that bring the device up */
> + assert(ioctl(dev->sock, SIOCGIFFLAGS, &ifr) >= 0);
> + ifr.ifr_flags |= (IFF_UP | IFF_RUNNING);
> + assert(ioctl(dev->sock, SIOCSIFFLAGS, &ifr) >= 0);
> +}
> +
> +static void vdev_send_packet(struct vdev_info *dev)
> +{
> + char *sendbuf = dev->test_buf + HDR_LEN;
> + struct sockaddr_ll saddrll = {0};
> + int sockfd = dev->sock;
> + int ret;
> +
> + saddrll.sll_family = PF_PACKET;
> + saddrll.sll_ifindex = dev->ifindex;
> + saddrll.sll_halen = ETH_ALEN;
> + saddrll.sll_protocol = htons(TEST_PTYPE);
> +
> + ret = sendto(sockfd, sendbuf, TEST_BUF_LEN, 0,
> + (struct sockaddr *)&saddrll,
> + sizeof(struct sockaddr_ll));
> + assert(ret >= 0);
> +}
> +
> +static bool vq_notify(struct virtqueue *vq)
> +{
> + struct vq_info *info = vq->priv;
> + unsigned long long v = 1;
> + int r;
> +
> + r = write(info->kick, &v, sizeof(v));
> + assert(r == sizeof(v));
> +
> + return true;
> +}
> +
> +static void vq_callback(struct virtqueue *vq)
> +{
> +}
> +
> +static void vhost_vq_setup(struct vdev_info *dev, struct vq_info *info)
> +{
> + struct vhost_vring_addr addr = {
> + .index = info->idx,
> + .desc_user_addr = (uint64_t)(unsigned long)info->vring.desc,
> + .avail_user_addr = (uint64_t)(unsigned long)info->vring.avail,
> + .used_user_addr = (uint64_t)(unsigned long)info->vring.used,
> + };
> + struct vhost_vring_state state = { .index = info->idx };
> + struct vhost_vring_file file = { .index = info->idx };
> + int r;
> +
> + state.num = info->vring.num;
> + r = ioctl(dev->control, VHOST_SET_VRING_NUM, &state);
> + assert(r >= 0);
> +
> + state.num = 0;
> + r = ioctl(dev->control, VHOST_SET_VRING_BASE, &state);
> + assert(r >= 0);
> +
> + r = ioctl(dev->control, VHOST_SET_VRING_ADDR, &addr);
> + assert(r >= 0);
> +
> + file.fd = info->kick;
> + r = ioctl(dev->control, VHOST_SET_VRING_KICK, &file);
> + assert(r >= 0);
> +
> + file.fd = info->call;
> + r = ioctl(dev->control, VHOST_SET_VRING_CALL, &file);
> + assert(r >= 0);
> +}
> +
> +static void vq_reset(struct vq_info *info, int num, struct virtio_device *vdev)
> +{
> + if (info->vq)
> + vring_del_virtqueue(info->vq);
> +
> + memset(info->ring, 0, vring_size(num, 4096));
> + vring_init(&info->vring, num, info->ring, 4096);
> + info->vq = vring_new_virtqueue(info->idx, num, 4096, vdev, true, false,
> + info->ring, vq_notify, vq_callback, "test");
> + assert(info->vq);
> + info->vq->priv = info;
> +}
> +
> +static void vq_info_add(struct vdev_info *dev, int idx, int num, int fd)
> +{
> + struct vhost_vring_file backend = { .index = idx, .fd = fd };
> + struct vq_info *info = &dev->vqs[idx];
> + int r;
> +
> + info->idx = idx;
> + info->kick = eventfd(0, EFD_NONBLOCK);
> + info->call = eventfd(0, EFD_NONBLOCK);
If we don't care about the callback, let's just avoid to set the call here?
(As I see vq_callback is a NULL)
> + r = posix_memalign(&info->ring, 4096, vring_size(num, 4096));
> + assert(r >= 0);
> + vq_reset(info, num, &dev->vdev);
> + vhost_vq_setup(dev, info);
> + info->fds.fd = info->call;
> + info->fds.events = POLLIN;
> +
> + r = ioctl(dev->control, VHOST_NET_SET_BACKEND, &backend);
> + assert(!r);
> +}
> +
> +static void vdev_info_init(struct vdev_info *dev, unsigned long long features)
> +{
> + struct ether_header *eh;
> + int i, r;
> +
> + dev->vdev.features = features;
> + INIT_LIST_HEAD(&dev->vdev.vqs);
> + spin_lock_init(&dev->vdev.vqs_list_lock);
> +
> + dev->buf_size = (HDR_LEN + TEST_BUF_LEN) * 2;
> + dev->buf = malloc(dev->buf_size);
> + assert(dev->buf);
> + dev->test_buf = dev->buf;
> + dev->res_buf = dev->test_buf + HDR_LEN + TEST_BUF_LEN;
> +
> + memset(dev->test_buf, 0, HDR_LEN + TEST_BUF_LEN);
> + eh = (struct ether_header *)(dev->test_buf + HDR_LEN);
> + eh->ether_type = htons(TEST_PTYPE);
> + memcpy(eh->ether_dhost, dev->mac, ETHER_ADDR_LEN);
> + memcpy(eh->ether_shost, dev->mac, ETHER_ADDR_LEN);
> +
> + for (i = sizeof(*eh); i < TEST_BUF_LEN; i++)
> + dev->test_buf[i + HDR_LEN] = (char)i;
> +
> + dev->control = open("/dev/vhost-net", O_RDWR);
> + assert(dev->control >= 0);
> +
> + r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
> + assert(r >= 0);
> +
> + dev->mem = malloc(offsetof(struct vhost_memory, regions) +
> + sizeof(dev->mem->regions[0]));
> + assert(dev->mem);
> + memset(dev->mem, 0, offsetof(struct vhost_memory, regions) +
> + sizeof(dev->mem->regions[0]));
> + dev->mem->nregions = 1;
> + dev->mem->regions[0].guest_phys_addr = (long)dev->buf;
> + dev->mem->regions[0].userspace_addr = (long)dev->buf;
> + dev->mem->regions[0].memory_size = dev->buf_size;
> +
> + r = ioctl(dev->control, VHOST_SET_MEM_TABLE, dev->mem);
> + assert(r >= 0);
> +
> + r = ioctl(dev->control, VHOST_SET_FEATURES, &features);
> + assert(r >= 0);
> +
> + dev->nvqs = 2;
> +}
> +
> +static void wait_for_interrupt(struct vq_info *vq)
> +{
> + unsigned long long val;
> +
> + poll(&vq->fds, 1, -1);
> +
> + if (vq->fds.revents & POLLIN)
> + read(vq->fds.fd, &val, sizeof(val));
> +}
> +
> +static void verify_res_buf(char *res_buf)
> +{
> + int i;
> +
> + for (i = ETHER_HDR_LEN; i < TEST_BUF_LEN; i++)
> + assert(res_buf[i] == (char)i);
> +}
> +
> +static void run_tx_test(struct vdev_info *dev, struct vq_info *vq,
> + bool delayed, int batch, int bufs)
It might be better to describe the test design briefly above as a
comment. Or we can start from simple test logic and add sophisticated
ones on top.
> +{
> + const bool random_batch = batch == RANDOM_BATCH;
> + long long spurious = 0;
> + struct scatterlist sl;
> + unsigned int len;
> + int r;
> +
> + for (;;) {
> + long started_before = vq->started;
> + long completed_before = vq->completed;
> +
> + virtqueue_disable_cb(vq->vq);
> + do {
> + if (random_batch)
> + batch = (random() % vq->vring.num) + 1;
> +
> + while (vq->started < bufs &&
> + (vq->started - vq->completed) < batch) {
> + sg_init_one(&sl, dev->test_buf, HDR_LEN + TEST_BUF_LEN);
> + r = virtqueue_add_outbuf(vq->vq, &sl, 1,
> + dev->test_buf + vq->started,
> + GFP_ATOMIC);
> + if (unlikely(r != 0)) {
> + if (r == -ENOSPC &&
> + vq->started > started_before)
> + r = 0;
> + else
> + r = -1;
> + break;
> + }
> +
> + ++vq->started;
> +
> + if (unlikely(!virtqueue_kick(vq->vq))) {
> + r = -1;
> + break;
> + }
> + }
> +
> + if (vq->started >= bufs)
> + r = -1;
> +
> + /* Flush out completed bufs if any */
> + while (virtqueue_get_buf(vq->vq, &len)) {
> + int n;
> +
> + n = recvfrom(dev->sock, dev->res_buf, TEST_BUF_LEN, 0, NULL, NULL);
> + assert(n == TEST_BUF_LEN);
> + verify_res_buf(dev->res_buf);
> +
> + ++vq->completed;
> + r = 0;
> + }
> + } while (r == 0);
> +
> + if (vq->completed == completed_before && vq->started == started_before)
> + ++spurious;
> +
> + assert(vq->completed <= bufs);
> + assert(vq->started <= bufs);
> + if (vq->completed == bufs)
> + break;
> +
> + if (delayed) {
> + if (virtqueue_enable_cb_delayed(vq->vq))
> + wait_for_interrupt(vq);
> + } else {
> + if (virtqueue_enable_cb(vq->vq))
> + wait_for_interrupt(vq);
> + }
> + }
> + printf("TX spurious wakeups: 0x%llx started=0x%lx completed=0x%lx\n",
> + spurious, vq->started, vq->completed);
> +}
> +
> +static void run_rx_test(struct vdev_info *dev, struct vq_info *vq,
> + bool delayed, int batch, int bufs)
> +{
> + const bool random_batch = batch == RANDOM_BATCH;
> + long long spurious = 0;
> + struct scatterlist sl;
> + unsigned int len;
> + int r;
> +
> + for (;;) {
> + long started_before = vq->started;
> + long completed_before = vq->completed;
> +
> + do {
> + if (random_batch)
> + batch = (random() % vq->vring.num) + 1;
> +
> + while (vq->started < bufs &&
> + (vq->started - vq->completed) < batch) {
> + sg_init_one(&sl, dev->res_buf, HDR_LEN + TEST_BUF_LEN);
> +
> + r = virtqueue_add_inbuf(vq->vq, &sl, 1,
> + dev->res_buf + vq->started,
> + GFP_ATOMIC);
> + if (unlikely(r != 0)) {
> + if (r == -ENOSPC &&
Drivers usually maintain a #free_slots, this can help to avoid the
trick for checking ENOSPC?
> + vq->started > started_before)
> + r = 0;
> + else
> + r = -1;
> + break;
> + }
> +
> + ++vq->started;
> +
> + vdev_send_packet(dev);
> +
> + if (unlikely(!virtqueue_kick(vq->vq))) {
> + r = -1;
> + break;
> + }
> + }
> +
> + if (vq->started >= bufs)
> + r = -1;
> +
> + /* Flush out completed bufs if any */
> + while (virtqueue_get_buf(vq->vq, &len)) {
> + struct ether_header *eh;
> +
> + eh = (struct ether_header *)(dev->res_buf + HDR_LEN);
> +
> + /* tun netdev is up and running, ignore the
> + * non-TEST_PTYPE packet.
> + */
I wonder if it's better to set up some kind of qdisc to avoid the
unexpected packet here, or is it too complicated?
Thanks
On 2024/2/2 12:05, Jason Wang wrote:
> On Tue, Jan 30, 2024 at 7:38 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>>
>> introduce vhost_net_test basing on virtio_test to test
>> vhost_net changing in the kernel.
>
> Let's describe what kind of test is being done and how it is done here.
How about something like below:
This patch introduces testing for both vhost_net tx and rx.
Steps for vhost_net tx testing:
1. Prepare a out buf
2. Kick the vhost_net to do tx processing
3. Do the receiving in the tun side
4. verify the data received by tun is correct
Steps for vhost_net rx testing::
1. Prepare a in buf
2. Do the sending in the tun side
3. Kick the vhost_net to do rx processing
4. verify the data received by vhost_net is correct
>> +
>> +static int tun_alloc(struct vdev_info *dev)
>> +{
>> + struct ifreq ifr;
>> + int len = HDR_LEN;
>
> Any reason you can't just use the virtio_net uapi?
I didn't find a macro for that in include/uapi/linux/virtio_net.h.
Did you mean using something like below?
sizeof(struct virtio_net_hdr_mrg_rxbuf)
>
>> + int fd, e;
>> +
>> + fd = open("/dev/net/tun", O_RDWR);
>> + if (fd < 0) {
>> + perror("Cannot open /dev/net/tun");
>> + return fd;
>> + }
>> +
>> + memset(&ifr, 0, sizeof(ifr));
>> +
>> + ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
>> + snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
>> +
>> + e = ioctl(fd, TUNSETIFF, &ifr);
>> + if (e < 0) {
>> + perror("ioctl[TUNSETIFF]");
>> + close(fd);
>> + return e;
>> + }
>> +
>> + e = ioctl(fd, TUNSETVNETHDRSZ, &len);
>> + if (e < 0) {
>> + perror("ioctl[TUNSETVNETHDRSZ]");
>> + close(fd);
>> + return e;
>> + }
>> +
>> + e = ioctl(fd, SIOCGIFHWADDR, &ifr);
>> + if (e < 0) {
>> + perror("ioctl[SIOCGIFHWADDR]");
>> + close(fd);
>> + return e;
>> + }
>> +
>> + memcpy(dev->mac, &ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN);
>> + return fd;
>> +}
>> +
>> +static void vdev_create_socket(struct vdev_info *dev)
>> +{
>> + struct ifreq ifr;
>> +
>> + dev->sock = socket(AF_PACKET, SOCK_RAW, htons(TEST_PTYPE));
>> + assert(dev->sock != -1);
>> +
>> + snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
>
> Nit: it might be better to accept the device name instead of repeating
> the snprintf trick here, this would facilitate the future changes.
I am not sure I understand what did you mean by "accept the device name"
here.
The above is used to get ifindex of the tun netdevice created in
tun_alloc(), so that we can use it in vdev_send_packet() to send
a packet using the tun netdevice created in tun_alloc(). Is there
anything obvious I missed here?
>
>> + assert(ioctl(dev->sock, SIOCGIFINDEX, &ifr) >= 0);
>> +
>> + dev->ifindex = ifr.ifr_ifindex;
>> +
>> + /* Set the flags that bring the device up */
>> + assert(ioctl(dev->sock, SIOCGIFFLAGS, &ifr) >= 0);
>> + ifr.ifr_flags |= (IFF_UP | IFF_RUNNING);
>> + assert(ioctl(dev->sock, SIOCSIFFLAGS, &ifr) >= 0);
>> +}
>> +
>> +static void vdev_send_packet(struct vdev_info *dev)
>> +{
>> + char *sendbuf = dev->test_buf + HDR_LEN;
>> + struct sockaddr_ll saddrll = {0};
>> + int sockfd = dev->sock;
>> + int ret;
>> +
>> + saddrll.sll_family = PF_PACKET;
>> + saddrll.sll_ifindex = dev->ifindex;
>> + saddrll.sll_halen = ETH_ALEN;
>> + saddrll.sll_protocol = htons(TEST_PTYPE);
>> +
>> + ret = sendto(sockfd, sendbuf, TEST_BUF_LEN, 0,
>> + (struct sockaddr *)&saddrll,
>> + sizeof(struct sockaddr_ll));
>> + assert(ret >= 0);
>> +}
>> +
...
>> +
>> +static void vq_info_add(struct vdev_info *dev, int idx, int num, int fd)
>> +{
>> + struct vhost_vring_file backend = { .index = idx, .fd = fd };
>> + struct vq_info *info = &dev->vqs[idx];
>> + int r;
>> +
>> + info->idx = idx;
>> + info->kick = eventfd(0, EFD_NONBLOCK);
>> + info->call = eventfd(0, EFD_NONBLOCK);
>
> If we don't care about the callback, let's just avoid to set the call here?
>
> (As I see vq_callback is a NULL)
Sure, will remove the vq_callback related code.
>
>> + r = posix_memalign(&info->ring, 4096, vring_size(num, 4096));
>> + assert(r >= 0);
>> + vq_reset(info, num, &dev->vdev);
>> + vhost_vq_setup(dev, info);
>> + info->fds.fd = info->call;
>> + info->fds.events = POLLIN;
>> +
>> + r = ioctl(dev->control, VHOST_NET_SET_BACKEND, &backend);
>> + assert(!r);
>> +}
>> +
>> +static void vdev_info_init(struct vdev_info *dev, unsigned long long features)
>> +{
>> + struct ether_header *eh;
>> + int i, r;
>> +
>> + dev->vdev.features = features;
>> + INIT_LIST_HEAD(&dev->vdev.vqs);
>> + spin_lock_init(&dev->vdev.vqs_list_lock);
>> +
>> + dev->buf_size = (HDR_LEN + TEST_BUF_LEN) * 2;
>> + dev->buf = malloc(dev->buf_size);
>> + assert(dev->buf);
>> + dev->test_buf = dev->buf;
>> + dev->res_buf = dev->test_buf + HDR_LEN + TEST_BUF_LEN;
>> +
>> + memset(dev->test_buf, 0, HDR_LEN + TEST_BUF_LEN);
>> + eh = (struct ether_header *)(dev->test_buf + HDR_LEN);
>> + eh->ether_type = htons(TEST_PTYPE);
>> + memcpy(eh->ether_dhost, dev->mac, ETHER_ADDR_LEN);
>> + memcpy(eh->ether_shost, dev->mac, ETHER_ADDR_LEN);
>> +
>> + for (i = sizeof(*eh); i < TEST_BUF_LEN; i++)
>> + dev->test_buf[i + HDR_LEN] = (char)i;
>> +
>> + dev->control = open("/dev/vhost-net", O_RDWR);
>> + assert(dev->control >= 0);
>> +
>> + r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
>> + assert(r >= 0);
>> +
>> + dev->mem = malloc(offsetof(struct vhost_memory, regions) +
>> + sizeof(dev->mem->regions[0]));
>> + assert(dev->mem);
>> + memset(dev->mem, 0, offsetof(struct vhost_memory, regions) +
>> + sizeof(dev->mem->regions[0]));
>> + dev->mem->nregions = 1;
>> + dev->mem->regions[0].guest_phys_addr = (long)dev->buf;
>> + dev->mem->regions[0].userspace_addr = (long)dev->buf;
>> + dev->mem->regions[0].memory_size = dev->buf_size;
>> +
>> + r = ioctl(dev->control, VHOST_SET_MEM_TABLE, dev->mem);
>> + assert(r >= 0);
>> +
>> + r = ioctl(dev->control, VHOST_SET_FEATURES, &features);
>> + assert(r >= 0);
>> +
>> + dev->nvqs = 2;
>> +}
>> +
>> +static void wait_for_interrupt(struct vq_info *vq)
>> +{
>> + unsigned long long val;
>> +
>> + poll(&vq->fds, 1, -1);
>> +
>> + if (vq->fds.revents & POLLIN)
>> + read(vq->fds.fd, &val, sizeof(val));
>> +}
>> +
>> +static void verify_res_buf(char *res_buf)
>> +{
>> + int i;
>> +
>> + for (i = ETHER_HDR_LEN; i < TEST_BUF_LEN; i++)
>> + assert(res_buf[i] == (char)i);
>> +}
>> +
>> +static void run_tx_test(struct vdev_info *dev, struct vq_info *vq,
>> + bool delayed, int batch, int bufs)
>
> It might be better to describe the test design briefly above as a
> comment. Or we can start from simple test logic and add sophisticated
> ones on top.
Does something described in the comment log as suggested by you make
sense to you?
Steps for vhost_net tx testing:
1. Prepare a out buf
2. Kick the vhost_net to do tx processing
3. Do the receiving in the tun side
4. verify the data received by tun is correct
>
>> +{
>> + const bool random_batch = batch == RANDOM_BATCH;
>> + long long spurious = 0;
>> + struct scatterlist sl;
>> + unsigned int len;
>> + int r;
>> +
>> + for (;;) {
>> + long started_before = vq->started;
>> + long completed_before = vq->completed;
>> +
>> + virtqueue_disable_cb(vq->vq);
>> + do {
>> + if (random_batch)
>> + batch = (random() % vq->vring.num) + 1;
>> +
>> + while (vq->started < bufs &&
>> + (vq->started - vq->completed) < batch) {
>> + sg_init_one(&sl, dev->test_buf, HDR_LEN + TEST_BUF_LEN);
>> + r = virtqueue_add_outbuf(vq->vq, &sl, 1,
>> + dev->test_buf + vq->started,
>> + GFP_ATOMIC);
>> + if (unlikely(r != 0)) {
>> + if (r == -ENOSPC &&
>> + vq->started > started_before)
>> + r = 0;
>> + else
>> + r = -1;
>> + break;
>> + }
>> +
>> + ++vq->started;
>> +
>> + if (unlikely(!virtqueue_kick(vq->vq))) {
>> + r = -1;
>> + break;
>> + }
>> + }
>> +
>> + if (vq->started >= bufs)
>> + r = -1;
>> +
>> + /* Flush out completed bufs if any */
>> + while (virtqueue_get_buf(vq->vq, &len)) {
>> + int n;
>> +
>> + n = recvfrom(dev->sock, dev->res_buf, TEST_BUF_LEN, 0, NULL, NULL);
>> + assert(n == TEST_BUF_LEN);
>> + verify_res_buf(dev->res_buf);
>> +
>> + ++vq->completed;
>> + r = 0;
>> + }
>> + } while (r == 0);
>> +
>> + if (vq->completed == completed_before && vq->started == started_before)
>> + ++spurious;
>> +
>> + assert(vq->completed <= bufs);
>> + assert(vq->started <= bufs);
>> + if (vq->completed == bufs)
>> + break;
>> +
>> + if (delayed) {
>> + if (virtqueue_enable_cb_delayed(vq->vq))
>> + wait_for_interrupt(vq);
>> + } else {
>> + if (virtqueue_enable_cb(vq->vq))
>> + wait_for_interrupt(vq);
>> + }
>> + }
>> + printf("TX spurious wakeups: 0x%llx started=0x%lx completed=0x%lx\n",
>> + spurious, vq->started, vq->completed);
>> +}
>> +
>> +static void run_rx_test(struct vdev_info *dev, struct vq_info *vq,
>> + bool delayed, int batch, int bufs)
>> +{
>> + const bool random_batch = batch == RANDOM_BATCH;
>> + long long spurious = 0;
>> + struct scatterlist sl;
>> + unsigned int len;
>> + int r;
>> +
>> + for (;;) {
>> + long started_before = vq->started;
>> + long completed_before = vq->completed;
>> +
>> + do {
>> + if (random_batch)
>> + batch = (random() % vq->vring.num) + 1;
>> +
>> + while (vq->started < bufs &&
>> + (vq->started - vq->completed) < batch) {
>> + sg_init_one(&sl, dev->res_buf, HDR_LEN + TEST_BUF_LEN);
>> +
>> + r = virtqueue_add_inbuf(vq->vq, &sl, 1,
>> + dev->res_buf + vq->started,
>> + GFP_ATOMIC);
>> + if (unlikely(r != 0)) {
>> + if (r == -ENOSPC &&
>
> Drivers usually maintain a #free_slots, this can help to avoid the
> trick for checking ENOSPC?
The above "(vq->started - vq->completed) < batch" seems to ensure that
the 'r' can't be '-ENOSPC'? We just need to ensure the batch <= desc_num,
and the 'r == -ENOSPC' checking seems to be unnecessary.
>
>> + vq->started > started_before)
>> + r = 0;
>> + else
>> + r = -1;
>> + break;
>> + }
>> +
>> + ++vq->started;
>> +
>> + vdev_send_packet(dev);
>> +
>> + if (unlikely(!virtqueue_kick(vq->vq))) {
>> + r = -1;
>> + break;
>> + }
>> + }
>> +
>> + if (vq->started >= bufs)
>> + r = -1;
>> +
>> + /* Flush out completed bufs if any */
>> + while (virtqueue_get_buf(vq->vq, &len)) {
>> + struct ether_header *eh;
>> +
>> + eh = (struct ether_header *)(dev->res_buf + HDR_LEN);
>> +
>> + /* tun netdev is up and running, ignore the
>> + * non-TEST_PTYPE packet.
>> + */
>
> I wonder if it's better to set up some kind of qdisc to avoid the
> unexpected packet here, or is it too complicated?
Yes, at least I don't know to do that yet.
>
> Thanks
>
> .
>
On Fri, Feb 2, 2024 at 8:24 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> On 2024/2/2 12:05, Jason Wang wrote:
> > On Tue, Jan 30, 2024 at 7:38 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
> >>
> >> introduce vhost_net_test basing on virtio_test to test
> >> vhost_net changing in the kernel.
> >
> > Let's describe what kind of test is being done and how it is done here.
>
> How about something like below:
>
> This patch introduces testing for both vhost_net tx and rx.
> Steps for vhost_net tx testing:
> 1. Prepare a out buf
> 2. Kick the vhost_net to do tx processing
> 3. Do the receiving in the tun side
> 4. verify the data received by tun is correct
>
> Steps for vhost_net rx testing::
> 1. Prepare a in buf
> 2. Do the sending in the tun side
> 3. Kick the vhost_net to do rx processing
> 4. verify the data received by vhost_net is correct
It looks like some important details were lost, e.g the logic for batching etc.
>
>
> >> +
> >> +static int tun_alloc(struct vdev_info *dev)
> >> +{
> >> + struct ifreq ifr;
> >> + int len = HDR_LEN;
> >
> > Any reason you can't just use the virtio_net uapi?
>
> I didn't find a macro for that in include/uapi/linux/virtio_net.h.
>
> Did you mean using something like below?
> sizeof(struct virtio_net_hdr_mrg_rxbuf)
Yes.
>
> >
> >> + int fd, e;
> >> +
> >> + fd = open("/dev/net/tun", O_RDWR);
> >> + if (fd < 0) {
> >> + perror("Cannot open /dev/net/tun");
> >> + return fd;
> >> + }
> >> +
> >> + memset(&ifr, 0, sizeof(ifr));
> >> +
> >> + ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
> >> + snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
> >> +
> >> + e = ioctl(fd, TUNSETIFF, &ifr);
> >> + if (e < 0) {
> >> + perror("ioctl[TUNSETIFF]");
> >> + close(fd);
> >> + return e;
> >> + }
> >> +
> >> + e = ioctl(fd, TUNSETVNETHDRSZ, &len);
> >> + if (e < 0) {
> >> + perror("ioctl[TUNSETVNETHDRSZ]");
> >> + close(fd);
> >> + return e;
> >> + }
> >> +
> >> + e = ioctl(fd, SIOCGIFHWADDR, &ifr);
> >> + if (e < 0) {
> >> + perror("ioctl[SIOCGIFHWADDR]");
> >> + close(fd);
> >> + return e;
> >> + }
> >> +
> >> + memcpy(dev->mac, &ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN);
> >> + return fd;
> >> +}
> >> +
> >> +static void vdev_create_socket(struct vdev_info *dev)
> >> +{
> >> + struct ifreq ifr;
> >> +
> >> + dev->sock = socket(AF_PACKET, SOCK_RAW, htons(TEST_PTYPE));
> >> + assert(dev->sock != -1);
> >> +
> >> + snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
> >
> > Nit: it might be better to accept the device name instead of repeating
> > the snprintf trick here, this would facilitate the future changes.
>
> I am not sure I understand what did you mean by "accept the device name"
> here.
>
> The above is used to get ifindex of the tun netdevice created in
> tun_alloc(), so that we can use it in vdev_send_packet() to send
> a packet using the tun netdevice created in tun_alloc(). Is there
> anything obvious I missed here?
I meant a const char *ifname for this function and let the caller to
pass the name.
>
> >
> >> + assert(ioctl(dev->sock, SIOCGIFINDEX, &ifr) >= 0);
> >> +
> >> + dev->ifindex = ifr.ifr_ifindex;
> >> +
> >> + /* Set the flags that bring the device up */
> >> + assert(ioctl(dev->sock, SIOCGIFFLAGS, &ifr) >= 0);
> >> + ifr.ifr_flags |= (IFF_UP | IFF_RUNNING);
> >> + assert(ioctl(dev->sock, SIOCSIFFLAGS, &ifr) >= 0);
> >> +}
> >> +
> >> +static void vdev_send_packet(struct vdev_info *dev)
> >> +{
> >> + char *sendbuf = dev->test_buf + HDR_LEN;
> >> + struct sockaddr_ll saddrll = {0};
> >> + int sockfd = dev->sock;
> >> + int ret;
> >> +
> >> + saddrll.sll_family = PF_PACKET;
> >> + saddrll.sll_ifindex = dev->ifindex;
> >> + saddrll.sll_halen = ETH_ALEN;
> >> + saddrll.sll_protocol = htons(TEST_PTYPE);
> >> +
> >> + ret = sendto(sockfd, sendbuf, TEST_BUF_LEN, 0,
> >> + (struct sockaddr *)&saddrll,
> >> + sizeof(struct sockaddr_ll));
> >> + assert(ret >= 0);
> >> +}
> >> +
>
> ...
>
> >> +
> >> +static void vq_info_add(struct vdev_info *dev, int idx, int num, int fd)
> >> +{
> >> + struct vhost_vring_file backend = { .index = idx, .fd = fd };
> >> + struct vq_info *info = &dev->vqs[idx];
> >> + int r;
> >> +
> >> + info->idx = idx;
> >> + info->kick = eventfd(0, EFD_NONBLOCK);
> >> + info->call = eventfd(0, EFD_NONBLOCK);
> >
> > If we don't care about the callback, let's just avoid to set the call here?
> >
> > (As I see vq_callback is a NULL)
>
> Sure, will remove the vq_callback related code.
>
> >
> >> + r = posix_memalign(&info->ring, 4096, vring_size(num, 4096));
> >> + assert(r >= 0);
> >> + vq_reset(info, num, &dev->vdev);
> >> + vhost_vq_setup(dev, info);
> >> + info->fds.fd = info->call;
> >> + info->fds.events = POLLIN;
> >> +
> >> + r = ioctl(dev->control, VHOST_NET_SET_BACKEND, &backend);
> >> + assert(!r);
> >> +}
> >> +
> >> +static void vdev_info_init(struct vdev_info *dev, unsigned long long features)
> >> +{
> >> + struct ether_header *eh;
> >> + int i, r;
> >> +
> >> + dev->vdev.features = features;
> >> + INIT_LIST_HEAD(&dev->vdev.vqs);
> >> + spin_lock_init(&dev->vdev.vqs_list_lock);
> >> +
> >> + dev->buf_size = (HDR_LEN + TEST_BUF_LEN) * 2;
> >> + dev->buf = malloc(dev->buf_size);
> >> + assert(dev->buf);
> >> + dev->test_buf = dev->buf;
> >> + dev->res_buf = dev->test_buf + HDR_LEN + TEST_BUF_LEN;
> >> +
> >> + memset(dev->test_buf, 0, HDR_LEN + TEST_BUF_LEN);
> >> + eh = (struct ether_header *)(dev->test_buf + HDR_LEN);
> >> + eh->ether_type = htons(TEST_PTYPE);
> >> + memcpy(eh->ether_dhost, dev->mac, ETHER_ADDR_LEN);
> >> + memcpy(eh->ether_shost, dev->mac, ETHER_ADDR_LEN);
> >> +
> >> + for (i = sizeof(*eh); i < TEST_BUF_LEN; i++)
> >> + dev->test_buf[i + HDR_LEN] = (char)i;
> >> +
> >> + dev->control = open("/dev/vhost-net", O_RDWR);
> >> + assert(dev->control >= 0);
> >> +
> >> + r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
> >> + assert(r >= 0);
> >> +
> >> + dev->mem = malloc(offsetof(struct vhost_memory, regions) +
> >> + sizeof(dev->mem->regions[0]));
> >> + assert(dev->mem);
> >> + memset(dev->mem, 0, offsetof(struct vhost_memory, regions) +
> >> + sizeof(dev->mem->regions[0]));
> >> + dev->mem->nregions = 1;
> >> + dev->mem->regions[0].guest_phys_addr = (long)dev->buf;
> >> + dev->mem->regions[0].userspace_addr = (long)dev->buf;
> >> + dev->mem->regions[0].memory_size = dev->buf_size;
> >> +
> >> + r = ioctl(dev->control, VHOST_SET_MEM_TABLE, dev->mem);
> >> + assert(r >= 0);
> >> +
> >> + r = ioctl(dev->control, VHOST_SET_FEATURES, &features);
> >> + assert(r >= 0);
> >> +
> >> + dev->nvqs = 2;
> >> +}
> >> +
> >> +static void wait_for_interrupt(struct vq_info *vq)
> >> +{
> >> + unsigned long long val;
> >> +
> >> + poll(&vq->fds, 1, -1);
> >> +
> >> + if (vq->fds.revents & POLLIN)
> >> + read(vq->fds.fd, &val, sizeof(val));
> >> +}
> >> +
> >> +static void verify_res_buf(char *res_buf)
> >> +{
> >> + int i;
> >> +
> >> + for (i = ETHER_HDR_LEN; i < TEST_BUF_LEN; i++)
> >> + assert(res_buf[i] == (char)i);
> >> +}
> >> +
> >> +static void run_tx_test(struct vdev_info *dev, struct vq_info *vq,
> >> + bool delayed, int batch, int bufs)
> >
> > It might be better to describe the test design briefly above as a
> > comment. Or we can start from simple test logic and add sophisticated
> > ones on top.
>
> Does something described in the comment log as suggested by you make
> sense to you?
> Steps for vhost_net tx testing:
> 1. Prepare a out buf
> 2. Kick the vhost_net to do tx processing
> 3. Do the receiving in the tun side
> 4. verify the data received by tun is correct
See my reply above.
>
> >
> >> +{
> >> + const bool random_batch = batch == RANDOM_BATCH;
> >> + long long spurious = 0;
> >> + struct scatterlist sl;
> >> + unsigned int len;
> >> + int r;
> >> +
> >> + for (;;) {
> >> + long started_before = vq->started;
> >> + long completed_before = vq->completed;
> >> +
> >> + virtqueue_disable_cb(vq->vq);
> >> + do {
> >> + if (random_batch)
> >> + batch = (random() % vq->vring.num) + 1;
> >> +
> >> + while (vq->started < bufs &&
> >> + (vq->started - vq->completed) < batch) {
> >> + sg_init_one(&sl, dev->test_buf, HDR_LEN + TEST_BUF_LEN);
> >> + r = virtqueue_add_outbuf(vq->vq, &sl, 1,
> >> + dev->test_buf + vq->started,
> >> + GFP_ATOMIC);
> >> + if (unlikely(r != 0)) {
> >> + if (r == -ENOSPC &&
> >> + vq->started > started_before)
> >> + r = 0;
> >> + else
> >> + r = -1;
> >> + break;
> >> + }
> >> +
> >> + ++vq->started;
> >> +
> >> + if (unlikely(!virtqueue_kick(vq->vq))) {
> >> + r = -1;
> >> + break;
> >> + }
> >> + }
> >> +
> >> + if (vq->started >= bufs)
> >> + r = -1;
> >> +
> >> + /* Flush out completed bufs if any */
> >> + while (virtqueue_get_buf(vq->vq, &len)) {
> >> + int n;
> >> +
> >> + n = recvfrom(dev->sock, dev->res_buf, TEST_BUF_LEN, 0, NULL, NULL);
> >> + assert(n == TEST_BUF_LEN);
> >> + verify_res_buf(dev->res_buf);
> >> +
> >> + ++vq->completed;
> >> + r = 0;
> >> + }
> >> + } while (r == 0);
> >> +
> >> + if (vq->completed == completed_before && vq->started == started_before)
> >> + ++spurious;
> >> +
> >> + assert(vq->completed <= bufs);
> >> + assert(vq->started <= bufs);
> >> + if (vq->completed == bufs)
> >> + break;
> >> +
> >> + if (delayed) {
> >> + if (virtqueue_enable_cb_delayed(vq->vq))
> >> + wait_for_interrupt(vq);
> >> + } else {
> >> + if (virtqueue_enable_cb(vq->vq))
> >> + wait_for_interrupt(vq);
> >> + }
> >> + }
> >> + printf("TX spurious wakeups: 0x%llx started=0x%lx completed=0x%lx\n",
> >> + spurious, vq->started, vq->completed);
> >> +}
> >> +
> >> +static void run_rx_test(struct vdev_info *dev, struct vq_info *vq,
> >> + bool delayed, int batch, int bufs)
> >> +{
> >> + const bool random_batch = batch == RANDOM_BATCH;
> >> + long long spurious = 0;
> >> + struct scatterlist sl;
> >> + unsigned int len;
> >> + int r;
> >> +
> >> + for (;;) {
> >> + long started_before = vq->started;
> >> + long completed_before = vq->completed;
> >> +
> >> + do {
> >> + if (random_batch)
> >> + batch = (random() % vq->vring.num) + 1;
> >> +
> >> + while (vq->started < bufs &&
> >> + (vq->started - vq->completed) < batch) {
> >> + sg_init_one(&sl, dev->res_buf, HDR_LEN + TEST_BUF_LEN);
> >> +
> >> + r = virtqueue_add_inbuf(vq->vq, &sl, 1,
> >> + dev->res_buf + vq->started,
> >> + GFP_ATOMIC);
> >> + if (unlikely(r != 0)) {
> >> + if (r == -ENOSPC &&
> >
> > Drivers usually maintain a #free_slots, this can help to avoid the
> > trick for checking ENOSPC?
>
> The above "(vq->started - vq->completed) < batch" seems to ensure that
> the 'r' can't be '-ENOSPC'?
Well, if this is true any reason we still check ENOSPEC here?
> We just need to ensure the batch <= desc_num,
> and the 'r == -ENOSPC' checking seems to be unnecessary.
>
> >
> >> + vq->started > started_before)
> >> + r = 0;
> >> + else
> >> + r = -1;
> >> + break;
> >> + }
> >> +
> >> + ++vq->started;
> >> +
> >> + vdev_send_packet(dev);
> >> +
> >> + if (unlikely(!virtqueue_kick(vq->vq))) {
> >> + r = -1;
> >> + break;
> >> + }
> >> + }
> >> +
> >> + if (vq->started >= bufs)
> >> + r = -1;
> >> +
> >> + /* Flush out completed bufs if any */
> >> + while (virtqueue_get_buf(vq->vq, &len)) {
> >> + struct ether_header *eh;
> >> +
> >> + eh = (struct ether_header *)(dev->res_buf + HDR_LEN);
> >> +
> >> + /* tun netdev is up and running, ignore the
> >> + * non-TEST_PTYPE packet.
> >> + */
> >
> > I wonder if it's better to set up some kind of qdisc to avoid the
> > unexpected packet here, or is it too complicated?
>
> Yes, at least I don't know to do that yet.
For example, the blackhole qdisc?
Thanks
On 2024/2/4 9:30, Jason Wang wrote:
> On Fri, Feb 2, 2024 at 8:24 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>>
>> On 2024/2/2 12:05, Jason Wang wrote:
>>> On Tue, Jan 30, 2024 at 7:38 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>>>>
>>>> introduce vhost_net_test basing on virtio_test to test
>>>> vhost_net changing in the kernel.
>>>
>>> Let's describe what kind of test is being done and how it is done here.
>>
>> How about something like below:
>>
>> This patch introduces testing for both vhost_net tx and rx.
>> Steps for vhost_net tx testing:
>> 1. Prepare a out buf
>> 2. Kick the vhost_net to do tx processing
>> 3. Do the receiving in the tun side
>> 4. verify the data received by tun is correct
>>
>> Steps for vhost_net rx testing::
>> 1. Prepare a in buf
>> 2. Do the sending in the tun side
>> 3. Kick the vhost_net to do rx processing
>> 4. verify the data received by vhost_net is correct
>
> It looks like some important details were lost, e.g the logic for batching etc.
I am supposeing you are referring to the virtio desc batch handling，
right?
It was a copy & paste code of virtio_test.c, I was thinking about removing
the virtio desc batch handling for now, as this patchset does not require
that to do the testing, it mainly depend on the "sock->sk->sk_sndbuf" to
be INT_MAX to call vhost_net_build_xdp(), which seems to be the default
case for vhost_net.
>
>>
...
>>>> +static void vdev_create_socket(struct vdev_info *dev)
>>>> +{
>>>> + struct ifreq ifr;
>>>> +
>>>> + dev->sock = socket(AF_PACKET, SOCK_RAW, htons(TEST_PTYPE));
>>>> + assert(dev->sock != -1);
>>>> +
>>>> + snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
>>>
>>> Nit: it might be better to accept the device name instead of repeating
>>> the snprintf trick here, this would facilitate the future changes.
>>
>> I am not sure I understand what did you mean by "accept the device name"
>> here.
>>
>> The above is used to get ifindex of the tun netdevice created in
>> tun_alloc(), so that we can use it in vdev_send_packet() to send
>> a packet using the tun netdevice created in tun_alloc(). Is there
>> anything obvious I missed here?
>
> I meant a const char *ifname for this function and let the caller to
> pass the name.
Sure.
>
>>
>>>> +
>>>> +static void run_rx_test(struct vdev_info *dev, struct vq_info *vq,
>>>> + bool delayed, int batch, int bufs)
>>>> +{
>>>> + const bool random_batch = batch == RANDOM_BATCH;
>>>> + long long spurious = 0;
>>>> + struct scatterlist sl;
>>>> + unsigned int len;
>>>> + int r;
>>>> +
>>>> + for (;;) {
>>>> + long started_before = vq->started;
>>>> + long completed_before = vq->completed;
>>>> +
>>>> + do {
>>>> + if (random_batch)
>>>> + batch = (random() % vq->vring.num) + 1;
>>>> +
>>>> + while (vq->started < bufs &&
>>>> + (vq->started - vq->completed) < batch) {
>>>> + sg_init_one(&sl, dev->res_buf, HDR_LEN + TEST_BUF_LEN);
>>>> +
>>>> + r = virtqueue_add_inbuf(vq->vq, &sl, 1,
>>>> + dev->res_buf + vq->started,
>>>> + GFP_ATOMIC);
>>>> + if (unlikely(r != 0)) {
>>>> + if (r == -ENOSPC &&
>>>
>>> Drivers usually maintain a #free_slots, this can help to avoid the
>>> trick for checking ENOSPC?
>>
>> The above "(vq->started - vq->completed) < batch" seems to ensure that
>> the 'r' can't be '-ENOSPC'?
>
> Well, if this is true any reason we still check ENOSPEC here?
As mentioned above, It was a copy & paste code of virtio_test.c.
Will remove 'r == -ENOSPC' checking.
>
>> We just need to ensure the batch <= desc_num,
>> and the 'r == -ENOSPC' checking seems to be unnecessary.
>>
>>>
>>>> + vq->started > started_before)
>>>> + r = 0;
>>>> + else
>>>> + r = -1;
>>>> + break;
>>>> + }
>>>> +
>>>> + ++vq->started;
>>>> +
>>>> + vdev_send_packet(dev);
>>>> +
>>>> + if (unlikely(!virtqueue_kick(vq->vq))) {
>>>> + r = -1;
>>>> + break;
>>>> + }
>>>> + }
>>>> +
>>>> + if (vq->started >= bufs)
>>>> + r = -1;
>>>> +
>>>> + /* Flush out completed bufs if any */
>>>> + while (virtqueue_get_buf(vq->vq, &len)) {
>>>> + struct ether_header *eh;
>>>> +
>>>> + eh = (struct ether_header *)(dev->res_buf + HDR_LEN);
>>>> +
>>>> + /* tun netdev is up and running, ignore the
>>>> + * non-TEST_PTYPE packet.
>>>> + */
>>>
>>> I wonder if it's better to set up some kind of qdisc to avoid the
>>> unexpected packet here, or is it too complicated?
>>
>> Yes, at least I don't know to do that yet.
>
> For example, the blackhole qdisc?
It seems the blackhole_enqueue() just drop everything, which includes
the packet sent using the raw socket in vdev_send_packet()?
We may bypass qdisc for the raw socket in vdev_send_packet()，but it
means other caller may bypass qdisc, and even cook up a packet with
ethertype being ETH_P_LOOPBACK, there is part of the reason I added a
simple payload verification in verify_res_buf().
>
> Thanks
>
> .
>
On Sun, Feb 4, 2024 at 11:50 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> On 2024/2/4 9:30, Jason Wang wrote:
> > On Fri, Feb 2, 2024 at 8:24 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
> >>
> >> On 2024/2/2 12:05, Jason Wang wrote:
> >>> On Tue, Jan 30, 2024 at 7:38 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
> >>>>
> >>>> introduce vhost_net_test basing on virtio_test to test
> >>>> vhost_net changing in the kernel.
> >>>
> >>> Let's describe what kind of test is being done and how it is done here.
> >>
> >> How about something like below:
> >>
> >> This patch introduces testing for both vhost_net tx and rx.
> >> Steps for vhost_net tx testing:
> >> 1. Prepare a out buf
> >> 2. Kick the vhost_net to do tx processing
> >> 3. Do the receiving in the tun side
> >> 4. verify the data received by tun is correct
> >>
> >> Steps for vhost_net rx testing::
> >> 1. Prepare a in buf
> >> 2. Do the sending in the tun side
> >> 3. Kick the vhost_net to do rx processing
> >> 4. verify the data received by vhost_net is correct
> >
> > It looks like some important details were lost, e.g the logic for batching etc.
>
> I am supposeing you are referring to the virtio desc batch handling，
> right?
Yes.
>
> It was a copy & paste code of virtio_test.c, I was thinking about removing
> the virtio desc batch handling for now, as this patchset does not require
> that to do the testing, it mainly depend on the "sock->sk->sk_sndbuf" to
> be INT_MAX to call vhost_net_build_xdp(), which seems to be the default
> case for vhost_net.
Ok.
>
> >
> >>
>
> ...
>
> >>>> +static void vdev_create_socket(struct vdev_info *dev)
> >>>> +{
> >>>> + struct ifreq ifr;
> >>>> +
> >>>> + dev->sock = socket(AF_PACKET, SOCK_RAW, htons(TEST_PTYPE));
> >>>> + assert(dev->sock != -1);
> >>>> +
> >>>> + snprintf(ifr.ifr_name, IFNAMSIZ, "tun_%d", getpid());
> >>>
> >>> Nit: it might be better to accept the device name instead of repeating
> >>> the snprintf trick here, this would facilitate the future changes.
> >>
> >> I am not sure I understand what did you mean by "accept the device name"
> >> here.
> >>
> >> The above is used to get ifindex of the tun netdevice created in
> >> tun_alloc(), so that we can use it in vdev_send_packet() to send
> >> a packet using the tun netdevice created in tun_alloc(). Is there
> >> anything obvious I missed here?
> >
> > I meant a const char *ifname for this function and let the caller to
> > pass the name.
>
> Sure.
>
> >
> >>
>
> >>>> +
> >>>> +static void run_rx_test(struct vdev_info *dev, struct vq_info *vq,
> >>>> + bool delayed, int batch, int bufs)
> >>>> +{
> >>>> + const bool random_batch = batch == RANDOM_BATCH;
> >>>> + long long spurious = 0;
> >>>> + struct scatterlist sl;
> >>>> + unsigned int len;
> >>>> + int r;
> >>>> +
> >>>> + for (;;) {
> >>>> + long started_before = vq->started;
> >>>> + long completed_before = vq->completed;
> >>>> +
> >>>> + do {
> >>>> + if (random_batch)
> >>>> + batch = (random() % vq->vring.num) + 1;
> >>>> +
> >>>> + while (vq->started < bufs &&
> >>>> + (vq->started - vq->completed) < batch) {
> >>>> + sg_init_one(&sl, dev->res_buf, HDR_LEN + TEST_BUF_LEN);
> >>>> +
> >>>> + r = virtqueue_add_inbuf(vq->vq, &sl, 1,
> >>>> + dev->res_buf + vq->started,
> >>>> + GFP_ATOMIC);
> >>>> + if (unlikely(r != 0)) {
> >>>> + if (r == -ENOSPC &&
> >>>
> >>> Drivers usually maintain a #free_slots, this can help to avoid the
> >>> trick for checking ENOSPC?
> >>
> >> The above "(vq->started - vq->completed) < batch" seems to ensure that
> >> the 'r' can't be '-ENOSPC'?
> >
> > Well, if this is true any reason we still check ENOSPEC here?
>
> As mentioned above, It was a copy & paste code of virtio_test.c.
> Will remove 'r == -ENOSPC' checking.
I see.
>
> >
> >> We just need to ensure the batch <= desc_num,
> >> and the 'r == -ENOSPC' checking seems to be unnecessary.
> >>
> >>>
> >>>> + vq->started > started_before)
> >>>> + r = 0;
> >>>> + else
> >>>> + r = -1;
> >>>> + break;
> >>>> + }
> >>>> +
> >>>> + ++vq->started;
> >>>> +
> >>>> + vdev_send_packet(dev);
> >>>> +
> >>>> + if (unlikely(!virtqueue_kick(vq->vq))) {
> >>>> + r = -1;
> >>>> + break;
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + if (vq->started >= bufs)
> >>>> + r = -1;
> >>>> +
> >>>> + /* Flush out completed bufs if any */
> >>>> + while (virtqueue_get_buf(vq->vq, &len)) {
> >>>> + struct ether_header *eh;
> >>>> +
> >>>> + eh = (struct ether_header *)(dev->res_buf + HDR_LEN);
> >>>> +
> >>>> + /* tun netdev is up and running, ignore the
> >>>> + * non-TEST_PTYPE packet.
> >>>> + */
> >>>
> >>> I wonder if it's better to set up some kind of qdisc to avoid the
> >>> unexpected packet here, or is it too complicated?
> >>
> >> Yes, at least I don't know to do that yet.
> >
> > For example, the blackhole qdisc?
>
> It seems the blackhole_enqueue() just drop everything, which includes
> the packet sent using the raw socket in vdev_send_packet()?
I vaguely remember there's a mode that af_packet will bypass the qdisc
but I might be wrong.
But rethink of this, though it facilitates the code but it introduces
unnecessary dependencies like black hole which seems to be suboptimal.
>
> We may bypass qdisc for the raw socket in vdev_send_packet()，but it
> means other caller may bypass qdisc, and even cook up a packet with
> ethertype being ETH_P_LOOPBACK, there is part of the reason I added a
> simple payload verification in verify_res_buf().
Fine.
Thanks
>
> >
> > Thanks
> >
> > .
> >
>
© 2016 - 2024 Red Hat, Inc.