From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 284EB8E92 for ; Wed, 19 Aug 2015 02:42:20 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP; 18 Aug 2015 17:42:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,705,1432623600"; d="scan'208";a="771404842" Received: from kmsmsx151.gar.corp.intel.com ([172.21.73.86]) by fmsmga001.fm.intel.com with ESMTP; 18 Aug 2015 17:42:18 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by KMSMSX151.gar.corp.intel.com (172.21.73.86) with Microsoft SMTP Server (TLS) id 14.3.224.2; Wed, 19 Aug 2015 08:42:17 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.206]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.143]) with mapi id 14.03.0224.002; Wed, 19 Aug 2015 08:42:16 +0800 From: "Lu, Wenzhuo" To: "Zhang, Helin" Thread-Topic: [dpdk-dev] [PATCH v1] ixgbe_pmd: forbid tx_rs_thresh above 1 for all NICs but 82598 Thread-Index: AQHQ1gay20DW3uwMHUGDbsbAnfMuLJ4KdIKAgAgOqGA= Date: Wed, 19 Aug 2015 00:42:15 +0000 Message-ID: <6A0DE07E22DDAD4C9103DF62FEBC0909D3E116@shsmsx102.ccr.corp.intel.com> References: <1439489195-31553-1-git-send-email-vladz@cloudius-systems.com> <55CD7EA5.6060100@cloudius-systems.com> In-Reply-To: <55CD7EA5.6060100@cloudius-systems.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v1] ixgbe_pmd: forbid tx_rs_thresh above 1 for all NICs but 82598 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, 19 Aug 2015 00:42:22 -0000 Hi Helin, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vlad Zolotarov > Sent: Friday, August 14, 2015 1:38 PM > To: Zhang, Helin; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1] ixgbe_pmd: forbid tx_rs_thresh above 1= for > all NICs but 82598 >=20 >=20 >=20 > On 08/13/15 23:28, Zhang, Helin wrote: > > Hi Vlad > > > > I don't think the changes are needed. It says in datasheet that the RS > > bit should be set on the last descriptor of every packet, ONLY WHEN > TXDCTL.WTHRESH equals to ZERO. >=20 > Of course it's needed! See below. > Exactly the same spec a few lines above the place u've just quoted states= : >=20 > "Software should not set the RS bit when TXDCTL.WTHRESH is greater than > zero." >=20 > And since all three (3) ixgbe xmit callbacks are utilizing RS bit notatio= n ixgbe PMD > is actually not supporting any value of WTHRESH different from zero. I think Vlad is right. We need to fix this issue. Any suggestion? If not, I= 'd like to ack this patch. >=20 > > > > Regards, > > Helin > > > >> -----Original Message----- > >> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com] > >> Sent: Thursday, August 13, 2015 11:07 AM > >> To: dev@dpdk.org > >> Cc: Zhang, Helin; Ananyev, Konstantin; avi@cloudius-systems.com; Vlad > >> Zolotarov > >> Subject: [dpdk-dev] [PATCH v1] ixgbe_pmd: forbid tx_rs_thresh above 1 = for > all > >> NICs but 82598 > >> > >> According to 82599 and x540 HW specifications RS bit *must* be set in = the > last > >> descriptor of *every* packet. > > There is a condition that if TXDCTL.WTHRESH equal to zero. >=20 > Right and ixgbe PMD requires this condition to be fulfilled in order to > function. See above. >=20 > > > >> This patch fixes the Tx hang we were constantly hitting with a seastar= -based > >> application on x540 NIC. > > Could you help to share with us how to reproduce the tx hang issue, wit= h using > > typical DPDK examples? >=20 > Sorry. I'm not very familiar with the typical DPDK examples to help u > here. However this is quite irrelevant since without this this patch > ixgbe PMD obviously abuses the HW spec as has been explained above. >=20 > We saw the issue when u stressed the xmit path with a lot of highly > fragmented TCP frames (packets with up to 33 fragments with non-headers > fragments as small as 4 bytes) with all offload features enabled. >=20 > Thanks, > vlad > > > >> Signed-off-by: Vlad Zolotarov > >> --- > >> drivers/net/ixgbe/ixgbe_ethdev.c | 9 +++++++++ > >> drivers/net/ixgbe/ixgbe_rxtx.c | 23 ++++++++++++++++++++++- > >> 2 files changed, 31 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > >> b/drivers/net/ixgbe/ixgbe_ethdev.c > >> index b8ee1e9..6714fd9 100644 > >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c > >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > >> @@ -2414,6 +2414,15 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, > struct > >> rte_eth_dev_info *dev_info) > >> .txq_flags =3D ETH_TXQ_FLAGS_NOMULTSEGS | > >> ETH_TXQ_FLAGS_NOOFFLOADS, > >> }; > >> + > >> + /* > >> + * According to 82599 and x540 specifications RS bit *must* be set o= n > the > >> + * last descriptor of *every* packet. Therefore we will not allow th= e > >> + * tx_rs_thresh above 1 for all NICs newer than 82598. > >> + */ > >> + if (hw->mac.type > ixgbe_mac_82598EB) > >> + dev_info->default_txconf.tx_rs_thresh =3D 1; > >> + > >> dev_info->hash_key_size =3D IXGBE_HKEY_MAX_INDEX * sizeof(uint32_t)= ; > >> dev_info->reta_size =3D ETH_RSS_RETA_SIZE_128; > >> dev_info->flow_type_rss_offloads =3D IXGBE_RSS_OFFLOAD_ALL; diff -- > git > >> a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c inde= x > >> 91023b9..8dbdffc 100644 > >> --- a/drivers/net/ixgbe/ixgbe_rxtx.c > >> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c > >> @@ -2085,11 +2085,19 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev > >> *dev, > >> struct ixgbe_tx_queue *txq; > >> struct ixgbe_hw *hw; > >> uint16_t tx_rs_thresh, tx_free_thresh; > >> + bool rs_deferring_allowed; > >> > >> PMD_INIT_FUNC_TRACE(); > >> hw =3D IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > >> > >> /* > >> + * According to 82599 and x540 specifications RS bit *must* be set o= n > the > >> + * last descriptor of *every* packet. Therefore we will not allow th= e > >> + * tx_rs_thresh above 1 for all NICs newer than 82598. > >> + */ > >> + rs_deferring_allowed =3D (hw->mac.type <=3D ixgbe_mac_82598EB); > >> + > >> + /* > >> * Validate number of transmit descriptors. > >> * It must not exceed hardware maximum, and must be multiple > >> * of IXGBE_ALIGN. > >> @@ -2110,6 +2118,8 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev > *dev, > >> * to transmit a packet is greater than the number of free TX > >> * descriptors. > >> * The following constraints must be satisfied: > >> + * tx_rs_thresh must be less than 2 for NICs for which RS deferring= is > >> + * forbidden (all but 82598). > >> * tx_rs_thresh must be greater than 0. > >> * tx_rs_thresh must be less than the size of the ring minus 2. > >> * tx_rs_thresh must be less than or equal to tx_free_thresh. > >> @@ -2121,9 +2131,20 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev > *dev, > >> * When set to zero use default values. > >> */ > >> tx_rs_thresh =3D (uint16_t)((tx_conf->tx_rs_thresh) ? > >> - tx_conf->tx_rs_thresh : DEFAULT_TX_RS_THRESH); > >> + tx_conf->tx_rs_thresh : > >> + (rs_deferring_allowed ? DEFAULT_TX_RS_THRESH : 1)); > >> tx_free_thresh =3D (uint16_t)((tx_conf->tx_free_thresh) ? > >> tx_conf->tx_free_thresh : DEFAULT_TX_FREE_THRESH); > >> + > >> + if (!rs_deferring_allowed && tx_rs_thresh > 1) { > >> + PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than 2 since RS > " > >> + "must be set for every packet for this HW. " > >> + "(tx_rs_thresh=3D%u port=3D%d queue=3D%d)", > >> + (unsigned int)tx_rs_thresh, > >> + (int)dev->data->port_id, (int)queue_idx); > >> + return -(EINVAL); > >> + } > >> + > >> if (tx_rs_thresh >=3D (nb_desc - 2)) { > >> PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than the > number " > >> "of TX descriptors minus 2. (tx_rs_thresh=3D%u " > >> -- > >> 2.1.0