On Wed, Feb 17, 2016 at 03:02:28PM +0200, Victor Kaplansky wrote:
> This patch adds to the vubr test the scattering of incoming
> packets to the chain of RX buffer. Also, this patch corrects the
> size of the header preceding the packet in RX buffers.
>
> Note that this patch doesn't add the support for mergeable
> buffers.
>
> Signed-off-by: Victor Kaplansky <vict...@redhat.com>
> ---
> tests/vhost-user-bridge.c | 95
> +++++++++++++++++++++++++++++++----------------
> 1 file changed, 63 insertions(+), 32 deletions(-)
>
> diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
> index 9fb09f1d..7f746317 100644
> --- a/tests/vhost-user-bridge.c
> +++ b/tests/vhost-user-bridge.c
> @@ -47,6 +47,7 @@
> #include <arpa/inet.h>
> #include <ctype.h>
> #include <netdb.h>
> +#include <qemu/osdep.h>
>
> #include <linux/vhost.h>
>
> @@ -186,6 +187,8 @@ typedef struct VubrVirtq {
>
> #define VHOST_MEMORY_MAX_NREGIONS 8
> #define VHOST_USER_F_PROTOCOL_FEATURES 30
> +/* v1.0 compliant. */
> +#define VIRTIO_F_VERSION_1 32
>
> #define VHOST_LOG_PAGE 4096
>
> @@ -294,6 +297,7 @@ typedef struct VubrDev {
> struct sockaddr_in backend_udp_dest;
> int ready;
> uint64_t features;
> + int hdrlen;
> } VubrDev;
>
> static const char *vubr_request_str[] = {
> @@ -484,7 +488,8 @@ vubr_backend_udp_recvbuf(VubrDev *dev, uint8_t *buf,
> size_t buflen)
> static void
> vubr_consume_raw_packet(VubrDev *dev, uint8_t *buf, uint32_t len)
> {
> - int hdrlen = sizeof(struct virtio_net_hdr_v1);
> + int hdrlen = dev->hdrlen;
> + DPRINT(" hdrlen = %d\n", dev->hdrlen);
>
> if (VHOST_USER_BRIDGE_DEBUG) {
> print_buffer(buf, len);
> @@ -546,6 +551,7 @@ vubr_post_buffer(VubrDev *dev, VubrVirtq *vq, uint8_t
> *buf, int32_t len)
> struct vring_avail *avail = vq->avail;
> struct vring_used *used = vq->used;
> uint64_t log_guest_addr = vq->log_guest_addr;
> + int32_t remained_len = len;
>
> unsigned int size = vq->size;
>
> @@ -560,36 +566,49 @@ vubr_post_buffer(VubrDev *dev, VubrVirtq *vq, uint8_t
> *buf, int32_t len)
> uint16_t d_index = avail->ring[a_index];
>
> int i = d_index;
> + uint32_t written_len = 0;
>
> - DPRINT("Post packet to guest on vq:\n");
> - DPRINT(" size = %d\n", vq->size);
> - DPRINT(" last_avail_index = %d\n", vq->last_avail_index);
> - DPRINT(" last_used_index = %d\n", vq->last_used_index);
> - DPRINT(" a_index = %d\n", a_index);
> - DPRINT(" u_index = %d\n", u_index);
> - DPRINT(" d_index = %d\n", d_index);
> - DPRINT(" desc[%d].addr = 0x%016"PRIx64"\n", i, desc[i].addr);
> - DPRINT(" desc[%d].len = %d\n", i, desc[i].len);
> - DPRINT(" desc[%d].flags = %d\n", i, desc[i].flags);
> - DPRINT(" avail->idx = %d\n", avail_index);
> - DPRINT(" used->idx = %d\n", used->idx);
> -
> - if (!(desc[i].flags & VRING_DESC_F_WRITE)) {
> - /* FIXME: we should find writable descriptor. */
> - fprintf(stderr, "Error: descriptor is not writable. Exiting.\n");
> - exit(1);
> - }
> + do {
> + DPRINT("Post packet to guest on vq:\n");
> + DPRINT(" size = %d\n", vq->size);
> + DPRINT(" last_avail_index = %d\n", vq->last_avail_index);
> + DPRINT(" last_used_index = %d\n", vq->last_used_index);
> + DPRINT(" a_index = %d\n", a_index);
> + DPRINT(" u_index = %d\n", u_index);
> + DPRINT(" d_index = %d\n", d_index);
> + DPRINT(" desc[%d].addr = 0x%016"PRIx64"\n", i, desc[i].addr);
> + DPRINT(" desc[%d].len = %d\n", i, desc[i].len);
> + DPRINT(" desc[%d].flags = %d\n", i, desc[i].flags);
> + DPRINT(" avail->idx = %d\n", avail_index);
> + DPRINT(" used->idx = %d\n", used->idx);
> +
> + if (!(desc[i].flags & VRING_DESC_F_WRITE)) {
> + /* FIXME: we should find writable descriptor. */
> + fprintf(stderr, "Error: descriptor is not writable. Exiting.\n");
> + exit(1);
> + }
>
> - void *chunk_start = (void *)gpa_to_va(dev, desc[i].addr);
> - uint32_t chunk_len = desc[i].len;
> + void *chunk_start = (void *)gpa_to_va(dev, desc[i].addr);
> + uint32_t chunk_len = desc[i].len;
> + uint32_t chunk_write_len = MIN(remained_len, chunk_len);
>
> - if (len <= chunk_len) {
> - memcpy(chunk_start, buf, len);
> - vubr_log_write(dev, desc[i].addr, len);
> - } else {
> - fprintf(stderr,
> - "Received too long packet from the backend. Dropping...\n");
> - return;
> + memcpy(chunk_start, buf + written_len, chunk_write_len);
> + vubr_log_write(dev, desc[i].addr, chunk_write_len);
> + remained_len -= chunk_write_len;
> + written_len += chunk_write_len;
> +
> + if ((remained_len == 0) || !(desc[i].flags & VRING_DESC_F_NEXT)) {
> + break;
> + }
> +
> + i = desc[i].next;
> + } while (1);
> +
> + if (remained_len > 0) {
> + fprintf(stderr,
> + "Too long packet for RX, remained remained_len = %d,
> Dropping...\n",
> + remained_len);
remained_len -> remaining_len?
> + return;
> }
>
> /* Add descriptor to the used ring. */
> @@ -697,7 +716,7 @@ vubr_backend_recv_cb(int sock, void *ctx)
> VubrVirtq *rx_vq = &dev->vq[0];
> uint8_t buf[4096];
> struct virtio_net_hdr_v1 *hdr = (struct virtio_net_hdr_v1 *)buf;
> - int hdrlen = sizeof(struct virtio_net_hdr_v1);
> + int hdrlen = dev->hdrlen;
> int buflen = sizeof(buf);
> int len;
>
> @@ -706,6 +725,7 @@ vubr_backend_recv_cb(int sock, void *ctx)
> }
>
> DPRINT("\n\n *** IN UDP RECEIVE CALLBACK ***\n\n");
> + DPRINT(" hdrlen = %d\n", hdrlen);
>
> uint16_t avail_index = atomic_mb_read(&rx_vq->avail->idx);
>
> @@ -717,10 +737,12 @@ vubr_backend_recv_cb(int sock, void *ctx)
> return;
> }
>
> + memset(buf, 0, hdrlen);
> + /* TODO: support mergeable buffers. */
> + if (hdrlen == 12)
> + hdr->num_buffers = 1;
> len = vubr_backend_udp_recvbuf(dev, buf + hdrlen, buflen - hdrlen);
>
> - *hdr = (struct virtio_net_hdr_v1) { };
> - hdr->num_buffers = 1;
> vubr_post_buffer(dev, rx_vq, buf, len + hdrlen);
> }
>
> @@ -754,7 +776,8 @@ vubr_get_features_exec(VubrDev *dev, VhostUserMsg *vmsg)
> ((1ULL << VIRTIO_NET_F_MRG_RXBUF) |
> (1ULL << VHOST_F_LOG_ALL) |
> (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) |
> - (1ULL << VHOST_USER_F_PROTOCOL_FEATURES));
> + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES) |
> + (1ULL << VIRTIO_F_VERSION_1));
>
> vmsg->size = sizeof(vmsg->payload.u64);
>
I'm not sure you should advertize VIRTIO_F_VERSION_1 as long as
you don't also do CPU to LE/LE to CPU transformations
when it's negotiated.
Also, you must advertize ANY_LAYOUT with VIRTIO_F_VERSION_1.
> @@ -768,7 +791,15 @@ static int
> vubr_set_features_exec(VubrDev *dev, VhostUserMsg *vmsg)
> {
> DPRINT("u64: 0x%016"PRIx64"\n", vmsg->payload.u64);
> +
> dev->features = vmsg->payload.u64;
> + if ((dev->features & (1ULL << VIRTIO_F_VERSION_1)) ||
> + (dev->features & (1ULL << VIRTIO_NET_F_MRG_RXBUF))) {
> + dev->hdrlen = 12;
> + } else {
> + dev->hdrlen = 10;
> + }
> +
> return 0;
> }
>
> --
> Victor
