From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <ivan.boule@6wind.com> Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id B230CAD86 for <dev@dpdk.org>; Wed, 15 Jun 2016 14:15:13 +0200 (CEST) Received: by mail-wm0-f49.google.com with SMTP id f126so19101863wma.1 for <dev@dpdk.org>; Wed, 15 Jun 2016 05:15:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=sLogYz9l9gIg2E5atq1g1PrmBBFJci6I90FxF0UQWB0=; b=ttskNP3jvbbSLck1AxtTMZtp6p6YreyGGnVaZ8AOOkU3RuGuq1aLDQSB2m9nZbduYR 2ujEv7J8SfFGc8sp3CsglR3Rd2begUU4DOngRGrPHpoAFYPhxekpuwFutno0lTP3Q29f HkRiOndGUwas9+phUGXvKenfGZ9gAAkHwE2jjf9nsDTTAz8zicHVwISne2QGfunZuCDw 9pFYQ7Od8+ZjuQWuTWUOOIUfvgVy+kseO2e3QeBKP5pGMCV51cIJipy7D9+vU/6qgWA3 xQkPEcKwZSmrfhsswiQPcH1p5iQkRdIiZZ2EfthCgWdzbxPx3BcWk9mAIWE/eLXB5SOm 1jZg== 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-transfer-encoding; bh=sLogYz9l9gIg2E5atq1g1PrmBBFJci6I90FxF0UQWB0=; b=F7Dt2Tj9QKxY3+83pfWbOGQ0+WlqDEXlgb9Y87MLsZtIOwlGZAQsO4vDmU9P5K6cKG /1PEzgMRdm5ExKr8avw29umfy30WwL32MVThRC6NEilMdVh25pbqwpYm3JjpwjidNEzo C9qqdbAK5DrInuEc7eKcgTF+T0z30iD4stgmApHr5IiFBY4nVJgJ43Aj76mK1Ej8KTs2 i6n/9n2tjDjKBEC7DMQGIKzTTDrIvcoW1O1Xw1+s+OQ/toLQp6btBgpKCnJE5V8EI3Am HFU0w8jp7pSLhE2deECcxt3E6fKqcc5T+xOz+ynRaFuiEIUHpL3WlVQftuB5MzEPxD1B IOYg== X-Gm-Message-State: ALyK8tLd8p6fFDubkmKUNqYPD9swCMSszDJN3rTg8lgKYp1+ALIe5XDz/i4o8pRGwV6CeYoO X-Received: by 10.194.172.36 with SMTP id az4mr12465145wjc.114.1465992911460; Wed, 15 Jun 2016 05:15:11 -0700 (PDT) Received: from [10.16.0.189] (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id o2sm11567811wjp.26.2016.06.15.05.15.09 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 15 Jun 2016 05:15:10 -0700 (PDT) To: Thomas Monjalon <thomas.monjalon@6wind.com>, "Ananyev, Konstantin" <konstantin.ananyev@intel.com> References: <1465575534-23605-1-git-send-email-reshma.pattan@intel.com> <10886152.VH5xYhdqG2@xps13> <2601191342CEEE43887BDE71AB97725836B714ED@irsmsx105.ger.corp.intel.com> <2907169.iIEIeOfXh7@xps13> Cc: "Pattan, Reshma" <reshma.pattan@intel.com>, dev@dpdk.org From: Ivan Boule <ivan.boule@6wind.com> Message-ID: <576146CD.8060008@6wind.com> Date: Wed, 15 Jun 2016 14:15:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.8.0 MIME-Version: 1.0 In-Reply-To: <2907169.iIEIeOfXh7@xps13> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v9 1/8] ethdev: use locks to protect Rx/Tx callback lists X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Wed, 15 Jun 2016 12:15:13 -0000 On 06/15/2016 10:48 AM, Thomas Monjalon wrote: >> >>> I think the read access would need locking but we do not want it >>> in fast path. >> >> I don't think it would be needed. >> As I said - read/write interaction didn't change from what we have right now. >> But if you have some particular scenario in mind that you believe would cause >> a race condition - please speak up. > > If we add/remove a callback during a burst? Is it possible that the next > pointer would have a wrong value leading to a crash? > Maybe we need a comment to state that we should not alter burst > callbacks while running burst functions. > Hi Reshma, You claim that the "rte_eth_rx_cb_lock" does not need to be hold in the function "rte_eth_rx_burst()" while parsing the "post_rx_burst_cbs" list of RX callbacks associated with the polled RX queue to safely remove RX callback(s) in parallel. The problem is not [only] with the setting and the loading of "cb->next" that you assume to be atomic operations, which is certainly true on most CPUs. I see the 2 important following issues: 1) the "rte_eth_rxtx_callback" data structure associated with a removed RX callback could still be accessed in the callback parsing loop of the function "rte_eth_rx_burst()" after having been freed in parallel. BUT SUCH A BAD SITUATION WILL NOT CURRENTLY HAPPEN, THANKS TO THE NICE MEMORY LEAK BUG in the function "rte_eth_remove_rx_callback()" that does not free the "rte_eth_rxtx_callback" data structure associated with the removed callback ! 2) As a consequence of 1), RX callbacks can be invoked/executed while/after being removed. If the application must free resources that it dynamically allocated to be used by the RX callback being removed, how to guarantee that the last invocation of that RX callback has been completed and that such a callback will never be invoked again, so that the resources can safely be freed? This is an example of a well-known more generic object deletion problem which must arrange to guarantee that a deleted object is not used and not accessible for use anymore before being actually deleted (freed, for instance). Note that a lock cannot be used in the execution path of the rte_eth_rx_burst() function to address this issue, as locks MUST NEVER be introduced in the RX/TX path of the DPDK framework. Of course, the same issues stand for TX callbacks. Regards, Ivan -- Ivan Boule 6WIND Development Engineer