| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACGkMEsKHOefArPd56RAYPsJE8kf=jGb6B-V6eNJiViCAD7GYA@mail.gmail.com>
Date: Tue, 6 Feb 2024 11:08:11 +0800
From: Jason Wang <jasowang@...hat.com>
To: Yunsheng Lin <linyunsheng@...wei.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
"Michael S. Tsirkin" <mst@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
virtualization@...ts.linux.dev
Subject: Re: [PATCH net-next v5 5/5] tools: virtio: introduce vhost_net_test
On Mon, Feb 5, 2024 at 8:46 PM Yunsheng Lin <linyunsheng@...wei.com> wrote:
>
> introduce vhost_net_test for both vhost_net tx and rx basing
> on virtio_test to test vhost_net changing in the kernel.
>
> 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.
>
> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>
> ---
> tools/virtio/.gitignore | 1 +
> tools/virtio/Makefile | 8 +-
> tools/virtio/linux/virtio_config.h | 4 +
> tools/virtio/vhost_net_test.c | 536 +++++++++++++++++++++++++++++
> 4 files changed, 546 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/linux/virtio_config.h b/tools/virtio/linux/virtio_config.h
> index 2a8a70e2a950..42a564f22f2d 100644
> --- a/tools/virtio/linux/virtio_config.h
> +++ b/tools/virtio/linux/virtio_config.h
> @@ -1,4 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef LINUX_VIRTIO_CONFIG_H
> +#define LINUX_VIRTIO_CONFIG_H
> #include <linux/virtio_byteorder.h>
> #include <linux/virtio.h>
> #include <uapi/linux/virtio_config.h>
> @@ -95,3 +97,5 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
> {
> return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
> }
> +
> +#endif
> diff --git a/tools/virtio/vhost_net_test.c b/tools/virtio/vhost_net_test.c
> new file mode 100644
> index 000000000000..6c41204e6707
> --- /dev/null
> +++ b/tools/virtio/vhost_net_test.c
> @@ -0,0 +1,536 @@
> +// 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/vhost.h>
> +#include <linux/if.h>
> +#include <linux/if_tun.h>
> +#include <linux/in.h>
> +#include <linux/if_packet.h>
> +#include <linux/virtio_net.h>
> +#include <netinet/ether.h>
> +
> +#define HDR_LEN sizeof(struct virtio_net_hdr_mrg_rxbuf)
> +#define TEST_BUF_LEN 256
> +#define TEST_PTYPE ETH_P_LOOPBACK
> +#define DESC_NUM 256
> +
> +/* 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, char *tun_name)
> +{
> + 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;
> + strncpy(ifr.ifr_name, tun_name, IFNAMSIZ);
> +
> + 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, char *tun_name)
> +{
> + struct ifreq ifr;
> +
> + dev->sock = socket(AF_PACKET, SOCK_RAW, htons(TEST_PTYPE));
> + assert(dev->sock != -1);
> +
> + strncpy(ifr.ifr_name, tun_name, IFNAMSIZ);
> + 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 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->vringavail,
> + .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);
> +}
> +
> +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, NULL, "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);
> + r = posix_memalign(&info->ring, 4096, vring_size(num, 4096));
> + assert(r >= 0);
> + vq_reset(info, num, &dev->vdev);
> + vhost_vq_setup(dev, info);
> +
> + 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);
It's not good to wait indefinitely.
> +
> + 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 bufs)
> +{
> + 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 {
> + while (vq->started < bufs &&
> + (vq->started - vq->completed) < 1) {
> + 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))
> + break;
> +
> + ++vq->started;
If we never decrease started/completed shouldn't we use unsigned here?
(as well as completed)
Otherwise we may get unexpected results for vq->started as well as
vq->completed.
> +
> + if (unlikely(!virtqueue_kick(vq->vq))) {
> + r = -1;
> + break;
> + }
> + }
> +
> + if (vq->started >= bufs)
> + r = -1;
Which condition do we reach here?
> +
> + /* 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);
> + }
This could be simplified with
if (delayed)
else
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 bufs)
> +{
> + long long spurious = 0;
> + struct scatterlist sl;
> + unsigned int len;
> + int r;
> +
> + for (;;) {
> + long started_before = vq->started;
> + long completed_before = vq->completed;
> +
> + do {
> + while (vq->started < bufs &&
> + (vq->started - vq->completed) < 1) {
> + 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))
> + 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);
Let's simply the logic here:
if (ether_type == htons()) {
assert()
verify_res_buf()
}
r = 0;
++vq->completed;
Others look good.
Thanks
Powered by blists - more mailing lists