Re: [dpdk-dev] [PATCH v2] ethdev: add new RX/TX queue state arrays in rte_eth_dev_data

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

> -----Original Message-----
> From: Qiu, Michael
> Sent: Wednesday, September 23, 2015 7:57 AM
> To: Stephen Hemminger; De Lara Guarch, Pablo
> Cc: dev@dpdk.org; thomas.monjalon@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v2] ethdev: add new RX/TX queue state
> arrays in rte_eth_dev_data
> 
> On 2015/9/22 6:40, Stephen Hemminger wrote:
> > On Wed, 16 Sep 2015 22:51:24 +0100
> > Pablo de Lara <pablo.de.lara.guarch@intel.com> wrote:
> >
> >> This is important to avoid trying to start/stop twice a queue,
> >> which will result in undefined behaviour
> >> (which may cause RX/TX disruption).
> >>
> >> Mind that only the PMDs which have queue_start/stop functions
> >> have been changed to update this field, as the functions will
> >> check the queue state before switching it.
> >>
> >> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > I agree that the DPDK API should check for buggy manipulation
> > in the control path. But this should be done in generic code.
> > Anything where you have to change any driver is making more work
> > than necessary.
> 

I see little way to set the queue state without touching the PMDs.
Basically, because queues can be started either calling directly
the function from the PMDs or calling the generic function from ethdev,
therefore some code must be placed in the PMDs.

I guess  there are two possible ways to overcome this:

 - Use in all PMDs rte_eth_rx_queue_start/stop, instead of directly 
  the internal functions: first, it is necessary the port id, which I am not sure if it is always available
 (plus, when it is, it is available in the specific queue of that PMD, so it does not have to be there).
 Anyway, this option requires changing the PMDs.

- Apparently, the only function that calls these queue_start/stop functions is dev_start/stop.
   In some PMDs, it is possible to defer the start of some queues, so it would not initialize
   these queues, but in other PMDs this is not implemented.
   We will also be depending at how it is going on within the PMDs,
   so the code will not be generic either.

Any ideas to make this truly generic?

> I agree with you, but I have a question, why we need expose the queue
> start and stop function to app?
> 
> In my opinion, user app will hardly to start a device but stop the
> device queue. what's the purpose of it?

This API was added in 1.7 if I am not wrong. The purpose was that user could decide
not to start some of the queues in a device at start up, and then a new API to start 
these queues was necessary, but there were no checks of their current state,
leading to undefined behaviour.

Thanks both Stephen and Michael,

Pablo
> 
> Thanks,
> Michael