Hi Jens,
On 12/21/18 4:29 PM, Jens Freimann wrote:
> This adds support to virtio-user for control virtqueues
> and reverts commit "5e4e7a752 net/virtio-user: fail if
> cq used with packed vq".
>
> I add a struct virtio_user_queue to have a place for wrap
> counters and avail/used index (which is not needed for split
> ring because it has those in shared memory).
>
> This is a RFC because it only supports in-order use of descriptors
> in the ring. I'm looking for ideas how to change this without the
> .next field in the descriptor as we have it with split virtqueues.
>
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> ---
> drivers/net/virtio/virtio_ethdev.c | 11 +-
> .../net/virtio/virtio_user/virtio_user_dev.c | 101 ++++++++++++++++--
> .../net/virtio/virtio_user/virtio_user_dev.h | 14 ++-
> drivers/net/virtio/virtio_user_ethdev.c | 25 ++++-
> 4 files changed, 135 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 446c338fc..010ab6489 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -153,6 +153,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
> uint16_t flags;
> int sum = 0;
> int k;
> + int ndescs = 0;
>
> /*
> * Format is enforced in qemu code:
> @@ -162,7 +163,6 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
> */
> head = vq->vq_avail_idx;
> wrap_counter = vq->avail_wrap_counter;
> - desc[head].flags = VRING_DESC_F_NEXT;
> desc[head].addr = cvq->virtio_net_hdr_mem;
> desc[head].len = sizeof(struct virtio_net_ctrl_hdr);
> vq->vq_free_cnt--;
> @@ -170,6 +170,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
> vq->vq_avail_idx -= vq->vq_nentries;
> vq->avail_wrap_counter ^= 1;
> }
> + ndescs++;
>
> for (k = 0; k < pkt_num; k++) {
> desc[vq->vq_avail_idx].addr = cvq->virtio_net_hdr_mem
> @@ -188,6 +189,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
> vq->vq_avail_idx -= vq->vq_nentries;
> vq->avail_wrap_counter ^= 1;
> }
> + ndescs++;
> }
>
>
> @@ -198,6 +200,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
> flags |= VRING_DESC_F_AVAIL(vq->avail_wrap_counter) |
> VRING_DESC_F_USED(!vq->avail_wrap_counter);
> desc[vq->vq_avail_idx].flags = flags;
> +
> flags = VRING_DESC_F_NEXT;
> flags |= VRING_DESC_F_AVAIL(wrap_counter) |
> VRING_DESC_F_USED(!wrap_counter);
> @@ -209,6 +212,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
> vq->vq_avail_idx -= vq->vq_nentries;
> vq->avail_wrap_counter ^= 1;
> }
> + ndescs++;
>
> virtqueue_notify(vq);
>
> @@ -220,8 +224,9 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
>
> /* now get used descriptors */
> while (desc_is_used(&desc[vq->vq_used_cons_idx], vq)) {
> - vq->vq_free_cnt++;
> - if (++vq->vq_used_cons_idx >= vq->vq_nentries) {
> + vq->vq_free_cnt += ndescs;
> + vq->vq_used_cons_idx += ndescs;
> + if (vq->vq_used_cons_idx >= vq->vq_nentries) {
> vq->vq_used_cons_idx -= vq->vq_nentries;
> vq->used_wrap_counter ^= 1;
> }
I'm not sure why this change is needed.
Why not getting used descriptor one by one as it was before?
If it does not work, doesn't that mean that the desc chain is not
marked as used properly by the device?
Thanks,
Maxime
@@ -153,6 +153,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
uint16_t flags;
int sum = 0;
int k;
+ int ndescs = 0;
/*
* Format is enforced in qemu code:
@@ -162,7 +163,6 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
*/
head = vq->vq_avail_idx;
wrap_counter = vq->avail_wrap_counter;
- desc[head].flags = VRING_DESC_F_NEXT;
desc[head].addr = cvq->virtio_net_hdr_mem;
desc[head].len = sizeof(struct virtio_net_ctrl_hdr);
vq->vq_free_cnt--;
@@ -170,6 +170,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
vq->vq_avail_idx -= vq->vq_nentries;
vq->avail_wrap_counter ^= 1;
}
+ ndescs++;
for (k = 0; k < pkt_num; k++) {
desc[vq->vq_avail_idx].addr = cvq->virtio_net_hdr_mem
@@ -188,6 +189,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
vq->vq_avail_idx -= vq->vq_nentries;
vq->avail_wrap_counter ^= 1;
}
+ ndescs++;
}
@@ -198,6 +200,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
flags |= VRING_DESC_F_AVAIL(vq->avail_wrap_counter) |
VRING_DESC_F_USED(!vq->avail_wrap_counter);
desc[vq->vq_avail_idx].flags = flags;
+
flags = VRING_DESC_F_NEXT;
flags |= VRING_DESC_F_AVAIL(wrap_counter) |
VRING_DESC_F_USED(!wrap_counter);
@@ -209,6 +212,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
vq->vq_avail_idx -= vq->vq_nentries;
vq->avail_wrap_counter ^= 1;
}
+ ndescs++;
virtqueue_notify(vq);
@@ -220,8 +224,9 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
/* now get used descriptors */
while (desc_is_used(&desc[vq->vq_used_cons_idx], vq)) {
- vq->vq_free_cnt++;
- if (++vq->vq_used_cons_idx >= vq->vq_nentries) {
+ vq->vq_free_cnt += ndescs;
+ vq->vq_used_cons_idx += ndescs;
+ if (vq->vq_used_cons_idx >= vq->vq_nentries) {
vq->vq_used_cons_idx -= vq->vq_nentries;
vq->used_wrap_counter ^= 1;
}
@@ -43,15 +43,26 @@ virtio_user_kick_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
struct vhost_vring_file file;
struct vhost_vring_state state;
struct vring *vring = &dev->vrings[queue_sel];
+ struct vring_packed *pq_vring = &dev->packed_vrings[queue_sel];
struct vhost_vring_addr addr = {
.index = queue_sel,
- .desc_user_addr = (uint64_t)(uintptr_t)vring->desc,
- .avail_user_addr = (uint64_t)(uintptr_t)vring->avail,
- .used_user_addr = (uint64_t)(uintptr_t)vring->used,
.log_guest_addr = 0,
.flags = 0, /* disable log */
};
+ if (dev->features & (1ULL << VIRTIO_F_RING_PACKED)) {
+ addr.desc_user_addr =
+ (uint64_t)(uintptr_t)pq_vring->desc_packed;
+ addr.avail_user_addr =
+ (uint64_t)(uintptr_t)pq_vring->driver_event;
+ addr.used_user_addr =
+ (uint64_t)(uintptr_t)pq_vring->device_event;
+ } else {
+ addr.desc_user_addr = (uint64_t)(uintptr_t)vring->desc;
+ addr.avail_user_addr = (uint64_t)(uintptr_t)vring->avail;
+ addr.used_user_addr = (uint64_t)(uintptr_t)vring->used;
+ }
+
state.index = queue_sel;
state.num = vring->num;
dev->ops->send_request(dev, VHOST_USER_SET_VRING_NUM, &state);
@@ -468,16 +479,10 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
if (!in_order)
dev->unsupported_features |= (1ull << VIRTIO_F_IN_ORDER);
- if (packed_vq) {
- if (cq) {
- PMD_INIT_LOG(ERR, "control vq not supported yet with "
- "packed virtqueues\n");
- return -1;
- }
+ if (packed_vq)
dev->device_features |= (1ull << VIRTIO_F_RING_PACKED);
- } else {
+ else
dev->device_features &= ~(1ull << VIRTIO_F_RING_PACKED);
- }
if (dev->mac_specified) {
dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
@@ -624,6 +629,80 @@ virtio_user_handle_ctrl_msg(struct virtio_user_dev *dev, struct vring *vring,
return n_descs;
}
+static inline int
+desc_is_avail(struct vring_packed_desc *desc, bool wrap_counter)
+{
+ return wrap_counter == !!(desc->flags & VRING_DESC_F_AVAIL(1)) &&
+ wrap_counter != !!(desc->flags & VRING_DESC_F_USED(1));
+}
+
+static uint32_t
+virtio_user_handle_ctrl_msg_pq(struct virtio_user_dev *dev,
+ struct vring_packed *vring,
+ uint16_t idx_hdr)
+{
+ struct virtio_net_ctrl_hdr *hdr;
+ virtio_net_ctrl_ack status = ~0;
+ uint16_t i, idx_data, idx_status;
+ uint32_t n_descs = 0;
+
+ /* locate desc for header, data, and status */
+ idx_data = idx_hdr + 1;
+ n_descs++;
+
+ i = idx_data;
+ while (vring->desc_packed[i].flags & VRING_DESC_F_NEXT) {
+ i++;
+ n_descs++;
+ }
+
+ /* locate desc for status */
+ idx_status = i;
+ n_descs++;
+
+ hdr = (void *)(uintptr_t)vring->desc_packed[idx_hdr].addr;
+ if (hdr->class == VIRTIO_NET_CTRL_MQ &&
+ hdr->cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
+ uint16_t queues;
+
+ queues = *(uint16_t *)(uintptr_t)vring->desc_packed[idx_data].addr;
+ status = virtio_user_handle_mq(dev, queues);
+ }
+
+ /* Update status */
+ *(virtio_net_ctrl_ack *)(uintptr_t)vring->desc_packed[idx_status].addr = status;
+
+ return n_descs;
+}
+
+void
+virtio_user_handle_cq_packed(struct virtio_user_dev *dev, uint16_t queue_idx)
+{
+ struct virtio_user_queue *vq = &dev->queues[queue_idx];
+ struct vring_packed *vring = &dev->packed_vrings[queue_idx];
+ uint16_t id, n_descs;
+ bool wrap_counter = vq->used_wrap_counter;
+
+ while (desc_is_avail(&vring->desc_packed[vq->used_idx],
+ vq->avail_wrap_counter)) {
+ id = vring->desc_packed[vq->used_idx].id;
+
+ n_descs = virtio_user_handle_ctrl_msg_pq(dev, vring, id);
+
+ vring->desc_packed[vq->used_idx].len = n_descs;
+ vring->desc_packed[vq->used_idx].id = vq->used_idx;
+ vq->used_idx += n_descs;
+ if (vq->used_idx >= dev->queue_size) {
+ vq->used_idx -= dev->queue_size;
+ vq->used_wrap_counter ^= 1;
+ }
+ }
+ desc->flags |= VRING_DESC_F_AVAIL(wrap_counter) |
+ VRING_DESC_F_USED(wrap_counter);
+
+ vq->avail_idx = vq->used_idx;
+}
+
void
virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx)
{
@@ -11,6 +11,12 @@
#include "../virtio_ring.h"
#include "vhost.h"
+struct virtio_user_queue {
+ uint16_t avail_idx, used_idx;
+ uint16_t used_wrap_counter;
+ uint16_t avail_wrap_counter;
+};
+
struct virtio_user_dev {
/* for vhost_user backend */
int vhostfd;
@@ -39,7 +45,12 @@ struct virtio_user_dev {
uint16_t port_id;
uint8_t mac_addr[ETHER_ADDR_LEN];
char path[PATH_MAX];
- struct vring vrings[VIRTIO_MAX_VIRTQUEUES];
+ union {
+ struct vring vrings[VIRTIO_MAX_VIRTQUEUES];
+ struct vring_packed packed_vrings[VIRTIO_MAX_VIRTQUEUES];
+ };
+ struct virtio_user_queue queues[VIRTIO_MAX_VIRTQUEUES];
+
struct virtio_user_backend_ops *ops;
pthread_mutex_t mutex;
bool started;
@@ -53,5 +64,6 @@ int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
int mrg_rxbuf, int in_order, int packed_vq);
void virtio_user_dev_uninit(struct virtio_user_dev *dev);
void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
+void virtio_user_handle_cq_packed(struct virtio_user_dev *dev, uint16_t queue_idx);
uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
#endif
@@ -277,6 +277,26 @@ virtio_user_setup_queue(struct virtio_hw *hw, struct virtqueue *vq)
struct virtio_user_dev *dev = virtio_user_get_dev(hw);
uint16_t queue_idx = vq->vq_queue_index;
uint64_t desc_addr, avail_addr, used_addr;
+ struct virtio_user_queue *vq_user = &dev->queues[queue_idx];
+
+ if (vtpci_packed_queue(hw)) {
+ desc_addr = (uintptr_t)vq->vq_ring_virt_mem;
+ avail_addr = desc_addr + vq->vq_nentries *
+ sizeof(struct vring_packed_desc);
+ used_addr = RTE_ALIGN_CEIL(avail_addr +
+ sizeof(struct vring_packed_desc_event),
+ VIRTIO_PCI_VRING_ALIGN);
+ dev->packed_vrings[queue_idx].num = vq->vq_nentries;
+ dev->packed_vrings[queue_idx].desc_packed =
+ (void *)(uintptr_t)desc_addr;
+ dev->packed_vrings[queue_idx].driver_event =
+ (void *)(uintptr_t)avail_addr;
+ dev->packed_vrings[queue_idx].device_event =
+ (void*)(uintptr_t)used_addr;
+ vq_user->avail_wrap_counter = true;
+ vq_user->used_wrap_counter = true;
+ return 0;
+ }
desc_addr = (uintptr_t)vq->vq_ring_virt_mem;
avail_addr = desc_addr + vq->vq_nentries * sizeof(struct vring_desc);
@@ -317,7 +337,10 @@ virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
struct virtio_user_dev *dev = virtio_user_get_dev(hw);
if (hw->cvq && (hw->cvq->vq == vq)) {
- virtio_user_handle_cq(dev, vq->vq_queue_index);
+ if (vtpci_packed_queue(vq->hw))
+ virtio_user_handle_cq_packed(dev, vq->vq_queue_index);
+ else
+ virtio_user_handle_cq(dev, vq->vq_queue_index);
return;
}