vhost: fix batch enqueue only handle few packets

Message ID 20191107143748.20649-1-yong.liu@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: fix batch enqueue only handle few packets |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Marvin Liu Nov. 7, 2019, 2:37 p.m. UTC
  After enqueue function finished, packet index has been increased. Batch
enqueue function should retrieve mbuf structure pointed by that index.

Fixes: 0294211bb6dc ("vhost: optimize packed ring enqueue")

Signed-off-by: Marvin Liu <yong.liu@intel.com>
---
 lib/librte_vhost/virtio_net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Maxime Coquelin Nov. 7, 2019, 8:11 a.m. UTC | #1
On 11/7/19 3:37 PM, Marvin Liu wrote:
> After enqueue function finished, packet index has been increased. Batch
> enqueue function should retrieve mbuf structure pointed by that index.
> 
> Fixes: 0294211bb6dc ("vhost: optimize packed ring enqueue")
> 
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
> ---
>  lib/librte_vhost/virtio_net.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

We definitely need Vhost-user and Virtio-user functional testing in the
CI.

Thanks,
Maxime
  
Maxime Coquelin Nov. 7, 2019, 8:20 a.m. UTC | #2
On 11/7/19 3:37 PM, Marvin Liu wrote:
> After enqueue function finished, packet index has been increased. Batch
> enqueue function should retrieve mbuf structure pointed by that index.
> 
> Fixes: 0294211bb6dc ("vhost: optimize packed ring enqueue")
> 
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
> ---
>  lib/librte_vhost/virtio_net.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to dpdk-next-virtio/master

Can you please run again the performance benchmarks, and see what is the
loss, if any?

Thanks,
Maxime
  
Marvin Liu Nov. 7, 2019, 8:29 a.m. UTC | #3
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, November 07, 2019 4:20 PM
> To: Liu, Yong <yong.liu@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>; Wang,
> Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] vhost: fix batch enqueue only handle few packets
> 
> 
> 
> On 11/7/19 3:37 PM, Marvin Liu wrote:
> > After enqueue function finished, packet index has been increased. Batch
> > enqueue function should retrieve mbuf structure pointed by that index.
> >
> > Fixes: 0294211bb6dc ("vhost: optimize packed ring enqueue")
> >
> > Signed-off-by: Marvin Liu <yong.liu@intel.com>
> > ---
> >  lib/librte_vhost/virtio_net.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Applied to dpdk-next-virtio/master
> 
> Can you please run again the performance benchmarks, and see what is the
> loss, if any?
> 
Sure, I will rerun the performance benchmarks.

Thanks,
Marvin

> Thanks,
> Maxime
  
Marvin Liu Nov. 7, 2019, 9:59 a.m. UTC | #4
> -----Original Message-----
> From: Liu, Yong
> Sent: Thursday, November 07, 2019 4:29 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Bie, Tiwei
> <tiwei.bie@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] vhost: fix batch enqueue only handle few packets
> 
> 
> 
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: Thursday, November 07, 2019 4:20 PM
> > To: Liu, Yong <yong.liu@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>;
> Wang,
> > Zhihong <zhihong.wang@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH] vhost: fix batch enqueue only handle few packets
> >
> >
> >
> > On 11/7/19 3:37 PM, Marvin Liu wrote:
> > > After enqueue function finished, packet index has been increased.
> Batch
> > > enqueue function should retrieve mbuf structure pointed by that index.
> > >
> > > Fixes: 0294211bb6dc ("vhost: optimize packed ring enqueue")
> > >
> > > Signed-off-by: Marvin Liu <yong.liu@intel.com>
> > > ---
> > >  lib/librte_vhost/virtio_net.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Applied to dpdk-next-virtio/master
> >
> > Can you please run again the performance benchmarks, and see what is the
> > loss, if any?
> >
> Sure, I will rerun the performance benchmarks.
> 

latest result with gcc9.0.1.
+---------------------------------------------------+
|                                   | 19.08 | + opt |
|-----------------------------------|-------|-------|
| 1518B PvP                         | 2.63M | 3.07M |
|-----------------------------------|-------|-------|
| 64B loopback                      | 7.81M | 12.3M |
|-----------------------------------|-------|-------|
| 1518B loopback                    | 3.59M | 4.54M |
|-----------------------------------|-------|-------|
| 16K chained loopback              | 297K  | 322K  |
|-----------------------------------|-------|-------|
| 50% 256B + 50% 16K                | 688K  | 959K  |
|-----------------------------------|-------|-------|
| pktgen_sample03_burst_single_flow | 5.78M | 5.80M |
+---------------------------------------------------+

> Thanks,
> Marvin
> 
> > Thanks,
> > Maxime
  

Patch

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index cde7498c7..f77944f5b 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -1155,7 +1155,8 @@  virtio_dev_rx_packed(struct virtio_net *dev,
 		rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
 
 		if (remained >= PACKED_BATCH_SIZE) {
-			if (!virtio_dev_rx_batch_packed(dev, vq, pkts)) {
+			if (!virtio_dev_rx_batch_packed(dev, vq,
+							&pkts[pkt_idx])) {
 				pkt_idx += PACKED_BATCH_SIZE;
 				remained -= PACKED_BATCH_SIZE;
 				continue;