From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id B848F5686 for ; Wed, 14 Jan 2015 10:47:00 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP; 14 Jan 2015 01:44:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="440375772" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by FMSMGA003.fm.intel.com with ESMTP; 14 Jan 2015 01:34:05 -0800 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.213]) by IRSMSX106.ger.corp.intel.com ([169.254.8.222]) with mapi id 14.03.0195.001; Wed, 14 Jan 2015 09:46:55 +0000 From: "Wodkowski, PawelX" To: "Ouyang, Changchun" , Vlad Zolotarov , "Jastrzebski, MichalX K" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 1/2] pmd: add DCB for VF for ixgbe Thread-Index: AQHQLnYf6hHH/oN/7EizLeiLa16Gspy91NsAgAD2pYCAAI5FYA== Date: Wed, 14 Jan 2015 09:46:55 +0000 Message-ID: References: <6c3329$jtfs2e@orsmga002.jf.intel.com> <54B4EEA2.1020300@cloudius-systems.com> In-Reply-To: Accept-Language: pl-PL, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 1/2] pmd: add DCB for VF for ixgbe X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Jan 2015 09:47:01 -0000 > > > > > > - split nb_q_per_pool to nb_rx_q_per_pool and nb_tx_q_per_pool > > > > > > Rationale: > > > > > > rx and tx number of queue might be different if RX and TX are > > > > > > configured in different mode. This allow to inform VF about > > > > > > proper number of queues. > > > > > > Nice move! Ouyang, this is a nice answer to my recent remarks about you= r > > PATCH4 in "Enable VF RSS for Niantic" series. >=20 > After I respond your last comments, I see this, :-), I am sure we both a= gree it is > the right way to resolve it in vmdq dcb case. >=20 I am now dividing this patch with your suggestions and I am little confused= . In this (DCB in SRIOV) case the primary cause for spliting nb_q_per_pool in= to nb_rx_q_per_pool and nb_tx_q_per_pool was because of this code: diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index af9e261..be3afe4 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -537,8 +537,8 @@ 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 =3D 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 =3D 1; + if (RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool > 1) + RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool =3D 1; break; } =20 @@ -553,17 +553,18 @@ 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 =3D ETH_MQ_TX_VMDQ_ONLY; - if (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool > 1) - RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool =3D 1; + if (RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool > 1) + RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool =3D 1; break; } =20 /* 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 > RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool) || + (nb_tx_q > RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool)) { PMD_DEBUG_TRACE("ethdev port_id=3D%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 equal to %d/%d\n", + port_id, RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool, + RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool); return (-EINVAL); } } else { -- This introduced an issue when RX and TX was configure in different way. The= problem was that the RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool as common for RX and TX and i= t is changed. So I did the above. But when testpmd was adjusted for DCB in SRIOV= there=20 was another issue. Testpmd is pre-configuring ports by default and since nb_rx_q_per_pool and nb_tx_q_per_pool was already reset to 1 there was no = way to=20 use it for DCB in SRIOV. So I did another modification: > + uint16_t nb_rx_q_per_pool =3D RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool; > + uint16_t nb_tx_q_per_pool =3D RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool; > + > switch (dev_conf->rxmode.mq_mode) { > - case ETH_MQ_RX_VMDQ_RSS: > case ETH_MQ_RX_VMDQ_DCB: > + break; > + case ETH_MQ_RX_VMDQ_RSS: > case ETH_MQ_RX_VMDQ_DCB_RSS: > - /* DCB/RSS VMDQ in SRIOV mode, not implement yet */ > + /* RSS, DCB+RSS VMDQ in SRIOV mode, not implement yet */ > PMD_DEBUG_TRACE("ethdev port_id=3D%" PRIu8 > " SRIOV active, " > "unsupported VMDQ mq_mode rx %u\n", > @@ -537,37 +560,32 @@ 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 =3D 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 =3D 1; > + if (nb_rx_q_per_pool > 1) > + nb_rx_q_per_pool =3D 1; > break; > } > =20 > switch (dev_conf->txmode.mq_mode) { > - case ETH_MQ_TX_VMDQ_DCB: > - /* DCB VMDQ in SRIOV mode, not implement yet */ > - PMD_DEBUG_TRACE("ethdev port_id=3D%" PRIu8 > - " SRIOV active, " > - "unsupported VMDQ mq_mode tx %u\n", > - port_id, dev_conf->txmode.mq_mode); > - return (-EINVAL); > + case ETH_MQ_TX_VMDQ_DCB: /* DCB VMDQ in SRIOV mode*/ > + break; > 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 =3D ETH_MQ_TX_VMDQ_ONLY; > - if (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool > 1) > - RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool =3D 1; > + if (nb_tx_q_per_pool > 1) > + nb_tx_q_per_pool =3D 1; > break; > } > =20 > /* 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=3D%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 equal to %d/%d\n", > + port_id, RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool, > + RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool); > return (-EINVAL); > } For this point I think that splitting RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool = might be not needed. From my point of view (DCB), since nb_q_per_pool is untouched, I t= hink I can stay with: > + uint16_t nb_rx_q_per_pool =3D RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool; > + uint16_t nb_tx_q_per_pool =3D RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool; > + What do you think? I noticed that you was discussing some issue about nb_q_= per_pool in face of RSS functionality. Can you spoke about my doubts in face of that RSS? Pawel