> 28/09/2020 10:59, Ferruh Yigit: > > On 9/27/2020 4:16 AM, Min Hu (Connor) wrote: > > > From: Huisong Li <lihuis...@huawei.com> > > > > > > Currently, only statistics of rx/tx queues with queue_id less than > > > RTE_ETHDEV_QUEUE_STAT_CNTRS can be displayed. If there is a certain > > > application scenario that it needs to use 256 or more than 256 queues > > > and display all statistics of rx/tx queue. At this moment, we have to > > > change the macro to be equaled to the queue number. > > > > > > However, modifying the macro to be greater than 256 will trigger > > > many errors and warnings from test-pmd, PMD drivers and librte_ethdev > > > during compiling dpdk project. But it is possible and permitted that > > > rx/tx queue number is greater than 256 and all statistics of rx/tx > > > queue need to be displayed. In addition, the data type of rx/tx queue > > > number in rte_eth_dev_configure API is 'uint16_t'. So It is unreasonable > > > to use the 'uint8_t' type for variables that control which per-queue > > > statistics can be displayed. > > The explanation is too much complex and misleading. > You mean you cannot increase RTE_ETHDEV_QUEUE_STAT_CNTRS > above 256 because it is an 8-bit type? > > [...] > > > --- a/lib/librte_ethdev/rte_ethdev.h > > > +++ b/lib/librte_ethdev/rte_ethdev.h > > > int rte_eth_dev_set_tx_queue_stats_mapping(uint16_t port_id, > > > - uint16_t tx_queue_id, uint8_t stat_idx); > > > + uint16_t tx_queue_id, uint16_t stat_idx); > [...] > > > int rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id, > > > uint16_t rx_queue_id, > > > - uint8_t stat_idx); > > > + uint16_t stat_idx); > [...] > > cc'ed tech-board, > > > > The patch breaks the ethdev ABI without a deprecation notice from previous > > release(s). > > > > It is mainly a fix to the port_id storage type, which we have updated from > > uint8_t to uint16_t in past but some seems remained for > > 'rte_eth_dev_set_tx_queue_stats_mapping()' & > > 'rte_eth_dev_set_rx_queue_stats_mapping()' APIs. > > No, it is not related to the port id, but the number of limited stats.it > it is not limited. In this case, we probably don't need the API > *_queue_stats_mapping which was invented for a limitation of ixgbe.
It is probably a bit unusual specially for modern NICs, but I think it still possible in principle. Let say NIC has up to 16K queues, but can map stats only for 1K, or so. So formally - yes, I think both queue_idx and stats_idx have to be same type. Also I can't foresee problems such change could introduce (except formal API/ABI break). So: Acked-by: Konstantin Ananyev <konstantin.anan...@intel.com> > > The problem is probably somewhere else (in testpmd), > that's why I am against this patch. >
