From: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
To: "Zhang, Yuying" <yuying.zhang@intel.com>,
"Singh, Aman Deep" <aman.deep.singh@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "Li, Xiaoyun" <xiaoyun.li@intel.com>,
"jing.d.chen@intel.com" <jing.d.chen@intel.com>,
"stable@dpdk.org" <stable@dpdk.org>,
Raslan Darawsheh <rasland@nvidia.com>
Subject: RE: [PATCH] app/testpmd: skip stopped queues when forwarding
Date: Wed, 9 Feb 2022 10:38:00 +0000 [thread overview]
Message-ID: <BL1PR12MB594501177E9D3A5D757D5CD9B92E9@BL1PR12MB5945.namprd12.prod.outlook.com> (raw)
In-Reply-To: <DM6PR11MB35163319E170CBFE67F1A1C18E2E9@DM6PR11MB3516.namprd11.prod.outlook.com>
Hi Aman, Yuying,
You share some concerns, so I'm answering in one thread.
> From: Zhang, Yuying <yuying.zhang@intel.com>
> Sent: Wednesday, February 9, 2022 12:00 PM
[...]
> > > +2225,20 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc,
> packet_fwd_t
> > pkt_fwd)
> > > static int
> > > start_pkt_forward_on_core(void *fwd_arg)
> > > {
> > > - run_pkt_fwd_on_lcore((struct fwd_lcore *) fwd_arg,
> > > - cur_fwd_config.fwd_eng->packet_fwd);
> > > + struct fwd_lcore *fc = fwd_arg;
> > > + struct fwd_stream *fsm = fwd_streams[fc->stream_idx];
> > > + struct queue_state *rxq = &ports[fsm->rx_port].rxq_state[fsm-
> > >rx_queue];
> > > + struct queue_state *txq = &ports[fsm->tx_port].txq_state[fsm-
> > >tx_queue];
> > > + struct fwd_engine *fwd_engine = cur_fwd_config.fwd_eng;
> > > + packet_fwd_t packet_fwd;
> > > +
> > > + /* Check if there will ever be any packets to send. */
> > > + if (rxq->stopped && (txq->stopped || fwd_engine !=
> > &tx_only_engine))
> > > + return 0;
> Have you considered other fwd_engines such as io_fwd_engine and
> mac_fwd_engine?
The only engine that can send packets without receiving them is "txonly".
All other engines call rte_eth_rx_burst(),
which is illegal for a stopped RxQ even if the packets are discarded.
> > > + /* Force rxonly mode if RxQ is started, but TxQ is stopped. */
> > > + packet_fwd = !rxq->stopped && txq->stopped ?
> > rx_only_engine.packet_fwd
> > > + : fwd_engine-
> >packet_fwd;
> > Should we have a print here for user info, that mode has been
> > changed or ignored.
Good idea.
> Why need to force rxonly mode for this situation? BTW, the value of
> cur_fwd_eng hasn't been updated after you changed forward mode.
It is logical to preserve as much workload as possible
so that stopping a TxQ does not reduce the Rx from NIC's perspective.
> > > + run_pkt_fwd_on_lcore(fc, packet_fwd);
> > > return 0;
> > > }
> > >
> > > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > > 2149ecd93a..2744fa4d76 100644
> > > --- a/app/test-pmd/testpmd.h
> > > +++ b/app/test-pmd/testpmd.h
> > > @@ -216,6 +216,12 @@ struct xstat_display_info {
> > > bool allocated;
> > > };
> > >
> > > +/** Application state of a queue. */
> > > +struct queue_state {
> > > + /** The queue is stopped and should not be used. */
> > > + bool stopped;
> > > +};
> > > +
> > > /**
> > > * The data structure associated with each port.
> > > */
> > > @@ -256,6 +262,10 @@ struct rte_port {
> > > uint64_t mbuf_dynf;
> > > const struct rte_eth_rxtx_callback
> > *tx_set_dynf_cb[RTE_MAX_QUEUES_PER_PORT+1];
> > > struct xstat_display_info xstats_info;
> > > + /** Per-Rx-queue state. */
> > > + struct queue_state rxq_state[RTE_MAX_QUEUES_PER_PORT];
> > > + /** Per-Tx-queue state. */
> > > + struct queue_state txq_state[RTE_MAX_QUEUES_PER_PORT];
> > Can we think of adding rxq_state/txq_state as part of existing
> structures
> > under rte_port->rte_eth_rxconf/rte_eth_txconf.
> > And if it helps, rather than bool can we use u8 with eth_dev
> defines- #define
> > RTE_ETH_QUEUE_STATE_STOPPED 0 /**< Queue stopped. */ #define
> > RTE_ETH_QUEUE_STATE_STARTED 1 /**< Queue started. */
> The same.
Will change to constants (even enum maybe?).
Struct rte_eth_{rx,tx}conf cannot be changed, because it's ethdev API,
and should not be changed, because it reflects queue configuration,
while the new arrays reflect the queue state.
What do you think of the following?
struct port_rxqueue {
struct rte_eth_rxconf conf;
uint8_t state;
};
struct port_txqueue {
struct rte_eth_txconf conf;
uint8_t state;
};
struct rte_port {
/* ... */
struct port_rxqueue rxq[RTE_MAX_QUEUES_PER_PORT];
struct port_txqueue txq[RTE_MAX_QUEUES_PER_PORT];
};
next prev parent reply other threads:[~2022-02-09 10:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-13 9:21 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 [this message]
2022-02-09 14:56 ` Li, Xiaoyun
[not found] ` <20220306232310.613552-1-dkozlyuk@nvidia.com>
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
[not found] ` <20220307125351.697936-1-dkozlyuk@nvidia.com>
2022-03-07 12:53 ` [PATCH v3 1/2] " 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=BL1PR12MB594501177E9D3A5D757D5CD9B92E9@BL1PR12MB5945.namprd12.prod.outlook.com \
--to=dkozlyuk@nvidia.com \
--cc=aman.deep.singh@intel.com \
--cc=dev@dpdk.org \
--cc=jing.d.chen@intel.com \
--cc=rasland@nvidia.com \
--cc=stable@dpdk.org \
--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 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).