From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pavan Nikhilesh Subject: Re: [PATCH v3 09/12] app/eventdev: add pipeline queue worker functions Date: Thu, 11 Jan 2018 19:22:02 +0530 Message-ID: <20180111135201.ufj3hqh6frncvjpx@Pavan-LT> References: <20171130072406.15605-1-pbhagavatula@caviumnetworks.com> <20180110145144.28403-1-pbhagavatula@caviumnetworks.com> <20180110145144.28403-9-pbhagavatula@caviumnetworks.com> <20180110201710.3uolm2hwzwcowoif@Pavan-LT> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: "Van Haaren, Harry" , "jerin.jacob@caviumnetworks.com" , "santosh.shukla@caviumnetworks.com" , "Eads, Gage" , "hemant.agrawal@nxp.com" , "nipun.gupta@nxp.com" , "Ma, Liang J" Return-path: Received: from NAM03-BY2-obe.outbound.protection.outlook.com (mail-by2nam03on0043.outbound.protection.outlook.com [104.47.42.43]) by dpdk.org (Postfix) with ESMTP id A286C12001 for ; Thu, 11 Jan 2018 14:52:12 +0100 (CET) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Jan 11, 2018 at 12:17:38PM +0000, Van Haaren, Harry wrote: > > > > > Thinking a little more about this, also in light of patch 11/12 of this > > series. > > > > > > The code here has a "safe" and "unsafe" version of TX. This involves > > adding a spinlock inside the code, which is being locked/unlocked before > > doing the actual TX action. > > > > > > I don't understand why this is necessary? DPDK's general stance on locking > > for data-path is DPDK functions do not provide locks, and that application > > level must implement thread-synchronization if it is required. > > > > > > In this case, the app/eventdev can be considered an App, but I don't like > > the idea of providing a sample application and code that duplicates core > > functionality with safe/unsafe versions.. > > > > > > > Some PMD's (net/octeontx) have capability to do multi-thread safe Tx where > > no > > thread-synchronization is required. This is exposed via the offload flag > > 'DEV_TX_OFFLOAD_MT_LOCKFREE'. > > Yes understood. > > > > So, the _safe Tx functions are selected based on the above offload > > capability > > and when the capability is absent _unsafe Tx functions are selected i.e. > > synchronized Tx via spin locks based on the Egress port id. > > > This part changes the current behavior of the sample app. > > Currently there is a (SINGLE_LINK | ATOMIC) stage at the end of the pipeline, which performs this "many-to-one" action, allowing a single core to dequeue all TX traffic, and perform the TX operation in a lock-free manner. > > Changing this to a locking mechanism is going to hurt performance on platforms that do not support TX_OFFLOAD_MT_LOCKFREE. > > In my opinion, the correct fix is to alter the overall pipeline, and always use lockless TX. Examples below; > > NO TX_OFFLOAD_MT_LOCKFREE: > > Eth RX adapter -> stage 1 -> stage 2...(N-1) -> stage N -> stage TX (Atomic | SINGLE_LINK) -> eth TX Agreed, when we detect that tx is not lockfree the workers would just forward the events to (Atomic | SINGLE_LINK) event queue which would be dequeued by a service(mt_unsafe) and Tx them lockfree. > > > WITH TX_OFFLOAD_MT_LOCKFREE: > > Eth RX adapter -> stage 1 -> stage 2...(N-1) -> stage N -> eth TX MT Capable The current lockfree pipeline would remain the same. > > > By configuring the pipeline based on MT_OFFLOAD_LOCKFREE capability flag, and adding the SINGLE_LINK at the end if required, we can support both models without resorting to locked TX functions. > > I think this will lead to a cleaner and more performant solution. > Thoughts? Pavan. >