Skip to contentSkip to conversation

vhost: fix iotlb mempool single-consumer flag

PATCH Superseded Eugenio Pérez 5 participants 13 messages Aug 10, 2020 – Sep 18, 2020

lore
Share & tools
lore

Eugenio Pérez

author
#0 lore raw

cover vhost: fix iotlb mempool single-consumer flag

Bugzilla bug: 523

Using testpmd as a vhost-user with iommu:

/home/dpdk/build/app/dpdk-testpmd -l 1,3 \
	--vdev net_vhost0,iface=/tmp/vhost-user1,queues=1,iommu-support=1 \
	-- --auto-start --stats-period 5 --forward-mode=txonly

And qemu with packed virtqueue:

    <interface type='vhostuser'>
      <mac address='88:67:11:5f:dd:02'/>
      <source type='unix' path='/tmp/vhost-user1' mode='client'/>
      <model type='virtio'/>
      <driver name='vhost' rx_queue_size='1024' iommu='on' ats='on'/>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
    </interface>

...

<qemu:commandline>

    <qemu:arg value='-set'/>
    <qemu:arg value='device.net1.packed=on'/>

</qemu:commandline>

--

Is it possible to consume the iotlb's entries of the mempoo from different threads. Thread sanitizer example output (after change rwlocks to POSIX ones):

WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 8 at 0x00017ffd5628 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:181 (dpdk-testpmd+0x769343)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous read of size 8 at 0x00017ffd5628 by thread T3:

    #0 vhost_user_iotlb_cache_find ../lib/librte_vhost/iotlb.c:252 (dpdk-testpmd+0x76ee96)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:42 (dpdk-testpmd+0x77488c)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7abeb3)
    #3 map_one_desc ../lib/librte_vhost/virtio_net.c:497 (dpdk-testpmd+0x7abeb3)
    #4 fill_vec_buf_packed ../lib/librte_vhost/virtio_net.c:751 (dpdk-testpmd+0x7abeb3)
    #5 vhost_enqueue_single_packed ../lib/librte_vhost/virtio_net.c:1170 (dpdk-testpmd+0x7abeb3)
    #6 virtio_dev_rx_single_packed ../lib/librte_vhost/virtio_net.c:1346 (dpdk-testpmd+0x7abeb3)
    #7 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1384 (dpdk-testpmd+0x7abeb3)
    #8 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
    #9 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
    #10 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
    #11 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
    #12 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
    #13 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
    #14 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
    #15 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
    #16 <null> <null> (libtsan.so.0+0x2a68d)

Location is global '<null>' at 0x000000000000 (rtemap_0+0x00003ffd5628)

