Re: [dpdk-dev] [PATCH v3 1/4] mbuf: detach mbuf with pinned external buffer

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 14 Jan 2020 09:15:02 +0000
Viacheslav Ovsiienko <viacheslavo@mellanox.com> wrote:

> +/**
> + * Returns TRUE if given mbuf has an pinned external buffer, or FALSE
> + * otherwise. The pinned external buffer is allocated at pool creation
> + * time and should not be freed.
> + *
> + * External buffer is a user-provided anonymous buffer.
> + */
> +#ifdef ALLOW_EXPERIMENTAL_API
> +#define RTE_MBUF_HAS_PINNED_EXTBUF(mb) rte_mbuf_has_pinned_extbuf(mb)
> +#else
> +#define RTE_MBUF_HAS_PINNED_EXTBUF(mb) false
> +#endif

This is worse than just letting new code in.
If you have to use conditional compilation, then please base it off
an config value.

And make the resulting function an inline, this avoid introducing yet
another macro. MACROS ARE HARDER TO READ.

#ifdef RTE_CONFIG_MBUF_PINNED
static inline bool
rte_mbuf_has_pinned_extbuf(const struct rte_mbuf *m)
{
...
}
#else
static inline bool
rte_mbuf_has_pinned_extbuf(const struct rte_mbuf *m)
{
	return false;
}
#endif

> +__rte_experimental
> +static inline uint32_t
> +rte_mbuf_has_pinned_extbuf(const struct rte_mbuf *m)
> +{
> +	if (RTE_MBUF_HAS_EXTBUF(m)) {
> +		/*
> +		 * The mbuf has the external attached buffer,
> +		 * we should check the type of the memory pool where
> +		 * the mbuf was allocated from.
> +		 */
> +		struct rte_pktmbuf_pool_private *priv =
> +			(struct rte_pktmbuf_pool_private *)
> +				rte_mempool_get_priv(m->pool);

Since rte_mempool_get_priv() returns void *, the cast is unnecessary
in standard C. Maybe you still need it for people using rte_mbuf.h in C++