Thread (32 messages) 32 messages, 6 authors, 2016-11-23

Re: [PATCH] Revert "bonding: use existing enslaved device queues"

From: Ilya Maximets <hidden>
Date: 2016-10-12 13:24:54

On 07.10.2016 05:02, Eric Kinzie wrote:
On Wed Sep 07 15:28:10 +0300 2016, Ilya Maximets wrote:
quoted
This reverts commit 5b7bb2bda5519b7800f814df64d4e015282140e5.

It is necessary to reconfigure all queues every time because configuration
can be changed.

For example, if we're reconfiguring bonding device with new memory pool,
already configured queues will still use the old one. And if the old
mempool be freed, application likely will panic in attempt to use
freed mempool.

This happens when we use the bonding device with OVS 2.6 while MTU
reconfiguration:

PANIC in rte_mempool_get_ops():
assert "(ops_index >= 0) && (ops_index < RTE_MEMPOOL_MAX_OPS_IDX)" failed

Cc: <redacted>
Signed-off-by: Ilya Maximets <redacted>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b20a272..eb5b6d1 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1305,8 +1305,6 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
 	struct bond_rx_queue *bd_rx_q;
 	struct bond_tx_queue *bd_tx_q;
 
-	uint16_t old_nb_tx_queues = slave_eth_dev->data->nb_tx_queues;
-	uint16_t old_nb_rx_queues = slave_eth_dev->data->nb_rx_queues;
 	int errval;
 	uint16_t q_id;
 
@@ -1347,9 +1345,7 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
 	}
 
 	/* Setup Rx Queues */
-	/* Use existing queues, if any */
-	for (q_id = old_nb_rx_queues;
-	     q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
+	for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
 		bd_rx_q = (struct bond_rx_queue *)bonded_eth_dev->data->rx_queues[q_id];
 
 		errval = rte_eth_rx_queue_setup(slave_eth_dev->data->port_id, q_id,
@@ -1365,9 +1361,7 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
 	}
 
 	/* Setup Tx Queues */
-	/* Use existing queues, if any */
-	for (q_id = old_nb_tx_queues;
-	     q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
+	for (q_id = 0; q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
 		bd_tx_q = (struct bond_tx_queue *)bonded_eth_dev->data->tx_queues[q_id];
 
 		errval = rte_eth_tx_queue_setup(slave_eth_dev->data->port_id, q_id,
-- 
2.7.4
NAK

There are still some users of this code.  Let's give them a chance to
comment before removing it.
Hi Eric,

Are these users in CC-list? If not, could you, please, add them?
This patch awaits in mail-list already more than a month. I think, it's enough
time period for all who wants to say something. Patch fixes a real bug that
prevent using of DPDK bonding in all applications that reconfigures devices
in runtime including OVS.

Best regards, Ilya Maximets.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help