Thread T5 'vhost-events' (tid=76933, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_ctrl_thread_create ../lib/librte_eal/common/eal_common_thread.c:216 (dpdk-testpmd+0xa289e7)
    #2 rte_vhost_driver_start ../lib/librte_vhost/socket.c:1190 (dpdk-testpmd+0x7728ef)
    #3 vhost_driver_setup ../drivers/net/vhost/rte_eth_vhost.c:1028 (dpdk-testpmd+0x1de233d)
    #4 eth_dev_configure ../drivers/net/vhost/rte_eth_vhost.c:1126 (dpdk-testpmd+0x1de29cc)
    #5 rte_eth_dev_configure ../lib/librte_ethdev/rte_ethdev.c:1439 (dpdk-testpmd+0x991ce2)
    #6 start_port ../app/test-pmd/testpmd.c:2450 (dpdk-testpmd+0x4f9b45)
    #7 main ../app/test-pmd/testpmd.c:3777 (dpdk-testpmd+0x4fe1ac)

Thread T3 'lcore-slave-3' (tid=76931, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_eal_init ../lib/librte_eal/linux/eal.c:1244 (dpdk-testpmd+0xa46e2b)
    #2 main ../app/test-pmd/testpmd.c:3673 (dpdk-testpmd+0x4fdd75)

--

Or:
WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 1 at 0x00017ffd00f8 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:182 (dpdk-testpmd+0x769370)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous write of size 1 at 0x00017ffd00f8 by thread T3:

    #0 vhost_user_iotlb_pending_insert ../lib/librte_vhost/iotlb.c:86 (dpdk-testpmd+0x75eb0c)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:58 (dpdk-testpmd+0x774926)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7a79d1)
    #3 virtio_dev_rx_batch_packed ../lib/librte_vhost/virtio_net.c:1295 (dpdk-testpmd+0x7a79d1)
    #4 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1376 (dpdk-testpmd+0x7a79d1)
    #5 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
    #6 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
    #7 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
    #8 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
    #9 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
    #10 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
    #11 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
    #12 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
    #13 <null> <null> (libtsan.so.0+0x2a68d)

--

As a consequence, the two threads can modify the same entry of the mempool. Usually, this cause a loop in iotlb_pending_entries list.

This behavior is only observed on packed vq + virtio-net kernel driver in the guest, so we could make the single-consumer flag optional. However, I have not found why I cannot see the issue in split, so the safer option is to never set it.

Any comments?

Thanks!

Eugenio Pérez (1):
vhost: fix iotlb mempool single-consumer flag

 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Signature · 1 line
2.18.1
author
#1 lore raw

PATCH 1/1 vhost: fix iotlb mempool single-consumer flag

Bugzilla bug: 523

Using testpmd as a vhost-user with iommu:

/home/dpdk/build/app/dpdk-testpmd -l 1,3 \
        --vdev net_vhost0,iface=/tmp/vhost-user1,queues=1,iommu-support=1 \
        -- --auto-start --stats-period 5 --forward-mode=txonly

And qemu with packed virtqueue:

    <interface type='vhostuser'>
      <mac address='88:67:11:5f:dd:02'/>
      <source type='unix' path='/tmp/vhost-user1' mode='client'/>
      <model type='virtio'/>
      <driver name='vhost' rx_queue_size='1024' iommu='on' ats='on'/>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
    </interface>

...

<qemu:commandline>

    <qemu:arg value='-set'/>
    <qemu:arg value='device.net1.packed=on'/>

</qemu:commandline>

--

Is it possible to consume the iotlb's entries of the mempoo from different threads. Thread sanitizer example output (after change rwlocks to POSIX ones):

WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 8 at 0x00017ffd5628 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:181 (dpdk-testpmd+0x769343)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous read of size 8 at 0x00017ffd5628 by thread T3:

    #0 vhost_user_iotlb_cache_find ../lib/librte_vhost/iotlb.c:252 (dpdk-testpmd+0x76ee96)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:42 (dpdk-testpmd+0x77488c)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7abeb3)
    #3 map_one_desc ../lib/librte_vhost/virtio_net.c:497 (dpdk-testpmd+0x7abeb3)
    #4 fill_vec_buf_packed ../lib/librte_vhost/virtio_net.c:751 (dpdk-testpmd+0x7abeb3)
    #5 vhost_enqueue_single_packed ../lib/librte_vhost/virtio_net.c:1170 (dpdk-testpmd+0x7abeb3)
    #6 virtio_dev_rx_single_packed ../lib/librte_vhost/virtio_net.c:1346 (dpdk-testpmd+0x7abeb3)
    #7 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1384 (dpdk-testpmd+0x7abeb3)
    #8 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
    #9 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
    #10 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
    #11 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
    #12 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
    #13 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
    #14 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
    #15 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
    #16 <null> <null> (libtsan.so.0+0x2a68d)

Location is global '<null>' at 0x000000000000 (rtemap_0+0x00003ffd5628)

Thread T5 'vhost-events' (tid=76933, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_ctrl_thread_create ../lib/librte_eal/common/eal_common_thread.c:216 (dpdk-testpmd+0xa289e7)
    #2 rte_vhost_driver_start ../lib/librte_vhost/socket.c:1190 (dpdk-testpmd+0x7728ef)
    #3 vhost_driver_setup ../drivers/net/vhost/rte_eth_vhost.c:1028 (dpdk-testpmd+0x1de233d)
    #4 eth_dev_configure ../drivers/net/vhost/rte_eth_vhost.c:1126 (dpdk-testpmd+0x1de29cc)
    #5 rte_eth_dev_configure ../lib/librte_ethdev/rte_ethdev.c:1439 (dpdk-testpmd+0x991ce2)
    #6 start_port ../app/test-pmd/testpmd.c:2450 (dpdk-testpmd+0x4f9b45)
    #7 main ../app/test-pmd/testpmd.c:3777 (dpdk-testpmd+0x4fe1ac)

Thread T3 'lcore-slave-3' (tid=76931, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_eal_init ../lib/librte_eal/linux/eal.c:1244 (dpdk-testpmd+0xa46e2b)
    #2 main ../app/test-pmd/testpmd.c:3673 (dpdk-testpmd+0x4fdd75)

--

Or:
WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 1 at 0x00017ffd00f8 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:182 (dpdk-testpmd+0x769370)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous write of size 1 at 0x00017ffd00f8 by thread T3:

    #0 vhost_user_iotlb_pending_insert ../lib/librte_vhost/iotlb.c:86 (dpdk-testpmd+0x75eb0c)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:58 (dpdk-testpmd+0x774926)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7a79d1)
    #3 virtio_dev_rx_batch_packed ../lib/librte_vhost/virtio_net.c:1295 (dpdk-testpmd+0x7a79d1)
    #4 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1376 (dpdk-testpmd+0x7a79d1)
    #5 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
    #6 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
    #7 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
    #8 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
    #9 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
    #10 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
    #11 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
    #12 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
    #13 <null> <null> (libtsan.so.0+0x2a68d)

--

As a consequence, the two threads can modify the same entry of the mempool. Usually, this cause a loop in iotlb_pending_entries list.

Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>
---
 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
lib/librte_vhost/iotlb.c +1 -2
index 5b3a0c090..e0b67721b 100644
@@ -321,8 +321,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, int vq_index)
 			IOTLB_CACHE_SIZE, sizeof(struct vhost_iotlb_entry), 0,
 			0, 0, NULL, NULL, NULL, socket,
 			MEMPOOL_F_NO_CACHE_ALIGN |
-			MEMPOOL_F_SP_PUT |
-			MEMPOOL_F_SC_GET);
+			MEMPOOL_F_SP_PUT);
 	if (!vq->iotlb_pool) {
 		VHOST_LOG_CONFIG(ERR,
 				"Failed to create IOTLB cache pool (%s)\n",
Signature · 1 line
2.18.1
#2 lore raw
Quoted 121 lines · Eugenio Pérez

Bugzilla bug: 523

Using testpmd as a vhost-user with iommu:

/home/dpdk/build/app/dpdk-testpmd -l 1,3 \
        --vdev net_vhost0,iface=/tmp/vhost-user1,queues=1,iommu-support=1 \
        -- --auto-start --stats-period 5 --forward-mode=txonly

And qemu with packed virtqueue:

    <interface type='vhostuser'>
      <mac address='88:67:11:5f:dd:02'/>
      <source type='unix' path='/tmp/vhost-user1' mode='client'/>
      <model type='virtio'/>
      <driver name='vhost' rx_queue_size='1024' iommu='on' ats='on'/>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
    </interface>

...

<qemu:commandline>

    <qemu:arg value='-set'/>
    <qemu:arg value='device.net1.packed=on'/>

</qemu:commandline>

--

Is it possible to consume the iotlb's entries of the mempoo from different threads. Thread sanitizer example output (after change rwlocks to POSIX ones):

WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 8 at 0x00017ffd5628 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:181 (dpdk-testpmd+0x769343)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous read of size 8 at 0x00017ffd5628 by thread T3:

    #0 vhost_user_iotlb_cache_find ../lib/librte_vhost/iotlb.c:252 (dpdk-testpmd+0x76ee96)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:42 (dpdk-testpmd+0x77488c)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7abeb3)
    #3 map_one_desc ../lib/librte_vhost/virtio_net.c:497 (dpdk-testpmd+0x7abeb3)
    #4 fill_vec_buf_packed ../lib/librte_vhost/virtio_net.c:751 (dpdk-testpmd+0x7abeb3)
    #5 vhost_enqueue_single_packed ../lib/librte_vhost/virtio_net.c:1170 (dpdk-testpmd+0x7abeb3)
    #6 virtio_dev_rx_single_packed ../lib/librte_vhost/virtio_net.c:1346 (dpdk-testpmd+0x7abeb3)
    #7 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1384 (dpdk-testpmd+0x7abeb3)
    #8 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
    #9 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
    #10 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
    #11 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
    #12 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
    #13 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
    #14 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
    #15 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
    #16 <null> <null> (libtsan.so.0+0x2a68d)

Location is global '<null>' at 0x000000000000 (rtemap_0+0x00003ffd5628)

Thread T5 'vhost-events' (tid=76933, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_ctrl_thread_create ../lib/librte_eal/common/eal_common_thread.c:216 (dpdk-testpmd+0xa289e7)
    #2 rte_vhost_driver_start ../lib/librte_vhost/socket.c:1190 (dpdk-testpmd+0x7728ef)
    #3 vhost_driver_setup ../drivers/net/vhost/rte_eth_vhost.c:1028 (dpdk-testpmd+0x1de233d)
    #4 eth_dev_configure ../drivers/net/vhost/rte_eth_vhost.c:1126 (dpdk-testpmd+0x1de29cc)
    #5 rte_eth_dev_configure ../lib/librte_ethdev/rte_ethdev.c:1439 (dpdk-testpmd+0x991ce2)
    #6 start_port ../app/test-pmd/testpmd.c:2450 (dpdk-testpmd+0x4f9b45)
    #7 main ../app/test-pmd/testpmd.c:3777 (dpdk-testpmd+0x4fe1ac)

Thread T3 'lcore-slave-3' (tid=76931, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_eal_init ../lib/librte_eal/linux/eal.c:1244 (dpdk-testpmd+0xa46e2b)
    #2 main ../app/test-pmd/testpmd.c:3673 (dpdk-testpmd+0x4fdd75)

--

Or:
WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 1 at 0x00017ffd00f8 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:182 (dpdk-testpmd+0x769370)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous write of size 1 at 0x00017ffd00f8 by thread T3:

    #0 vhost_user_iotlb_pending_insert ../lib/librte_vhost/iotlb.c:86 (dpdk-testpmd+0x75eb0c)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:58 (dpdk-testpmd+0x774926)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7a79d1)
    #3 virtio_dev_rx_batch_packed ../lib/librte_vhost/virtio_net.c:1295 (dpdk-testpmd+0x7a79d1)
    #4 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1376 (dpdk-testpmd+0x7a79d1)
    #5 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
    #6 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
    #7 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
    #8 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
    #9 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
    #10 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
    #11 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
    #12 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
    #13 <null> <null> (libtsan.so.0+0x2a68d)

--

As a consequence, the two threads can modify the same entry of the mempool. Usually, this cause a loop in iotlb_pending_entries list.

Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>
---
 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
lib/librte_vhost/iotlb.c +0 -0
index 5b3a0c090..e0b67721b 100644

lib/librte_vhost/iotlb.c vhost_user_iotlb_init(struct virtio_net *dev, int vq_index) · line 327

Quoted 9 lines
lib/librte_vhost/iotlb.c +1 -2
@@ -321,8 +321,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, int vq_index)
 			IOTLB_CACHE_SIZE, sizeof(struct vhost_iotlb_entry), 0,
 			0, 0, NULL, NULL, NULL, socket,
 			MEMPOOL_F_NO_CACHE_ALIGN |
-			MEMPOOL_F_SP_PUT |
-			MEMPOOL_F_SC_GET);
+			MEMPOOL_F_SP_PUT);
 	if (!vq->iotlb_pool) {
 		VHOST_LOG_CONFIG(ERR,
 				"Failed to create IOTLB cache pool (%s)\n",

Looks ok to me, but would need review from vhost maintainer.

#3 lore raw
In reply to Eugenio Pérez

Hi Eugenio,

Quoted 26 lines

-----Original Message-----
From: Eugenio Pérez <[email protected]>
Sent: Monday, August 10, 2020 10:11 PM
To: [email protected]
Cc: Adrian Moreno Zapata <[email protected]>; Maxime Coquelin <[email protected]>; [email protected]; Wang, Zhihong <[email protected]>; Xia, Chenbo <[email protected]>
Subject: [PATCH 1/1] vhost: fix iotlb mempool single-consumer flag

Bugzilla bug: 523

Using testpmd as a vhost-user with iommu:

/home/dpdk/build/app/dpdk-testpmd -l 1,3 \
        --vdev net_vhost0,iface=/tmp/vhost-user1,queues=1,iommu-support=1
\
        -- --auto-start --stats-period 5 --forward-mode=txonly

And qemu with packed virtqueue:

    <interface type='vhostuser'>
      <mac address='88:67:11:5f:dd:02'/>
      <source type='unix' path='/tmp/vhost-user1' mode='client'/>
      <model type='virtio'/>
      <driver name='vhost' rx_queue_size='1024' iommu='on' ats='on'/>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00'
Quoted 9 lines

function='0x0'/>
</interface>
...

<qemu:commandline>

    <qemu:arg value='-set'/>
    <qemu:arg value='device.net1.packed=on'/>

</qemu:commandline>

The fix looks fine to me. But the commit message is a little bit complicated to me (also, some lines too long). Since this bug is clear and could be described by something like 'control thread which handles iotlb msg and forwarding thread which uses iotlb to translate address may modify same entry of mempool and may cause a loop in iotlb_pending_entries list'. Do you think it makes sense?

Thanks for the fix!
Chenbo

Quoted 157 lines

--

Is it possible to consume the iotlb's entries of the mempoo from different threads. Thread sanitizer example output (after change rwlocks to POSIX ones):

WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 8 at 0x00017ffd5628 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:181
(dpdk-testpmd+0x769343)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-
testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-
testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-
testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-
testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193
(dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous read of size 8 at 0x00017ffd5628 by thread T3:

    #0 vhost_user_iotlb_cache_find ../lib/librte_vhost/iotlb.c:252 (dpdk-
testpmd+0x76ee96)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:42 (dpdk-
testpmd+0x77488c)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-
testpmd+0x7abeb3)
    #3 map_one_desc ../lib/librte_vhost/virtio_net.c:497 (dpdk-
testpmd+0x7abeb3)
    #4 fill_vec_buf_packed ../lib/librte_vhost/virtio_net.c:751 (dpdk-
testpmd+0x7abeb3)
    #5 vhost_enqueue_single_packed ../lib/librte_vhost/virtio_net.c:1170
(dpdk-testpmd+0x7abeb3)
    #6 virtio_dev_rx_single_packed ../lib/librte_vhost/virtio_net.c:1346
(dpdk-testpmd+0x7abeb3)
    #7 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1384 (dpdk-
testpmd+0x7abeb3)
    #8 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-
