On Mon, Nov 28, 2016 at 11:13 AM, Thomas Monjalon <thomas.monjalon at 6wind.com > wrote:
> 2016-11-24 17:59, Olivier Matz: > > Hi, > > > > On Mon, 2016-11-21 at 09:59 +0000, Alejandro Lucero wrote: > > > From: Bert van Leeuwen <bert.vanleeuwen at netronome.com> > > > > > > Arrays inside rte_eth_stats have size=RTE_ETHDEV_QUEUE_STAT_CNTRS. > > > Some devices report more queues than that and this code blindly uses > > > the reported number of queues by the device to fill those arrays up. > > > This patch fixes the problem using MIN between the reported number of > > > queues and RTE_ETHDEV_QUEUE_STAT_CNTRS. > > > > > > Signed-off-by: Alejandro Lucero <alejandro.lucero at netronome.com> > > > > > > > Reviewed-by: Olivier Matz <olivier.matz at 6wind.com> > > > > > > As a next step, I'm wondering if it would be possible to remove > > this limitation. We could replace the tables in struct rte_eth_stats > > by a pointer to an array allocated dynamically at pmd setup. > > Yes that's definitely the right way to handle these statistics. > > Agree. > > It would break the API, so it should be announced first. I'm thinking > > of something like: > > > > struct rte_eth_generic_stats { > > uint64_t ipackets; > > uint64_t opackets; > > uint64_t ibytes; > > uint64_t obytes; > > uint64_t imissed; > > uint64_t ierrors; > > uint64_t oerrors; > > uint64_t rx_nombuf > > }; > > > > struct rte_eth_stats { > > struct rte_eth_generic_stats port_stats; > > struct rte_eth_generic_stats *queue_stats; > > }; > > > > The queue_stats array would always be indexed by queue_id. > > The xstats would continue to report the generic stats per-port and > > per-queue. > > > > About the mapping API, either we keep it as-is, or it could > > become a driver-specific API. > > Yes I agree to remove the queue statistics mapping which is very specific. > I will send a patch with a deprecation notice to move the mapping API > to a driver-specific API. > > Any objection? > No from my side.