diff mbox series

eventdev: fix symbol export for port maintenance

Message ID 20231010140029.66159-1-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series eventdev: fix symbol export for port maintenance |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/intel-Testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS

Commit Message

David Marchand Oct. 10, 2023, 2 p.m. UTC 
  Trying to call rte_event_maintain out of the eventdev library triggers
a link failure, as the tracepoint symbol associated to this inline
helper was not exported.

Fixes: 54f17843a887 ("eventdev: add port maintenance API")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Caught by the CI when testing the dispatcher library.
See for example:
https://github.com/ovsrobot/dpdk/actions/runs/6460514355/job/17538348529#step:19:5506

---
 lib/eventdev/version.map | 1 +
 1 file changed, 1 insertion(+)

Comments

Mattias Rönnblom Oct. 11, 2023, 6:46 a.m. UTC | #1 
On 2023-10-10 16:00, David Marchand wrote:
> Trying to call rte_event_maintain out of the eventdev library triggers
> a link failure, as the tracepoint symbol associated to this inline
> helper was not exported.
> 
> Fixes: 54f17843a887 ("eventdev: add port maintenance API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Caught by the CI when testing the dispatcher library.
> See for example:
> https://github.com/ovsrobot/dpdk/actions/runs/6460514355/job/17538348529#step:19:5506
> 
> ---
>   lib/eventdev/version.map | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/lib/eventdev/version.map b/lib/eventdev/version.map
> index b03c10d99f..249eb115b1 100644
> --- a/lib/eventdev/version.map
> +++ b/lib/eventdev/version.map
> @@ -5,6 +5,7 @@ DPDK_24 {
>   	__rte_eventdev_trace_deq_burst;
>   	__rte_eventdev_trace_enq_burst;
>   	__rte_eventdev_trace_eth_tx_adapter_enqueue;
> +	__rte_eventdev_trace_maintain;
>   	__rte_eventdev_trace_timer_arm_burst;
>   	__rte_eventdev_trace_timer_arm_tmo_tick_burst;
>   	__rte_eventdev_trace_timer_cancel_burst;

I can't say I know why it's needed, but the change seems consistent with 
other Eventdev trace points.

Maybe Jerin can comment on this? If not, I can dig into the details.
Jerin Jacob Oct. 11, 2023, 6:51 a.m. UTC | #2 
On Tue, Oct 10, 2023 at 7:30 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> Trying to call rte_event_maintain out of the eventdev library triggers
> a link failure, as the tracepoint symbol associated to this inline
> helper was not exported.
>
> Fixes: 54f17843a887 ("eventdev: add port maintenance API")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>

David, If it is stopping dispatcher library integration, please take
this patch through main tree, if not, I will add through event tree
for rc2.


> ---
> Caught by the CI when testing the dispatcher library.
> See for example:
> https://github.com/ovsrobot/dpdk/actions/runs/6460514355/job/17538348529#step:19:5506
>
> ---
>  lib/eventdev/version.map | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lib/eventdev/version.map b/lib/eventdev/version.map
> index b03c10d99f..249eb115b1 100644
> --- a/lib/eventdev/version.map
> +++ b/lib/eventdev/version.map
> @@ -5,6 +5,7 @@ DPDK_24 {
>         __rte_eventdev_trace_deq_burst;
>         __rte_eventdev_trace_enq_burst;
>         __rte_eventdev_trace_eth_tx_adapter_enqueue;
> +       __rte_eventdev_trace_maintain;
>         __rte_eventdev_trace_timer_arm_burst;
>         __rte_eventdev_trace_timer_arm_tmo_tick_burst;
>         __rte_eventdev_trace_timer_cancel_burst;
> --
> 2.41.0
>
David Marchand Oct. 11, 2023, 7:03 a.m. UTC | #3 
On Wed, Oct 11, 2023 at 8:47 AM Mattias Rönnblom <hofors@lysator.liu.se> wrote:
>
> On 2023-10-10 16:00, David Marchand wrote:
> > Trying to call rte_event_maintain out of the eventdev library triggers
> > a link failure, as the tracepoint symbol associated to this inline
> > helper was not exported.
> >
> > Fixes: 54f17843a887 ("eventdev: add port maintenance API")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> > Caught by the CI when testing the dispatcher library.
> > See for example:
> > https://github.com/ovsrobot/dpdk/actions/runs/6460514355/job/17538348529#step:19:5506
> >
> > ---
> >   lib/eventdev/version.map | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/eventdev/version.map b/lib/eventdev/version.map
> > index b03c10d99f..249eb115b1 100644
> > --- a/lib/eventdev/version.map
> > +++ b/lib/eventdev/version.map
> > @@ -5,6 +5,7 @@ DPDK_24 {
> >       __rte_eventdev_trace_deq_burst;
> >       __rte_eventdev_trace_enq_burst;
> >       __rte_eventdev_trace_eth_tx_adapter_enqueue;
> > +     __rte_eventdev_trace_maintain;
> >       __rte_eventdev_trace_timer_arm_burst;
> >       __rte_eventdev_trace_timer_arm_tmo_tick_burst;
> >       __rte_eventdev_trace_timer_cancel_burst;
>
> I can't say I know why it's needed, but the change seems consistent with
> other Eventdev trace points.

The trace point framework in DPDK relies on a per trace point global
variable (whose name is __<trace point name>):

#define __RTE_TRACE_POINT(_mode, _tp, _args, ...) \
extern rte_trace_point_t __##_tp; \
static __rte_always_inline void \
_tp _args \
{ \
        __rte_trace_point_emit_header_##_mode(&__##_tp); \
        __VA_ARGS__ \
}

When tracepoints are called from within a shared library code, and
because all symbols of a group of objects are visible, the tracepoint
symbols are resolved by the linker.
But when this tracepoint is called via an inline helper from some code
out of the shared library, this symbol must be exported in the shared
library map or it won't be visible to "external" users.
David Marchand Oct. 11, 2023, 7:03 a.m. UTC | #4 
Hello Jerin,

On Wed, Oct 11, 2023 at 8:51 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Tue, Oct 10, 2023 at 7:30 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > Trying to call rte_event_maintain out of the eventdev library triggers
> > a link failure, as the tracepoint symbol associated to this inline
> > helper was not exported.
> >
> > Fixes: 54f17843a887 ("eventdev: add port maintenance API")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
>
> Acked-by: Jerin Jacob <jerinj@marvell.com>
>
> David, If it is stopping dispatcher library integration, please take
> this patch through main tree, if not, I will add through event tree
> for rc2.

I was going to suggest merging directly in main :-).
I will delegate it to me in patchwork.


Thanks.
David Marchand Oct. 11, 2023, 7:21 a.m. UTC | #5 
Jerin,

On Wed, Oct 11, 2023 at 9:03 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Wed, Oct 11, 2023 at 8:47 AM Mattias Rönnblom <hofors@lysator.liu.se> wrote:
> >
> > On 2023-10-10 16:00, David Marchand wrote:
> > > Trying to call rte_event_maintain out of the eventdev library triggers
> > > a link failure, as the tracepoint symbol associated to this inline
> > > helper was not exported.
> > >
> > > Fixes: 54f17843a887 ("eventdev: add port maintenance API")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > > Caught by the CI when testing the dispatcher library.
> > > See for example:
> > > https://github.com/ovsrobot/dpdk/actions/runs/6460514355/job/17538348529#step:19:5506
> > >
> > > ---
> > >   lib/eventdev/version.map | 1 +
> > >   1 file changed, 1 insertion(+)
> > >
> > > diff --git a/lib/eventdev/version.map b/lib/eventdev/version.map
> > > index b03c10d99f..249eb115b1 100644
> > > --- a/lib/eventdev/version.map
> > > +++ b/lib/eventdev/version.map
> > > @@ -5,6 +5,7 @@ DPDK_24 {
> > >       __rte_eventdev_trace_deq_burst;
> > >       __rte_eventdev_trace_enq_burst;
> > >       __rte_eventdev_trace_eth_tx_adapter_enqueue;
> > > +     __rte_eventdev_trace_maintain;
> > >       __rte_eventdev_trace_timer_arm_burst;
> > >       __rte_eventdev_trace_timer_arm_tmo_tick_burst;
> > >       __rte_eventdev_trace_timer_cancel_burst;
> >
> > I can't say I know why it's needed, but the change seems consistent with
> > other Eventdev trace points.
>
> The trace point framework in DPDK relies on a per trace point global
> variable (whose name is __<trace point name>):
>
> #define __RTE_TRACE_POINT(_mode, _tp, _args, ...) \
> extern rte_trace_point_t __##_tp; \
> static __rte_always_inline void \
> _tp _args \
> { \
>         __rte_trace_point_emit_header_##_mode(&__##_tp); \
>         __VA_ARGS__ \
> }
>
> When tracepoints are called from within a shared library code, and
> because all symbols of a group of objects are visible, the tracepoint
> symbols are resolved by the linker.
> But when this tracepoint is called via an inline helper from some code
> out of the shared library, this symbol must be exported in the shared
> library map or it won't be visible to "external" users.

Could we describe / mention this in the trace point library doc?
Or maybe I read too quickly and there is already something but it was
not obvious to me.
David Marchand Oct. 11, 2023, 12:11 p.m. UTC | #6 
On Wed, Oct 11, 2023 at 8:51 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> On Tue, Oct 10, 2023 at 7:30 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > Trying to call rte_event_maintain out of the eventdev library triggers
> > a link failure, as the tracepoint symbol associated to this inline
> > helper was not exported.
> >
> > Fixes: 54f17843a887 ("eventdev: add port maintenance API")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>

Applied thanks.
Jerin Jacob Oct. 12, 2023, 12:32 p.m. UTC | #7 
> > >
> > > I can't say I know why it's needed, but the change seems consistent with
> > > other Eventdev trace points.
> >
> > The trace point framework in DPDK relies on a per trace point global
> > variable (whose name is __<trace point name>):
> >
> > #define __RTE_TRACE_POINT(_mode, _tp, _args, ...) \
> > extern rte_trace_point_t __##_tp; \
> > static __rte_always_inline void \
> > _tp _args \
> > { \
> >         __rte_trace_point_emit_header_##_mode(&__##_tp); \
> >         __VA_ARGS__ \
> > }
> >
> > When tracepoints are called from within a shared library code, and
> > because all symbols of a group of objects are visible, the tracepoint
> > symbols are resolved by the linker.
> > But when this tracepoint is called via an inline helper from some code
> > out of the shared library, this symbol must be exported in the shared
> > library map or it won't be visible to "external" users.
>
> Could we describe / mention this in the trace point library doc?
> Or maybe I read too quickly and there is already something but it was
> not obvious to me.

Following text is available in
https://doc.dpdk.org/guides/prog_guide/trace_lib.html as NOTE.
We may need to update to very specific on FP trace points.


The RTE_TRACE_POINT_REGISTER defines the placeholder for the
rte_trace_point_t tracepoint object. For generic tracepoint or for
tracepoint used in public header files, the user must export a
__<trace_function_name> symbol in the library .map file for this
tracepoint to be used out of the library, in shared builds. For
example, __app_trace_string will be the exported symbol in the above
example.

>
>
> --
> David Marchand
>
diff mbox series

Patch

diff --git a/lib/eventdev/version.map b/lib/eventdev/version.map
index b03c10d99f..249eb115b1 100644
--- a/lib/eventdev/version.map
+++ b/lib/eventdev/version.map
@@ -5,6 +5,7 @@  DPDK_24 {
 	__rte_eventdev_trace_deq_burst;
 	__rte_eventdev_trace_enq_burst;
 	__rte_eventdev_trace_eth_tx_adapter_enqueue;
+	__rte_eventdev_trace_maintain;
 	__rte_eventdev_trace_timer_arm_burst;
 	__rte_eventdev_trace_timer_arm_tmo_tick_burst;
 	__rte_eventdev_trace_timer_cancel_burst;