testpmd+0x7b0654)
    #9 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465
(dpdk-testpmd+0x7b0654)
    #10 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-
testpmd+0x1ddfbd8)
    #11 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-
testpmd+0x505fdb)
    #12 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-
testpmd+0x5106ad)
    #13 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-
testpmd+0x4f8951)
    #14 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-
testpmd+0x4f89d7)
    #15 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-
testpmd+0xa5b20a)
    #16 <null> <null> (libtsan.so.0+0x2a68d)

Location is global '<null>' at 0x000000000000 (rtemap_0+0x00003ffd5628)

Thread T5 'vhost-events' (tid=76933, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1
rte_ctrl_thread_create ../lib/librte_eal/common/eal_common_thread.c:216
(dpdk-testpmd+0xa289e7)
    #2 rte_vhost_driver_start ../lib/librte_vhost/socket.c:1190 (dpdk-
testpmd+0x7728ef)
    #3 vhost_driver_setup ../drivers/net/vhost/rte_eth_vhost.c:1028 (dpdk-
testpmd+0x1de233d)
    #4 eth_dev_configure ../drivers/net/vhost/rte_eth_vhost.c:1126 (dpdk-
testpmd+0x1de29cc)
    #5 rte_eth_dev_configure ../lib/librte_ethdev/rte_ethdev.c:1439 (dpdk-
testpmd+0x991ce2)
    #6 start_port ../app/test-pmd/testpmd.c:2450 (dpdk-testpmd+0x4f9b45)
    #7 main ../app/test-pmd/testpmd.c:3777 (dpdk-testpmd+0x4fe1ac)

Thread T3 'lcore-slave-3' (tid=76931, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_eal_init ../lib/librte_eal/linux/eal.c:1244 (dpdk-
testpmd+0xa46e2b)
    #2 main ../app/test-pmd/testpmd.c:3673 (dpdk-testpmd+0x4fdd75)

--

Or:
WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 1 at 0x00017ffd00f8 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:182
(dpdk-testpmd+0x769370)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-
testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-
testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-
testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-
testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193
(dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous write of size 1 at 0x00017ffd00f8 by thread T3:
#0 vhost_user_iotlb_pending_insert ../lib/librte_vhost/iotlb.c:86
(dpdk-testpmd+0x75eb0c)

    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:58 (dpdk-
testpmd+0x774926)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-
testpmd+0x7a79d1)
    #3 virtio_dev_rx_batch_packed ../lib/librte_vhost/virtio_net.c:1295
(dpdk-testpmd+0x7a79d1)
    #4 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1376 (dpdk-
testpmd+0x7a79d1)
    #5 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-
testpmd+0x7b0654)
    #6 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465
(dpdk-testpmd+0x7b0654)
    #7 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-
testpmd+0x1ddfbd8)
    #8 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-
testpmd+0x505fdb)
    #9 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-
testpmd+0x5106ad)
    #10 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-
testpmd+0x4f8951)
    #11 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-
testpmd+0x4f89d7)
    #12 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-
