[dpdk-dev] [PATCH v5 2/2] ethdev: change queue release callback

[Ferruh Yigit]

On 9/29/2021 2:57 PM, Xueming(Steven) Li wrote:
> On Wed, 2021-09-22 at 12:54 +0000, Xueming(Steven) Li wrote:
>> On Wed, 2021-09-22 at 11:57 +0100, Ferruh Yigit wrote:
>>>>>
>>>>> <...>
>>>>>
>>>>>>   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.
>>>
>>>>
>>
>> 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.

> 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?

Thanks,
ferruh