From patchwork Wed Jan 14 09:46:55 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Wodkowski, PawelX" X-Patchwork-Id: 2268 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id CF2F05A1F; Wed, 14 Jan 2015 10:47:03 +0100 (CET) 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] 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > > > > > > - 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 your > > PATCH4 in "Enable VF RSS for Niantic" series. > > After I respond your last comments, I see this, :-), I am sure we both agree it is > the right way to resolve it in vmdq dcb case. > 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 into nb_rx_q_per_pool and nb_tx_q_per_pool was because of this code: --- 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 it is changed. So I did the above. But when testpmd was adjusted for DCB in SRIOV there 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 use it for DCB in SRIOV. So I did another modification: > + uint16_t nb_rx_q_per_pool = RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool; > + uint16_t nb_tx_q_per_pool = 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=%" 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 = 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; > } > > 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=%" 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 = 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 = 1; > + 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 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 think I can stay with: > + 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; > + 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 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 = 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 (RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool > 1) + RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool = 1; break; } @@ -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 = 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 = 1; + if (RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool > 1) + RTE_ETH_DEV_SRIOV(dev).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 > 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=%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 {