From: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
To: "Qiu, Michael" <michael.qiu@intel.com>,
Stephen Hemminger <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2] ethdev: add new RX/TX queue state arrays in rte_eth_dev_data
Date: Wed, 23 Sep 2015 07:39:02 +0000 [thread overview]
Message-ID: <E115CCD9D858EF4F90C690B0DCB4D8973C819817@IRSMSX108.ger.corp.intel.com> (raw)
In-Reply-To: <533710CFB86FA344BFBF2D6802E602861989289E@SHSMSX101.ccr.corp.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
next prev parent reply other threads:[~2015-09-23 7:40 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-16 21:51 Pablo de Lara
2015-09-21 22:40 ` Stephen Hemminger
2015-09-23 6:56 ` Qiu, Michael
2015-09-23 7:39 ` De Lara Guarch, Pablo [this message]
2015-09-23 8:22 ` Ananyev, Konstantin
2015-09-23 11:49 ` Ananyev, Konstantin
2015-11-04 16:53 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E115CCD9D858EF4F90C690B0DCB4D8973C819817@IRSMSX108.ger.corp.intel.com \
--to=pablo.de.lara.guarch@intel.com \
--cc=dev@dpdk.org \
--cc=michael.qiu@intel.com \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).