Re: [dpdk-dev] [PATCH v6 2/8] net/ice: add pointer for queue buffer release

["Lu, Wenzhuo" <wenzhuo.lu@intel.com>]

Hi Maxime,


> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Monday, March 25, 2019 9:24 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/8] net/ice: add pointer for queue buffer
> release
> 
> 
> 
> On 3/25/19 7:06 AM, Wenzhuo Lu wrote:
> > Add function pointers of buffer releasing for RX and TX queues, for
> > vector functions will be added for RX and TX.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> >   drivers/net/ice/ice_rxtx.c | 24 +++++++++++++++---------
> >   drivers/net/ice/ice_rxtx.h |  5 +++++
> >   2 files changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> > index c794ee8..d540ed1 100644
> > --- a/drivers/net/ice/ice_rxtx.c
> > +++ b/drivers/net/ice/ice_rxtx.c
> > @@ -366,7 +366,7 @@
> >   		PMD_DRV_LOG(ERR, "Failed to switch RX queue %u on",
> >   			    rx_queue_id);
> >
> > -		ice_rx_queue_release_mbufs(rxq);
> > +		rxq->rx_rel_mbufs(rxq);
> >   		ice_reset_rx_queue(rxq);
> >   		return -EINVAL;
> >   	}
> > @@ -393,7 +393,7 @@
> >   				    rx_queue_id);
> >   			return -EINVAL;
> >   		}
> > -		ice_rx_queue_release_mbufs(rxq);
> > +		rxq->rx_rel_mbufs(rxq);
> >   		ice_reset_rx_queue(rxq);
> >   		dev->data->rx_queue_state[rx_queue_id] =
> >   			RTE_ETH_QUEUE_STATE_STOPPED;
> > @@ -555,7 +555,7 @@
> >   		return -EINVAL;
> >   	}
> >
> > -	ice_tx_queue_release_mbufs(txq);
> > +	txq->tx_rel_mbufs(txq);
> >   	ice_reset_tx_queue(txq);
> >   	dev->data->tx_queue_state[tx_queue_id] =
> > RTE_ETH_QUEUE_STATE_STOPPED;
> >
> > @@ -669,6 +669,7 @@
> >   	ice_reset_rx_queue(rxq);
> >   	rxq->q_set = TRUE;
> >   	dev->data->rx_queues[queue_idx] = rxq;
> > +	rxq->rx_rel_mbufs = ice_rx_queue_release_mbufs;
> >
> >   	use_def_burst_func =
> > ice_check_rx_burst_bulk_alloc_preconditions(rxq);
> >
> > @@ -701,7 +702,7 @@
> >   		return;
> >   	}
> >
> > -	ice_rx_queue_release_mbufs(q);
> > +	q->rx_rel_mbufs(q);
> >   	rte_free(q->sw_ring);
> >   	rte_free(q);
> >   }
> > @@ -866,6 +867,7 @@
> >   	ice_reset_tx_queue(txq);
> >   	txq->q_set = TRUE;
> >   	dev->data->tx_queues[queue_idx] = txq;
> > +	txq->tx_rel_mbufs = ice_tx_queue_release_mbufs;
> 
> I think it could be cleaner to have ice_rx_queue_release_mbufs() to call the
> callback. So you would rename current ice_rx_queue_release_mbufs to
> something else.
> 
> I would make the code more consistent IMHO, and would avoid to patch all
> call sites.
Good suggestion. I'll change it.