From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id C38599A92 for ; Thu, 19 May 2016 15:26:47 +0200 (CEST) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1b3NyI-0004bL-9Q; Thu, 19 May 2016 09:26:45 -0400 Date: Thu, 19 May 2016 09:26:32 -0400 From: Neil Horman To: Panu Matilainen Cc: dev@dpdk.org, Bruce Richardson , Thomas Monjalon , Stephen Hemminger Message-ID: <20160519132632.GE4128@hmsreliant.think-freely.org> References: <1463431287-4551-1-git-send-email-nhorman@tuxdriver.com> <1463431287-4551-5-git-send-email-nhorman@tuxdriver.com> <3ee4159d-fd29-1a20-1417-4c0a40c18779@redhat.com> <20160518120300.GA29900@hmsreliant.think-freely.org> <20160518134817.GB29900@hmsreliant.think-freely.org> <4e9a5124-8ea0-7f23-9268-fde1b5d4af02@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4e9a5124-8ea0-7f23-9268-fde1b5d4af02@redhat.com> User-Agent: Mutt/1.6.1 (2016-04-27) X-Spam-Score: -1.0 (-) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCH 4/4] pmd_hw_support.py: Add tool to query binaries for hw 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: Thu, 19 May 2016 13:26:48 -0000 On Thu, May 19, 2016 at 09:08:52AM +0300, Panu Matilainen wrote: > On 05/18/2016 04:48 PM, Neil Horman wrote: > > On Wed, May 18, 2016 at 03:48:12PM +0300, Panu Matilainen wrote: > > > On 05/18/2016 03:03 PM, Neil Horman wrote: > > > > On Wed, May 18, 2016 at 02:48:30PM +0300, Panu Matilainen wrote: > > > > > On 05/16/2016 11:41 PM, 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 > > > > > > > > > > > > Signed-off-by: Neil Horman > > > > > > CC: Bruce Richardson > > > > > > CC: Thomas Monjalon > > > > > > CC: Stephen Hemminger > > > > > > CC: Panu Matilainen > > > > > > --- > > > > > > tools/pmd_hw_support.py | 174 ++++++++++++++++++++++++++++++++++++++++++++++++ > > > > > > 1 file changed, 174 insertions(+) > > > > > > create mode 100755 tools/pmd_hw_support.py > > > > > > > > > > > > diff --git a/tools/pmd_hw_support.py b/tools/pmd_hw_support.py > > > > > > new file mode 100755 > > > > > > index 0000000..0669aca > > > > > > --- /dev/null > > > > > > +++ b/tools/pmd_hw_support.py > > > > > > @@ -0,0 +1,174 @@ > > > > > > +#!/usr/bin/python3 > > > > > > > > > > I think this should use /usr/bin/python to be consistent with the other > > > > > python scripts, and like the others work with python 2 and 3. I only tested > > > > > it with python2 after changing this and it seemed to work fine so the > > > > > compatibility side should be fine as-is. > > > > > > > > > Sure, I can change the python executable, that makes sense. > > > > > > > > > On the whole, AFAICT the patch series does what it promises, and works for > > > > > both static and shared linkage. Using JSON formatted strings in an ELF > > > > > section is a sound working technical solution for the storage of the data. > > > > > But the difference between the two cases makes me wonder about this all... > > > > You mean the difference between checking static binaries and dynamic binaries? > > > > yes, there is some functional difference there > > > > > > > > > > > > > > For static library build, you'd query the application executable, eg > > > > Correct. > > > > > > > > > testpmd, to get the data out. For a shared library build, that method gives > > > > > absolutely nothing because the data is scattered around in individual > > > > > libraries which might be just about wherever, and you need to somehow > > > > Correct, I figured that users would be smart enough to realize that with > > > > dynamically linked executables, they would need to look at DSO's, but I agree, > > > > its a glaring diffrence. > > > > > > Being able to look at DSOs is good, but expecting the user to figure out > > > which DSOs might be loaded and not and where to look is going to be well > > > above many users. At very least it's not what I would call user-friendly. > > > > > I disagree, there is no linkage between an application and the dso's it opens > > via dlopen that is exportable. The only way to handle that is to have a > > standard search path for the pmd_hw_info python script. Thats just like modinfo > > works (i.e. "modinfo bnx2" finds the bnx2 module for the running kernel). We > > can of course do something simmilar, but we have no existing implicit path > > information to draw from to do that (because you can have multiple dpdk installs > > side by side). The only way around that is to explicitly call out the path on > > the command line. > > There's no telling what libraries user might load at runtime with -D, that > is true for both static and shared libraries. > I agree. > When CONFIG_RTE_EAL_PMD_PATH is set, as it is likely to be on distro builds, > you *know* that everything in that path will be loaded on runtime regardless > of what commandline options there might be so the situation is actually on > par with static builds. Of course you still dont know about ones added with > -D but that's a limitation of any solution that works without actually > running the app. > Its not on ours, as the pmd libraries get placed in the same directory as every other dpdk library, and no one wants to try (and fail to load rte_sched/rte_acl/etc twice, or deal with the fallout of trying to do so, or adjust the packaging so that pmds are placed in their own subdirectory, or handle the need for multiuser support. Using CONFIG_RTE_EAL_PMD_PATH also doesn't account for directory changes. This use case: 1) run pmdinfo 2) remove DSOs from RTE_EAL_PMD_PATH 3) execute leads to erroneous results, as hardware support that was reported in (1) is no longer available at (3) It also completely misses any libraries that we load via the -d option on the command line, which won't be included in RTE_EAL_PMD_PATH, so following that path is a half measure at best, and I think that leads to erroneous results. > > > > > > > discover the location + correct library files to be able to query that. For > > > > > the shared case, perhaps the script could be taught to walk files in > > > > > CONFIG_RTE_EAL_PMD_PATH to give in-the-ballpark correct/identical results > > > > My initial thought would be to run ldd on the executable, and use a heuristic to > > > > determine relevant pmd DSO's, and then feed each of those through the python > > > > script. I didn't want to go to that trouble unless there was consensus on it > > > > though. > > > > > > Problem is, ldd doesn't know about them either because the pmds are not > > > linked to the executables at all anymore. They could be force-linked of > > > course, but that means giving up the flexibility of plugins, which IMO is a > > > no-go. Except maybe as an option, but then that would be a third case to > > > support. > > > > > Thats not true at all, or at least its a perfectly valid way to link the DSO's > > in at link time via -lrte_pmd_. Its really just the dlopen case we need > > to worry about. I would argue that, if they're not explicitly linked in like > > that, then its correct to indicate that an application supports no hardware, > > because it actually doesn't, it only supports the pmds that it chooses to list > > on the command line. And if a user is savy enough to specify a pmd on the > > application command line, then they are perfectly capable of specifying the same > > path to the hw_info script. > > Yes you can force-link apps to every driver on existence, but it requires > not just linking but using --whole-archive. For the static case, yes, and thats what DPDK does, and likely will in perpituity, unless there is a major architectural change in the project (see commit 20afd76a504155e947c770783ef5023e87136ad8). > The apps in DPDK itself dont in > shared link setup (take a look at testpmd) and I think its for a damn good > reason - the drivers are plugins and that's how plugins are expected to > work: they are not linked to, they reside in a specific path which is I know, I'm the one that made that change when we introduced the PMD_REGISTER_DRIVER macro :). That doesn't mean its not a valid case when building apps, and one that we can take advantage of opportunistically. There are three cases we have to handle: 1) Static linking - This is taken care of 2) Dynamic linking via DT_NEEDED entries - this is taken care of 3) Dynamic linking via dlopen - This is what we're discussing here > scanned at runtime and plugins loaded to provide extra functionality. Its the runtime part that makes this non-functional. Just because you scan all the DSO's in the RTE_EAL_PATH, doesn't mean they will be there when the app is run, nor does it mean you will get a comprehensive list of hardware support, because it doesn't include additional paths/DSO's added via -d. I would much rather have users understand that an app has _no_ hardware support if it uses DSO's, because the hardware support is included with the DSO's themself, not the application (saving for the DT_NEEDED case above, where application execution is predicated on the availability of those shared objects) > > > > > > > > > > > > > when querying the executable as with static builds. If identical operation > > > > > between static and shared versions is a requirement (without running the app > > > > > in question) then query through the executable itself is practically the > > > > > only option. Unless some kind of (auto-generated) external config file > > > > > system ala kernel depmod / modules.dep etc is brought into the picture. > > > > Yeah, I'm really trying to avoid that, as I think its really not a typical part > > > > of how user space libraries are interacted with. > > > > > > > > > > > > > > For shared library configurations, having the data in the individual pmds is > > > > > valuable as one could for example have rpm autogenerate provides from the > > > > > data to ease/automate installation (in case of split packaging and/or 3rd > > > > > party drivers). And no doubt other interesting possibilities. With static > > > > > builds that kind of thing is not possible. > > > > Right. > > > > > > > > Note, this also leaves out PMD's that are loaded dynamically (i.e. via dlopen). > > > > For those situations I don't think we have any way of 'knowing' that the > > > > application intends to use them. > > > > > > Hence my comment about CONFIG_RTE_EAL_PMD_PATH above, it at least provides a > > > reasonable heuristic of what would be loaded by the app when run. But > > > ultimately the only way to know what hardware is supported at a given time > > > is to run an app which calls rte_eal_init() to load all the drivers that are > > > present and work from there, because besides CONFIG_RTE_EAL_PMD_PATH this > > > can be affected by runtime commandline switches and applies to both shared > > > and static builds. > > > > > I'm not sure I agree with that. Its clearly tempting to use, but its not > > at all guaranteed to be accurate (the default is just set to "", and there is no > > promise anyone will set it properly). > > The promise is that shared builds are barely functional unless its set > correctly, because zero drivers are linked to testpmd in shared config. So > you're kinda likely to notice if its not set. > You're twisting the meaning of 'barely functional' here. I agree that shared builds are barely functional, because they have no self-contained hardware support, and as such, running pmdinfo.py on such an application should report exactly that. That said, in order to run, and DPDK application built to use shared libraries has to use one of two methods to obtain hardware support A) direct shared linking (the DT_NEEDED case) - This case is handled, and we report hardware support when found, as the application won't run unless those libraries are resolved b) dynamic loading via dlopen - This case shouldn't be handled, because the application in reality doesn't support any hardware. Hardware support is garnered at run time when the EAL_PMD_PATH (and any other paths added via the -d option) are scanned. In this case, pmdinfo shouldn't report any hardware support, it should only do so if the pmd DSO is queried directly. > It defaults to empty because at the time there was no standard installation > available at that time. Setting a reasonable default is tricky still because > it needs to be set before build whereas install path is set at install time. > Exactly, which is why distros don't use it. It also neglects the multiuser case (in which different users may want to load different hardware support). > > And it also requires that the binary will > > be tied to a specific release. I really think that, given the fact that > > distributions generally try to package dpdk in such a way that multiple dpdk > > versions might be available, the right solution is to just require a full path > > specification if you want to get hw info for a DSO that is dynamically loaded > > via dlopen from the command line. Otherwise you're going to fall into this trap > > where you might be looking implicitly at an older version of the PMD while your > > application may use a newer version. > > If there are multiple dpdk versions available then they just need to have > separate PMD paths, but that's not a problem. > > > > > > > > > > > Calling up on the list of requirements from > > > > > http://dpdk.org/ml/archives/dev/2016-May/038324.html, I see a pile of > > > > > technical requirements but perhaps we should stop for a moment to think > > > > > about the use-cases first? > > > > > > > > To ennumerate the list: > > > > > > > > - query all drivers in static binary or shared library (works) > > > > - stripping resiliency (works) > > > > - human friendly (works) > > > > - script friendly (works) > > > > - show driver name (works) > > > > - list supported device id / name (works) > > > > - list driver options (not yet, but possible) > > > > - show driver version if available (nope, but possible) > > > > - show dpdk version (nope, but possible) > > > > - show kernel dependencies (vfio/uio_pci_generic/etc) (nope) > > > > - room for extra information? (works) > > > > > > > > Of the items that are missing, I've already got a V2 started that can do driver > > > > options, and is easier to expand. Adding in the the DPDK and PMD version should > > > > be easy (though I think they can be left out, as theres currently no globaly > > > > defined DPDK release version, its all just implicit, and driver versions aren't > > > > really there either). I'm also hesitant to include kernel dependencies without > > > > defining exactly what they mean (just module dependencies, or feature > > > > enablement, or something else?). Once we define it though, adding it can be > > > > easy. > > > > > > Yup. I just think the shared/static difference needs to be sorted out > > > somehow, eg requiring user to know about DSOs is not human-friendly at all. > > > That's why I called for the higher level use-cases in my previous email. > > > > > > > I disagree with that. While its reasonable to give users the convienience of > > scanning the DT_NEEDED entries of a binary and scanning those DSO's. If a user > > Scanning DT_NEEDED is of course ok sane and right thing to do, its just not > sufficient. > But its the only sane thing we can do implicitly in the shared case, because we know those drivers have to be resolved for the app to run. In the RTE_EAL_PMD_PATH or -d cases, we dont' know until runtime what drivers that will include, and so reporting on hardware support prior to run time via scanning of the application is erroneous. The sane thing to do is scan the pmd DSO, which is where the hardware support resides, and make it clear that, for an application to get that hardware support, they need to either link dynamically (via a DT_NEEDED entry), or specify it on the command line, or make sure its in RTE_EAL_PMD_PATH (if the distribution set it, which so far, no one does). > > has to specify the PMD to load in an application (either on the command line or > > via a configuration file), then its reasonable assume that they (a) know where > > But when the PMD path is set (as it should be on a distro build), this is > all automatic with zero action or extra config required from the user. > Its not, RHEL doesn't do it, Fedora Doesn't do it, Ubuntu doesn't do it. Even if it is, it doesn't imply an application will get that support, as the directory contents may change between scanning and application run time. > > to find that pmd and (b) are savy enough to pass that same path to a hardware > > info tool. Thats the exact same way that modinfo works (save for the fact that > > modinfo can implicitly check the running kernel version to find the appropriate > > path for a modular driver). > > > > The only other thing that seems reasonable to me would be to scan > > LD_LIBRARY_PATH. I would assume that, if an application is linked dynamically, > > the individual DSO's (librte_sched.so, etc), need to be in LD_LIBRARY_PATH. If > > thats the case, then we can assume that the appropriate PMD DSO's are there too, > > and we can search there. We can also check the standard /usr/lib and /lib paths > > with that. I think that would make fairly good sense. > > You really don't want go crashing through the potentially thousands of > libraries in the standard library path going "is it a pmd, no, is it a pmd, > no..." What? No. You misunderstand. In the above, all I'm saying is that if you specify an application, you can scan LD_LIBRARY_PATH for libraries in the DT_NEEDED list. I don't mean to say that we shoudl check _every_ library in LD_LIBRARY_PATH, that would be crazy. Of course, the same problem exists with RTE_EAL_PMD_PATH. In the current installation, all pmd libraries are co-located with the rest of the dpdk library set, so RTE_EAL_PMD_PATH would have to be set to whatever that installation path was. And once thats done, we would have to go crashing through every DPDK library to see if it was a pmd. Thats just as insane. Neil