From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v6 4/6] ethdev: adjust APIs removal error report Date: Fri, 19 Jan 2018 19:16:55 +0100 Message-ID: <3230393.d19WHEAjBn@xps> References: <1516220357-13013-1-git-send-email-matan@mellanox.com> <3568963.dcr6cRumit@xps> <7d77f38b-1259-d4b8-9d19-a21d80de9c04@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, Matan Azrad , Adrien Mazarguil , Gaetan Rivet , Andrew Rybchenko , "Ananyev, Konstantin" , Alejandro Lucero , Jerin Jacob , Hemant Agrawal , Shahaf Shuler , Olivier MATZ , "Zhang, Helin" To: Ferruh Yigit Return-path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id DA320A48C for ; Fri, 19 Jan 2018 19:17:31 +0100 (CET) In-Reply-To: <7d77f38b-1259-d4b8-9d19-a21d80de9c04@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 19/01/2018 19:13, Ferruh Yigit: > On 1/19/2018 5:54 PM, Thomas Monjalon wrote: > > 19/01/2018 17:19, Ferruh Yigit: > >> On 1/18/2018 6:10 PM, Matan Azrad wrote: > >>> From: Ferruh Yigit, Thursday, January 18, 2018 7:31 PM > >>>> This patch updates *all* ethdev public APIs to add if device is removed > >>>> check? > >>> > >>> Yes. > >>> > >>>> And each check goes to ethdev is_removed() dev_ops to ask if dev is > >>>> removed. > >>> Probably, if the REMOVED state setted in will not call device is_remove. > >>> > >>>> These must be better way of doing this, am I missing something. > >>> > >>> Suggest. > >> > >> With a silly analogy, this is like a blind person asking each time if he is dead > >> before talking to other person. > >> > >> At first glance I can think of a kind of watchdog timer can be implemented in > >> ethdev layer. It provides periodic checks and if device is dead it calls the > >> registered user callback function. > >> > >> This method presented as synchronous method but not triggered from side where > >> event happens, I mean not triggered from PMD but from application. > >> So does application doing polling continuously if device is dead? > >> Or if application is relying this patch to add a check in each API, what happens > >> if device removed during data processing, will app rely on asynchronous method? > > > > We cannot put a mutex on hardware removal :) > > So we have to live with errors du to removal. > > If we are trying to configure a removed device, > > the error will be not related to the root cause. > > This patch is just trying to improve the situation by returning > > an appropriate error code if removal can be detected. > > Note: the check is run only if there is an error > > and if the removal is not already detected. > > > > If I understand well, you prefer relying only on asynchronous > > hotplug events? Even if they come really late? > > I think asynchronous hotplug events are better approach, but if they are late I > understand you need a solution. > > I assume issue is when device is removed, application can make API calls until > it detects the removal. > > For that case what do you think instead of ethdev abstraction layer doing a > translation, define a specific error type and return it from PMDs to indicate > that error is related to the missing device and application will know device is > no more there? And consistently use that error type in all PMDs. Yes it is also a good solution. I think Matan proposed to integrate it in ethdev to avoid duplicating the same mechanism in every PMDs. > Or what do you think above suggested flexible watchdog timer implementation in > ethdev, so applications may be informed about missing device faster. I think the watchdog would compete with hotplug events. So I prefer not integrating one more asynchronous mechanism for the same purpose. If we want polling, it can be an option to enable in EAL hotplug.