[dpdk-dev,v2,2/4] ethdev: prevent changing of nb_q_per_pool in rte_eth_dev_check_mq_mode()

Message ID 1421672551-11652-3-git-send-email-pawelx.wodkowski@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Wodkowski, PawelX Jan. 19, 2015, 1:02 p.m. UTC
  If SRIOV is used and device configuration does not use MQ the
RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool is set to 1 in
rte_eth_dev_check_mq_mode().
This is bad becouse of two reasons:
1. Port reconfiguration from non-MQ mode to MQ mode is impossible
2. Confguring RX and TX side in different way is impossible.

This patch fix first issue by not changing
RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool
and second by comparing nb_q_per_pool separately for RX
(nb_rx_q_per_pool) and
for TX (nb_tx_q_per_pool).

Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
---
 lib/librte_ether/rte_ethdev.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
  

Comments

Ouyang Changchun Jan. 20, 2015, 1:32 a.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pawel Wodkowski
> Sent: Monday, January 19, 2015 9:02 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 2/4] ethdev: prevent changing of
> nb_q_per_pool in rte_eth_dev_check_mq_mode()
> 
> If SRIOV is used and device configuration does not use MQ the
> RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool is set to 1 in
> rte_eth_dev_check_mq_mode().
> This is bad becouse of two reasons:
> 1. Port reconfiguration from non-MQ mode to MQ mode is impossible 2.
> Confguring RX and TX side in different way is impossible.
> 

This case is possible:
rxmode.mq_mode is ETH_MQ_RX_VMDQ_RSS, and txmode.mq_mode is ETH_MQ_TX_NONE.
  
Wodkowski, PawelX Jan. 20, 2015, 9:09 a.m. UTC | #2
> -----Original Message-----
> From: Ouyang, Changchun
> Sent: Tuesday, January 20, 2015 2:33 AM
> To: Wodkowski, PawelX; dev@dpdk.org
> Cc: Ouyang, Changchun
> Subject: RE: [dpdk-dev] [PATCH v2 2/4] ethdev: prevent changing of
> nb_q_per_pool in rte_eth_dev_check_mq_mode()
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pawel Wodkowski
> > Sent: Monday, January 19, 2015 9:02 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 2/4] ethdev: prevent changing of
> > nb_q_per_pool in rte_eth_dev_check_mq_mode()
> >
> > If SRIOV is used and device configuration does not use MQ the
> > RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool is set to 1 in
> > rte_eth_dev_check_mq_mode().
> > This is bad becouse of two reasons:
> > 1. Port reconfiguration from non-MQ mode to MQ mode is impossible 2.
> > Confguring RX and TX side in different way is impossible.
> >
> 
> This case is possible:
> rxmode.mq_mode is ETH_MQ_RX_VMDQ_RSS, and txmode.mq_mode is
> ETH_MQ_TX_NONE.
> 
but ETH_MQ_RX_NONE -> ETH_MQ_RX_VMDQ_RSS is not. 

I have 8 VFs
In testpmd

testpmd> port config all rxq 2
port config all rxq 2

testpmd> port start 0
port start 0

Configuring Port 0 (socket 0)
Fail to configure port 0
testpmd> port config all rxq 4
port config all rxq 4

testpmd> port start 0
port start 0

Configuring Port 0 (socket 0)
Fail to configure port 0
testpmd> port config all rxq 8
port config all rxq 8

testpmd> port start all
port start all

Configuring Port 0 (socket 0)
Fail to configure port 0
testpmd> port config all rxq 1
port config all rxq 1

testpmd> port start 0
port start 0

Configuring Port 0 (socket 0)
PMD: ixgbe_dev_tx_queue_setup(): sw_ring=0x7ffec0ae9140 hw_ring=0x7ffec2c0bf00 dma_addr=0x102c0bf00
PMD: set_tx_function(): Using full-featured tx code path
PMD: set_tx_function():  - txq_flags = 0 [IXGBE_SIMPLE_FLAGS=f01]
PMD: set_tx_function():  - tx_rs_thresh = 32 [RTE_PMD_IXGBE_TX_MAX_BURST=32]
PMD: ixgbe_dev_rx_queue_setup(): sw_ring=0x7ffec0ae88c0 hw_ring=0x7ffec2c1bf00 dma_addr=0x102c1bf00
PMD: ixgbe_dev_rx_queue_setup(): Rx Burst Bulk Alloc Preconditions are satisfied. Rx Burst Bulk Alloc function will be used on port=0, queue=0.
PMD: ixgbe_dev_rx_queue_setup(): Vector rx enabled, please make sure RX burst size no less than 32.
Port 0: 00:1B:21:C7:33:B0
Checking link statuses...
Port 0 Link Up - speed 10000 Mbps - full-duplex
Port 1 Link Down
Done
testpmd>

Please refer to RSS patch thread. I will post there second reply.

Pawel
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 62d7f6e..85385f8 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -548,6 +548,9 @@  rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 			return (-EINVAL);
 		}
 
+		uint16_t nb_rx_q_per_pool = RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool;
+		uint16_t nb_tx_q_per_pool = RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool;
+
 		switch (dev_conf->rxmode.mq_mode) {
 		case ETH_MQ_RX_VMDQ_DCB:
 		case ETH_MQ_RX_VMDQ_DCB_RSS:
@@ -580,8 +583,8 @@  rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 		default: /* ETH_MQ_RX_VMDQ_ONLY or ETH_MQ_RX_NONE */
 			/* if nothing mq mode configure, use default scheme */
 			dev->data->dev_conf.rxmode.mq_mode = ETH_MQ_RX_VMDQ_ONLY;
-			if (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool > 1)
-				RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool = 1;
+			if (nb_rx_q_per_pool > 1)
+				nb_rx_q_per_pool = 1;
 			break;
 		}
 
@@ -596,15 +599,16 @@  rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 		default: /* ETH_MQ_TX_VMDQ_ONLY or ETH_MQ_TX_NONE */
 			/* if nothing mq mode configure, use default scheme */
 			dev->data->dev_conf.txmode.mq_mode = ETH_MQ_TX_VMDQ_ONLY;
+			if (nb_tx_q_per_pool > 1)
+				nb_tx_q_per_pool = 1;
 			break;
 		}
 
 		/* check valid queue number */
-		if ((nb_rx_q > RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool) ||
-		    (nb_tx_q > RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool)) {
+		if (nb_rx_q > nb_rx_q_per_pool || nb_tx_q > nb_tx_q_per_pool) {
 			PMD_DEBUG_TRACE("ethdev port_id=%d SRIOV active, "
-				    "queue number must less equal to %d\n",
-					port_id, RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool);
+					"rx/tx queue number must less or equal to %d/%d\n",
+					port_id, nb_rx_q_per_pool, nb_tx_q_per_pool);
 			return (-EINVAL);
 		}
 	} else {