Re: [dpdk-dev] [PATCH] net/tap: set queue started and stopped

["Eads, Gage" <gage.eads@intel.com>]

> -----Original Message-----
> From: Wiles, Keith
> Sent: Monday, July 9, 2018 5:07 PM
> To: Eads, Gage <gage.eads@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/tap: set queue started and stopped
> 
> 
> 
> > On Jul 9, 2018, at 5:00 PM, Wiles, Keith <keith.wiles@intel.com> wrote:
> >
> >
> >
> >> On Jul 9, 2018, at 4:51 PM, Eads, Gage <gage.eads@intel.com> wrote:
> >>
> >> <snip>
> >>
> >>>>
> >>>> +static int
> >>>> +tap_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
> >>>> +{
> >>>> +	dev->data->rx_queue_state[rx_queue_id] =
> >>> RTE_ETH_QUEUE_STATE_STARTED;
> >>>
> >>> We need to verify the rx_queue_id is valid before setting the state.
> >>>
> >>> if (rx_queue_id < dev->data>nb_rx_queues)
> >>> 	dev->data->rx_queue_state[rx_queue_id] =
> >>> RTE_ETH_QUEUE_STATE_STARTED; else
> >>> 	return -1;
> >>>
> >>> This needs to be done for each of these routines.
> >>>
> >>
> >> The ethdev layer function (rte_eth_dev_{rx, tx}_queue_{start, stop}) already
> does the queue ID bounds check -- do you prefer to duplicate it here?
> >
> > I looked in ixgb driver and it was checking I then assumed needed it. I should
> check in the ethdev layer. We do not need to duplicate more checks.
> >
> > Thanks for spotting that one.
> 
> Looks like a number of the Intel drivers check the queue_id in the PMD :-(

Well, these queue start/stop functions shouldn't be called in the main loop, so a redundant check should be fine performance-wise. Perhaps not ok from a code maintenance perspective.

> 
> >
> >>
> >> Thanks,
> >> Gage
> >
> > Regards,
> > Keith
> 
> Regards,
> Keith