Re: [dpdk-dev] [dpdk-dev, PATCHv5, 1/8] ethdev: add new API to retrieve RX/TX queue information

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Hi Amine,

> -----Original Message-----
> From: Amine Kherbouche [mailto:amine.kherbouche@6wind.com]
> Sent: Wednesday, October 14, 2015 12:40 PM
> To: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [dpdk-dev, PATCHv5, 1/8] ethdev: add new API to retrieve RX/TX queue information
> 
> 
> 
> Hi Konstantin
> > +/**
> > + * Ethernet device RX queue information structure.
> > + * Used to retieve information about configured queue.
> > + */
> > +struct rte_eth_rxq_info {
> > +	struct rte_mempool *mp;     /**< mempool used by that queue. */
> > +	struct rte_eth_rxconf conf; /**< queue config parameters. */
> > +	uint8_t scattered_rx;       /**< scattered packets RX supported. */
> > +	uint16_t nb_desc;           /**< configured number of RXDs. */
> Here i need two more fields in this struct :
>      uint16_t free_desc : for free queue descriptors
>      uint16_t used_desc : for used queue descriptors
> > +} __rte_cache_aligned;

Yep, that’s good thing to have.
But to fill them properly extra work is required, so my thought was to left it for future expansion.

> > +
> > +/**
> > + * Ethernet device TX queue information structure.
> > + * Used to retieve information about configured queue.
> > + */
> > +struct rte_eth_txq_info {
> > +	struct rte_eth_txconf conf; /**< queue config parameters. */
> > +	uint16_t nb_desc;           /**< configured number of TXDs. */
> And also here.
> > +} __rte_cache_aligned;
> > +
> >   struct rte_eth_dev;
> How to add them without breaking API ?

As I said in the patch comments:
"left extra free space in the queue info structures,
so extra fields could be added later without ABI breakage."
As you can see both queue info structures have size 64B each.
So there is plenty of space for expansion without ABI breakage.  

 I would prefer to see them now, so
> I'll send an update on your patch series that i'll use this 2 more
> fields. The purpose will be to provide analysis of the usage of the RX
> and TX queues.

Well, in general I am not opposed to add these fields to the structures.
In fact, I think it is a good thing to have.
But if we'll add them without making queue_info_get() feeling them properly -
it might create a lot of controversy.
So unless you prepared to make changes in all queue_info_get() too,
my opinion - let's leave it as it is for 2.2, and add new fields in later releases.
As I said above - ABI wouldn't be affected.

Konstantin