testpmd+0xa5b20a)
    #13 <null> <null> (libtsan.so.0+0x2a68d)

--

As a consequence, the two threads can modify the same entry of the mempool. Usually, this cause a loop in iotlb_pending_entries list.

Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>
---
 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
lib/librte_vhost/iotlb.c +1 -2
index 5b3a0c090..e0b67721b 100644
@@ -321,8 +321,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, int
 vq_index)
 			IOTLB_CACHE_SIZE, sizeof(struct vhost_iotlb_entry), 0,
 			0, 0, NULL, NULL, NULL, socket,
 			MEMPOOL_F_NO_CACHE_ALIGN |
-			MEMPOOL_F_SP_PUT |
-			MEMPOOL_F_SC_GET);
+			MEMPOOL_F_SP_PUT);
 	if (!vq->iotlb_pool) {
 		VHOST_LOG_CONFIG(ERR,

"Failed to create IOTLB cache pool (%s)\n",
--
2.18.1

author
#4 lore raw

Hi Chenbo.

Quoted 39 lines · Chenbo

Hi Eugenio,

Quoted 26 lines

-----Original Message-----
From: Eugenio Pérez <[email protected]>
Sent: Monday, August 10, 2020 10:11 PM
To: [email protected]
Cc: Adrian Moreno Zapata <[email protected]>; Maxime Coquelin <[email protected]>; [email protected]; Wang, Zhihong <[email protected]>; Xia, Chenbo <[email protected]>
Subject: [PATCH 1/1] vhost: fix iotlb mempool single-consumer flag

Bugzilla bug: 523

Using testpmd as a vhost-user with iommu:

/home/dpdk/build/app/dpdk-testpmd -l 1,3 \
        --vdev net_vhost0,iface=/tmp/vhost-user1,queues=1,iommu-support=1
\
        -- --auto-start --stats-period 5 --forward-mode=txonly

And qemu with packed virtqueue:

    <interface type='vhostuser'>
      <mac address='88:67:11:5f:dd:02'/>
      <source type='unix' path='/tmp/vhost-user1' mode='client'/>
      <model type='virtio'/>
      <driver name='vhost' rx_queue_size='1024' iommu='on' ats='on'/>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00'
Quoted 9 lines

function='0x0'/>
</interface>
...

<qemu:commandline>

    <qemu:arg value='-set'/>
    <qemu:arg value='device.net1.packed=on'/>

</qemu:commandline>

Quoted 6 lines

The fix looks fine to me. But the commit message is a little bit complicated to me (also, some lines too long). Since this bug is clear and could be described by something like 'control thread which handles iotlb msg and forwarding thread which uses iotlb to translate address may modify same entry of mempool and may cause a loop in iotlb_pending_entries list'. Do you think it makes sense?

Sure, I just wanted to give enough information to reproduce it, but that can be in the bugzilla case too if you prefer. Do you need me to send a v2?

Thanks!

Quoted 162 lines

Thanks for the fix!
Chenbo

Quoted 157 lines

--

Is it possible to consume the iotlb's entries of the mempoo from different threads. Thread sanitizer example output (after change rwlocks to POSIX ones):

WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 8 at 0x00017ffd5628 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:181
(dpdk-testpmd+0x769343)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-
testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-
testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-
testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-
testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193
(dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous read of size 8 at 0x00017ffd5628 by thread T3:

    #0 vhost_user_iotlb_cache_find ../lib/librte_vhost/iotlb.c:252 (dpdk-
testpmd+0x76ee96)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:42 (dpdk-
testpmd+0x77488c)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-
testpmd+0x7abeb3)
    #3 map_one_desc ../lib/librte_vhost/virtio_net.c:497 (dpdk-
testpmd+0x7abeb3)
    #4 fill_vec_buf_packed ../lib/librte_vhost/virtio_net.c:751 (dpdk-
testpmd+0x7abeb3)
    #5 vhost_enqueue_single_packed ../lib/librte_vhost/virtio_net.c:1170
(dpdk-testpmd+0x7abeb3)
    #6 virtio_dev_rx_single_packed ../lib/librte_vhost/virtio_net.c:1346
(dpdk-testpmd+0x7abeb3)
    #7 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1384 (dpdk-
testpmd+0x7abeb3)
    #8 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-
testpmd+0x7b0654)
    #9 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465
(dpdk-testpmd+0x7b0654)
    #10 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-
testpmd+0x1ddfbd8)
    #11 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-
testpmd+0x505fdb)
    #12 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-
testpmd+0x5106ad)
    #13 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-
testpmd+0x4f8951)
    #14 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-
testpmd+0x4f89d7)
    #15 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-
testpmd+0xa5b20a)
    #16 <null> <null> (libtsan.so.0+0x2a68d)

Location is global '<null>' at 0x000000000000 (rtemap_0+0x00003ffd5628)

Thread T5 'vhost-events' (tid=76933, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1
rte_ctrl_thread_create ../lib/librte_eal/common/eal_common_thread.c:216
(dpdk-testpmd+0xa289e7)
    #2 rte_vhost_driver_start ../lib/librte_vhost/socket.c:1190 (dpdk-
testpmd+0x7728ef)
    #3 vhost_driver_setup ../drivers/net/vhost/rte_eth_vhost.c:1028 (dpdk-
testpmd+0x1de233d)
    #4 eth_dev_configure ../drivers/net/vhost/rte_eth_vhost.c:1126 (dpdk-
testpmd+0x1de29cc)
    #5 rte_eth_dev_configure ../lib/librte_ethdev/rte_ethdev.c:1439 (dpdk-
testpmd+0x991ce2)
    #6 start_port ../app/test-pmd/testpmd.c:2450 (dpdk-testpmd+0x4f9b45)
    #7 main ../app/test-pmd/testpmd.c:3777 (dpdk-testpmd+0x4fe1ac)

Thread T3 'lcore-slave-3' (tid=76931, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_eal_init ../lib/librte_eal/linux/eal.c:1244 (dpdk-
testpmd+0xa46e2b)
    #2 main ../app/test-pmd/testpmd.c:3673 (dpdk-testpmd+0x4fdd75)

--

Or:
WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 1 at 0x00017ffd00f8 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:182
(dpdk-testpmd+0x769370)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-
testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-
testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-
testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-
testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193
(dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous write of size 1 at 0x00017ffd00f8 by thread T3:
#0 vhost_user_iotlb_pending_insert ../lib/librte_vhost/iotlb.c:86
(dpdk-testpmd+0x75eb0c)

    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:58 (dpdk-
testpmd+0x774926)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-
testpmd+0x7a79d1)
    #3 virtio_dev_rx_batch_packed ../lib/librte_vhost/virtio_net.c:1295
(dpdk-testpmd+0x7a79d1)
    #4 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1376 (dpdk-
testpmd+0x7a79d1)
    #5 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-
testpmd+0x7b0654)
    #6 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465
(dpdk-testpmd+0x7b0654)
    #7 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-
testpmd+0x1ddfbd8)
    #8 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-
testpmd+0x505fdb)
    #9 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-
testpmd+0x5106ad)
    #10 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-
testpmd+0x4f8951)
    #11 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-
testpmd+0x4f89d7)
    #12 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-
testpmd+0xa5b20a)
    #13 <null> <null> (libtsan.so.0+0x2a68d)

--

As a consequence, the two threads can modify the same entry of the mempool. Usually, this cause a loop in iotlb_pending_entries list.

Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>
---
 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
