On Thu, 2021-09-16 at 10:09 +0200, Thomas Monjalon wrote: > 15/09/2021 15:02, Xueming Li: > > To align with other eth device queue configuration callbacks, cleanup > > queue release callback API by changing RX and TX queue release callback > > API parameter from queue object to device and queue index. > > Please split the explanation in multiple sentences, > so it allows taking a breath while reading :) > A good method is to start with explaining the status: > most callbacks use queue ID > queue release callbacks use queue object > Then explain what is done: > queue release callbacks are changed to use queue ID > all drivers are adapted > empty callbacks are removed in some drivers > > > This patch allows NULL callback to avoid defining empty callbacks. > > > > Signed-off-by: Xueming Li <xuemi...@nvidia.com> > > Cc: Ferruh Yigit <ferruh.yi...@intel.com> > > Cc: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> > > Cc: "Singh, Aman Deep" <aman.deep.si...@intel.com> > > --- > > v2: included new NFP PMD driver > > > > This patch is a preparation of shared Rx queue feature since the rxq > > object is subject to be used by PMD as shared rxq object which is shared > > among rx queues in same share group: > > https://mails.dpdk.org/archives/dev/2021-July/215575.html > > This note does not fully explain why using the queue ID helps. > > > > --- a/lib/ethdev/ethdev_driver.h > > +++ b/lib/ethdev/ethdev_driver.h > > -typedef void (*eth_queue_release_t)(void *queue); > > +typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev, > > + uint16_t rx_queue_id); > > > --- a/lib/ethdev/rte_ethdev.c > > +++ b/lib/ethdev/rte_ethdev.c > > - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_release, > > -ENOTSUP); > > Why removing this check? > It is changing the behaviour.
Some PMD implemented a dummy callback, so I think maybe it good to make this callback optional to make PMD clean, how do you think? > > > - > > + if (dev->dev_ops->rx_queue_release != NULL) > > + for (i = nb_queues; i < old_nb_queues; i++) > > + (*dev->dev_ops->rx_queue_release)(dev, i); > > rxq = dev->data->rx_queues; > > - > > - for (i = nb_queues; i < old_nb_queues; i++) > > - (*dev->dev_ops->rx_queue_release)(rxq[i]); > > rxq = rte_realloc(rxq, sizeof(rxq[0]) * nb_queues, > > RTE_CACHE_LINE_SIZE); > > >
