|SUCCESS| [RFC] eal: add destructor to unregister tailq on unload

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163238

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-22

Reviewing commit from series.patch:

git sha: 20260422205449.289861-1-stephen at networkplumber.org
Author: Stephen Hemminger <stephen at networkplumber.org>
Subject: [RFC] eal: add destructor to unregister tailq on unload

The patch adds cleanup for EAL_REGISTER_TAILQ to prevent dangling
pointers when libraries are unloaded via dlclose(). It introduces
rte_eal_tailq_unregister() and modifies the EAL_REGISTER_TAILQ macro to
emit an RTE_FINI destructor.

> diff --git a/lib/eal/common/eal_common_tailqs.c b/lib/eal/common/eal_common_tailqs.c
> index c581f43b6f..714f91d0ec 100644
> --- a/lib/eal/common/eal_common_tailqs.c
> +++ b/lib/eal/common/eal_common_tailqs.c
> @@ -148,6 +148,14 @@ rte_eal_tailq_register(struct rte_tailq_elem *t)
>  	return -1;
>  }
>  
> +RTE_EXPORT_SYMBOL(rte_eal_tailq_unregister)
> +void
> +rte_eal_tailq_unregister(struct rte_tailq_elem *t)
> +{
> +	TAILQ_REMOVE(&rte_tailq_elem_head, t, next);
> +	t->head = NULL;
> +}

Does this code need locking? The rte_eal_tailq_register() function
appears to use EAL_TAILQ_RWLOCK for consistency when accessing
rte_tailq_elem_head. Should rte_eal_tailq_unregister() acquire the same
lock around TAILQ_REMOVE()?

Can this code race during dlclose() if another thread is concurrently
traversing rte_tailq_elem_head via rte_eal_tailq_lookup() or similar?

> diff --git a/lib/eal/include/rte_tailq.h b/lib/eal/include/rte_tailq.h
> index e7caed6812..c5d5cb782f 100644
> --- a/lib/eal/include/rte_tailq.h
> +++ b/lib/eal/include/rte_tailq.h
> @@ -117,11 +117,28 @@ struct rte_tailq_head *rte_eal_tailq_lookup(const char *name);
>   */
>  int rte_eal_tailq_register(struct rte_tailq_elem *t);
>  
> +/**
> + * Remove a tail queue element from the local list.
> + * This function is mainly used for EAL_REGISTER_TAILQ macro which pairs
> + * an RTE_FINI destructor with the existing RTE_INIT constructor.
> + * The destructor calls this function during dlclose() to prevent
> + * dangling pointers to unmapped library data.
> + *
> + * @param t
> + *   The tailq element which contains the name of the tailq you want to
> + *   delete
> + */
> +void rte_eal_tailq_unregister(struct rte_tailq_elem *t);
> +
>  #define EAL_REGISTER_TAILQ(t) \
>  RTE_INIT(tailqinitfn_ ##t) \
>  { \
>  	if (rte_eal_tailq_register(&t) < 0) \
>  		rte_panic("Cannot initialize tailq: %s\n", t.name); \
> +} \
> +RTE_FINI(tailqfinifn_ ##t) \
> +{ \
> +	rte_eal_tailq_unregister(&t); \
>  }

Does this code handle cleanup of the tailq contents? The comment
mentions preventing dangling pointers to unmapped library data, but does
the destructor only remove the rte_tailq_elem from rte_tailq_elem_head,
or does it also handle the case where the actual tailq (pointed to by
t->head) may contain entries that should be freed?

Should the API documentation warn about ordering guarantees during
unload? If library A depends on library B and both register tailqs, can
the unload order cause B's destructor to run while A still references
data in B's tailq?

Does rte_eal_tailq_unregister() need an @experimental tag since this is
new API in an RFC patch, or does the commit indicate it should be stable
immediately?