From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 34F2D1B492 for ; Mon, 7 Jan 2019 11:30:53 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8AD2F85362; Mon, 7 Jan 2019 10:30:52 +0000 (UTC) Received: from [10.36.112.12] (unknown [10.36.112.12]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7F3F7608C1; Mon, 7 Jan 2019 10:30:49 +0000 (UTC) To: Jens Freimann , dev@dpdk.org Cc: tiwei.bie@intel.com References: <20181221152948.20714-1-jfreimann@redhat.com> From: Maxime Coquelin Message-ID: <19e9ef95-714e-7516-8479-2681318f5525@redhat.com> Date: Mon, 7 Jan 2019 11:30:47 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <20181221152948.20714-1-jfreimann@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 07 Jan 2019 10:30:52 +0000 (UTC) Subject: Re: [dpdk-dev] [RFC] virtio-user: ctrl vq support for packed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Jan 2019 10:30:53 -0000 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 > --- > 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