From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f175.google.com (mail-wi0-f175.google.com [209.85.212.175]) by dpdk.org (Postfix) with ESMTP id 4BEF28D94 for ; Fri, 14 Aug 2015 07:37:44 +0200 (CEST) Received: by wijp15 with SMTP id p15so8108084wij.0 for ; Thu, 13 Aug 2015 22:37: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:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=nY+/RXfn3qPxGW/zuR1vS3C/4jtxQ3tBv+NNW20nM3k=; b=WNO1Wg0XUG2hhtr2OmMeCDJydZTJ4ez261ucTajI61zDV2DFoxgUbT+AzbjFWuoxAn VriXW1xICzgRo60ULSFAAjYob3pdPk71MAozWkenm7W7PRjS6+NwFsemIli/3V818BHU rUNeV9IVRHa+p7w+KYKYBTOcG0GI82V1o7wX3pWoIsGdqbdSgLW9XdSJ3gJhSEX/c9SX P4UBNYQUbAD++B+/z1oNl4FGLP9WvPx0RvnGWr9QIEqnA9z9PPaTjJiX3e+MgG2MDdaL HCp9VTVXg4RsuFFaestKTWTKIN/K9G/ekUlu0wnynfKSl3C3119xnzVciWEAqY5/7yTE Elkg== X-Gm-Message-State: ALoCoQnLLgBOFc841nJdwR/J24laLUD2+2D9Wj7ne2IJcXfnuIEyGfum98Hrvn1+/RBSsuj+G5mm X-Received: by 10.180.85.74 with SMTP id f10mr2777621wiz.45.1439530664157; Thu, 13 Aug 2015 22:37:44 -0700 (PDT) Received: from [10.0.0.4] (bzq-109-64-112-195.red.bezeqint.net. [109.64.112.195]) by smtp.googlemail.com with ESMTPSA id h6sm1326798wiy.3.2015.08.13.22.37.42 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Aug 2015 22:37:43 -0700 (PDT) To: "Zhang, Helin" , "dev@dpdk.org" References: <1439489195-31553-1-git-send-email-vladz@cloudius-systems.com> From: Vlad Zolotarov Message-ID: <55CD7EA5.6060100@cloudius-systems.com> Date: Fri, 14 Aug 2015 08:37:41 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: Fri, 14 Aug 2015 05:37:44 -0000 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. > > 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