From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0F361A04F3; Wed, 8 Jan 2020 15:28:03 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 439A81DA3C; Wed, 8 Jan 2020 15:28:02 +0100 (CET) Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 596911C1D0 for ; Wed, 8 Jan 2020 15:28:00 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id D88B121C39; Wed, 8 Jan 2020 09:27:59 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 08 Jan 2020 09:27:59 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=xObyaoJUYtlu6Z96Ut6pYGIp+MlF3ZrlwoaEz0wRR2Y=; b=g4VX1ER77mkl ppsSHpBQEXFv6yxVC9+4JPU6kFI+iNvu3uMEVG2jZHqSlCveSyoElCt/anQfMCIX 5tZp/Ewl7aTIttbcf23xBMS3vTbB1THLbrj7AP8HDowMQ9PSwtVdk8tZ8gaZ1OIB wFF19YGJhOBmADRMN8jF0spePe4Sx2Y= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=xObyaoJUYtlu6Z96Ut6pYGIp+MlF3ZrlwoaEz0wRR 2Y=; b=XRIFrJNr/Vufmg6gOqNZAb2VwylsS5bsU3RHM27xNVw/eiRucsWh+tpjo 88qaW0Bu8AUhOYbvwSb/EEhwpfm1g+7cI3j8xfiiYeynJU8Ct9znaYzoZd8kUKPl orjLeR6b1egD9EA7WL7o18fQ59UPojzcApddIGXeLk67mwHP+k13woZHFWpfY4H7 ahIdhansE+N8YNagTcH2KLdZ8rMoetAhBkrha6ZLQXcLbjkC1DD1z3Y3oMf7aQ6q +SaiS51SVqDl7aZ0ZV4QVFN87QK+M4rqlMmo4slx7+uuuQ4w2SxSZtAAxcwN+IlS tV2sVF9vQldmm204F89yV2RAy4EwQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrvdehkedgieegucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecukf hppeejjedrudefgedrvddtfedrudekgeenucfrrghrrghmpehmrghilhhfrhhomhepthhh ohhmrghssehmohhnjhgrlhhonhdrnhgvthenucevlhhushhtvghrufhiiigvpedt X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id F403F8005A; Wed, 8 Jan 2020 09:27:57 -0500 (EST) From: Thomas Monjalon To: Ferruh Yigit , Laurent Hardy , David Marchand , Andrew Rybchenko Cc: dev , Olivier Matz Date: Wed, 08 Jan 2020 15:27:57 +0100 Message-ID: <2017012.Icojqenx9y@xps> In-Reply-To: <817d82f8-18b4-1f2a-e531-3052fd8c48f3@solarflare.com> References: <20200107145637.8922-1-laurent.hardy@6wind.com> <736913fa-3ad9-af6e-c1ad-bc6e72b8059e@intel.com> <817d82f8-18b4-1f2a-e531-3052fd8c48f3@solarflare.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] librte_ethdev: extend dpdk api led control to query capability 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 08/01/2020 15:15, Andrew Rybchenko: > On 1/8/20 4:52 PM, Ferruh Yigit wrote: > > On 1/8/2020 1:25 PM, Thomas Monjalon wrote: > >> 08/01/2020 14:20, Ferruh Yigit: > >>> On 1/8/2020 1:06 PM, Thomas Monjalon wrote: > >>>> 08/01/2020 13:59, Ferruh Yigit: > >>>>> But for dev_ops, instead of having another capabilities indicator, which > >>>>> requires PMDs to keep this synchronized, I think it is better if we can self > >>>>> contain this information within dev_ops, like not implementing dev_ops would > >>>>> mean it is not supported, this way it is easier to maintain and less error prone. > >>>> > >>>> It means the dev_ops is resetted at init if a device does not support the feature. > >>>> It is against having const dev_ops. > >>> > >>> I didn't get your comment. > >>> For example getting FW version, I am saying instead of keeping another piece of > >>> information to say if it is supported by device/driver, better to grasp this > >>> from if the driver implemented 'fw_version_get' dev_ops or not. > >> > >> I like this approach. > >> Capabilities should be expressed by setting the function pointer or not (NULL). > >> But a driver may support a feature for a subset of devices. > > > > In that case dev_ops itself can return the '-ENOTSUP', since application > > interaction will be through the ethdev API, either API send '-ENOTSUP' because > > the dev_ops is NULL or dev_ops itself send the '-ENOTSUP' because of the > > underlying version of the device, for application it will be clear that that > > feature is not supported. > > I think it is a good illustration why deriving the capability > from dev_ops pointer is not that good idea. > > >> If a device does not support a feature, the function pointer must be set to NULL. > >> The only issue is having dev_ops as a const struct. > > > > Not sure about changing the dev_ops on runtime, it can be very hard to debug. > > I hope it was just an idea to copy dev_ops and adjust in > accordance with the device capabilities on register. > I.e. not fully dynamic changes in runtime. Changing a function pointer in runtime is tough :) I was thinking about changing it during init but I don't really see a great value. Probably better to return ENOTSUP. Anyway it does not address the capability info need.