From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 6CFBEC620 for ; Wed, 15 Jun 2016 14:40:20 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 15 Jun 2016 05:40:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,475,1459839600"; d="scan'208";a="987952632" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by fmsmga001.fm.intel.com with ESMTP; 15 Jun 2016 05:40:18 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.51]) by IRSMSX153.ger.corp.intel.com ([169.254.9.105]) with mapi id 14.03.0248.002; Wed, 15 Jun 2016 13:40:17 +0100 From: "Ananyev, Konstantin" To: Ivan Boule , Thomas Monjalon CC: "Pattan, Reshma" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v9 1/8] ethdev: use locks to protect Rx/Tx callback lists Thread-Index: AQHRxsb8c+QpcxnBPE2qkSBe1aWjuJ/qHtqAgAAUOJD///QYAIAAOaKAgAAWL0A= Date: Wed, 15 Jun 2016 12:40:16 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836B717B0@irsmsx105.ger.corp.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> <576146CD.8060008@6wind.com> In-Reply-To: <576146CD.8060008@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Jun 2016 12:40:21 -0000 Hi Ivan, > -----Original Message----- > From: Ivan Boule [mailto:ivan.boule@6wind.com] > Sent: Wednesday, June 15, 2016 1:15 PM > To: Thomas Monjalon; Ananyev, Konstantin > Cc: Pattan, Reshma; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v9 1/8] ethdev: use locks to protect Rx/Tx= callback lists >=20 > On 06/15/2016 10:48 AM, Thomas Monjalon wrote: >=20 > >> > >>> 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 rig= ht now. > >> But if you have some particular scenario in mind that you believe woul= d cause > >> a race condition - please speak up. > > > > If we add/remove a callback during a burst? Is it possible that the nex= t > > 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. > > >=20 > Hi Reshma, >=20 > 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: >=20 > 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. >=20 > 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 ! Yes, though it is documented behaviour, someone can probably refer it as a feature, not a bug ;) >=20 > 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? >=20 > 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). Yes, and as I wrote in other mail, IMO it needs to be addressed. But again it is already existing problem in rte_ethdev, and I think it shouldn't stop pdump integration. Konstantin >=20 > 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. >=20 > Of course, the same issues stand for TX callbacks. >=20 > Regards, > Ivan >=20 >=20 >=20 > -- > Ivan Boule > 6WIND Development Engineer