| Message ID | 20210407090656.29176-1-david.marchand@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | David Marchand |
| Headers | show |
| Series | service: clean references to removed symbol | expand |
| Context | Check | Description |
|---|---|---|
| ci/iol-testing | success | Testing PASS |
| ci/iol-abi-testing | success | Testing PASS |
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/intel-Testing | success | Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/github-robot | success | github build: passed |
| ci/travis-robot | success | travis build: passed |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | success | coding style OK |
> -----Original Message----- > From: David Marchand <david.marchand@redhat.com> > Sent: Wednesday, April 7, 2021 10:07 AM > To: dev@dpdk.org > Cc: stable@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>; Ray > Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Pavan > Nikhilesh <pbhagavatula@caviumnetworks.com> > Subject: [PATCH] service: clean references to removed symbol > > rte_service_get_id() was removed in v17.11 but the API description > still referenced it and a version node was still present in EAL map. > > Fixes: 8edc9aaaf217 ("service: use id in get by name function") > Cc: stable@dpdk.org > > Signed-off-by: David Marchand <david.marchand@redhat.com> For rte_service.h doc changes I'm all fine. For ABI consistency, this removes a function from the DPDK_21 version map that didn't exist in the shared object itself. I'm not sure if that's an ABI break or not... I see ABI experts on CC, and will let them comment on that topic. Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com> > --- > lib/librte_eal/include/rte_service.h | 5 +---- > lib/librte_eal/version.map | 1 - > 2 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/lib/librte_eal/include/rte_service.h > b/lib/librte_eal/include/rte_service.h > index ca9950d091..c7d037d862 100644 > --- a/lib/librte_eal/include/rte_service.h > +++ b/lib/librte_eal/include/rte_service.h > @@ -47,10 +47,7 @@ extern "C" { > #define RTE_SERVICE_CAP_MT_SAFE (1 << 0) > > /** > - * Return the number of services registered. > - * > - * The number of services registered can be passed to *rte_service_get_by_id*, > - * enabling the application to retrieve the specification of each service. > + * Return the number of services registered. > * > * @return The number of services registered. > */ > diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map > index e23745ae6e..0f2a9228b3 100644 > --- a/lib/librte_eal/version.map > +++ b/lib/librte_eal/version.map > @@ -159,7 +159,6 @@ DPDK_21 { > rte_service_component_unregister; > rte_service_dump; > rte_service_finalize; > - rte_service_get_by_id; > rte_service_get_by_name; > rte_service_get_count; > rte_service_get_name; > -- > 2.23.0
On Thu, Apr 8, 2021 at 2:54 PM Van Haaren, Harry <harry.van.haaren@intel.com> wrote: > > > -----Original Message----- > > From: David Marchand <david.marchand@redhat.com> > > Sent: Wednesday, April 7, 2021 10:07 AM > > To: dev@dpdk.org > > Cc: stable@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>; Ray > > Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Pavan > > Nikhilesh <pbhagavatula@caviumnetworks.com> > > Subject: [PATCH] service: clean references to removed symbol > > > > rte_service_get_id() was removed in v17.11 but the API description > > still referenced it and a version node was still present in EAL map. > > > > Fixes: 8edc9aaaf217 ("service: use id in get by name function") > > Cc: stable@dpdk.org > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > For rte_service.h doc changes I'm all fine. > > For ABI consistency, this removes a function from the DPDK_21 version map that didn't exist in > the shared object itself. I'm not sure if that's an ABI break or not... I see ABI experts on CC, > and will let them comment on that topic. The symbol is not in the API anymore. Applications can't have a reference to this symbol without a definition.
> -----Original Message----- > From: David Marchand <david.marchand@redhat.com> > Sent: Thursday, April 8, 2021 1:58 PM > To: Van Haaren, Harry <harry.van.haaren@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman > <nhorman@tuxdriver.com>; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com> > Subject: Re: [PATCH] service: clean references to removed symbol > > On Thu, Apr 8, 2021 at 2:54 PM Van Haaren, Harry > <harry.van.haaren@intel.com> wrote: > > > > > -----Original Message----- > > > From: David Marchand <david.marchand@redhat.com> > > > Sent: Wednesday, April 7, 2021 10:07 AM > > > To: dev@dpdk.org > > > Cc: stable@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>; Ray > > > Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Pavan > > > Nikhilesh <pbhagavatula@caviumnetworks.com> > > > Subject: [PATCH] service: clean references to removed symbol > > > > > > rte_service_get_id() was removed in v17.11 but the API description > > > still referenced it and a version node was still present in EAL map. > > > > > > Fixes: 8edc9aaaf217 ("service: use id in get by name function") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > > > For rte_service.h doc changes I'm all fine. > > > > For ABI consistency, this removes a function from the DPDK_21 version map that > didn't exist in > > the shared object itself. I'm not sure if that's an ABI break or not... I see ABI > experts on CC, > > and will let them comment on that topic. > > The symbol is not in the API anymore. > Applications can't have a reference to this symbol without a definition. Agreed, however I'm not familiar enough with linking/ABI stability to know if removing a symbol (even when not available in the API) could affect the resulting shared object's ABI. If somebody says there's no risk in ABI break, go ahead and merge. -Harry
On Thu, Apr 8, 2021 at 3:08 PM Van Haaren, Harry <harry.van.haaren@intel.com> wrote: > > > For rte_service.h doc changes I'm all fine. > > > > > > For ABI consistency, this removes a function from the DPDK_21 version map that > > didn't exist in > > > the shared object itself. I'm not sure if that's an ABI break or not... I see ABI > > experts on CC, > > > and will let them comment on that topic. > > > > The symbol is not in the API anymore. > > Applications can't have a reference to this symbol without a definition. > > Agreed, however I'm not familiar enough with linking/ABI stability to > know if removing a symbol (even when not available in the API) could > affect the resulting shared object's ABI. After a discussion with Dodji and looking at binutils, I understand that the versioning is applied for each symbol found in an elf object. So at least with binutils, non-existent nodes in a version script are harmless and have no impact on the generated elf.
08/04/2021 16:04, David Marchand: > On Thu, Apr 8, 2021 at 3:08 PM Van Haaren, Harry > <harry.van.haaren@intel.com> wrote: > > > > For rte_service.h doc changes I'm all fine. > > > > > > > > For ABI consistency, this removes a function from the DPDK_21 version map that > > > didn't exist in > > > > the shared object itself. I'm not sure if that's an ABI break or not... I see ABI > > > experts on CC, > > > > and will let them comment on that topic. > > > > > > The symbol is not in the API anymore. > > > Applications can't have a reference to this symbol without a definition. > > > > Agreed, however I'm not familiar enough with linking/ABI stability to > > know if removing a symbol (even when not available in the API) could > > affect the resulting shared object's ABI. > > After a discussion with Dodji and looking at binutils, I understand > that the versioning is applied for each symbol found in an elf object. > So at least with binutils, non-existent nodes in a version script are > harmless and have no impact on the generated elf. Yes I don't see how it could break ABI. Acked-by: Thomas Monjalon <thomas@monjalon.net>
On Wed, Apr 7, 2021 at 11:08 AM David Marchand <david.marchand@redhat.com> wrote: > > rte_service_get_id() was removed in v17.11 but the API description > still referenced it and a version node was still present in EAL map. > > Fixes: 8edc9aaaf217 ("service: use id in get by name function") > Cc: stable@dpdk.org > > Signed-off-by: David Marchand <david.marchand@redhat.com> Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com> Acked-by: Thomas Monjalon <thomas@monjalon.net> Applied, thanks.
diff --git a/lib/librte_eal/include/rte_service.h b/lib/librte_eal/include/rte_service.h index ca9950d091..c7d037d862 100644 --- a/lib/librte_eal/include/rte_service.h +++ b/lib/librte_eal/include/rte_service.h @@ -47,10 +47,7 @@ extern "C" { #define RTE_SERVICE_CAP_MT_SAFE (1 << 0) /** - * Return the number of services registered. - * - * The number of services registered can be passed to *rte_service_get_by_id*, - * enabling the application to retrieve the specification of each service. + * Return the number of services registered. * * @return The number of services registered. */ diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map index e23745ae6e..0f2a9228b3 100644 --- a/lib/librte_eal/version.map +++ b/lib/librte_eal/version.map @@ -159,7 +159,6 @@ DPDK_21 { rte_service_component_unregister; rte_service_dump; rte_service_finalize; - rte_service_get_by_id; rte_service_get_by_name; rte_service_get_count; rte_service_get_name;
rte_service_get_id() was removed in v17.11 but the API description still referenced it and a version node was still present in EAL map. Fixes: 8edc9aaaf217 ("service: use id in get by name function") Cc: stable@dpdk.org Signed-off-by: David Marchand <david.marchand@redhat.com> --- lib/librte_eal/include/rte_service.h | 5 +---- lib/librte_eal/version.map | 1 - 2 files changed, 1 insertion(+), 5 deletions(-)