From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 6D53D9AD8 for ; Fri, 20 May 2016 07:22:38 +0200 (CEST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 433C5C05B1DB; Fri, 20 May 2016 05:22:37 +0000 (UTC) Received: from sopuli.koti.laiskiainen.org (vpn1-6-156.ams2.redhat.com [10.36.6.156]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4K5MY6A013119; Fri, 20 May 2016 01:22:35 -0400 From: Panu Matilainen To: Neil Horman References: <1463431287-4551-1-git-send-email-nhorman@tuxdriver.com> <1463605687-649-1-git-send-email-nhorman@tuxdriver.com> <1463605687-649-5-git-send-email-nhorman@tuxdriver.com> <20160519120031.GC4128@hmsreliant.think-freely.org> Cc: dev@dpdk.org, Bruce Richardson , Thomas Monjalon , Stephen Hemminger Message-ID: Date: Fri, 20 May 2016 08:22:34 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <20160519120031.GC4128@hmsreliant.think-freely.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 20 May 2016 05:22:37 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCHv2 4/4] pmdinfo.py: Add tool to query binaries for hw and other support information 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: Fri, 20 May 2016 05:22:38 -0000 On 05/19/2016 03:00 PM, Neil Horman wrote: > On Thu, May 19, 2016 at 12:02:27PM +0300, Panu Matilainen wrote: >> On 05/19/2016 12:08 AM, Neil Horman wrote: >>> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF binary, >>> and, if found parses the remainder of the string as a json encoded string, >>> outputting the results in either a human readable or raw, script parseable >>> format >>> >>> Note that, in the case of dynamically linked applications, pmdinfo.py will scan >>> for implicitly linked PMDs by searching the specified binaries .dynamic section >>> for DT_NEEDED entries that contain the substring librte_pmd. The DT_RUNPATH, >>> LD_LIBRARY_PATH, /usr/lib and /lib are searched for these libraries, in that >>> order >> >> Scanning /usr/lib and /lib does little good on systems where /usr/lib64 and >> /lib64 are the standard path, such as x86_64 Fedora / RHEL and derivates. >> > Ah, sorry, forgot the 64 bit variants, I can add those in. > >> With the path changed (or LD_LIBRARY_PATH set manually), I can confirm it >> works for a shared binary which is --whole-archive linked to all of DPDK >> such as ovs-vswitchd currently is (because it needs to for static DPDK >> linkage and is not aware of plugin autoloading). >> > Right, thats why it works, because DPDK always requires --whole-archive for > static linking, and likely always will (see commit > 20afd76a504155e947c770783ef5023e87136ad8) > >> It doesn't help testpmd though because its not linked with --whole-archive >> in the shared case, so its not working for the main DPDK executable... >> > This sentence doesn't make sense --whole-archive is only applicable in the > static binary case, and only when linking archive files. Okay sorry I was indeed mixing up things here. 1) Testpmd doesn't get linked to those pmds at all, because of this in mk/rte.app.mk: ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) # plugins (link only if static libraries) ... endif 2) I'm could swear I've seen --whole-archive it make a difference on newer toolchains with the linker script, but I can't reproduce that now no matter what. Must've been hallucinating ;) > >> In any case, using --whole-archive is a sledgehammer solution at best, and >> against the spirit of shared libs and plugins in particular. >> > It may be a sledgehammer solution, but its the one dpdk uses, and will likely > use in perpituity. > >> I think the shared linkage case can be solved by exporting the PMD path from >> librte_eal (either through an elf section or c-level symbol) and teach the >> script to detect the case of an executable dynamically linked to librte_eal, >> fish the path from there and then process everything in that path. >> > I really disagree with this, because its a half-measure at best. Yes, if its > set, you will definately get all the shared objects in that directory loaded, > but that is in no way a guarantee that those are the only libraries that get > loaded (the application may load its own independently). That is in no way different from statically linked apps loading additional drivers with the EAL -d switch. > So you're left in this > situation in which you get maybe some of the hardware support an application > offers. Its also transient. That is to say, if you configure a plugin > directory and search it when you scan an application, its contents may change > post scan, leading to erroneous results. Yes, changing system state such as installing or removing packages between scanning and running can "do stuff" such as change results. People are quite aware of this because that's how a huge number of things in the system works, you install plugins for multimedia formats you need, maybe remove others you dont to clean up system etc. I fail to see how that is a problem when it's the expected behavior with plugins. > The way I see it, we have 3 cases that we need to handle: > > 1) Statically linked application - in this case, all pmds that are statically > linked in to the application will be reported, so we're good here > > 2) Dynamically loaded via DT_NEEDED entries - This is effectively the same as a > static linking case, in that we have a list of libraries that must be resolved > at run time, so we are safe to search for and scan the DSO's that the > application ennumerates > > 3) Dynamically loaded via dlopen - In this case, we don't actually know until > runtime what DSO's are going to get loaded, even if RTE_EAL_PMD_PATH is set, > because the contents of that path can change at arbitrary times. In this case, > its correct to indicate that the application itself _doesn't_ actually support > the hardware of the PMD's in that path, because until the application is > executed, it has none of the support embodied in any DSO that it loads via > dlopen. The hardware support travels with the DSO itself, and so its correct to > only display hardware support when the PMD shared library itself is scanned. > > Handling case 3 the way I'm proposing is exactly the way the OS does it (that is > to say, it only details hardware support for the module being queried, and you > have to specify the module name to get that). I don't see there being any > problem with that. Ability to query individual DSOs is a building block for other things like automation (you dont expect normal users to go manually loading hw support modules for the OS either), but its not an end-user solution. Thomas said in http://dpdk.org/ml/archives/dev/2016-May/038324.html: "This tool should not behave differently depending of how DPDK was compiled (static or shared)." Its entirely possible to handle all the three above cases virtually identically (point the tool to the app executable), so I have a hard time understanding the level of resistance to handling the plugin case. - Panu - > >>> >>> If a file is specified with no path, it is assumed to be a PMD DSO, and the >>> LD_LIBRARY_PATH, /usr/lib/ and /lib is searched for it >> >> Same as above, /usr/lib/ and /lib is incorrect for a large number of >> systems. >> > Yeah, I'll add 64 bit detection and correct that path accordingly > >>> >>> Currently the tool can output data in 3 formats: >>> >>> a) raw, suitable for scripting, where the raw JSON strings are dumped out >>> b) table format (default) where hex pci ids are dumped in a table format >>> c) pretty, where a user supplied pci.ids file is used to print out vendor and >>> device strings >> >> c) is a nice addition. Would be even nicer if it knew the most common >> pci.ids locations so it doesn't need extra arguments in those cases, ie see >> if /usr/share/hwdata/pci.ids or /usr/share/misc/pci.ids exists and use that >> unless overridden on the cli. >> > Yeah, I can do that. > >> - Panu - >> >> >>