lib/librte_vhost/iotlb.c +1 -2
index 5b3a0c090..e0b67721b 100644
@@ -321,8 +321,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, int
 vq_index)
                      IOTLB_CACHE_SIZE, sizeof(struct vhost_iotlb_entry), 0,
                      0, 0, NULL, NULL, NULL, socket,
                      MEMPOOL_F_NO_CACHE_ALIGN |
-                     MEMPOOL_F_SP_PUT |
-                     MEMPOOL_F_SC_GET);
+                     MEMPOOL_F_SP_PUT);
      if (!vq->iotlb_pool) {
              VHOST_LOG_CONFIG(ERR,

"Failed to create IOTLB cache pool (%s)\n",
--
2.18.1

#5 lore raw

Hi Eugenio,

Quoted 58 lines

-----Original Message-----
From: Eugenio Perez Martin <[email protected]>
Sent: Wednesday, August 26, 2020 8:51 PM
To: Xia, Chenbo <[email protected]>
Cc: [email protected]; Adrian Moreno Zapata <[email protected]>; Maxime Coquelin <[email protected]>; [email protected]; Wang, Zhihong <[email protected]>
Subject: Re: [PATCH 1/1] vhost: fix iotlb mempool single-consumer flag

Hi Chenbo.

Quoted 18 lines · Chenbo

Hi Eugenio,

Quoted 15 lines

-----Original Message-----
From: Eugenio Pérez <[email protected]>
Sent: Monday, August 10, 2020 10:11 PM
To: [email protected]
Cc: Adrian Moreno Zapata <[email protected]>; Maxime Coquelin <[email protected]>; [email protected]; Wang, Zhihong <[email protected]>; Xia, Chenbo <[email protected]>
Subject: [PATCH 1/1] vhost: fix iotlb mempool single-consumer flag

Bugzilla bug: 523

Using testpmd as a vhost-user with iommu:

/home/dpdk/build/app/dpdk-testpmd -l 1,3 \
        --vdev net_vhost0,iface=/tmp/vhost-user1,queues=1,iommu-

support=1

Quoted 15 lines
Quoted 15 lines
\
        -- --auto-start --stats-period 5 --forward-mode=txonly

And qemu with packed virtqueue:

    <interface type='vhostuser'>
      <mac address='88:67:11:5f:dd:02'/>
      <source type='unix' path='/tmp/vhost-user1' mode='client'/>
      <model type='virtio'/>
      <driver name='vhost' rx_queue_size='1024' iommu='on' ats='on'/>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00'

function='0x0'/>
</interface>
...

Quoted 7 lines
Quoted 5 lines

<qemu:commandline>

    <qemu:arg value='-set'/>
    <qemu:arg value='device.net1.packed=on'/>

</qemu:commandline>

The fix looks fine to me. But the commit message is a little bit

complicated

Quoted 2 lines

to me (also, some lines too long). Since this bug is clear and could be described by something like 'control thread which handles iotlb msg and

forwarding

Quoted 1 line

thread which uses iotlb to translate address may modify same entry of

Quoted 9 lines

mempool

Quoted 1 line

and may cause a loop in iotlb_pending_entries list'. Do you think it

makes

Quoted 1 line

sense?

Sure, I just wanted to give enough information to reproduce it, but that can be in the bugzilla case too if you prefer. Do you need me to send a v2?

Yes, the information is very detailed for review! Since there's already one warning for commits message in patchwork, I'd like a brief description with bugzilla link and the details could be in that link. Is this ok for you?

Thanks!
Chenbo

Quoted 193 lines

Thanks!

Quoted 7 lines

Thanks for the fix!
Chenbo

Quoted 3 lines

--

Is it possible to consume the iotlb's entries of the mempoo from

different

Quoted 1 line
Quoted 1 line

threads. Thread sanitizer example output (after change rwlocks to

POSIX

Quoted 7 lines
Quoted 7 lines

ones):

WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 8 at 0x00017ffd5628 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:181
(dpdk-testpmd+0x769343)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380

(dpdk-

Quoted 2 lines
Quoted 2 lines

testpmd+0x78e4bf)

    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848

(dpdk-

Quoted 6 lines
Quoted 6 lines

testpmd+0x78fcf8)

    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-
testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-
testpmd+0x7591c2)
    #5
ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193
> > (dpdk-testpmd+0xa2890b)
> >     #6 <null> <null> (libtsan.so.0+0x2a68d)
Quoted 3 lines
Quoted 3 lines

Previous read of size 8 at 0x00017ffd5628 by thread T3:

    #0 vhost_user_iotlb_cache_find ../lib/librte_vhost/iotlb.c:252

(dpdk-

Quoted 10 lines
Quoted 10 lines

testpmd+0x76ee96)

    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:42 (dpdk-
testpmd+0x77488c)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-
testpmd+0x7abeb3)
    #3 map_one_desc ../lib/librte_vhost/virtio_net.c:497 (dpdk-
testpmd+0x7abeb3)
    #4 fill_vec_buf_packed ../lib/librte_vhost/virtio_net.c:751 (dpdk-
testpmd+0x7abeb3)
    #5
vhost_enqueue_single_packed ../lib/librte_vhost/virtio_net.c:1170
> > (dpdk-testpmd+0x7abeb3)
Quoted 1 line
Quoted 1 line
    #6
virtio_dev_rx_single_packed ../lib/librte_vhost/virtio_net.c:1346
> > (dpdk-testpmd+0x7abeb3)
Quoted 1 line
Quoted 1 line
    #7 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1384

(dpdk-

Quoted 6 lines
Quoted 6 lines

testpmd+0x7abeb3)

    #8 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-
testpmd+0x7b0654)
    #9 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465
(dpdk-testpmd+0x7b0654)
    #10 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-
Quoted 8 lines
Quoted 8 lines

testpmd+0x1ddfbd8)

    #11 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-
testpmd+0x505fdb)
    #12 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-
testpmd+0x5106ad)
    #13 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-
testpmd+0x4f8951)
    #14 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106

(dpdk-

Quoted 2 lines
Quoted 2 lines

testpmd+0x4f89d7)

    #15 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127

(dpdk-

Quoted 4 lines
Quoted 4 lines

testpmd+0xa5b20a)

    #16 <null> <null> (libtsan.so.0+0x2a68d)

Location is global '<null>' at 0x000000000000

(rtemap_0+0x00003ffd5628)

Quoted 2 lines
Quoted 2 lines

Thread T5 'vhost-events' (tid=76933, running) created by main thread

at:

Quoted 3 lines
Quoted 3 lines
    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1
rte_ctrl_thread_create ../lib/librte_eal/common/eal_common_thread.c:216
> > (dpdk-testpmd+0xa289e7)
Quoted 3 lines
Quoted 3 lines
    #2 rte_vhost_driver_start ../lib/librte_vhost/socket.c:1190 (dpdk-
testpmd+0x7728ef)
    #3 vhost_driver_setup ../drivers/net/vhost/rte_eth_vhost.c:1028

(dpdk-

Quoted 2 lines
Quoted 2 lines

testpmd+0x1de233d)

    #4 eth_dev_configure ../drivers/net/vhost/rte_eth_vhost.c:1126

(dpdk-

Quoted 2 lines
Quoted 2 lines

testpmd+0x1de29cc)

    #5 rte_eth_dev_configure ../lib/librte_ethdev/rte_ethdev.c:1439

