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

["Di, ChenxuX" <chenxux.di@intel.com>]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 15, 2020 10:34 PM
> To: Di, ChenxuX <chenxux.di@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v9 2/4] net/ice: cleanup Tx buffers
> 
> 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?
> 
Hi, Xiaolong, Ferruh
Sorry about that, it may be an error while I delete the parentheses of the  if ... else ...

And what should I do now, update a new version patch? Or add another patch with only removing the #ifdef?


> Thanks,
> ferruh