Re: [dpdk-dev] [Bug 383] dpdk virtio_user lack of notifications make vhost_net+napi stops tx buffers

["Liu, Yong" <yong.liu@intel.com>]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of eperezma@redhat.com
> Sent: Thursday, January 09, 2020 11:56 PM
> To: bugzilla@dpdk.org; dev@dpdk.org; Maxime Coquelin <mcoqueli@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>; Michael S. Tsirkin <mst@redhat.com>;
> Adrian Moreno Zapata <amorenoz@redhat.com>
> Subject: Re: [dpdk-dev] [Bug 383] dpdk virtio_user lack of notifications
> make vhost_net+napi stops tx buffers
> 
> Proposal for patch - Requesting For Comments.
> 
> Just running the shadow copy-flush-call unconditionally in
> vhost_flush_dequeue_packed solve the issue, and it gives the best
> latency I can get in the tests (8101.959 trans/sec if I run netperf
> TCP_RR, 1331.735 trans/sec if I run TCP_STREAM at the same time). Apart
> from that, testpmd is able to tx about 820Kpps to the guest.
> 

Hi Eugenio,

Shadow method is aimed to maximum the throughput. In our experimental, there's no 
clear performance gain when shadowed size over half of the ring size (eg.256).

Unconditional do shadow flush will harm the performance a lot with virtio-user frontend.
Checking next descriptors will has less impact in performance, I prefer this solution.

Thanks,
Marvin


> However, to still do a little bit of batching I replace the condition
> for the one attached here. Although it implies a read barrier, I am
> able to achieve a little more of throughput (about 890Kpps), reducing
> to 8048.919 the numbers of transactions/sec in TCP_RR test (1372.327 if
> it runs in parallel with TCP_STREAM).
> 
> I also tried to move the vhost_flush_dequeue_shadow_packed and
> host_flush_dequeue_shadow_packed after the do_data_copy_dequeue in
> virtio_dev_tx_packed, more or less the same way virtio_dev_rx_packed
> do, but I repeatedly find less throughput in this case, even if I add
> the !next_desc_is_avail(vq) test. Not sure why, since both ways should
> be very similar. About 836Kpps are achieved this way, and TCP_RR is
> able to do 8120.154 trans/sec by itself and 1363.341 trans/sec if it
> runs with another TCP_STREAM test in parallel.
> 
> So, is there room for improvement, either in the patches or in the
> tests? Is one of the solutions preferred over another?
> 
> All tests were run with:
> * producer in a different processor than consumer (host testpmd and VM
> never run in the same core)
> * 256 descriptors queues in guest's testpmd and tx vq
> 
> Thanks!
> 
> PS: Sorry for the from mail address change, DPDK bugzilla doesn't send
> me the confirmation mail to this account.
> 
> diff --git a/lib/librte_vhost/virtio_net.c
> b/lib/librte_vhost/virtio_net.c
> index 21c311732..f7137149c 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -382,6 +382,20 @@ vhost_shadow_enqueue_single_packed(struct
> virtio_net *dev,
>         }
>  }
> 
> +static __rte_always_inline bool
> +next_desc_is_avail(const struct vhost_virtqueue *vq)
> +{
> +       bool wrap_counter = vq->avail_wrap_counter;
> +       uint16_t next_used_idx = vq->last_used_idx + 1;
> +
> +       if (next_used_idx >= vq->size) {
> +               next_used_idx -= vq->size;
> +               wrap_counter ^= 1;
> +       }
> +
> +       return desc_is_avail(&vq->desc_packed[next_used_idx],
> wrap_counter);
> +}
> +
>  static __rte_always_inline void
>  vhost_flush_dequeue_packed(struct virtio_net *dev,
>                            struct vhost_virtqueue *vq)
> @@ -394,7 +408,8 @@ vhost_flush_dequeue_packed(struct virtio_net *dev,
>         if (shadow_count <= 0)
>                 shadow_count += vq->size;
> 
> -       if ((uint32_t)shadow_count >= (vq->size - MAX_PKT_BURST)) {
> +       if ((uint32_t)shadow_count >= (vq->size - MAX_PKT_BURST)
> +               || !next_desc_is_avail(vq)) {
>                 do_data_copy_dequeue(vq);
>                 vhost_flush_dequeue_shadow_packed(dev, vq);
>                 vhost_vring_call_packed(dev, vq);
> 
> On Thu, 2020-01-09 at 15:47 +0000, bugzilla@dpdk.org wrote:
> > https://bugs.dpdk.org/show_bug.cgi?id=383
> >
> >             Bug ID: 383
> >            Summary: dpdk virtio_user lack of notifications make
> >                     vhost_net+napi stops tx buffers
> >            Product: DPDK
> >            Version: unspecified
> >           Hardware: All
> >                 OS: Linux
> >             Status: UNCONFIRMED
> >           Severity: normal
> >           Priority: Normal
> >          Component: vhost/virtio
> >           Assignee: dev@dpdk.org
> >           Reporter: eupm90@gmail.com
> >   Target Milestone: ---
> >
> > Using the current testpmd vhost_user as:
> >
> > ./app/testpmd -l 6,7,8 --vdev='net_vhost1,iface=/tmp/vhost-user1'
> > --vdev='net_vhost2,iface=/tmp/vhost-user2' -- -a -i --rxq=1 --txq=1
> > --txd=1024
> > --forward-mode=rxonly
> >
> > And starting qemu using packed=on on the interface:
> >
> > -netdev vhost-user,chardev=charnet1,id=hostnet1 -device
> > virtio-net-pci,rx_queue_size=256,...,packed=on
> >
> > And start to tx in the guest using:
> >
> > ./dpdk/build/app/testpmd -l 1,2 --vdev=eth_af_packet0,iface=eth0 -- \
> >     --forward-mode=txonly --txq=1 --txd=256 --auto-start --txpkts
> > 1500 \
> >     --stats-period 1
> >
> > After first burst of packets (512 or a little more), sendto() will
> > start to
> > return EBUSY. kernel NAPI is refusing to send more packets to
> > virtio_net device
> > until it free old skbs.
> >
> > However, virtio_net driver is unable to free old buffers since host
> > does not return them in `vhost_flush_dequeue_packed` until shadow
> > queue is full
> > except for MAX_PKT_BURST (32) packets.
> >
> > Sometimes we are lucky and reach this point, or packets are small
> > enough to
> > fill the queue and flush, but if the packets and the virtqueue are
> > big enough,
> > we will not be able to tx anymore.
> >