All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	Dmitry Kozlyuk <dkozlyuk@nvidia.com>
Cc: dev@dpdk.org, Raslan Darawsheh <rasland@nvidia.com>,
	Slava Ovsiienko <viacheslavo@nvidia.com>,
	"Li, Xiaoyun" <xiaoyun.li@intel.com>,
	"Zhang, Yuying" <yuying.zhang@intel.com>,
	"Singh, Aman Deep" <aman.deep.singh@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [PATCH] app/testpmd: skip stopped queues when forwarding
Date: Thu, 24 Feb 2022 10:28:46 +0100	[thread overview]
Message-ID: <109715067.nniJfEyVGO@thomas> (raw)
In-Reply-To: <BL1PR12MB59453495C01B7F4C1C0D7B17B93A9@BL1PR12MB5945.namprd12.prod.outlook.com>

21/02/2022 09:58, Dmitry Kozlyuk:
> Andrew, Ferruh, Thomas, which behavior does ethdev assume (see below)?

For the whole device stop, this is the documentation:
"
  The transmit and receive functions should not be invoked
  when the device is stopped.
"

There is also this comment on rte_eth_dev_reset:
"
  Note: To avoid unexpected behavior, the application should stop calling
  Tx and Rx functions before calling rte_eth_dev_reset( ). For thread
  safety, all these controlling functions should be called from the same
  thread.
"

For queue stop, there is no documented expectation.

There is this comment for queue callback removal:
"
  The memory for the callback can be
  subsequently freed back by the application by calling rte_free():
 
  - Immediately - if the port is stopped, or the user knows that no
    callbacks are in flight e.g. if called from the thread doing Rx/Tx
    on that queue.
 
  - After a short delay - where the delay is sufficient to allow any
    in-flight callbacks to complete. Alternately, the RCU mechanism can be
    used to detect when data plane threads have ceased referencing the
    callback memory.
"

> > This patch was created with the assumption
> > that stopped queues may not be used for Rx/Tx.
> > No-op behavior of rte_eth_rx/tx_burst()
> > for a stopped queue is not documented.

Indeed, it is not documented.
I suggest working on this documentation first.
testpmd could be adjusted later if needed.

> > Yes, at least some PMDs implement it this way.
> > But is this behavior intended?
> > 
> > It is the application that stops the queue or starts it deferred.
> > Stopping is non-atomic, so polling the queue is not allowed during it.
> > Hence, if the application intends to stop queues, it must either:
> > 
> > a) Know the queue is not polled before stopping it.
> >    Use case: a secondary that was polling the queue has crashed,
> >    the primary is doing a recovery to free all mbufs.
> >    There is no issue since there is no poller to touch the queue.
> > 
> > b) Tell the poller to skip the queue for the time of stopping it.
> >    Use case: deferred queue start or queue reconfiguration.
> >    But if the poller is cooperating anyway,
> >    what prevents it from not touching the queue for longer?
> > 
> > The same considerations apply to starting a queue.
> > 
> > No-op behavior is convenient from the application point of view.
> > But it also means that pollers of stopped queues
> > will go all the way down to PMD Rx/Tx routines, wasting cycles,
> > and some PMDs will do a check for the queue state,
> > even though it may never be needed for a particular application.

Yes that's the question: Do we want
1/ to allow apps to poll stopped queues, adding checks in PMDs,
or do we prefer
2/ saving check cycles and expect apps to not poll?




  reply	other threads:[~2022-02-24  9:28 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13  9:21 [PATCH] app/testpmd: skip stopped queues when forwarding Dmitry Kozlyuk
2022-02-02 10:02 ` Dmitry Kozlyuk
2022-02-03 13:52 ` Singh, Aman Deep
2022-02-09  8:59   ` Zhang, Yuying
2022-02-09 10:38     ` Dmitry Kozlyuk
2022-02-09 14:56     ` Li, Xiaoyun
2022-02-11 10:42       ` Dmitry Kozlyuk
2022-02-21  8:58         ` Dmitry Kozlyuk
2022-02-24  9:28           ` Thomas Monjalon [this message]
2022-03-06 23:23 ` [PATCH v2 0/2] " Dmitry Kozlyuk
2022-03-06 23:23   ` [PATCH v2 1/2] ethdev: prohibit polling of a stopped queue Dmitry Kozlyuk
2022-03-06 23:23   ` [PATCH v2 2/2] app/testpmd: do not poll stopped queues Dmitry Kozlyuk
2022-03-07 12:53   ` [PATCH v3 0/2] app/testpmd: skip stopped queues when forwarding Dmitry Kozlyuk
2022-03-07 12:53     ` [PATCH v3 1/2] app/testpmd: do not poll stopped queues Dmitry Kozlyuk
2022-03-09 10:36       ` Dmitry Kozlyuk
2023-07-08  1:54         ` Stephen Hemminger
2022-05-25 15:46       ` Thomas Monjalon
2022-06-10 11:28         ` Jiang, YuX
2022-03-07 12:53     ` [PATCH v3 2/2] ethdev: prohibit polling of a stopped queue Dmitry Kozlyuk

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=109715067.nniJfEyVGO@thomas \
    --to=thomas@monjalon.net \
    --cc=aman.deep.singh@intel.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=dkozlyuk@nvidia.com \
    --cc=ferruh.yigit@intel.com \
    --cc=rasland@nvidia.com \
    --cc=viacheslavo@nvidia.com \
    --cc=xiaoyun.li@intel.com \
    --cc=yuying.zhang@intel.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.