From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f46.google.com (mail-wm0-f46.google.com [74.125.82.46]) by dpdk.org (Postfix) with ESMTP id 3428DDE5 for ; Tue, 17 Jan 2017 09:24:14 +0100 (CET) Received: by mail-wm0-f46.google.com with SMTP id r126so190575881wmr.0 for ; Tue, 17 Jan 2017 00:24:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=OB9fOwtO4Am6dfKtNBqVsQyx87b2PjbT3b8LqUDoG2Y=; b=k07IaVtioA6NmQYtbqm08BLnQ6ue5xJYT27cbH7xcnsfECZuuBbg0KtxiEW/u33dzK 4mVfvoMFdUo0OCsex59JklUGkpfWnw7/ew1syrrK8UDXwulj80SmgGYd7KMHwRK3dUAq qsg41wtYXworwkEFfBun4LyZJFZ16A+qlJbrb8ZfzBIT5I8Ke+S44vZhjgyFk9BiBiAf MIEzmodyQd0KbOTz5ArqYWlyxGiEqnCBV8olDf52zZZqEgaBaCdYibl9N4gVCTnR2SEb tNNmmQeUZmkpFvwE85ipkwxxcDlCssHRXy2GdfxBpAzNa7DV2X60gLXUzZpZlR5FFcKS 4WmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=OB9fOwtO4Am6dfKtNBqVsQyx87b2PjbT3b8LqUDoG2Y=; b=ODyZKdsQCYpw0Haclm0FTXRvGqL+ysuomhoXHd7WThQXw2X7NV8lZ7nSajDXui3ilJ YmuPRH6wmz9DGk4QKq03WEIsvx1fJDJTplqLUBsvxzDjDyZMfyI/W/TICrNgnLSLsoRg vlmaVADVdrjsFO7wx/o9ZBadEhkRYQZw30gwFJlew8hDF5T9JU7jUu00WCg+9P0Y8b2z tkENDqz76q1F5THKB8uhGFeDjoU1i5Yrh3aKsrrTVcx3quZ4WFcts/XWhOypxcFCDnc+ Xq9RSfWtjZgP16jjFflSyKUBs0tD0Pc2xDegWhUS7fyglShFXrSmOoNZ981X6D5QCgNF AlhQ== X-Gm-Message-State: AIkVDXJcnDepwpNgwaVifFckujR2pI7NeaWx4cxW3eQSlFPNOkb50V++rNSj8hjZ3hxXZS1h X-Received: by 10.223.130.46 with SMTP id 43mr25711257wrb.41.1484641453818; Tue, 17 Jan 2017 00:24:13 -0800 (PST) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id c132sm29776893wme.21.2017.01.17.00.24.12 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 17 Jan 2017 00:24:13 -0800 (PST) Date: Tue, 17 Jan 2017 09:24:10 +0100 From: Olivier Matz To: "Richardson, Bruce" Cc: "dev@dpdk.org" , "thomas.monjalon@6wind.com" , "Ananyev, Konstantin" , "Lu, Wenzhuo" , "Zhang, Helin" Message-ID: <20170117092410.5f5950d7@platinum> In-Reply-To: <59AF69C657FD0841A61C55336867B5B035B88D5D@IRSMSX103.ger.corp.intel.com> References: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com> <20170113174409.2f1be0b5@platinum> <59AF69C657FD0841A61C55336867B5B035B88D5D@IRSMSX103.ger.corp.intel.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC 0/9] get Rx and Tx used descriptors 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: Tue, 17 Jan 2017 08:24:14 -0000 Hi, Thanks Bruce for the comments. On Fri, 13 Jan 2017 17:32:38 +0000, "Richardson, Bruce" wrote: > > -----Original Message----- > > From: Olivier Matz [mailto:olivier.matz@6wind.com] > > Sent: Friday, January 13, 2017 4:44 PM > > To: dev@dpdk.org > > Cc: thomas.monjalon@6wind.com; Ananyev, Konstantin > > ; Lu, Wenzhuo ; > > Zhang, Helin ; Richardson, Bruce > > > > Subject: Re: [dpdk-dev] [RFC 0/9] get Rx and Tx used descriptors > > > > Hi, > > > > On Thu, 24 Nov 2016 10:54:12 +0100, Olivier Matz > > wrote: > > > This RFC patchset introduces a new ethdev API function > > > rte_eth_tx_queue_count() which is the tx counterpart of > > > rte_eth_rx_queue_count(). It implements this API on some Intel > > > drivers for reference, and it also optimizes the implementation of > > > rte_eth_rx_queue_count(). > > > > > > > I'm planning to send a new version of this patchset, fixing the > > issues seen by Ferruh, plus a bug fix in the e1000 implementation. > > > > Does anyone have any comment about the new API or about questions > > raised in the cover letter? Especially about the real meaning of > > "used descriptor": should it include the descriptors hold by the > > driver? > For TX, I think we just need used/unused, since for TX any driver > will reuse a slot that has been completed by the NIC, and doesn't > hold the mbufs back for buffering at all. Agree > For RX, strictly speaking, we should have three categories, rather > than trying to work it into 2. I don't see why we can't report a slot > as used/unused/unavailable. With the rte_eth_rx_queue_count() API, we don't have this opportunity since it just returns an int. Something I found a bit strange when doing this patchset is that the user does not have the full control of the number of hold buffers. With default parameters, the effective size of a ring of 128 is 64. So it is, we could introduce an API to retrieve the status: used/unused/unavailable. > > Any comment about the method (binary search to find the used > > descriptors)? > > I think binary search should work ok, though linear search may work > better for smaller ranges as we can prefetch ahead since we know what > we will check next. Linear can also go backward only if we want > accuracy (going forward risks having race conditions between read and > NIC write). Overall, though I think binary search should work well > enough. > > > > > I'm also wondering about adding rte_eth_tx_descriptor_done() in the > > API at the same time. > > > > Let me switch the question around - do we need the queue_count APIs at > all, and is it not more efficient to just supply the > descriptor_done() APIs? If an app wants to know the use of the ring, > and take some action based on it, that app is going to have one or > more thresholds for taking the action, right? In that case, rather > than scanning descriptors to find the absolute number of free/used > descriptors, it would be more efficient for the app to just check the > descriptor on the threshold - and take action based just on that > value. Yes, I reached the same conclusion (...after posting the RFC patchset unfortunatly). > Any app that really does need the absolute value of the ring > capacity can presumably do its own binary search or linear search to > determine the value itself. However, I think just doing a done > function should encourage people to use the more efficient solution > of just checking the minimum number of descriptors needed. The question is: now that the work is done, is there any application that would require this absolute values? For instance, monitoring. If not, I have no problem to the patchset, I just need to validate my application with a descriptor_done() API. In this case we can also deprecate rx_queue_count() and tx_queue_count(). The rte_eth_rx_descriptor_done() function could be updated into: /** * Check the status of a RX descriptor in the queue. * * @param port_id * The port identifier of the Ethernet device. * @param queue_id * The queue id on the specific port. * @param offset * The offset of the descriptor ID from tail (0 is the next packet to * be received by the driver). * - (2) Descriptor is unavailable (hold by driver, not yet returned to hw) * - (1) Descriptor is done (filled by hw, but not processed by the driver, * i.e. in the receive queue) * - (0) Descriptor is available for the hardware to receive a packet. * - (-ENODEV) if *port_id* invalid. * - (-ENOTSUP) if the device does not support this function */ static inline int rte_eth_rx_descriptor_done(uint8_t port_id, uint16_t queue_id, uint16_t offset) A similar rte_eth_tx_descriptor_done() would be introduced: /** * Check the status of a TX descriptor in the queue. * * @param port_id * The port identifier of the Ethernet device. * @param queue_id * The queue id on the specific port. * @param offset * The offset of the descriptor ID from tail (0 is the place where the next * packet will be send). * - (1) Descriptor is beeing processed by the hw, i.e. in the transmit queue * - (0) Descriptor is available for the driver to send a packet. * - (-ENODEV) if *port_id* invalid. * - (-ENOTSUP) if the device does not support this function */ static inline int rte_eth_tx_descriptor_done(uint8_t port_id, uint16_t queue_id, uint16_t offset) An alternative would be to rename these functions in descriptor_status() instead of descriptor_done(). Regards, Olivier