Re: [dpdk-dev] [PATCH v10 6/9] net/virtio: add vectorized packed ring Rx path

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

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, April 28, 2020 9:46 PM
> To: Liu, Yong <yong.liu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>;
> Wang, Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; jerinj@marvell.com
> Subject: Re: [PATCH v10 6/9] net/virtio: add vectorized packed ring Rx path
> 
> 
> 
> On 4/28/20 3:01 PM, Liu, Yong wrote:
> >>> Maxime,
> >>> Thanks for point it out, it will add extra cache miss in datapath.
> >>> And its impact on performance is around 1% in loopback case.
> >> Ok, thanks for doing the test. I'll try to run some PVP benchmarks
> >> on my side because when doing IO loopback, the cache pressure is
> >> much less important.
> >>
> >>> While benefit of vectorized path will be more than that number.
> >> Ok, but I disagree for two reasons:
> >>  1. You have to keep in mind than non-vectorized is the default and
> >> encouraged mode to use. Indeed, it takes a lot of shortcuts like not
> >> checking header length (so no error stats), etc...
> >>
> > Ok, I will keep non-vectorized same as before.
> >
> >>  2. It's like saying it's OK it degrades by 5% on $CPU_VENDOR_A because
> >> the gain is 20% on $CPU_VENDOR_B.
> >>
> >> In the case we see more degradation in real-world scenario, you might
> >> want to consider using ifdefs to avoid adding padding in the non-
> >> vectorized case, like you did to differentiate Virtio PMD to Virtio-user
> >> PMD in patch 7.
> >>
> > Maxime,
> > The performance difference is so slight, so I ignored for it look like a
> sampling error.
> 
> Agree for IO loopback, but it adds one more cache line access per burst,
> which might be see in some real-life use cases.
> 
> > It maybe not suitable to add new configuration for such setting which
> only used inside driver.
> 
> Wait, the Virtio-user #ifdef is based on the defconfig options? How can
> it work since both Virtio PMD and Virtio-user PMD can be selected at the
> same time?
> 
> I thought it was a define set before the headers inclusion and unset
> afterwards, but I didn't checked carefully.
> 

Maxime,
The difference between virtio PMD and Virtio-user PMD addresses is handled by vq->offset. 

When virtio PMD is running, offset will be set to buf_iova.
vq->offset = offsetof(struct rte_mbuf, buf_iova);

When virtio_user PMD is running, offset will be set to buf_addr.
vq->offset = offsetof(struct rte_mbuf, buf_addr);

> > Virtio driver can check whether virtqueue is using vectorized path when
> initialization, will use padded structure if it is.
> > I have added some tested code and now performance came back.  Since
> code has changed in initialization process,  it need some time for regression
> check.
> 
> Ok, works for me.
> 
> I am investigating a linkage issue with your series, which does not
> happen systematically (see below, it happens also with clang). David
> pointed me to some Intel patches removing the usage if __rte_weak,
> could it be related?
> 

I checked David's patch, it only changed i40e driver. Meanwhile attribute __rte_weak should still be in virtio_rxtx.c. 
I will follow David's patch, eliminate the usage of weak attribute. 

