Message ID | 20230122183406.9679-1-cleviatan@marvell.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev] netdev-dpdk: config tx queue number to be the minimum between device and ovs request | expand |
On 1/22/23 19:34, cleviatan@marvell.com wrote: > From: Chava Leviatan <cleviatan@marvell.com> Hi. Thanks for the patch. See some comments below. > > Tx queue are set each time port is added/removed or the cmask changes by > reconfigure_datapath. The amount of TX queues is set according to PMD > thread and does not take into consideration the device capabilities . It does. If you look at dpdk_eth_dev_port_config(), first it called with the minimum of requested and maximum available number of queues. And then it calls rte_eth_dev_configure() and tries to create all the queues with rte_eth_tx_queue_setup() gradualy decreasing the number of requested queues until all calls are successful. Once this function succeeds, we can be sure that all the queues are created. The number is stored in the netdev->n_txq field. After each netdev_reconfigure() call dpif-netdev re-checks the number of available queues and it should never use queues higher than netdev's n_txq field. > As a result , when transmitting packet from OVS to device driver by > rte_eth_tx_burst , the device driver is called with > dev->data->tx_queues[queue_id] which is not a legal address , hence > can cause a crash . This should not be possible according to the explanation above. If dpif-netdev is calling netdev_send() for the queue number higher than netdev->n_txq, we should fix that instead. Do you have a more detailed call graph on how did that happen in you case? Or maybe a stack trace? Best regards, Ilya Maximets.
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ab5b8223e..278da12d4 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -508,6 +508,7 @@ struct netdev_dpdk { int requested_n_rxq; int requested_rxq_size; int requested_txq_size; + int device_n_txq; /* Number of rx/tx descriptors for physical devices */ int rxq_size; @@ -1124,6 +1125,7 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int n_txq) dev->up.n_rxq = n_rxq; dev->up.n_txq = n_txq; + dev->device_n_txq = n_txq; return 0; } @@ -2133,15 +2135,20 @@ netdev_dpdk_get_numa_id(const struct netdev *netdev) static int netdev_dpdk_set_tx_multiq(struct netdev *netdev, unsigned int n_txq) { + unsigned int req_n_txq; struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); ovs_mutex_lock(&dev->mutex); - if (dev->requested_n_txq == n_txq) { + req_n_txq = MIN(n_txq, dev->device_n_txq); + if (dev->requested_n_txq == req_n_txq) { goto out; } - - dev->requested_n_txq = n_txq; + if (!dev->device_n_txq) { + dev->device_n_txq = 1; + req_n_txq = 1; + } + dev->requested_n_txq = req_n_txq; netdev_request_reconfigure(netdev); out: