From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id A6CBE5A08 for ; Thu, 25 Aug 2016 10:39:52 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP; 25 Aug 2016 01:39:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,575,1464678000"; d="scan'208";a="870863204" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga003.jf.intel.com with ESMTP; 25 Aug 2016 01:39:51 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.71]) by IRSMSX102.ger.corp.intel.com ([169.254.2.123]) with mapi id 14.03.0248.002; Thu, 25 Aug 2016 09:39:50 +0100 From: "Iremonger, Bernard" To: "Lu, Wenzhuo" , "dev@dpdk.org" CC: "De Lara Guarch, Pablo" Thread-Topic: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe Thread-Index: AQHR7tkwzgh/3Jc9HEiKF7r1kuB6RaBX9MEwgABfmfCAAJ/1gIAAgc4A Date: Thu, 25 Aug 2016 08:39:49 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C21A06A70A@IRSMSX108.ger.corp.intel.com> References: <1470374429-14848-1-git-send-email-wenzhuo.lu@intel.com> <8CEF83825BEC744B83065625E567D7C21A06A32E@IRSMSX108.ger.corp.intel.com> <8CEF83825BEC744B83065625E567D7C21A06A573@IRSMSX108.ger.corp.intel.com> <6A0DE07E22DDAD4C9103DF62FEBC09090349EA68@shsmsx102.ccr.corp.intel.com> In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09090349EA68@shsmsx102.ccr.corp.intel.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGY2MjEwNmQtYzFkYi00NTNkLTg1ZDItNWUwNmE4YzYwYjIwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IlJ6ejc4YnA2WVpPT1NcL0Q1NmNDb1dmOGlHSVRTa1lQUUpSWFZkN29TY05nPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on 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: Thu, 25 Aug 2016 08:39:53 -0000 Hi Wenzhuo, > > > > Subject: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on > > > > ixgbe > > > > > > > > An issue is found that DCB cannot be configured on ixgbe NICs. > > > > It's said the TX queue number is not right. > > > > On ixgbe the max TX queue number is not fixed, it depends on the > > > > multi- queue mode. > > > > > > > > This patch adds the device configuration before getting info in > > > > the DCB configuration process. So the right info can be got > > > > depending on the configuration. > > > > > > > > Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported > > > > Tx > > > > queues) > > > > As the fix in this patch is to testpmd, I don't think the fixes line is= correct. > The bug is introduced by this patch. Before this patch, APP need not care > about the multi-queue mode when it want to get the max queue number. >=20 > > > > > > Signed-off-by: Wenzhuo Lu > > > > --- > > > > app/test-pmd/testpmd.c | 39 > > > > +++++++++++++++++++++------------------ > > > > 1 file changed, 21 insertions(+), 18 deletions(-) > > > > > > > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index > > > > 1428974..ba41bea 100644 > > > > --- a/app/test-pmd/testpmd.c > > > > +++ b/app/test-pmd/testpmd.c > > > > @@ -1962,17 +1962,30 @@ init_port_dcb_config(portid_t pid, > > > > uint8_t pfc_en) > > > > { > > > > struct rte_eth_conf port_conf; > > > > - struct rte_eth_dev_info dev_info; > > > > struct rte_port *rte_port; > > > > int retval; > > > > uint16_t i; > > > > > > > > - rte_eth_dev_info_get(pid, &dev_info); > > > > + rte_port =3D &ports[pid]; > > > > + > > > > + memset(&port_conf, 0, sizeof(struct rte_eth_conf)); > > > > + /* Enter DCB configuration status */ > > > > + dcb_config =3D 1; > > > > + > > > > + /*set configuration of DCB in vt mode and DCB in non-vt mode*/ > > > > + retval =3D get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, > > > > pfc_en); > > > > + if (retval < 0) > > > > + return retval; > > > > + port_conf.rxmode.hw_vlan_filter =3D 1; > > > > + > > > > + (void)rte_eth_dev_configure(pid, 0, 0, &port_conf); > > > > > > The return value of rte_eth_dev_configure() should be checked. > > > Calling rte_eth_dev_configure() with parameters nb_rx_q and nb_tx_q > > > equal to 0 returns -EINVAL, and does nothing. > > > Should the values of nb_rx_q and nb_tx_q be non zero? > The 0 is used on purpose. Because I don't want to configure the queues. T= he > only purpose is to make all the configuration to write into the device. = And > that's why the return value is ignored by (void). It might be useful to add a comment to explain why rte_eth_dev_configure() = is being called in this way. =20 > > The call to rte_eth_dev_configure() may not be necessary, as it is > > also called when the port is started. > In rte_ethdev.h, we can see it's said that rte_eth_dev_configure should b= e > called before any other function of ethernet API. As the DCB configuratio= n is > changed here. We have to call rte_eth_dev_configure to make the > configuration is right before we can use rte_eth_dev_info_get. >=20 > > > > > > + rte_eth_dev_info_get(pid, &rte_port->dev_info); > > > > > > > > /* If dev_info.vmdq_pool_base is greater than 0, > > > > * the queue id of vmdq pools is started after pf queues. > > > > */ > > > > - if (dcb_mode =3D=3D DCB_VT_ENABLED && dev_info.vmdq_pool_base > > > > > 0) { > > > > + if (dcb_mode =3D=3D DCB_VT_ENABLED && > > > > + rte_port->dev_info.vmdq_pool_base > 0) { > > > > printf("VMDQ_DCB multi-queue mode is nonsensical" > > > > " for port %d.", pid); > > > > return -1; > > > > @@ -1982,13 +1995,13 @@ init_port_dcb_config(portid_t pid, > > > > * and has the same number of rxq and txq in dcb mode > > > > */ > > > > if (dcb_mode =3D=3D DCB_VT_ENABLED) { > > > > - nb_rxq =3D dev_info.max_rx_queues; > > > > - nb_txq =3D dev_info.max_tx_queues; > > > > + nb_rxq =3D rte_port->dev_info.max_rx_queues; > > > > + nb_txq =3D rte_port->dev_info.max_tx_queues; > > > > If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues > > respectively, there is a failure when the port is started in > > ixgbe_check_mq_mode() at line > > 1990 in ixgbe_ethdev.c. > > SRIOV is active, nb_rx_q=3D128 nb_tx_q=3D128 queue number must be less > > than or equal to 1. > I haven't hit this issue. Would you like to give more details about how t= o hit > it? I'll check if I miss something. There is a Niantic PF and VF bound to igb_uio. Port 0 is the PF and Port 1 = is the VF. ./testpmd -c 3f -l 1-5 -n 4 -- -i testpmd> set corelist 2,3,4,5 testpmd> port stop 0 /* PF is 0 */ testpmd> port config 0 dcb vt on 4 pfc on testpmd> port start 0 /* PF is 0 */ line 1990 ixgbe_ethdev.c SRIOV is active, nb_rx_q=3D128 nb_tx_q=3D128 queue number must be less than= or equal to 1. /* Works if nb_rx_q and nb_tx_q set to 1 */ testpmd> show port dcb_tc 0 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D DCB infos for port 0 =3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D TC NUMBER: 4 (showing 8 a lot of the time ??) /* port start should be = called first */=20 TC : 0 1 2 3 Priority : 0 1 2 3 BW percent : 25% 25% 25% 25% RXQ base : 0 1 2 3 RXQ number : 1 1 1 1 TXQ base : 0 1 2 3 TXQ number : 1 1 1 1=20 testpmd> show port dcb_tc 1 Failed to get dcb infos on port 1 =20 > > > > nb_rxq and nb_txq are equal to 1 at this point, if they are not > > changed, port start completes successfully. > > > > > > > > } else { > > > > /*if vt is disabled, use all pf queues */ > > > > - if (dev_info.vmdq_pool_base =3D=3D 0) { > > > > - nb_rxq =3D dev_info.max_rx_queues; > > > > - nb_txq =3D dev_info.max_tx_queues; > > > > + if (rte_port->dev_info.vmdq_pool_base =3D=3D 0) { > > > > + nb_rxq =3D rte_port->dev_info.max_rx_queues; > > > > + nb_txq =3D rte_port->dev_info.max_tx_queues; > > > > } else { > > > > nb_rxq =3D (queueid_t)num_tcs; > > > > nb_txq =3D (queueid_t)num_tcs; > > > > @@ -1997,16 +2010,6 @@ init_port_dcb_config(portid_t pid, > > > > } > > > > rx_free_thresh =3D 64; > > > > > > > > - memset(&port_conf, 0, sizeof(struct rte_eth_conf)); > > > > - /* Enter DCB configuration status */ > > > > - dcb_config =3D 1; > > > > - > > > > - /*set configuration of DCB in vt mode and DCB in non-vt mode*/ > > > > - retval =3D get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, > > > > pfc_en); > > > > - if (retval < 0) > > > > - return retval; > > > > - > > > > - rte_port =3D &ports[pid]; > > > > memcpy(&rte_port->dev_conf, &port_conf, sizeof(struct > > > > rte_eth_conf)); > > > > > > > > rxtx_port_config(rte_port); > > > > -- > > > > 1.9.3 Regards, Bernard