From mboxrd@z Thu Jan  1 00:00:00 1970
From: Olivier Matz <olivier.matz@6wind.com>
Subject: Re: [PATCH v1 2/6] mempool: implement abstract mempool
	info API
Date: Thu, 19 Apr 2018 18:42:40 +0200
Message-ID: <20180419164240.dgqwzsc6rubngo3s@platinum>
References: <1516713372-10572-1-git-send-email-arybchenko@solarflare.com>
 <1522080779-25472-1-git-send-email-arybchenko@solarflare.com>
 <1522080779-25472-3-git-send-email-arybchenko@solarflare.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, "Artem V. Andreev" <Artem.Andreev@oktetlabs.ru>
To: Andrew Rybchenko <arybchenko@solarflare.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])
 by dpdk.org (Postfix) with ESMTP id BA843D00F
 for <dev@dpdk.org>; Thu, 19 Apr 2018 18:42:42 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <1522080779-25472-3-git-send-email-arybchenko@solarflare.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Mar 26, 2018 at 05:12:55PM +0100, Andrew Rybchenko wrote:
> From: "Artem V. Andreev" <Artem.Andreev@oktetlabs.ru>
> 
> Primarily, it is intended as a way for the mempool driver to provide
> additional information on how it lays up objects inside the mempool.
> 
> Signed-off-by: Artem V. Andreev <Artem.Andreev@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

I think it's a good idea to have a way to query mempool features
or parameters. The approach chosen in this patch looks similar
to what we have with ethdev devinfo, right?

[...]

>  /**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Additional information about the mempool
> + */
> +struct rte_mempool_info;
> +

[...]

> +/* wrapper to get additional mempool info */
> +int
> +rte_mempool_ops_get_info(const struct rte_mempool *mp,
> +			 struct rte_mempool_info *info)
> +{
> +	struct rte_mempool_ops *ops;
> +
> +	ops = rte_mempool_get_ops(mp->ops_index);
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(ops->get_info, -ENOTSUP);
> +	return ops->get_info(mp, info);
> +}

Thinking in terms of ABI compatibility, it looks that each time we will
add or remove a field, it will break the ABI because the info structure
will change.

Well, it's maybe nitpicking, because most of the time adding a field in
info structure goes with adding a field in the mempool struct, which
will anyway break the ABI.

Another approach is to have a function
rte_mempool_info_contig_block_size(mp) whose ABI can be more easily
wrapped with VERSION_SYMBOL().

On my side I'm fine with your current approach, especially given how few
usages of VERSION_SYMBOL() we can find in DPDK. But in case you feel
it's better to have a function...