* [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL @ 2020-11-24 15:13 Timothy Redaelli 2020-11-24 15:13 ` [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection Timothy Redaelli ` (6 more replies) 0 siblings, 7 replies; 14+ messages in thread From: Timothy Redaelli @ 2020-11-24 15:13 UTC (permalink / raw) To: Anatoly Burakov; +Cc: bruce.richardson, stable Currently eal only tries to find shared lib by looking for .so files, but on Fedora, CentOS and RHEL the .so file is not installed by dpdk package, but it's only installed by dpdk-devel package, since .so files should not be necessary in order to run a program [1]. This series fix that by checking for .so.ABI_VERSION that should be available on any linux distribution. [1] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Timothy Redaelli (2): eal: fix shared lib mode detection eal: fix loading of shared libs from driver plugin directories lib/librte_eal/common/eal_common_options.c | 9 ++++++--- lib/librte_eal/common/meson.build | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) -- 2.28.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection 2020-11-24 15:13 [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli @ 2020-11-24 15:13 ` Timothy Redaelli 2020-11-24 15:14 ` Timothy Redaelli ` (2 more replies) 2020-11-24 15:14 ` [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli ` (5 subsequent siblings) 6 siblings, 3 replies; 14+ messages in thread From: Timothy Redaelli @ 2020-11-24 15:13 UTC (permalink / raw) To: Anatoly Burakov; +Cc: bruce.richardson, stable Commit 06c7871dde01 ("eal: restrict default plugin path to shared lib mode") introduced a check that enabled shared lib mode when librte_eal.so can be loaded, but it can't work, at least, on Fedora/CentOS/RHEL since .so symlinks are not installed when you install dpdk package, but only when you install dpdk-devel package. This commit uses librte_eal.so.ABI_VERSION to check for shared lib, since it exists on any linux distributions. See Fedora Packaging Guidelines for more informations: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Fixes: 06c7871dde01 ("eal: restrict default plugin path to shared lib mode") Cc: bruce.richardson@intel.com Cc: stable@dpdk.org Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> --- lib/librte_eal/common/eal_common_options.c | 3 ++- lib/librte_eal/common/meson.build | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c index fc6f0cea93..e6f77ad217 100644 --- a/lib/librte_eal/common/eal_common_options.c +++ b/lib/librte_eal/common/eal_common_options.c @@ -503,7 +503,8 @@ eal_plugins_init(void) * (Using dlopen with NOLOAD flag on EAL, will return NULL if the EAL * shared library is not already loaded i.e. it's statically linked.) */ - if (dlopen("librte_eal.so", RTLD_LAZY | RTLD_NOLOAD) != NULL && + if (dlopen("librte_eal.so."ABI_VERSION, RTLD_LAZY | RTLD_NOLOAD) + != NULL && *default_solib_dir != '\0' && stat(default_solib_dir, &sb) == 0 && S_ISDIR(sb.st_mode)) diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build index 9f32262450..39abf7a0a4 100644 --- a/lib/librte_eal/common/meson.build +++ b/lib/librte_eal/common/meson.build @@ -3,6 +3,8 @@ includes += include_directories('.') +cflags += [ '-DABI_VERSION="@0@"'.format(abi_version) ] + if is_windows sources += files( 'eal_common_bus.c', -- 2.28.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection 2020-11-24 15:13 ` [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection Timothy Redaelli @ 2020-11-24 15:14 ` Timothy Redaelli 2020-11-24 15:22 ` Bruce Richardson 2020-11-25 9:10 ` David Marchand 2 siblings, 0 replies; 14+ messages in thread From: Timothy Redaelli @ 2020-11-24 15:14 UTC (permalink / raw) To: Anatoly Burakov; +Cc: bruce.richardson, dev, stable Commit 06c7871dde01 ("eal: restrict default plugin path to shared lib mode") introduced a check that enabled shared lib mode when librte_eal.so can be loaded, but it can't work, at least, on Fedora/CentOS/RHEL since .so symlinks are not installed when you install dpdk package, but only when you install dpdk-devel package. This commit uses librte_eal.so.ABI_VERSION to check for shared lib, since it exists on any linux distributions. See Fedora Packaging Guidelines for more informations: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Fixes: 06c7871dde01 ("eal: restrict default plugin path to shared lib mode") Cc: bruce.richardson@intel.com Cc: stable@dpdk.org Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> --- lib/librte_eal/common/eal_common_options.c | 3 ++- lib/librte_eal/common/meson.build | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c index fc6f0cea93..e6f77ad217 100644 --- a/lib/librte_eal/common/eal_common_options.c +++ b/lib/librte_eal/common/eal_common_options.c @@ -503,7 +503,8 @@ eal_plugins_init(void) * (Using dlopen with NOLOAD flag on EAL, will return NULL if the EAL * shared library is not already loaded i.e. it's statically linked.) */ - if (dlopen("librte_eal.so", RTLD_LAZY | RTLD_NOLOAD) != NULL && + if (dlopen("librte_eal.so."ABI_VERSION, RTLD_LAZY | RTLD_NOLOAD) + != NULL && *default_solib_dir != '\0' && stat(default_solib_dir, &sb) == 0 && S_ISDIR(sb.st_mode)) diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build index 9f32262450..39abf7a0a4 100644 --- a/lib/librte_eal/common/meson.build +++ b/lib/librte_eal/common/meson.build @@ -3,6 +3,8 @@ includes += include_directories('.') +cflags += [ '-DABI_VERSION="@0@"'.format(abi_version) ] + if is_windows sources += files( 'eal_common_bus.c', -- 2.28.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection 2020-11-24 15:13 ` [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection Timothy Redaelli 2020-11-24 15:14 ` Timothy Redaelli @ 2020-11-24 15:22 ` Bruce Richardson 2020-11-25 9:10 ` David Marchand 2 siblings, 0 replies; 14+ messages in thread From: Bruce Richardson @ 2020-11-24 15:22 UTC (permalink / raw) To: Timothy Redaelli; +Cc: Anatoly Burakov, dev, stable On Tue, Nov 24, 2020 at 04:14:14PM +0100, Timothy Redaelli wrote: > Commit 06c7871dde01 ("eal: restrict default plugin path to shared lib mode") > introduced a check that enabled shared lib mode when librte_eal.so can > be loaded, but it can't work, at least, on Fedora/CentOS/RHEL since .so > symlinks are not installed when you install dpdk package, but only when > you install dpdk-devel package. > > This commit uses librte_eal.so.ABI_VERSION to check for shared lib, > since it exists on any linux distributions. > > See Fedora Packaging Guidelines for more informations: > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages > > Fixes: 06c7871dde01 ("eal: restrict default plugin path to shared lib mode") > Cc: bruce.richardson@intel.com > Cc: stable@dpdk.org > Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> > --- > lib/librte_eal/common/eal_common_options.c | 3 ++- > lib/librte_eal/common/meson.build | 2 ++ > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c > index fc6f0cea93..e6f77ad217 100644 > --- a/lib/librte_eal/common/eal_common_options.c > +++ b/lib/librte_eal/common/eal_common_options.c > @@ -503,7 +503,8 @@ eal_plugins_init(void) > * (Using dlopen with NOLOAD flag on EAL, will return NULL if the EAL > * shared library is not already loaded i.e. it's statically linked.) > */ > - if (dlopen("librte_eal.so", RTLD_LAZY | RTLD_NOLOAD) != NULL && > + if (dlopen("librte_eal.so."ABI_VERSION, RTLD_LAZY | RTLD_NOLOAD) > + != NULL && Since we are not so strict on the 80-char lines, I think I'd keep the != NULL on the original line to improve readability. > *default_solib_dir != '\0' && > stat(default_solib_dir, &sb) == 0 && > S_ISDIR(sb.st_mode)) > diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build > index 9f32262450..39abf7a0a4 100644 > --- a/lib/librte_eal/common/meson.build > +++ b/lib/librte_eal/common/meson.build > @@ -3,6 +3,8 @@ > > includes += include_directories('.') > > +cflags += [ '-DABI_VERSION="@0@"'.format(abi_version) ] > + > if is_windows > sources += files( > 'eal_common_bus.c', > -- Acked-by: Bruce Richardson <bruce.richardson@intel.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection 2020-11-24 15:13 ` [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection Timothy Redaelli 2020-11-24 15:14 ` Timothy Redaelli 2020-11-24 15:22 ` Bruce Richardson @ 2020-11-25 9:10 ` David Marchand 2 siblings, 0 replies; 14+ messages in thread From: David Marchand @ 2020-11-25 9:10 UTC (permalink / raw) To: Timothy Redaelli; +Cc: Anatoly Burakov, Bruce Richardson, dev, dpdk stable On Tue, Nov 24, 2020 at 4:15 PM Timothy Redaelli <tredaelli@redhat.com> wrote: > > Commit 06c7871dde01 ("eal: restrict default plugin path to shared lib mode") > introduced a check that enabled shared lib mode when librte_eal.so can > be loaded, but it can't work, at least, on Fedora/CentOS/RHEL since .so > symlinks are not installed when you install dpdk package, but only when > you install dpdk-devel package. > > This commit uses librte_eal.so.ABI_VERSION to check for shared lib, > since it exists on any linux distributions. > > See Fedora Packaging Guidelines for more informations: > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages > > Fixes: 06c7871dde01 ("eal: restrict default plugin path to shared lib mode") > Cc: bruce.richardson@intel.com > Cc: stable@dpdk.org > Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Same comment as Bruce on the != NULL location, but otherwise lgtm. Acked-by: David Marchand <david.marchand@redhat.com> -- David Marchand ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL 2020-11-24 15:13 [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli 2020-11-24 15:13 ` [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection Timothy Redaelli @ 2020-11-24 15:14 ` Timothy Redaelli 2020-11-24 15:14 ` [dpdk-stable] [PATCH 2/2] eal: fix loading of shared libs from driver plugin directories Timothy Redaelli ` (4 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: Timothy Redaelli @ 2020-11-24 15:14 UTC (permalink / raw) To: Anatoly Burakov; +Cc: bruce.richardson, dev, stable Currently eal only tries to find shared lib by looking for .so files, but on Fedora, CentOS and RHEL the .so file is not installed by dpdk package, but it's only installed by dpdk-devel package, since .so files should not be necessary in order to run a program [1]. This series fix that by checking for .so.ABI_VERSION that should be available on any linux distribution. [1] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Timothy Redaelli (2): eal: fix shared lib mode detection eal: fix loading of shared libs from driver plugin directories lib/librte_eal/common/eal_common_options.c | 9 ++++++--- lib/librte_eal/common/meson.build | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) -- 2.28.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-stable] [PATCH 2/2] eal: fix loading of shared libs from driver plugin directories 2020-11-24 15:13 [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli 2020-11-24 15:13 ` [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection Timothy Redaelli 2020-11-24 15:14 ` [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli @ 2020-11-24 15:14 ` Timothy Redaelli 2020-11-24 15:22 ` Bruce Richardson 2020-11-25 10:01 ` David Marchand 2020-11-25 10:57 ` [dpdk-stable] [dpdk-dev] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Maxime Coquelin ` (3 subsequent siblings) 6 siblings, 2 replies; 14+ messages in thread From: Timothy Redaelli @ 2020-11-24 15:14 UTC (permalink / raw) To: Anatoly Burakov; +Cc: bruce.richardson, dev, stable Commit 49b536fc3060 ("eal: load only shared libs from driver plugin directories") introduced a check that any shared library must ends with .so, but it can't work, at least, on Fedora/CentOS/RHEL since .so symlinks are not installed when you install dpdk package, but only when you install dpdk-devel package. This commit adds also a check for .so.ABI_VERSION to check for shared lib. See Fedora Packaging Guidelines for more informations: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Fixes: 49b536fc3060 ("eal: load only shared libs from driver plugin directories") Cc: bruce.richardson@intel.com Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> --- lib/librte_eal/common/eal_common_options.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c index e6f77ad217..e9e2362c53 100644 --- a/lib/librte_eal/common/eal_common_options.c +++ b/lib/librte_eal/common/eal_common_options.c @@ -402,8 +402,10 @@ eal_plugindir_init(const char *path) struct stat sb; int nlen = strnlen(dent->d_name, sizeof(dent->d_name)); - /* check if name ends in .so */ - if (strcmp(&dent->d_name[nlen - 3], ".so") != 0) + /* check if name ends in .so or .so.ABI_VERSION */ + if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 && + strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)], + ".so."ABI_VERSION) != 0) continue; snprintf(sopath, sizeof(sopath), "%s/%s", path, dent->d_name); -- 2.28.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] eal: fix loading of shared libs from driver plugin directories 2020-11-24 15:14 ` [dpdk-stable] [PATCH 2/2] eal: fix loading of shared libs from driver plugin directories Timothy Redaelli @ 2020-11-24 15:22 ` Bruce Richardson 2020-11-25 10:01 ` David Marchand 1 sibling, 0 replies; 14+ messages in thread From: Bruce Richardson @ 2020-11-24 15:22 UTC (permalink / raw) To: Timothy Redaelli; +Cc: Anatoly Burakov, dev, stable On Tue, Nov 24, 2020 at 04:14:15PM +0100, Timothy Redaelli wrote: > Commit 49b536fc3060 ("eal: load only shared libs from driver plugin directories") > introduced a check that any shared library must ends with .so, but it can't > work, at least, on Fedora/CentOS/RHEL since .so symlinks are not installed > when you install dpdk package, but only when you install dpdk-devel package. > > This commit adds also a check for .so.ABI_VERSION to check for shared > lib. > > See Fedora Packaging Guidelines for more informations: > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages > > Fixes: 49b536fc3060 ("eal: load only shared libs from driver plugin directories") > Cc: bruce.richardson@intel.com > Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> > --- Acked-by: Bruce Richardson <bruce.richardson@intel.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] eal: fix loading of shared libs from driver plugin directories 2020-11-24 15:14 ` [dpdk-stable] [PATCH 2/2] eal: fix loading of shared libs from driver plugin directories Timothy Redaelli 2020-11-24 15:22 ` Bruce Richardson @ 2020-11-25 10:01 ` David Marchand 2020-11-25 11:05 ` Ferruh Yigit 1 sibling, 1 reply; 14+ messages in thread From: David Marchand @ 2020-11-25 10:01 UTC (permalink / raw) To: Timothy Redaelli; +Cc: Anatoly Burakov, Bruce Richardson, dev, dpdk stable On Tue, Nov 24, 2020 at 4:15 PM Timothy Redaelli <tredaelli@redhat.com> wrote: > > Commit 49b536fc3060 ("eal: load only shared libs from driver plugin directories") > introduced a check that any shared library must ends with .so, but it can't > work, at least, on Fedora/CentOS/RHEL since .so symlinks are not installed > when you install dpdk package, but only when you install dpdk-devel package. > > This commit adds also a check for .so.ABI_VERSION to check for shared > lib. > > See Fedora Packaging Guidelines for more informations: > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages > > Fixes: 49b536fc3060 ("eal: load only shared libs from driver plugin directories") > Cc: bruce.richardson@intel.com > Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> > --- > lib/librte_eal/common/eal_common_options.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c > index e6f77ad217..e9e2362c53 100644 > --- a/lib/librte_eal/common/eal_common_options.c > +++ b/lib/librte_eal/common/eal_common_options.c > @@ -402,8 +402,10 @@ eal_plugindir_init(const char *path) > struct stat sb; > int nlen = strnlen(dent->d_name, sizeof(dent->d_name)); > > - /* check if name ends in .so */ > - if (strcmp(&dent->d_name[nlen - 3], ".so") != 0) > + /* check if name ends in .so or .so.ABI_VERSION */ > + if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 && > + strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)], > + ".so."ABI_VERSION) != 0) > continue; > > snprintf(sopath, sizeof(sopath), "%s/%s", path, dent->d_name); A first doubt I had about this patch is about multiple loading of the same driver, and its effects. So I first had a look at the main branch current state (using devtools/test-null.sh) and I can see multiple loading already happens for statically linked drivers like the bond driver. $ LD_LIBRARY_PATH=build/lib gdb build/app/dpdk-testpmd -ex "b main" -ex "run -c 3 --no-huge -m 20 -d build/drivers '--log-level=*:debug' -a 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia" ... (gdb) info sharedlibrary ... 0x00007ffff796d5b0 0x00007ffff797d2d6 Yes /home/dmarchan/dpdk/build/app/../drivers/librte_net_bond.so.21 ... (gdb) b rte_eal_init (gdb) c (gdb) finish ... EAL: open shared lib build/drivers/librte_net_avp.so EAL: open shared lib build/drivers/librte_net_bond.so EAL: open shared lib build/drivers/librte_net_ixgbe.so ... (gdb) set $elm=*(struct shared_driver *)solib_list.tqh_first (gdb) while $elm p $elm set $elm=*(struct shared_driver *)($elm.next.tqe_next) end ... $67 = {next = {tqe_next = 0x5c3350, tqe_prev = 0x5c1310}, name = "build/drivers/librte_net_avp.so", '\000' <repeats 4064 times>, lib_handle = 0x616870} $68 = {next = {tqe_next = 0x5c4370, tqe_prev = 0x5c2330}, name = "build/drivers/librte_net_bond.so", '\000' <repeats 4063 times>, lib_handle = 0x7ffff7995a80} $69 = {next = {tqe_next = 0x5c5390, tqe_prev = 0x5c3350}, name = "build/drivers/librte_net_ixgbe.so", '\000' <repeats 4062 times>, lib_handle = 0x7ffff7942fc0} ... I could not confirm the 0x7ffff7995a80 handle points at the first load of the bond driver. gdb does not seem to know of the linker internal structures, a bit hard to tell but the addresses are in the same range, so likely the same object. I confirmed the bond constructor only being called once when the bond driver is loaded because of static link: $ LD_LIBRARY_PATH=build/lib gdb build/app/dpdk-testpmd -ex "b vdrvinitfn_pmd_bond_drv" -ex "run -c 3 --no-huge -m 20 -d build/drivers '--log-level=*:debug' -a 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia" GNU gdb (GDB) Fedora 8.3.50.20190824-30.fc31 ... Reading symbols from build/app/dpdk-testpmd... Function "vdrvinitfn_pmd_bond_drv" not defined. Make breakpoint pending on future shared library load? (y or [n]) y Breakpoint 1 (vdrvinitfn_pmd_bond_drv) pending. Starting program: /home/dmarchan/dpdk/build/app/dpdk-testpmd -c 3 --no-huge -m 20 -d build/drivers '--log-level=*:debug' -a 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia [Thread debugging using libthread_db enabled] Using host libthread_db library "/usr/lib64/libthread_db.so.1". Breakpoint 1, vdrvinitfn_pmd_bond_drv () at ../drivers/net/bonding/rte_eth_bond_pmd.c:3755 3755 RTE_PMD_REGISTER_VDEV(net_bonding, pmd_bond_drv); Missing separate debuginfos, use: dnf debuginfo-install elfutils-libelf-0.179-2.fc31.x86_64 glibc-2.30-13.fc31.x86_64 jansson-2.12-4.fc31.x86_64 libbsd-0.9.1-4.fc31.x86_64 libfdt-1.6.0-1.fc31.x86_64 numactl-libs-2.0.12-3.fc31.x86_64 zlib-1.2.11-20.fc31.x86_64 (gdb) c Continuing. ... EAL: open shared lib build/drivers/librte_net_avp.so EAL: open shared lib build/drivers/librte_net_bond.so EAL: open shared lib build/drivers/librte_net_ixgbe.so ... testpmd> Now, about this patch. In this test, it has the effect of loading all drivers twice (or thrice if statically linked). I see no problem with it since the linker is intelligent enough and constructors are only called once. My only remaining doubt is whether we should be looking for .ABI_VERSION or .ABI_MAJOR extension. This could be discussed, but in its current form, this patch lgtm. Acked-by: David Marchand <david.marchand@redhat.com> -- David Marchand ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] eal: fix loading of shared libs from driver plugin directories 2020-11-25 10:01 ` David Marchand @ 2020-11-25 11:05 ` Ferruh Yigit 0 siblings, 0 replies; 14+ messages in thread From: Ferruh Yigit @ 2020-11-25 11:05 UTC (permalink / raw) To: David Marchand, Timothy Redaelli Cc: Anatoly Burakov, Bruce Richardson, dev, dpdk stable On 11/25/2020 10:01 AM, David Marchand wrote: > On Tue, Nov 24, 2020 at 4:15 PM Timothy Redaelli <tredaelli@redhat.com> wrote: >> >> Commit 49b536fc3060 ("eal: load only shared libs from driver plugin directories") >> introduced a check that any shared library must ends with .so, but it can't >> work, at least, on Fedora/CentOS/RHEL since .so symlinks are not installed >> when you install dpdk package, but only when you install dpdk-devel package. >> >> This commit adds also a check for .so.ABI_VERSION to check for shared >> lib. >> >> See Fedora Packaging Guidelines for more informations: >> https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages >> >> Fixes: 49b536fc3060 ("eal: load only shared libs from driver plugin directories") >> Cc: bruce.richardson@intel.com >> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> >> --- >> lib/librte_eal/common/eal_common_options.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c >> index e6f77ad217..e9e2362c53 100644 >> --- a/lib/librte_eal/common/eal_common_options.c >> +++ b/lib/librte_eal/common/eal_common_options.c >> @@ -402,8 +402,10 @@ eal_plugindir_init(const char *path) >> struct stat sb; >> int nlen = strnlen(dent->d_name, sizeof(dent->d_name)); >> >> - /* check if name ends in .so */ >> - if (strcmp(&dent->d_name[nlen - 3], ".so") != 0) >> + /* check if name ends in .so or .so.ABI_VERSION */ >> + if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 && >> + strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)], >> + ".so."ABI_VERSION) != 0) >> continue; >> >> snprintf(sopath, sizeof(sopath), "%s/%s", path, dent->d_name); > > A first doubt I had about this patch is about multiple loading of the > same driver, and its effects. > So I first had a look at the main branch current state (using > devtools/test-null.sh) and I can see multiple loading already happens > for statically linked drivers like the bond driver. > > $ LD_LIBRARY_PATH=build/lib gdb build/app/dpdk-testpmd -ex "b main" > -ex "run -c 3 --no-huge -m 20 -d build/drivers '--log-level=*:debug' > -a 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall > --total-num-mbufs=2048 -ia" > ... > (gdb) info sharedlibrary > ... > 0x00007ffff796d5b0 0x00007ffff797d2d6 Yes > /home/dmarchan/dpdk/build/app/../drivers/librte_net_bond.so.21 > ... > (gdb) b rte_eal_init > (gdb) c > (gdb) finish > ... > EAL: open shared lib build/drivers/librte_net_avp.so > EAL: open shared lib build/drivers/librte_net_bond.so > EAL: open shared lib build/drivers/librte_net_ixgbe.so > ... > (gdb) set $elm=*(struct shared_driver *)solib_list.tqh_first > (gdb) while $elm > p $elm > set $elm=*(struct shared_driver *)($elm.next.tqe_next) > end > ... > $67 = {next = {tqe_next = 0x5c3350, tqe_prev = 0x5c1310}, name = > "build/drivers/librte_net_avp.so", '\000' <repeats 4064 times>, > lib_handle = 0x616870} > $68 = {next = {tqe_next = 0x5c4370, tqe_prev = 0x5c2330}, name = > "build/drivers/librte_net_bond.so", '\000' <repeats 4063 times>, > lib_handle = 0x7ffff7995a80} > $69 = {next = {tqe_next = 0x5c5390, tqe_prev = 0x5c3350}, name = > "build/drivers/librte_net_ixgbe.so", '\000' <repeats 4062 times>, > lib_handle = 0x7ffff7942fc0} > ... > > I could not confirm the 0x7ffff7995a80 handle points at the first load > of the bond driver. > gdb does not seem to know of the linker internal structures, a bit > hard to tell but the addresses are in the same range, so likely the > same object. > > I confirmed the bond constructor only being called once when the bond > driver is loaded because of static link: > > $ LD_LIBRARY_PATH=build/lib gdb build/app/dpdk-testpmd -ex "b > vdrvinitfn_pmd_bond_drv" -ex "run -c 3 --no-huge -m 20 -d > build/drivers '--log-level=*:debug' -a 0:0.0 --vdev net_null1 --vdev > net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia" > GNU gdb (GDB) Fedora 8.3.50.20190824-30.fc31 > ... > Reading symbols from build/app/dpdk-testpmd... > Function "vdrvinitfn_pmd_bond_drv" not defined. > Make breakpoint pending on future shared library load? (y or [n]) y > Breakpoint 1 (vdrvinitfn_pmd_bond_drv) pending. > Starting program: /home/dmarchan/dpdk/build/app/dpdk-testpmd -c 3 > --no-huge -m 20 -d build/drivers '--log-level=*:debug' -a 0:0.0 --vdev > net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia > [Thread debugging using libthread_db enabled] > Using host libthread_db library "/usr/lib64/libthread_db.so.1". > > Breakpoint 1, vdrvinitfn_pmd_bond_drv () at > ../drivers/net/bonding/rte_eth_bond_pmd.c:3755 > 3755 RTE_PMD_REGISTER_VDEV(net_bonding, pmd_bond_drv); > Missing separate debuginfos, use: dnf debuginfo-install > elfutils-libelf-0.179-2.fc31.x86_64 glibc-2.30-13.fc31.x86_64 > jansson-2.12-4.fc31.x86_64 libbsd-0.9.1-4.fc31.x86_64 > libfdt-1.6.0-1.fc31.x86_64 numactl-libs-2.0.12-3.fc31.x86_64 > zlib-1.2.11-20.fc31.x86_64 > > (gdb) c > Continuing. > ... > EAL: open shared lib build/drivers/librte_net_avp.so > EAL: open shared lib build/drivers/librte_net_bond.so > EAL: open shared lib build/drivers/librte_net_ixgbe.so > ... > testpmd> > > > Now, about this patch. > In this test, it has the effect of loading all drivers twice (or > thrice if statically linked). > I see no problem with it since the linker is intelligent enough and > constructors are only called once. > > > My only remaining doubt is whether we should be looking for > .ABI_VERSION or .ABI_MAJOR extension. > This could be discussed, but in its current form, this patch lgtm. > > Acked-by: David Marchand <david.marchand@redhat.com> > > I also checked the same, yes same file processed twice, with actual name and symlink, but since eal using 'realpath()' before 'dlopen()' the path passed to the 'dlopen()' is exact same for both symlink and actual file. And 'dlopen()' has following in the man page: " If the same shared object is opened again with dlopen(), the same object handle is returned. The dynamic linker maintains reference counts for object handles, so a dynamically loaded shared object is not deallocated until dlclose() has been called on it as many times as dlopen() has succeeded on it. Constructors (see below) are called only when the object is actually loaded into memory (i.e., when the reference count increases to 1). " As I run/debug with or without the symlinks, it looks OK with patch, Tested-by: Ferruh Yigit <ferruh.yigit@intel.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL 2020-11-24 15:13 [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli ` (2 preceding siblings ...) 2020-11-24 15:14 ` [dpdk-stable] [PATCH 2/2] eal: fix loading of shared libs from driver plugin directories Timothy Redaelli @ 2020-11-25 10:57 ` Maxime Coquelin 2020-11-25 15:56 ` [dpdk-stable] " Ali Alnubani ` (2 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: Maxime Coquelin @ 2020-11-25 10:57 UTC (permalink / raw) To: Timothy Redaelli, Anatoly Burakov; +Cc: bruce.richardson, dev, stable On 11/24/20 4:14 PM, Timothy Redaelli wrote: > Currently eal only tries to find shared lib by looking for .so files, > but on Fedora, CentOS and RHEL the .so file is not installed by dpdk > package, but it's only installed by dpdk-devel package, since .so files > should not be necessary in order to run a program [1]. > > This series fix that by checking for .so.ABI_VERSION that should be > available on any linux distribution. > > [1] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages > > Timothy Redaelli (2): > eal: fix shared lib mode detection > eal: fix loading of shared libs from driver plugin directories > > lib/librte_eal/common/eal_common_options.c | 9 ++++++--- > lib/librte_eal/common/meson.build | 2 ++ > 2 files changed, 8 insertions(+), 3 deletions(-) > Looks good to me. For the series: Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com> Thanks, Maxime ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL 2020-11-24 15:13 [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli ` (3 preceding siblings ...) 2020-11-25 10:57 ` [dpdk-stable] [dpdk-dev] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Maxime Coquelin @ 2020-11-25 15:56 ` Ali Alnubani 2020-11-25 15:59 ` [dpdk-stable] [dpdk-dev] " Ali Alnubani 2020-11-25 22:54 ` [dpdk-stable] " Thomas Monjalon 6 siblings, 0 replies; 14+ messages in thread From: Ali Alnubani @ 2020-11-25 15:56 UTC (permalink / raw) To: Timothy Redaelli, Anatoly Burakov; +Cc: bruce.richardson, stable > -----Original Message----- > From: stable <stable-bounces@dpdk.org> On Behalf Of Timothy Redaelli > Sent: Tuesday, November 24, 2020 5:14 PM > To: Anatoly Burakov <anatoly.burakov@intel.com> > Cc: bruce.richardson@intel.com; stable@dpdk.org > Subject: [dpdk-stable] [PATCH 0/2] Fix shared lib detection on > Fedora/CentOS/RHEL > Tested-by: Ali Alnubani <alialnu@nvidia.com> Thanks, Ali ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL 2020-11-24 15:13 [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli ` (4 preceding siblings ...) 2020-11-25 15:56 ` [dpdk-stable] " Ali Alnubani @ 2020-11-25 15:59 ` Ali Alnubani 2020-11-25 22:54 ` [dpdk-stable] " Thomas Monjalon 6 siblings, 0 replies; 14+ messages in thread From: Ali Alnubani @ 2020-11-25 15:59 UTC (permalink / raw) To: Timothy Redaelli, Anatoly Burakov; +Cc: bruce.richardson, dev, stable > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Timothy Redaelli > Sent: Tuesday, November 24, 2020 5:14 PM > To: Anatoly Burakov <anatoly.burakov@intel.com> > Cc: bruce.richardson@intel.com; dev@dpdk.org; stable@dpdk.org > Subject: [dpdk-dev] [PATCH 0/2] Fix shared lib detection on > Fedora/CentOS/RHEL > Tested-by: Ali Alnubani <alialnu@nvidia.com> Thanks, Ali ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL 2020-11-24 15:13 [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli ` (5 preceding siblings ...) 2020-11-25 15:59 ` [dpdk-stable] [dpdk-dev] " Ali Alnubani @ 2020-11-25 22:54 ` Thomas Monjalon 6 siblings, 0 replies; 14+ messages in thread From: Thomas Monjalon @ 2020-11-25 22:54 UTC (permalink / raw) To: Timothy Redaelli Cc: Anatoly Burakov, stable, bruce.richardson, dev, david.marchand, maxime.coquelin, ferruh.yigit, Ali Alnubani, ktraynor 24/11/2020 16:13, Timothy Redaelli: > Currently eal only tries to find shared lib by looking for .so files, > but on Fedora, CentOS and RHEL the .so file is not installed by dpdk > package, but it's only installed by dpdk-devel package, since .so files > should not be necessary in order to run a program [1]. > > This series fix that by checking for .so.ABI_VERSION that should be > available on any linux distribution. > > [1] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages > > Timothy Redaelli (2): > eal: fix shared lib mode detection > eal: fix loading of shared libs from driver plugin directories This is very late to fix such core behaviour, but it has been well reviewed and tested: Acked-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: David Marchand <david.marchand@redhat.com> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com> Tested-by: Ferruh Yigit <ferruh.yigit@intel.com> Tested-by: Ali Alnubani <alialnu@nvidia.com> Applied, thanks ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-11-25 22:54 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-24 15:13 [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli 2020-11-24 15:13 ` [dpdk-stable] [PATCH 1/2] eal: fix shared lib mode detection Timothy Redaelli 2020-11-24 15:14 ` Timothy Redaelli 2020-11-24 15:22 ` Bruce Richardson 2020-11-25 9:10 ` David Marchand 2020-11-24 15:14 ` [dpdk-stable] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Timothy Redaelli 2020-11-24 15:14 ` [dpdk-stable] [PATCH 2/2] eal: fix loading of shared libs from driver plugin directories Timothy Redaelli 2020-11-24 15:22 ` Bruce Richardson 2020-11-25 10:01 ` David Marchand 2020-11-25 11:05 ` Ferruh Yigit 2020-11-25 10:57 ` [dpdk-stable] [dpdk-dev] [PATCH 0/2] Fix shared lib detection on Fedora/CentOS/RHEL Maxime Coquelin 2020-11-25 15:56 ` [dpdk-stable] " Ali Alnubani 2020-11-25 15:59 ` [dpdk-stable] [dpdk-dev] " Ali Alnubani 2020-11-25 22:54 ` [dpdk-stable] " Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).