Re: [dpdk-dev] [PATCH v9 2/4] net/ice: cleanup Tx buffers

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/14/2020 12:40 PM, Ferruh Yigit wrote:
> On 1/13/2020 9:57 AM, Chenxu Di wrote:
>> Add support to the ice driver for the API rte_eth_tx_done_cleanup
>> to force free consumed buffers on Tx ring.
>>
>> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> 
> <...>
> 
>> +static int
>> +ice_tx_done_cleanup_vec(struct ice_tx_queue *txq __rte_unused,
>> +			uint32_t free_cnt __rte_unused)
>> +{
>> +	return -ENOTSUP;
>> +}
>> +
>> +static int
>> +ice_tx_done_cleanup_simple(struct ice_tx_queue *txq,
>> +			uint32_t free_cnt)
>> +{
>> +	int i, n, cnt;
>> +
>> +	if (free_cnt == 0 || free_cnt > txq->nb_tx_desc)
>> +		free_cnt = txq->nb_tx_desc;
>> +
>> +	cnt = free_cnt - free_cnt % txq->tx_rs_thresh;
>> +
>> +	for (i = 0; i < cnt; i += n) {
>> +		if (txq->nb_tx_desc - txq->nb_tx_free < txq->tx_rs_thresh)
>> +			break;
>> +
>> +		n = ice_tx_free_bufs(txq);
>> +
>> +		if (n == 0)
>> +			break;
>> +	}
>> +
>> +	return i;
>> +}
>> +
>> +int
>> +ice_tx_done_cleanup(void *txq, uint32_t free_cnt)
>> +{
>> +	struct ice_tx_queue *q = (struct ice_tx_queue *)txq;
>> +	struct rte_eth_dev *dev = &rte_eth_devices[q->port_id];
>> +	struct ice_adapter *ad =
>> +		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
>> +
>> +#ifdef RTE_ARCH_X86
>> +	if (ad->tx_vec_allowed)
>> +		return ice_tx_done_cleanup_vec(q, free_cnt);
>> +#endif
> 
> Hi Chenxu,
> 
> This is causing build error for non x86 builds [1], wrapping the
> 'ice_tx_done_cleanup_vec()' with #ifdef can solve the error, but instead why not
> remove the #ifdef completely.
> 
> Would the 'tx_vec_allowed' be set when it is non x86, I think it shouldn't, IF
> so #ifdef can go away.
> 
> [1]
> .../dpdk/drivers/net/ice/ice_rxtx.c:2709:1: error: ‘ice_tx_done_cleanup_vec’
> defined but not used [-Werror=unused-function]
>  ice_tx_done_cleanup_vec(struct ice_tx_queue *txq __rte_unused,
>  ^~~~~~~~~~~~~~~~~~~~~~~
> 

Hi Chenxu, Xiaolong,

I will fix the build error while merging, by wrapping 'ice_tx_done_cleanup_vec'
with "#ifdef RTE_ARCH_X86",
BUT can you please make an incremental patch to remove the #ifdef?

Thanks,
ferruh