(dpdk-

Quoted 2 lines
Quoted 2 lines

testpmd+0x991ce2)

    #6 start_port ../app/test-pmd/testpmd.c:2450 (dpdk-

testpmd+0x4f9b45)

Quoted 3 lines
Quoted 3 lines
    #7 main ../app/test-pmd/testpmd.c:3777 (dpdk-testpmd+0x4fe1ac)

Thread T3 'lcore-slave-3' (tid=76931, running) created by main

thread at:

Quoted 5 lines
Quoted 5 lines
    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_eal_init ../lib/librte_eal/linux/eal.c:1244 (dpdk-
testpmd+0xa46e2b)
    #2 main ../app/test-pmd/testpmd.c:3673 (dpdk-testpmd+0x4fdd75)
Quoted 8 lines
Quoted 8 lines

--

Or:
WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 1 at 0x00017ffd00f8 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:182
(dpdk-testpmd+0x769370)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380

(dpdk-

Quoted 2 lines
Quoted 2 lines

testpmd+0x78e4bf)

    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848

(dpdk-

Quoted 6 lines
Quoted 6 lines

testpmd+0x78fcf8)

    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-
testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-
testpmd+0x7591c2)
    #5
ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193
> > (dpdk-testpmd+0xa2890b)
> >     #6 <null> <null> (libtsan.so.0+0x2a68d)
Quoted 9 lines
Quoted 9 lines

Previous write of size 1 at 0x00017ffd00f8 by thread T3:
#0 vhost_user_iotlb_pending_insert ../lib/librte_vhost/iotlb.c:86
(dpdk-testpmd+0x75eb0c)

    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:58 (dpdk-
testpmd+0x774926)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-
testpmd+0x7a79d1)
    #3
virtio_dev_rx_batch_packed ../lib/librte_vhost/virtio_net.c:1295
> > (dpdk-testpmd+0x7a79d1)
Quoted 1 line
Quoted 1 line
    #4 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1376

(dpdk-

Quoted 6 lines
Quoted 6 lines

testpmd+0x7a79d1)

    #5 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-
testpmd+0x7b0654)
    #6 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465
(dpdk-testpmd+0x7b0654)
    #7 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-
Quoted 8 lines
Quoted 8 lines

testpmd+0x1ddfbd8)

    #8 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-
testpmd+0x505fdb)
    #9 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-
testpmd+0x5106ad)
    #10 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-
testpmd+0x4f8951)
    #11 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106

(dpdk-

Quoted 2 lines
Quoted 2 lines

testpmd+0x4f89d7)

    #12 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127

(dpdk-

Quoted 6 lines
Quoted 6 lines

testpmd+0xa5b20a)

    #13 <null> <null> (libtsan.so.0+0x2a68d)

--

As a consequence, the two threads can modify the same entry of the

mempool.

Quoted 8 lines
Quoted 8 lines

Usually, this cause a loop in iotlb_pending_entries list.

Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>
---
 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Quoted 7 lines

lib/librte_vhost/iotlb.c vhost_user_iotlb_init(struct virtio_net *dev, int · line 322

Quoted 7 lines
lib/librte_vhost/iotlb.c +0 -0
index 5b3a0c090..e0b67721b 100644
@@ -321,8 +321,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, int
 vq_index)
                      IOTLB_CACHE_SIZE, sizeof(struct

vhost_iotlb_entry), 0,

Quoted 8 lines
Quoted 8 lines
lib/librte_vhost/iotlb.c +1 -2
                      0, 0, NULL, NULL, NULL, socket,
                      MEMPOOL_F_NO_CACHE_ALIGN |
