[dpdk-dev,6/9] net/virtio: fix queue setup consistency
Checks
Commit Message
In rx/tx queue setup functions, some code is executed only if
use_simple_rxtx == 1. The value of this variable can change depending on
the offload flags or sse support. If Rx queue setup is called before Tx
queue setup, it can result in an invalid configuration:
- dev_configure is called: use_simple_rxtx is initialized to 0
- rx queue setup is called: queues are initialized without simple path
support
- tx queue setup is called: use_simple_rxtx switch to 1, and simple
Rx/Tx handlers are selected
Fix this by postponing a part of Rx/Tx queue initialization in
dev_start(), as it was the case in the initial implementation.
Fixes: 48cec290a3d2 ("net/virtio: move queue configure code to proper place")
Cc: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/virtio/virtio_ethdev.c | 13 +++++++++++++
drivers/net/virtio/virtio_ethdev.h | 6 ++++++
drivers/net/virtio/virtio_rxtx.c | 40 ++++++++++++++++++++++++++++++--------
3 files changed, 51 insertions(+), 8 deletions(-)
Comments
Platform description
--------------------
guest (dpdk)
+----------------+
| |
| |
| port0 |
+----------------+
|
| virtio
|
+----------------+
| tap0 |
| |
| |
+----------------+
host (linux, vhost-net)
Host configuration
------------------
Start qemu with:
- a ne2k management interface to avoi any conflict with dpdk
- a virtio net device, connected to a tap interface through vhost-net
- mergeable buffers disabled
/usr/bin/qemu-system-x86_64 -k fr -daemonize --enable-kvm -m 2G -cpu host \
-smp 3 -serial telnet::40564,server,nowait -serial null \
-qmp tcp::44340,server,nowait -monitor telnet::49229,server,nowait \
-device ne2k_pci,mac=de:ad:de:01:02:03,netdev=user.0,addr=03 \
-netdev user,id=user.0,hostfwd=tcp::34965-:22 \
-netdev type=tap,id=vhostnet0,script=no,vhost=on,queues=8 \
-device virtio-net-pci,mrg_rxbuf=off,netdev=vhostnet0,mq=on,vectors=17 \
-hda "${VM_PATH}/ubuntu-16.04-template.qcow2" \
-snapshot -vga none -display none
Guest configuration
-------------------
Apply a patch that reverts initialization of queues in testpmd
(initialize rx queue first), and displays some logs in virtio:
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1461,34 +1461,10 @@ start_port(portid_t pid)
}
if (port->need_reconfig_queues > 0) {
port->need_reconfig_queues = 0;
- /* setup tx queues */
- for (qi = 0; qi < nb_txq; qi++) {
- if ((numa_support) &&
- (txring_numa[pi] != NUMA_NO_CONFIG))
- diag = rte_eth_tx_queue_setup(pi, qi,
- nb_txd,txring_numa[pi],
- &(port->tx_conf));
- else
- diag = rte_eth_tx_queue_setup(pi, qi,
- nb_txd,port->socket_id,
- &(port->tx_conf));
-
- if (diag == 0)
- continue;
-
- /* Fail to setup tx queue, return */
- if (rte_atomic16_cmpset(&(port->port_status),
- RTE_PORT_HANDLING,
- RTE_PORT_STOPPED) == 0)
- printf("Port %d can not be set back "
- "to stopped\n", pi);
- printf("Fail to configure port %d tx queues\n", pi);
- /* try to reconfigure queues next time */
- port->need_reconfig_queues = 1;
- return -1;
- }
/* setup rx queues */
for (qi = 0; qi < nb_rxq; qi++) {
+ printf("rte_eth_rx_queue_setup %d %d\n",
+ pi, qi);
if ((numa_support) &&
(rxring_numa[pi] != NUMA_NO_CONFIG)) {
struct rte_mempool * mp =
@@ -1500,7 +1476,6 @@ start_port(portid_t pid)
rxring_numa[pi]);
return -1;
}
-
diag = rte_eth_rx_queue_setup(pi, qi,
nb_rxd,rxring_numa[pi],
&(port->rx_conf),mp);
@@ -1532,6 +1507,34 @@ start_port(portid_t pid)
port->need_reconfig_queues = 1;
return -1;
}
+ /* setup tx queues */
+ for (qi = 0; qi < nb_txq; qi++) {
+ printf("rte_eth_tx_queue_setup %d %d\n",
+ pi, qi);
+ if ((numa_support) &&
+ (txring_numa[pi] != NUMA_NO_CONFIG))
+ diag = rte_eth_tx_queue_setup(pi, qi,
+ nb_txd,txring_numa[pi],
+ &(port->tx_conf));
+ else
+ diag = rte_eth_tx_queue_setup(pi, qi,
+ nb_txd,port->socket_id,
+ &(port->tx_conf));
+
+ if (diag == 0)
+ continue;
+
+ /* Fail to setup tx queue, return */
+ if (rte_atomic16_cmpset(&(port->port_status),
+ RTE_PORT_HANDLING,
+ RTE_PORT_STOPPED) == 0)
+ printf("Port %d can not be set back "
+ "to stopped\n", pi);
+ printf("Fail to configure port %d tx queues\n", pi);
+ /* try to reconfigure queues next time */
+ port->need_reconfig_queues = 1;
+ return -1;
+ }
}
for (event_type = RTE_ETH_EVENT_UNKNOWN;
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -445,6 +445,8 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
nbufs = 0;
error = ENOSPC;
+ printf("rx_queue_setup() use_simple_rxtx=%d\n",
+ hw->use_simple_rxtx);
if (hw->use_simple_rxtx) {
for (desc_idx = 0; desc_idx < vq->vq_nentries;
desc_idx++) {
@@ -563,6 +565,8 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
vq->vq_free_thresh = tx_free_thresh;
+ printf("tx_queue_setup() use_simple_rxtx=%d\n",
+ hw->use_simple_rxtx);
if (hw->use_simple_rxtx) {
uint16_t mid_idx = vq->vq_nentries >> 1;
Compile dpdk:
cd dpdk.org
make config T=x86_64-native-linuxapp-gcc
sed -i 's,CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_INIT=n,CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_INIT=y,' build/.config
sed -i 's,CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n,CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=y,' build/.config
make -j4
Prepare environment:
mkdir -p /mnt/huge
mount -t hugetlbfs nodev /mnt/huge
echo 256 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
modprobe uio_pci_generic
python usertools/dpdk-devbind.py -b uio_pci_generic 0000:00:02.0
./build/app/testpmd -l 0,1 --log-level 7 -- --total-num-mbufs=16384 \
-i --port-topology=chained --disable-hw-vlan-filter \
--disable-hw-vlan-strip --txqflags=0xf01
...
Configuring Port 0 (socket 0)
rte_eth_rx_queue_setup 0 0
rx_queue_setup() use_simple_rxtx=0
rte_eth_tx_queue_setup 0 0
PMD: virtio_update_rxtx_handler(): Using simple rx/tx path
tx_queue_setup() use_simple_rxtx=1
...
Configure testpmd:
set fwd rxonly
set verbose 1
start
Without the fix, there is a segfault in virtio_recv_pkts_vec()
It works ok with the patch.
@@ -1737,6 +1737,19 @@ virtio_dev_start(struct rte_eth_dev *dev)
struct virtnet_rx *rxvq;
struct virtnet_tx *txvq __rte_unused;
struct virtio_hw *hw = dev->data->dev_private;
+ int ret;
+
+ /* Finish the initialization of the queues */
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ ret = virtio_dev_rx_queue_setup_finish(dev, i);
+ if (ret < 0)
+ return ret;
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ ret = virtio_dev_tx_queue_setup_finish(dev, i);
+ if (ret < 0)
+ return ret;
+ }
/* check if lsc interrupt feature is enabled */
if (dev->data->dev_conf.intr_conf.lsc) {
@@ -92,10 +92,16 @@ int virtio_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
const struct rte_eth_rxconf *rx_conf,
struct rte_mempool *mb_pool);
+int virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev,
+ uint16_t rx_queue_id);
+
int virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
uint16_t nb_tx_desc, unsigned int socket_id,
const struct rte_eth_txconf *tx_conf);
+int virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev,
+ uint16_t tx_queue_id);
+
uint16_t virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
uint16_t nb_pkts);
@@ -421,9 +421,6 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
struct virtio_hw *hw = dev->data->dev_private;
struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
struct virtnet_rx *rxvq;
- int error, nbufs;
- struct rte_mbuf *m;
- uint16_t desc_idx;
PMD_INIT_FUNC_TRACE();
@@ -440,10 +437,24 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
}
dev->data->rx_queues[queue_idx] = rxvq;
+ return 0;
+}
+
+int
+virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev, uint16_t queue_idx)
+{
+ uint16_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
+ struct virtio_hw *hw = dev->data->dev_private;
+ struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
+ struct virtnet_rx *rxvq = &vq->rxq;
+ struct rte_mbuf *m;
+ uint16_t desc_idx;
+ int error, nbufs;
+
+ PMD_INIT_FUNC_TRACE();
/* Allocate blank mbufs for the each rx descriptor */
nbufs = 0;
- error = ENOSPC;
if (hw->use_simple_rxtx) {
for (desc_idx = 0; desc_idx < vq->vq_nentries;
@@ -534,7 +545,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
struct virtnet_tx *txvq;
uint16_t tx_free_thresh;
- uint16_t desc_idx;
PMD_INIT_FUNC_TRACE();
@@ -563,9 +573,24 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
vq->vq_free_thresh = tx_free_thresh;
- if (hw->use_simple_rxtx) {
- uint16_t mid_idx = vq->vq_nentries >> 1;
+ dev->data->tx_queues[queue_idx] = txvq;
+ return 0;
+}
+
+int
+virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev,
+ uint16_t queue_idx)
+{
+ uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+ struct virtio_hw *hw = dev->data->dev_private;
+ struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
+ uint16_t mid_idx = vq->vq_nentries >> 1;
+ struct virtnet_tx *txvq = &vq->txq;
+ uint16_t desc_idx;
+ PMD_INIT_FUNC_TRACE();
+
+ if (hw->use_simple_rxtx) {
for (desc_idx = 0; desc_idx < mid_idx; desc_idx++) {
vq->vq_ring.avail->ring[desc_idx] =
desc_idx + mid_idx;
@@ -587,7 +612,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
VIRTQUEUE_DUMP(vq);
- dev->data->tx_queues[queue_idx] = txvq;
return 0;
}