From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 4AB3F2B9D for ; Tue, 8 Aug 2017 10:32:11 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga105.jf.intel.com with ESMTP; 08 Aug 2017 01:32:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,342,1498546800"; d="scan'208";a="116829646" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.57]) ([10.237.220.57]) by orsmga004.jf.intel.com with ESMTP; 08 Aug 2017 01:32:08 -0700 To: Bruce Richardson Cc: Thomas Monjalon , dev@dpdk.org, Stephen Hemminger , "Chilikin, Andrey" , "Ananyev, Konstantin" , "Wu, Jingjing" References: <20170803132138.GA8732@bricha3-MOBL3.ger.corp.intel.com> <20170803091531.5902f86c@xeon-e3> <1581480.IJArXVfUmc@xps> <12c296b7-0696-6c12-7fb2-e43bcd49d784@intel.com> <20170804125635.GA23392@bricha3-MOBL3.ger.corp.intel.com> From: Ferruh Yigit Message-ID: Date: Tue, 8 Aug 2017 09:32:07 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170804125635.GA23392@bricha3-MOBL3.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [RFC] ethdev: add ioctl-like API to control device specific features 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: , X-List-Received-Date: Tue, 08 Aug 2017 08:32:11 -0000 On 8/4/2017 1:56 PM, Bruce Richardson wrote: > On Fri, Aug 04, 2017 at 12:58:01PM +0100, Ferruh Yigit wrote: >> On 8/3/2017 8:53 PM, Thomas Monjalon wrote: >>> 03/08/2017 18:15, Stephen Hemminger: >>>> On Thu, 3 Aug 2017 14:21:38 +0100 >>>> Bruce Richardson wrote: >>>> >>>>> On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote: >>>>>> To control some device-specific features public device-specific functions >>>>>> rte_pmd_*.h are used. >>>>>> >>>>>> But this solution requires applications to distinguish devices at runtime >>>>>> and, depending on the device type, call corresponding device-specific >>>>>> functions even if functions' parameters are the same. >>>>>> >>>>>> IOCTL-like API can be added to ethdev instead of public device-specific >>>>>> functions to address the following: >>>>>> >>>>>> * allow more usable support of features across a range of NIC from >>>>>> one vendor, but not others >>>>>> * allow features to be implemented by multiple NIC drivers without >>>>>> relying on a critical mass to get the functionality in ethdev >>>>>> * there are a large number of possible device specific functions, and >>>>>> creating individual APIs for each one is not a good solution >>>>>> * IOCTLs are a proven method for solving this problem in other areas, >>>>>> i.e. OS kernels. >>>>>> >>>>>> Control requests for this API will be globally defined at ethdev level, so >>>>>> an application will use single API call to control different devices from >>>>>> one/multiple vendors. >>>>>> >>>>>> API call may look like as a classic ioctl with an extra parameter for >>>>>> argument length for better sanity checks: >>>>>> >>>>>> int >>>>>> rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp, >>>>>> unsigned arg_length); >>>>>> >>>>>> Regards, >>>>>> Andrey >>>>> >>>>> I think we need to start putting in IOCTLs for ethdevs, much as I hate >>>>> to admit it, since I dislike IOCTLs and other functions with opaque >>>>> arguments! Having driver specific functions I don't think will scale >>>>> well as each vendor tries to expose as much of their driver specific >>>>> functionality as possible. >>>>> >>>>> One other additional example: I discovered just this week another issue >>>>> with driver specific functions and testpmd, when I was working on the >>>>> meson build rework. >>>>> >>>>> * With shared libraries, when we do "ninja install" we want our DPDK >>>>> libs moved to e.g. /usr/local/lib, but the drivers moved to a separate >>>>> driver folder, so that they can be automatically loaded from that >>>>> single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH]. >>>>> * However, testpmd, as well as using the drivers as plugins, uses >>>>> driver-specific functions, which means that it explicitly links >>>>> against the pmd .so files. >>>>> * Those driver .so files are not in with the other libraries, so ld.so >>>>> does not find the pmd, and the installed testpmd fails to run due to >>>>> missing library dependencies. >>>>> * The workaround is to add the drivers path to the ld load path, but we >>>>> should not require ld library path changes just to get DPDK apps to >>>>> work. >>>>> >>>>> Using ioctls instead of driver-specific functions would solve this. >>>>> >>>>> My 2c. >>>> >>>> My 2c. No. >>>> >>>> Short answer: >>>> Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now >>>> despised by Linux kernel developers. They provide an unstructured, unsecured, >>>> back door for device driver abuse. Try to get a new driver in Linux with >>>> a unique ioctl, and it will be hard to get accepted. >>>> >>>> Long answer: >>>> So far every device specific feature has fit into ethdev model. Doing ioctl >>>> is admitting "it is too hard to be general, we need need an out". For something >>>> that is a flag, it should fit into existing config model; ignoring silly ABI constraints. >>>> For a real feature (think flow direction), we want a first class API for that. >>>> For a wart, then devargs will do. >>>> >>>> Give a good example of something that should be an ioctl. Don't build the >>>> API first and then let it get cluttered. >>> >>> I agree with Stephen. >>> >>> And please do not forget that ioctl still requires an API: >>> the argument that you put in ioctl is the API of the feature. >>> So it is the same thing as defining a new function. >> >> I am also not fan of the ioctl usage. I believe it hides APIs behind ids >> and prevent argument check by compiler. >> >> BUT, the number of the increasing PMD specific APIs are also worrying, >> it is becoming harder to maintain, and I believe this is something NOT >> sustainable in long run. >> >> >> What about having *eth_dev_extended_ops* ? >> >> >> As a part of the rte_eth_dev. This can be in the librte_ether library >> but in a separated file. >> >> And the APIs for these ops can be less strict on compatibility, and >> easier to add. >> >> Benefits of having this new dev_ops: >> >> * Having an abstraction layer for common checks. >> >> * Even feature is not generic for all NICs, still a few NICs can share >> the ops. >> >> * All APIs are in the same file makes it easy to see PMD specific APIs >> comparing to scattered into various PMDs. >> >> * This is very like ioctl approach, but APIs are more clear and >> arguments can be verified. >> > > Sounds like an ethdev-staging library, where features can be put until > such time as they get critical mass for acceptance and promoted to > ethdev? It's sounds better than IOCTL, while giving the same benefits. > > I'd be happy enough with any solution that allows NIC features to be > exposed that does not have functions limited to each individual driver, > so that common functionality can be exposed to apps via an API even if > only 2 drivers support it. This is not decided yet, but to enable working on this for next release, is a deprecation notice required to add a new field to "struct rte_eth_dev" ? "struct rte_eth_dev" is marked as "@internal", so I believe deprecation notice is NOT required, but I would like to confirm. > >> Thanks, >> ferruh >> >> >>> >>> The real debate is to decide if we want to continue adding more >>> control path features in DPDK or focus on Rx/Tx. > I don't see dropping control path as an option. It would severely limit > the usefulness of DPDK. > > /Bruce >