From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk0-f182.google.com (mail-qk0-f182.google.com [209.85.220.182]) by dpdk.org (Postfix) with ESMTP id E3E438D38 for ; Wed, 19 Aug 2015 06:55:44 +0200 (CEST) Received: by qkfj126 with SMTP id j126so66462014qkf.0 for ; Tue, 18 Aug 2015 21:55:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=3uZZO6xKQZ2wy1bnUuuf2m+aw7RNyhfVpQQpKU00miY=; b=PcoM7ImUzdhJYM1iWPcMV1N8Od14OrTnGsKeTED5wS2P736LEWmmQw+FBr1aawYPYq O6UtahN3zVlyNqwyApT3+ewjg1SD+cf0w2ayzPz0TrKgiSzrOSoIsnXpYkdOr3Uw7mdA dh1rBSquMvkEjFv8AXA06YhvT2PNacbRDnLtlmOH8LrzdSJyZ71jD0kIFa5FrjFlRQCT GVMcy3TjbZFe7aL/k/PP0pkvAZdet9PtZI1MHcNiNv50ALKoLOaGvpd5J4Tb6kD0zA8v vDe09gSsFmMRlidemfL/Gl9pui7bcxG9qFWCnQ3ShR+YhuHHkuQEXw535EW1Uf0zldBp p13Q== X-Gm-Message-State: ALoCoQmERJcJpBaWq2XEaFhL+N0zv63B48GlyeZTrJxXRjccTnxTDt4I7pmFNQ9yT6+ShUh8CpzL MIME-Version: 1.0 X-Received: by 10.55.40.166 with SMTP id o38mr19928077qko.105.1439960144368; Tue, 18 Aug 2015 21:55:44 -0700 (PDT) Received: by 10.55.114.5 with HTTP; Tue, 18 Aug 2015 21:55:44 -0700 (PDT) Received: by 10.55.114.5 with HTTP; Tue, 18 Aug 2015 21:55:44 -0700 (PDT) In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC0909D3E116@shsmsx102.ccr.corp.intel.com> References: <1439489195-31553-1-git-send-email-vladz@cloudius-systems.com> <55CD7EA5.6060100@cloudius-systems.com> <6A0DE07E22DDAD4C9103DF62FEBC0909D3E116@shsmsx102.ccr.corp.intel.com> Date: Wed, 19 Aug 2015 07:55:44 +0300 Message-ID: From: Vladislav Zolotarov To: "Lu, Wenzhuo" Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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 04:55:45 -0000 On Aug 19, 2015 03:42, "Lu, Wenzhuo" wrote: > > 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 > > > > > > > > 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. > > > > Of course it's needed! See below. > > Exactly the same spec a few lines above the place u've just quoted states: > > > > "Software should not set the RS bit when TXDCTL.WTHRESH is greater than > > zero." > > > > And since all three (3) ixgbe xmit callbacks are utilizing RS bit notation 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. Pls., note that there is a v2 of this patch on the list. I forgot to patch ixgbevf_dev_info_get() in v1. > > > > > > > > > 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. > > > > Right and ixgbe PMD requires this condition to be fulfilled in order to > > function. See above. > > > > > > > >> 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, with using > > > typical DPDK examples? > > > > 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. > > > > 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. > > > > 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 = ETH_TXQ_FLAGS_NOMULTSEGS | > > >> ETH_TXQ_FLAGS_NOOFFLOADS, > > >> }; > > >> + > > >> + /* > > >> + * According to 82599 and x540 specifications RS bit *must* be set on > > the > > >> + * last descriptor of *every* packet. Therefore we will not allow the > > >> + * 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 = 1; > > >> + > > >> dev_info->hash_key_size = IXGBE_HKEY_MAX_INDEX * sizeof(uint32_t); > > >> dev_info->reta_size = ETH_RSS_RETA_SIZE_128; > > >> dev_info->flow_type_rss_offloads = IXGBE_RSS_OFFLOAD_ALL; diff -- > > git > > >> a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c index > > >> 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 = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > > >> > > >> /* > > >> + * According to 82599 and x540 specifications RS bit *must* be set on > > the > > >> + * last descriptor of *every* packet. Therefore we will not allow the > > >> + * tx_rs_thresh above 1 for all NICs newer than 82598. > > >> + */ > > >> + rs_deferring_allowed = (hw->mac.type <= 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 = (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 = (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=%u port=%d queue=%d)", > > >> + (unsigned int)tx_rs_thresh, > > >> + (int)dev->data->port_id, (int)queue_idx); > > >> + return -(EINVAL); > > >> + } > > >> + > > >> if (tx_rs_thresh >= (nb_desc - 2)) { > > >> PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than the > > number " > > >> "of TX descriptors minus 2. (tx_rs_thresh=%u " > > >> -- > > >> 2.1.0 >