> 
> gcc  -o app/test/dpdk-test
> 'app/test/3062f5d@@dpdk-test@exe/commands.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/packet_burst_generator.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_acl.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_alarm.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_atomic.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_barrier.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_bpf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_byteorder.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cmdline.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cmdline_cirbuf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cmdline_etheraddr.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cmdline_ipaddr.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cmdline_lib.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cmdline_num.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cmdline_portlist.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cmdline_string.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_common.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cpuflags.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_crc.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cryptodev.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cryptodev_asym.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cryptodev_blockcipher.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cryptodev_security_pdcp.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_cycles.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_debug.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_distributor.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_distributor_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_eal_flags.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_eal_fs.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_efd.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_efd_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_errno.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_event_crypto_adapter.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_event_eth_rx_adapter.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_event_ring.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_event_timer_adapter.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_eventdev.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_external_mem.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_fbarray.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_fib.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_fib_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_fib6.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_fib6_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_func_reentrancy.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_flow_classify.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_hash.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_hash_functions.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_hash_multiwriter.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_hash_readwrite.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_hash_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_hash_readwrite_lf_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_interrupts.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ipfrag.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ipsec.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ipsec_sad.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_kni.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_kvargs.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_link_bonding.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_link_bonding_rssconf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_logs.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_lpm.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_lpm6.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_lpm6_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_lpm_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_malloc.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_mbuf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_member.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_member_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_memcpy.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_memcpy_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_memory.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_mempool.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_mempool_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_memzone.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_meter.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_metrics.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_mcslock.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_mp_secondary.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_per_lcore.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_pmd_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_power.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_power_cpufreq.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_power_kvm_vm.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_prefetch.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_rand_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_rawdev.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_rcu_qsbr.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_rcu_qsbr_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_reciprocal_division.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_reciprocal_division_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_red.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_reorder.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_rib.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_rib6.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ring.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ring_mpmc_stress.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ring_hts_stress.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ring_peek_stress.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ring_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ring_rts_stress.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ring_stress.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_rwlock.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_sched.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_security.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_service_cores.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_spinlock.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_stack.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_stack_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_string_fns.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_table.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_table_acl.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_table_combined.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_table_pipeline.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_table_ports.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_table_tables.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_tailq.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_thash.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_timer.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_timer_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_timer_racecond.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_timer_secondary.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_ticketlock.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_trace.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_trace_register.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_trace_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_version.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/virtual_pmd.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_pmd_ring_perf.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_pmd_ring.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_event_eth_tx_adapter.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_bitratestats.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_latencystats.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_link_bonding_mode4.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/sample_packet_forward.c.o'
> 'app/test/3062f5d@@dpdk-test@exe/test_pdump.c.o' -Wl,--no-undefined
> -Wl,--as-needed -Wl,-O1 -Wl,--whole-archive -Wl,--start-group
> drivers/librte_common_cpt.a drivers/librte_common_dpaax.a
> drivers/librte_common_iavf.a drivers/librte_common_octeontx.a
> drivers/librte_common_octeontx2.a drivers/librte_bus_dpaa.a
> drivers/librte_bus_fslmc.a drivers/librte_bus_ifpga.a
> drivers/librte_bus_pci.a drivers/librte_bus_vdev.a
> drivers/librte_bus_vmbus.a drivers/librte_mempool_bucket.a
> drivers/librte_mempool_dpaa.a drivers/librte_mempool_dpaa2.a
> drivers/librte_mempool_octeontx.a drivers/librte_mempool_octeontx2.a
> drivers/librte_mempool_ring.a drivers/librte_mempool_stack.a
> drivers/librte_pmd_af_packet.a drivers/librte_pmd_ark.a
> drivers/librte_pmd_atlantic.a drivers/librte_pmd_avp.a
> drivers/librte_pmd_axgbe.a drivers/librte_pmd_bond.a
> drivers/librte_pmd_bnxt.a drivers/librte_pmd_cxgbe.a
> drivers/librte_pmd_dpaa.a drivers/librte_pmd_dpaa2.a
> drivers/librte_pmd_e1000.a drivers/librte_pmd_ena.a
> drivers/librte_pmd_enetc.a drivers/librte_pmd_enic.a
> drivers/librte_pmd_failsafe.a drivers/librte_pmd_fm10k.a
> drivers/librte_pmd_i40e.a drivers/librte_pmd_hinic.a
> drivers/librte_pmd_hns3.a drivers/librte_pmd_iavf.a
> drivers/librte_pmd_ice.a drivers/librte_pmd_igc.a
> drivers/librte_pmd_ixgbe.a drivers/librte_pmd_kni.a
> drivers/librte_pmd_liquidio.a drivers/librte_pmd_memif.a
> drivers/librte_pmd_netvsc.a drivers/librte_pmd_nfp.a
> drivers/librte_pmd_null.a drivers/librte_pmd_octeontx.a
> drivers/librte_pmd_octeontx2.a drivers/librte_pmd_pfe.a
> drivers/librte_pmd_qede.a drivers/librte_pmd_ring.a
> drivers/librte_pmd_sfc.a drivers/librte_pmd_softnic.a
> drivers/librte_pmd_tap.a drivers/librte_pmd_thunderx.a
> drivers/librte_pmd_vdev_netvsc.a drivers/librte_pmd_vhost.a
> drivers/librte_pmd_virtio.a drivers/librte_pmd_vmxnet3.a
> drivers/librte_rawdev_dpaa2_cmdif.a drivers/librte_rawdev_dpaa2_qdma.a
> drivers/librte_rawdev_ioat.a drivers/librte_rawdev_ntb.a
> drivers/librte_rawdev_octeontx2_dma.a
> drivers/librte_rawdev_octeontx2_ep.a drivers/librte_rawdev_skeleton.a
> drivers/librte_pmd_caam_jr.a drivers/librte_pmd_dpaa_sec.a
> drivers/librte_pmd_dpaa2_sec.a drivers/librte_pmd_nitrox.a
> drivers/librte_pmd_null_crypto.a drivers/librte_pmd_octeontx_crypto.a
> drivers/librte_pmd_octeontx2_crypto.a
> drivers/librte_pmd_crypto_scheduler.a drivers/librte_pmd_virtio_crypto.a
> drivers/librte_pmd_octeontx_compress.a drivers/librte_pmd_qat.a
> drivers/librte_pmd_ifc.a drivers/librte_pmd_dpaa_event.a
> drivers/librte_pmd_dpaa2_event.a drivers/librte_pmd_octeontx2_event.a
> drivers/librte_pmd_opdl_event.a drivers/librte_pmd_skeleton_event.a
> drivers/librte_pmd_sw_event.a drivers/librte_pmd_dsw_event.a
> drivers/librte_pmd_octeontx_event.a drivers/librte_pmd_bbdev_null.a
> drivers/librte_pmd_bbdev_turbo_sw.a
> drivers/librte_pmd_bbdev_fpga_lte_fec.a
> drivers/librte_pmd_bbdev_fpga_5gnr_fec.a -Wl,--no-whole-archive
> -Wl,--no-as-needed -pthread -lm -ldl -lnuma lib/librte_acl.a
> lib/librte_eal.a lib/librte_kvargs.a lib/librte_bitratestats.a
> lib/librte_ethdev.a lib/librte_net.a lib/librte_mbuf.a
> lib/librte_mempool.a lib/librte_ring.a lib/librte_meter.a
> lib/librte_metrics.a lib/librte_bpf.a lib/librte_cfgfile.a
> lib/librte_cmdline.a lib/librte_cryptodev.a lib/librte_distributor.a
> lib/librte_efd.a lib/librte_hash.a lib/librte_eventdev.a
> lib/librte_timer.a lib/librte_fib.a lib/librte_rib.a
> lib/librte_flow_classify.a lib/librte_table.a lib/librte_port.a
> lib/librte_sched.a lib/librte_ip_frag.a lib/librte_kni.a
> lib/librte_pci.a lib/librte_lpm.a lib/librte_ipsec.a
> lib/librte_security.a lib/librte_latencystats.a lib/librte_member.a
> lib/librte_pipeline.a lib/librte_rawdev.a lib/librte_rcu.a
> lib/librte_reorder.a lib/librte_stack.a lib/librte_power.a
> lib/librte_pdump.a lib/librte_gso.a lib/librte_vhost.a
> lib/librte_compressdev.a lib/librte_bbdev.a -Wl,--end-group
> '-Wl,-rpath,$ORIGIN/../../lib:$ORIGIN/../../drivers'
> -Wl,-rpath-
> link,/tmp/dpdk_build/meson_buildir_gcc/lib:/tmp/dpdk_build/meson_buil
> dir_gcc/drivers
> drivers/librte_pmd_virtio.a(net_virtio_virtio_ethdev.c.o): In function
> `set_rxtx_funcs':
> virtio_ethdev.c:(.text.unlikely+0x6f): undefined reference to
> `virtio_xmit_pkts_packed_vec'
> collect2: error: ld returned 1 exit status
> ninja: build stopped: subcommand failed.
> 
> > Regards,
> > Marvin
> >