On 2024/1/5 0:17, Eugenio Perez Martin wrote: > On Wed, Jan 3, 2024 at 11:00 AM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote: ... >> + >> +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, i; >> + >> + n = recvfrom(dev->sock, dev->res_buf, TEST_BUF_LEN, 0, NULL, NULL); >> + assert(n == TEST_BUF_LEN); >> + >> + for (i = ETHER_HDR_LEN; i < n; i++) >> + assert(dev->res_buf[i] == (char)i); >> + >> + ++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; >> + int i; >> + >> + 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); >> + >> + for (i = ETHER_HDR_LEN; i < TEST_BUF_LEN; i++) >> + assert(dev->res_buf[i + HDR_LEN] == (char)i); >> + >> + ++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); >> +} > > Hi! > > I think the tests are great! Maybe both run_rx_test and run_tx_test > (and virtio_test.c:run_test) can be merged into a common function with > some callbacks? Not sure if it will complicate the code more. I looked at it more closely, it seems using callback just make the code more complicated without obvious benefit, as the main steps is different for tx and rx. For tx: 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 For rx: 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 I could refactor out a common function to do the data verifying for step 4 of both tx and rx. For virtio_test.c:run_test, it does not seems to be using the vhost_net directly, it uses shim vhost_test.ko module, and it seems to only have step 1 & 3 of vhost_net_test' rx testing. The new vhost_net_test should be able to replace the virtio_test, I thought about deleting the virtio_test after adding the vhost_net_test, but I am not familiar enough with virtio to do the judgement yet. > > Thanks! >