[PATCH] eventdev: fix symbol export for port maintenance

[PATCH] eventdev: fix symbol export for port maintenance

[David Marchand]

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;
-- 
2.41.0

Re: [PATCH] eventdev: fix symbol export for port maintenance

[Mattias Rönnblom]

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.

Re: [PATCH] eventdev: fix symbol export for port maintenance

[David Marchand]

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

Re: [PATCH] eventdev: fix symbol export for port maintenance

[David Marchand]

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

Re: [PATCH] eventdev: fix symbol export for port maintenance

[Jerin Jacob]

> > >
> > > 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
>

Re: [PATCH] eventdev: fix symbol export for port maintenance

[Jerin Jacob]

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
>

Re: [PATCH] eventdev: fix symbol export for port maintenance

[David Marchand]

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

Re: [PATCH] eventdev: fix symbol export for port maintenance

[David Marchand]

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.


-- 
David Marchand