From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])
 by dpdk.org (Postfix) with ESMTP id 33ECA1D7
 for <dev@dpdk.org>; Wed, 25 Apr 2018 12:26:22 +0200 (CEST)
Received: from lfbn-lil-1-700-92.w81-254.abo.wanadoo.fr ([81.254.37.92]
 helo=droids-corp.org)
 by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.89) (envelope-from <olivier.matz@6wind.com>)
 id 1fBHd4-0007o3-SL; Wed, 25 Apr 2018 12:26:28 +0200
Received: by droids-corp.org (sSMTP sendmail emulation);
 Wed, 25 Apr 2018 12:26:18 +0200
Date: Wed, 25 Apr 2018 12:26:18 +0200
From: Olivier Matz <olivier.matz@6wind.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: dev@dpdk.org, "Artem V. Andreev" <Artem.Andreev@oktetlabs.ru>
Message-ID: <20180425102618.a7omcbokz5uyefkq@neon>
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>
 <20180419164240.dgqwzsc6rubngo3s@platinum>
 <a5124802-42dd-5fcf-1c37-37ac218a9e74@solarflare.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a5124802-42dd-5fcf-1c37-37ac218a9e74@solarflare.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Subject: Re: [dpdk-dev] [PATCH v1 2/6] mempool: implement abstract mempool
	info API
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
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>
X-List-Received-Date: Wed, 25 Apr 2018 10:26:22 -0000

Hi Andrew,

> > > + * @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...
> 
> I'd prefer to keep current solution. Otherwise it could result in too many
> different functions to get various information about mempool driver
> features/characteristics. Also it could be not very convenient to get
> many parameters.
> 
> May be we should align info structure size to cache line to avoid size
> changes in many cases? Typically it will be used on slow path and
> located on caller stack and adding some bytes more should not
> be a problem.

Yes, that could be a good thing to do.

Thanks,
Olivier