On Tue, 2021-10-05 at 17:38 +0100, Ferruh Yigit wrote:

On 9/29/2021 2:57 PM, Xueming(Steven) Li wrote:

quoted

On Wed, 2021-09-22 at 12:54 +0000, Xueming(Steven) Li wrote:

quoted

On Wed, 2021-09-22 at 11:57 +0100, Ferruh Yigit wrote:

quoted

quoted

quoted

<...>

quoted

  void
-i40e_dev_rx_queue_release(void *rxq)
+i40e_dev_rx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
+{
+	i40e_rx_queue_release(dev->data->rx_queues[qid]);
+}
+
+void
+i40e_dev_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
+{
+	i40e_tx_queue_release(dev->data->tx_queues[qid]);
+}
+

Is there any specific reason to not update driver but add wrappers for it?

Some caller don't have queue ID on hand, adding wrapper seems more
convinient.

Convinient for the patch, but not sure convinient for the driver.

As mentioned before, not sure about approach to update some driver and add
wrappers for some others.

qede, ice and i40e seems not updated, I am for syncronizing with their
maintainers before proceed.

quoted

For qede, qede_tx_queue_release(txq_obj) is called by
qede_alloc_tx_queue_mem(dev, qid), while upper caller
qede_tx_queue_setup() doesn't always save txq_obj to dev->data->txqs[].

For ice and i40e, it's similar, ice_tx_queue_release() is used to free
txq, but some txq isn't saved into dev, please refer to
ice_fdir_setup(), wrapper is needed.

These 3 PMDs create rxq/txq that not saved in dev->data, can't change
parameter to dev+qid for such case, that's why wrapper was there.

Hi Ferruh,

No response from qede, ice and i40e. Basically the original queue
release api is shared by private queues(not registered to ethdev),
can't access by index, that why a warpper was there. To avoid more
rebase in last minute for this big patch, do you think we could close
it?

I see the reason and since there is no update from maintainers, to keep
the ball rolling agree to continue with wrappers, those PMDs can send
incremental patches if required.

quoted

BTW, from feedback from hns3, I will post a new version to add the
macro.

I have concern about this one, how accessing to the global variable
'rte_eth_devices' via a macro improves the situation?

Can you please make wrappers for hns3 driver too, we can follow it later
with driver maintainer?

hns3 doesn't need a wrapper. The macro isn't related to wrapper, just
for the rte_eth_devices access as you suggested:
	&rte_eth_devices[hw->data->port_id]

Thanks,
ferruh