From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 0BF761B860 for ; Thu, 1 Feb 2018 00:13:18 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 7E5C6208C1; Wed, 31 Jan 2018 18:13:18 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Wed, 31 Jan 2018 18:13:18 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=OhLDohNiyVVLS9TF/bxae020Pe T5JUR8RB36roD8tM8=; b=PKg4yTvIljw1rUQkfSV5AQi3uLEWuBICiqr+ahZtXT CvwHEqdUwOZWCcs9v4KPFM9QGovFdCQ/33FFKJJqAfOAMBI+dzbcJRoOaX+/E0wR V0v3tcz9d7/G5mQYGOAdBc/A4MHmdNzrfIwITkwhL8qYYwy9c8F0xHnyCKGMGZFU M= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=OhLDoh NiyVVLS9TF/bxae020PeT5JUR8RB36roD8tM8=; b=JPHjaHkrx/lXm7EeBMHzHz pg/c3EKtuN9h1mRB8WTI8yhvv8ex2XIqfPTQx+pshoU2+Ij2uwBKL1nwfF0WOGmc MTUJwF5RviDTZ6H27ghm9WazpwuABZofpM/Ompn4SRZkjWGGA9syuiAxgPr1r107 w9bfGFNPA6NTqA9n654prf1VXOa/cmQyOtoBRLVnOTgk8s5dzsrUf/UZzdGQ7Oqr GoMUr4ukTembwAWDDRbs4clXmwPqDkBgVxXsoj+anf4m/GyD5f+k5ChFK0d28Kip 0UU52hkWavYdwgAGRIVcnCjoWkwITqFir5V063aod1lXxOtTg3CUyBYIDKLMJtAA == X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 2F6DD24608; Wed, 31 Jan 2018 18:13:18 -0500 (EST) From: Thomas Monjalon To: Kevin Laatz Cc: dev@dpdk.org, Adrien Mazarguil , Bruce Richardson Date: Thu, 01 Feb 2018 00:12:32 +0100 Message-ID: <2804169.Dxd2SmVN9f@xps> In-Reply-To: <20180130134828.GL4256@6wind.com> References: <20180112103053.47110-1-kevin.laatz@intel.com> <20180129162500.GA7904@bricha3-MOBL3.ger.corp.intel.com> <20180130134828.GL4256@6wind.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2 0/3] Increase default RX/TX ring sizes X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Jan 2018 23:13:19 -0000 30/01/2018 14:48, Adrien Mazarguil: > On Mon, Jan 29, 2018 at 04:25:00PM +0000, Bruce Richardson wrote: > > On Tue, Jan 16, 2018 at 02:13:19PM +0100, Adrien Mazarguil wrote: > > > Hi Kevin, > > > > > > On Fri, Jan 12, 2018 at 10:48:43AM +0000, Kevin Laatz wrote: > > > > Increasing the RX/TX default ring size to 1024/1024 to accommodate for > > > > faster NICs. With the increase of number of PPS, a larger RX buffer is > > > > required in order to avoid packet loss. While a ring size of 128 may be > > > > large enough for 1G and possibly 10G NICs, this is not going to scale to > > > > small packet sizes at 25G and 40G line rates. As we are increasing the RX > > > > buffer size to 1024, we also need to increase the TX buffer size to ensure > > > > that the TX side does not become the bottleneck. > > > > > > > > v2 > > > > - fixed typos in commit messages > > > > - fixed typo in Cc email address > > > > > > I agree with the above and this series contents but would like to comment > > > anyway. > > > > > > Since typical TX/RX bursts are usually somewhere between 16 to 64 packets > > > depending on the application, increasing ring size instead of burst size to > > > keep up with packet rate may mean that software (PMD/application) is too > > > slow on the RX side or hardware is too slow on the TX side (rings always > > > full basically), and this is worked around by introducing latency to absorb > > > packet loss. This is not necessarily a good trade-off. > > > > Well, if RX burst size of 64 is in use, the existing default of 128 is > > definely very much too low - though point taken about slowness of RX. > > Agreed, I just wanted to stress that increasing TX/RX ring sizes may result > in rings still full, but thanks to their FIFO nature, now with increased > latency and resource consumption while still dropping packets in case of > HW/SW slowness. This is not the proper workaround for such a scenario (not > uncommon). > > > > Granted the most appropriate burst/ring/threshold values always depend on > > > the application and underlying hardware, and each vendor is responsible for > > > documenting ideal values for typical applications by providing performance > > > results. > > > > I actually think it probably depends a lot on the NIC hardware in > > question. What is the optimal size for an app with a 1G NIC will be > > different for a 25G or 100G NIC. I therefore think in a future release > > we should have an ethdev API to allow each driver to propose it's > > recommended ring sizes. The app can perhaps provide a burst size hint as > > parameter. What do you think? > > Sounds like a good idea. It could also be implemented without hurting any > API by making 0 descriptors a special value for rte_eth_[rt]x_queue_setup(), > so that being lazy translates to optimized defaults at the cost of some > uncertainty regarding mbuf pool sizing. > > PMDs that do not implement this would reject queue creation as they likely > already do. > > > > My concern is that modifying defaults makes performance comparison > > > with past DPDK releases more difficult for existing automated tests > > > that do not provide ring size and other parameters. There should an > > > impact given that larger rings require more buffers, use more cache, > > > and access more memory in general. > > > > > I'm actually not too concerned about that, as I would expect most > > serious performance comparisons to be done with individually tuned rx > > and tx ring size parameters. For zero loss throughput tests, larger ring > > sizes are needed for any of the NIC I've tested anyway. > > > > Overall, I feel this change is long overdue. > > > > Series Acked-by: Bruce Richardson > > True, therefore: > > Acked-by: Adrien Mazarguil Applied, thanks