From: Tiwei Bie <tiwei.bie@intel.com>
To: Jens Freimann <jfreimann@redhat.com>
Cc: dev@dpdk.org, maxime.coquelin@redhat.com, Gavin.Hu@arm.com,
zhihong.wang@intel.com
Subject: Re: [dpdk-dev] [PATCH v6 06/11] net/virtio: implement transmit path for packed queues
Date: Fri, 21 Sep 2018 20:49:05 +0800 [thread overview]
Message-ID: <20180921124905.GA26178@debian> (raw)
In-Reply-To: <20180921123732.ftt7weewxxe753o2@jenstp.localdomain>
On Fri, Sep 21, 2018 at 02:37:32PM +0200, Jens Freimann wrote:
> On Fri, Sep 21, 2018 at 08:26:58PM +0800, Tiwei Bie wrote:
> > On Fri, Sep 21, 2018 at 12:33:03PM +0200, Jens Freimann wrote:
> > [...]
> > >
> > > static inline int
> > > -desc_is_used(struct vring_desc_packed *desc, struct vring *vr)
> > > +_desc_is_used(struct vring_desc_packed *desc)
> > > {
> > > uint16_t used, avail;
> > >
> > > used = !!(desc->flags & VRING_DESC_F_USED(1));
> > > avail = !!(desc->flags & VRING_DESC_F_AVAIL(1));
> > >
> > > - return used == avail && used == vr->used_wrap_counter;
> > > + return used == avail;
> > > +
> > > +}
> > > +
> > > +static inline int
> > > +desc_is_used(struct vring_desc_packed *desc, struct vring *vr)
> > > +{
> > > + uint16_t used;
> > > +
> > > + used = !!(desc->flags & VRING_DESC_F_USED(1));
> > > +
> > > + return _desc_is_used(desc) && used == vr->used_wrap_counter;
> > > }
> > >
> > > /* The standard layout for the ring is a continuous chunk of memory which
> > > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> > > index eb891433e..ea6300563 100644
> > > --- a/drivers/net/virtio/virtio_rxtx.c
> > > +++ b/drivers/net/virtio/virtio_rxtx.c
> > > @@ -38,6 +38,7 @@
> > > #define VIRTIO_DUMP_PACKET(m, len) do { } while (0)
> > > #endif
> > >
> > > +
> > > int
> > > virtio_dev_rx_queue_done(void *rxq, uint16_t offset)
> > > {
> > > @@ -165,6 +166,31 @@ virtqueue_dequeue_rx_inorder(struct virtqueue *vq,
> > > #endif
> > >
> > > /* Cleanup from completed transmits. */
> > > +static void
> > > +virtio_xmit_cleanup_packed(struct virtqueue *vq)
> > > +{
> > > + uint16_t idx;
> > > + uint16_t size = vq->vq_nentries;
> > > + struct vring_desc_packed *desc = vq->vq_ring.desc_packed;
> > > + struct vq_desc_extra *dxp;
> > > +
> > > + idx = vq->vq_used_cons_idx;
> > > + while (_desc_is_used(&desc[idx]) &&
> >
> > We can't just compare the AVAIL bit and USED bit to
> > check whether a desc is used.
>
> We can't compare with the current wrap counter value as well
> because it won't match the flags in descriptors. So check against
> used_wrap_counter ^= 1 then?
I haven't looked into this series yet, so I'm not sure what's
the best way to get the wrap-counter we need here. But, yes,
you need some way to get the wrap-counter we should use here.
> >
> > > + vq->vq_free_cnt < size) {
> > > + dxp = &vq->vq_descx[idx];
> >
> > The code is still assuming the descs will be written
> > back by device in order. The vq->vq_descx[] needs to
> > be managed e.g. as a list to support the out-of-order
> > processing. IOW, we can't assume vq->vq_descx[idx]
> > is corresponding to desc[idx] when device may write
> > back the descs out of order.
>
> I changed it to not assume this in other spots but missed this one. I
> will check more carefully and add code to make vq_descx entries a list.
After making it support the out-of-order, we may want to do
some performance test for the Tx path only. Because I suspect
we may not be able to get the expected performance improvements
in packed ring due to this when device is faster than driver.
Thanks
next prev parent reply other threads:[~2018-09-21 12:50 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-21 10:32 [dpdk-dev] [PATCH v6 00/11] implement packed virtqueues Jens Freimann
2018-09-21 10:32 ` [dpdk-dev] [PATCH v6 01/11] net/virtio: vring init for packed queues Jens Freimann
2018-09-21 10:32 ` [dpdk-dev] [PATCH v6 02/11] net/virtio: add packed virtqueue defines Jens Freimann
2018-09-21 10:33 ` [dpdk-dev] [PATCH v6 03/11] net/virtio: add packed virtqueue helpers Jens Freimann
2018-09-21 10:33 ` [dpdk-dev] [PATCH v6 04/11] net/virtio: flush packed receive virtqueues Jens Freimann
2018-09-21 10:33 ` [dpdk-dev] [PATCH v6 05/11] net/virtio: dump packed virtqueue data Jens Freimann
2018-09-21 10:33 ` [dpdk-dev] [PATCH v6 06/11] net/virtio: implement transmit path for packed queues Jens Freimann
2018-09-21 12:26 ` Tiwei Bie
2018-09-21 12:37 ` Jens Freimann
2018-09-21 12:49 ` Tiwei Bie [this message]
2018-09-21 10:33 ` [dpdk-dev] [PATCH v6 07/11] net/virtio: implement receive " Jens Freimann
2018-09-21 10:33 ` [dpdk-dev] [PATCH v6 08/11] net/virtio: add support for mergeable buffers with packed virtqueues Jens Freimann
2018-09-21 10:33 ` [dpdk-dev] [PATCH v6 09/11] net/virtio: add virtio send command packed queue support Jens Freimann
2018-09-21 12:37 ` Tiwei Bie
2018-09-21 10:33 ` [dpdk-dev] [PATCH v6 10/11] net/virtio-user: add option to use packed queues Jens Freimann
2018-09-21 10:33 ` [dpdk-dev] [PATCH v6 11/11] net/virtio: enable packed virtqueues by default Jens Freimann
2018-09-21 12:32 ` [dpdk-dev] [PATCH v6 00/11] implement packed virtqueues Tiwei Bie
2018-09-21 14:06 ` Jens Freimann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180921124905.GA26178@debian \
--to=tiwei.bie@intel.com \
--cc=Gavin.Hu@arm.com \
--cc=dev@dpdk.org \
--cc=jfreimann@redhat.com \
--cc=maxime.coquelin@redhat.com \
--cc=zhihong.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).