-                     MEMPOOL_F_SP_PUT |
-                     MEMPOOL_F_SC_GET);
+                     MEMPOOL_F_SP_PUT);
      if (!vq->iotlb_pool) {
              VHOST_LOG_CONFIG(ERR,

"Failed to create IOTLB cache pool

(%s)\n",

Quoted 3 lines
Quoted 2 lines

--
2.18.1

Reviewed-by on v1 1/1
#6 lore raw
In reply to Eugenio Pérez

Hi Eugenio,

Quoted 106 lines · Eugenio Pérez

Bugzilla bug: 523

Using testpmd as a vhost-user with iommu:

/home/dpdk/build/app/dpdk-testpmd -l 1,3 \
       --vdev net_vhost0,iface=/tmp/vhost-user1,queues=1,iommu-support=1 \
       -- --auto-start --stats-period 5 --forward-mode=txonly

And qemu with packed virtqueue:

   <interface type='vhostuser'>
     <mac address='88:67:11:5f:dd:02'/>
     <source type='unix' path='/tmp/vhost-user1' mode='client'/>
     <model type='virtio'/>
     <driver name='vhost' rx_queue_size='1024' iommu='on' ats='on'/>
     <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
   </interface>

...

<qemu:commandline>
<qemu:arg value='-set'/>
<qemu:arg value='device.net1.packed=on'/>
</qemu:commandline>

--

Is it possible to consume the iotlb's entries of the mempoo from different threads. Thread sanitizer example output (after change rwlocks to POSIX ones):

WARNING: ThreadSanitizer: data race (pid=76927)

 Write of size 8 at 0x00017ffd5628 by thread T5:
   #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:181 (dpdk-testpmd+0x769343)
   #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
   #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
   #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
   #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
   #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
   #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous read of size 8 at 0x00017ffd5628 by thread T3:

   #0 vhost_user_iotlb_cache_find ../lib/librte_vhost/iotlb.c:252 (dpdk-testpmd+0x76ee96)
   #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:42 (dpdk-testpmd+0x77488c)
   #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7abeb3)
   #3 map_one_desc ../lib/librte_vhost/virtio_net.c:497 (dpdk-testpmd+0x7abeb3)
   #4 fill_vec_buf_packed ../lib/librte_vhost/virtio_net.c:751 (dpdk-testpmd+0x7abeb3)
   #5 vhost_enqueue_single_packed ../lib/librte_vhost/virtio_net.c:1170 (dpdk-testpmd+0x7abeb3)
   #6 virtio_dev_rx_single_packed ../lib/librte_vhost/virtio_net.c:1346 (dpdk-testpmd+0x7abeb3)
   #7 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1384 (dpdk-testpmd+0x7abeb3)
   #8 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
   #9 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
   #10 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
   #11 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
   #12 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
   #13 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
   #14 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
   #15 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
   #16 <null> <null> (libtsan.so.0+0x2a68d)

Location is global '<null>' at 0x000000000000 (rtemap_0+0x00003ffd5628)

Thread T5 'vhost-events' (tid=76933, running) created by main thread at:

   #0 pthread_create <null> (libtsan.so.0+0x2cd42)
   #1 rte_ctrl_thread_create ../lib/librte_eal/common/eal_common_thread.c:216 (dpdk-testpmd+0xa289e7)
   #2 rte_vhost_driver_start ../lib/librte_vhost/socket.c:1190 (dpdk-testpmd+0x7728ef)
   #3 vhost_driver_setup ../drivers/net/vhost/rte_eth_vhost.c:1028 (dpdk-testpmd+0x1de233d)
   #4 eth_dev_configure ../drivers/net/vhost/rte_eth_vhost.c:1126 (dpdk-testpmd+0x1de29cc)
   #5 rte_eth_dev_configure ../lib/librte_ethdev/rte_ethdev.c:1439 (dpdk-testpmd+0x991ce2)
   #6 start_port ../app/test-pmd/testpmd.c:2450 (dpdk-testpmd+0x4f9b45)
   #7 main ../app/test-pmd/testpmd.c:3777 (dpdk-testpmd+0x4fe1ac)

Thread T3 'lcore-slave-3' (tid=76931, running) created by main thread at:

   #0 pthread_create <null> (libtsan.so.0+0x2cd42)
   #1 rte_eal_init ../lib/librte_eal/linux/eal.c:1244 (dpdk-testpmd+0xa46e2b)
   #2 main ../app/test-pmd/testpmd.c:3673 (dpdk-testpmd+0x4fdd75)

--

Or:
WARNING: ThreadSanitizer: data race (pid=76927)

 Write of size 1 at 0x00017ffd00f8 by thread T5:
   #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:182 (dpdk-testpmd+0x769370)
   #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
   #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
   #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
   #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
   #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
   #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous write of size 1 at 0x00017ffd00f8 by thread T3:

   #0 vhost_user_iotlb_pending_insert ../lib/librte_vhost/iotlb.c:86 (dpdk-testpmd+0x75eb0c)
   #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:58 (dpdk-testpmd+0x774926)
   #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7a79d1)
   #3 virtio_dev_rx_batch_packed ../lib/librte_vhost/virtio_net.c:1295 (dpdk-testpmd+0x7a79d1)
   #4 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1376 (dpdk-testpmd+0x7a79d1)
   #5 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
   #6 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
   #7 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
   #8 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
   #9 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
   #10 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
   #11 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
   #12 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
   #13 <null> <null> (libtsan.so.0+0x2a68d)

--

Quoted 9 lines

As a consequence, the two threads can modify the same entry of the mempool. Usually, this cause a loop in iotlb_pending_entries list.

Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>

---
lib/librte_vhost/iotlb.c | 3 +--

1 file changed, 1 insertion(+), 2 deletions(-)

looks good to me.

Reviewed-by Jens Freimann <[email protected]>

regards,
Jens

author
#7 lore raw
In reply to Eugenio Pérez
Quoted 119 lines · Eugenio Pérez

Bugzilla bug: 523

Using testpmd as a vhost-user with iommu:

/home/dpdk/build/app/dpdk-testpmd -l 1,3 \
        --vdev net_vhost0,iface=/tmp/vhost-user1,queues=1,iommu-support=1 \
        -- --auto-start --stats-period 5 --forward-mode=txonly

And qemu with packed virtqueue:

    <interface type='vhostuser'>
      <mac address='88:67:11:5f:dd:02'/>
      <source type='unix' path='/tmp/vhost-user1' mode='client'/>
      <model type='virtio'/>
      <driver name='vhost' rx_queue_size='1024' iommu='on' ats='on'/>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
    </interface>

...

<qemu:commandline>

    <qemu:arg value='-set'/>
    <qemu:arg value='device.net1.packed=on'/>

</qemu:commandline>

--

Is it possible to consume the iotlb's entries of the mempoo from different threads. Thread sanitizer example output (after change rwlocks to POSIX ones):

WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 8 at 0x00017ffd5628 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:181 (dpdk-testpmd+0x769343)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous read of size 8 at 0x00017ffd5628 by thread T3:

    #0 vhost_user_iotlb_cache_find ../lib/librte_vhost/iotlb.c:252 (dpdk-testpmd+0x76ee96)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:42 (dpdk-testpmd+0x77488c)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7abeb3)
    #3 map_one_desc ../lib/librte_vhost/virtio_net.c:497 (dpdk-testpmd+0x7abeb3)
    #4 fill_vec_buf_packed ../lib/librte_vhost/virtio_net.c:751 (dpdk-testpmd+0x7abeb3)
    #5 vhost_enqueue_single_packed ../lib/librte_vhost/virtio_net.c:1170 (dpdk-testpmd+0x7abeb3)
    #6 virtio_dev_rx_single_packed ../lib/librte_vhost/virtio_net.c:1346 (dpdk-testpmd+0x7abeb3)
    #7 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1384 (dpdk-testpmd+0x7abeb3)
    #8 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
    #9 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
    #10 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
    #11 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
    #12 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
    #13 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
    #14 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
    #15 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
    #16 <null> <null> (libtsan.so.0+0x2a68d)

Location is global '<null>' at 0x000000000000 (rtemap_0+0x00003ffd5628)

Thread T5 'vhost-events' (tid=76933, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_ctrl_thread_create ../lib/librte_eal/common/eal_common_thread.c:216 (dpdk-testpmd+0xa289e7)
    #2 rte_vhost_driver_start ../lib/librte_vhost/socket.c:1190 (dpdk-testpmd+0x7728ef)
    #3 vhost_driver_setup ../drivers/net/vhost/rte_eth_vhost.c:1028 (dpdk-testpmd+0x1de233d)
    #4 eth_dev_configure ../drivers/net/vhost/rte_eth_vhost.c:1126 (dpdk-testpmd+0x1de29cc)
    #5 rte_eth_dev_configure ../lib/librte_ethdev/rte_ethdev.c:1439 (dpdk-testpmd+0x991ce2)
    #6 start_port ../app/test-pmd/testpmd.c:2450 (dpdk-testpmd+0x4f9b45)
    #7 main ../app/test-pmd/testpmd.c:3777 (dpdk-testpmd+0x4fe1ac)

Thread T3 'lcore-slave-3' (tid=76931, running) created by main thread at:

    #0 pthread_create <null> (libtsan.so.0+0x2cd42)
    #1 rte_eal_init ../lib/librte_eal/linux/eal.c:1244 (dpdk-testpmd+0xa46e2b)
    #2 main ../app/test-pmd/testpmd.c:3673 (dpdk-testpmd+0x4fdd75)

--

Or:
WARNING: ThreadSanitizer: data race (pid=76927)

  Write of size 1 at 0x00017ffd00f8 by thread T5:
    #0 vhost_user_iotlb_cache_insert ../lib/librte_vhost/iotlb.c:182 (dpdk-testpmd+0x769370)
    #1 vhost_user_iotlb_msg ../lib/librte_vhost/vhost_user.c:2380 (dpdk-testpmd+0x78e4bf)
    #2 vhost_user_msg_handler ../lib/librte_vhost/vhost_user.c:2848 (dpdk-testpmd+0x78fcf8)
    #3 vhost_user_read_cb ../lib/librte_vhost/socket.c:311 (dpdk-testpmd+0x770162)
    #4 fdset_event_dispatch ../lib/librte_vhost/fd_man.c:286 (dpdk-testpmd+0x7591c2)
    #5 ctrl_thread_init ../lib/librte_eal/common/eal_common_thread.c:193 (dpdk-testpmd+0xa2890b)
    #6 <null> <null> (libtsan.so.0+0x2a68d)

Previous write of size 1 at 0x00017ffd00f8 by thread T3:

    #0 vhost_user_iotlb_pending_insert ../lib/librte_vhost/iotlb.c:86 (dpdk-testpmd+0x75eb0c)
    #1 __vhost_iova_to_vva ../lib/librte_vhost/vhost.c:58 (dpdk-testpmd+0x774926)
    #2 vhost_iova_to_vva ../lib/librte_vhost/vhost.h:753 (dpdk-testpmd+0x7a79d1)
    #3 virtio_dev_rx_batch_packed ../lib/librte_vhost/virtio_net.c:1295 (dpdk-testpmd+0x7a79d1)
    #4 virtio_dev_rx_packed ../lib/librte_vhost/virtio_net.c:1376 (dpdk-testpmd+0x7a79d1)
    #5 virtio_dev_rx ../lib/librte_vhost/virtio_net.c:1435 (dpdk-testpmd+0x7b0654)
    #6 rte_vhost_enqueue_burst ../lib/librte_vhost/virtio_net.c:1465 (dpdk-testpmd+0x7b0654)
    #7 eth_vhost_tx ../drivers/net/vhost/rte_eth_vhost.c:470 (dpdk-testpmd+0x1ddfbd8)
    #8 rte_eth_tx_burst ../lib/librte_ethdev/rte_ethdev.h:4800 (dpdk-testpmd+0x505fdb)
    #9 pkt_burst_transmit ../app/test-pmd/txonly.c:365 (dpdk-testpmd+0x5106ad)
    #10 run_pkt_fwd_on_lcore ../app/test-pmd/testpmd.c:2080 (dpdk-testpmd+0x4f8951)
    #11 start_pkt_forward_on_core ../app/test-pmd/testpmd.c:2106 (dpdk-testpmd+0x4f89d7)
    #12 eal_thread_loop ../lib/librte_eal/linux/eal_thread.c:127 (dpdk-testpmd+0xa5b20a)
    #13 <null> <null> (libtsan.so.0+0x2a68d)

--

As a consequence, the two threads can modify the same entry of the mempool. Usually, this cause a loop in iotlb_pending_entries list.

This behavior is only observed on packed vq + virtio-net kernel driver in the guest, so we could make the single-consumer flag optional. However, I have not found why I cannot see the issue in split, so the safer option is to never set it.

Any comments?

Thanks!

Quoted 9 lines

Eugenio Pérez (1):
vhost: fix iotlb mempool single-consumer flag

 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--
2.18.1

Hi! Just a friendly ping on this. Should I CC somebody else?

Thanks!

author
#8 lore raw

v2 cover vhost: Make iotlb mempool not single-consumer

In reply to Eugenio Pérez

Bugzilla bug: 523

This behavior is only observed on packed vq + virtio-net kernel driver in the guest, so we could make the single-consumer flag optional. However, I have not found why I cannot see the issue in split, so the safer option is to never set it.

Any comments?

Thanks!

v2: Modify commit message to not to include all trace and commands.

Eugenio Pérez (1):
vhost: fix iotlb mempool single-consumer flag

 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Signature · 1 line
2.18.1
author
#9 lore raw

PATCH v2 1/1 vhost: fix iotlb mempool single-consumer flag

Bugzilla bug: 523

Control thread (which handles iotlb msg) and forwarding thread both use iotlb to translate address. The former may modify the same entry of mempool and may cause a loop in iotlb_pending_entries list.

Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>
---
 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
lib/librte_vhost/iotlb.c +1 -2
index 5b3a0c090..e0b67721b 100644
@@ -321,8 +321,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, int vq_index)
 			IOTLB_CACHE_SIZE, sizeof(struct vhost_iotlb_entry), 0,
 			0, 0, NULL, NULL, NULL, socket,
 			MEMPOOL_F_NO_CACHE_ALIGN |
-			MEMPOOL_F_SP_PUT |
-			MEMPOOL_F_SC_GET);
+			MEMPOOL_F_SP_PUT);
 	if (!vq->iotlb_pool) {
 		VHOST_LOG_CONFIG(ERR,
 				"Failed to create IOTLB cache pool (%s)\n",
Signature · 1 line
2.18.1
Reviewed-by on v2 1/1
#10 lore raw
Quoted 38 lines

-----Original Message-----
From: Eugenio Pérez <[email protected]>
Sent: Monday, August 31, 2020 3:59 PM
To: [email protected]
Cc: Adrian Moreno Zapata <[email protected]>; Xia, Chenbo <[email protected]>; Wang, Zhihong <[email protected]>; Jens Freimann <[email protected]>; [email protected]; Maxime Coquelin <[email protected]>; Kevin Traynor <[email protected]>
Subject: [PATCH v2 1/1] vhost: fix iotlb mempool single-consumer flag

Bugzilla bug: 523

Control thread (which handles iotlb msg) and forwarding thread both use iotlb to translate address. The former may modify the same entry of mempool and may cause a loop in iotlb_pending_entries list.

Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>
---
 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
lib/librte_vhost/iotlb.c +1 -2
index 5b3a0c090..e0b67721b 100644
@@ -321,8 +321,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, int
 vq_index)
 			IOTLB_CACHE_SIZE, sizeof(struct vhost_iotlb_entry), 0,
 			0, 0, NULL, NULL, NULL, socket,
 			MEMPOOL_F_NO_CACHE_ALIGN |
-			MEMPOOL_F_SP_PUT |
-			MEMPOOL_F_SC_GET);
+			MEMPOOL_F_SP_PUT);
 	if (!vq->iotlb_pool) {
 		VHOST_LOG_CONFIG(ERR,
Quoted 3 lines

"Failed to create IOTLB cache pool (%s)\n",
--
2.18.1

Reviewed-by Chenbo Xia <[email protected]>
Reviewed-by on v2 1/1
#11 lore raw
In reply to Eugenio Pérez
Quoted 6 lines · Eugenio Pérez

Bugzilla bug: 523

Control thread (which handles iotlb msg) and forwarding thread both use iotlb to translate address. The former may modify the same entry of mempool and may cause a loop in iotlb_pending_entries list.

Bugzilla ID: 523

Quoted 12 lines
Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>
---
 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
lib/librte_vhost/iotlb.c +0 -0
index 5b3a0c090..e0b67721b 100644

lib/librte_vhost/iotlb.c vhost_user_iotlb_init(struct virtio_net *dev, int vq_index) · line 327

Quoted 9 lines
lib/librte_vhost/iotlb.c +1 -2
@@ -321,8 +321,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, int vq_index)
 			IOTLB_CACHE_SIZE, sizeof(struct vhost_iotlb_entry), 0,
 			0, 0, NULL, NULL, NULL, socket,
 			MEMPOOL_F_NO_CACHE_ALIGN |
-			MEMPOOL_F_SP_PUT |
-			MEMPOOL_F_SC_GET);
+			MEMPOOL_F_SP_PUT);
 	if (!vq->iotlb_pool) {
 		VHOST_LOG_CONFIG(ERR,
 				"Failed to create IOTLB cache pool (%s)\n",

I'll fix commit message while applying.

Thanks for your contribution:

Reviewed-by Maxime Coquelin <[email protected]>

Maxime

Applied
#12 lore raw
In reply to Eugenio Pérez
Quoted 7 lines · Eugenio Pérez

Bugzilla bug: 523

Control thread (which handles iotlb msg) and forwarding thread both use iotlb to translate address. The former may modify the same entry of mempool and may cause a loop in iotlb_pending_entries list.

Quoted 5 lines
Fixes d012d1f293f4 ("vhost: add IOTLB helper functions")
Signed-off-by Eugenio Pérez <[email protected]>
---
 lib/librte_vhost/iotlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Applied to dpdk-next-virtio/master.

Thanks,
Maxime