On 7/6/22 05:00, lihuisong (C) wrote:
在 2022/7/5 18:12, Zhang, Peng1X 写道:
Hi Song,
Currently this patch is just fix the issue detected for rx queue on
secondary process.
Later patch for tx queue will be submit.
Firstly, the Rx packet dump failure is not displayed in your commit log.
In addition, even if this patch is aimed to fix the Rx issue on
secondary process,
it doesn't work well when if testpmd run in 'mac' or 'swap' forwarding
mode, right?
Because it is also affected by the Tx queue state. Therefore, it is
better to place
the modification of Rx and Tx queue state in the same patch to fix this
issue.
If the problem is generic, it is better to find generic solution
rather than concentrate on one aspect of the problem.
@Peng1X, @Andrew and @Thomas
Currently, the secondary process cannot send and receive packets.
It is suggested that this problem should be solved quickly.
After all, 22.07 will be released soon.
@Andrew, what's your opinion about the solution of this patch?
IMHO the patch is simply incorrect since it uses
rx_deferred_start incorrectly. The attribute does not say if
an Rx queue is started or stopped right now.
Thanks,
Peng
-----Original Message-----
From: lihuisong (C) <lihuis...@huawei.com>
Sent: Monday, July 4, 2022 10:37 AM
To: Zhang, Peng1X <peng1x.zh...@intel.com>; Andrew Rybchenko
<andrew.rybche...@oktetlabs.ru>; dev@dpdk.org
Cc: Singh, Aman Deep <aman.deep.si...@intel.com>; Zhang, Yuying
<yuying.zh...@intel.com>; sta...@dpdk.org
Subject: Re: [PATCH] app/testpmd: fix secondary process cannot dump
packet
Hi Peng1X,
在 2022/7/1 19:36, Zhang, Peng1X 写道:
Hi,
In fact, the patch is aim to fix this issue that secondary process
cannot dump
packet after start testpmd.
This issue is induced by commit id is 3c4426db54fc ("app/testpmd: do
not poll stopped queues"). After secondary process start, the default
value of Rx/Tx queue state maintained by testpmd is
'RTE_ETH_QUEUE_STATE_STOPPED', the 'fsm[sm_id]->disabled' flag will set
true according to queues state, then packet cannot forward and dump.
I get your meaning.
However, failing to dump packet isn't the first exception, and the
first one is
that testpmd doesn't call 'struct fwd_engine::packet_fwd()' to
receive or send
packet.
So, I think you should describe and resolve this problem from this
point. This
patch cannot completely resolve this problem. The Tx queue state
should also
be added here.
The reason why not use 'dev->data->rx_queue_state' is whether queue
state is start or stop in primary process depend on
rx_conf->rx_deferred_start after start testpmd. And after having
started
testpmd, queue state can be controlled by command for example 'port x
rxq x
start'.
Should we align with the same behavior of queues state for primary and
secondary process after start testpmd?
If primary process stops a queue, but secondary doesn't know.
we have to simplify this queue state problem like you momentioned if
we don't
have a good idea.
@Andrew, what do you think?
I'm sorry to say, but multi-process support in testpmd is not well-
thought. So, it is hard to suggest a good solution. I think that trying
to mirror state in secondary testpmd process is a bad idea. It could
become out-of-sync by definition. So, I'd either share testpmd state or
use state from ethdev (which is located in shared memory).
However, it sounds like a long story and non-trivial changes which are
too late at the current release cycle state. See below.
Thanks,
Huisong
-----Original Message-----
From: lihuisong (C) <lihuis...@huawei.com>
Sent: Wednesday, June 29, 2022 10:55 AM
To: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>; Zhang, Peng1X
<peng1x.zh...@intel.com>; dev@dpdk.org
Cc: Singh, Aman Deep <aman.deep.si...@intel.com>; Zhang, Yuying
<yuying.zh...@intel.com>; sta...@dpdk.org
Subject: Re: [PATCH] app/testpmd: fix secondary process cannot dump
packet
在 2022/6/23 20:10, Andrew Rybchenko 写道:
On 6/23/22 21:15, peng1x.zh...@intel.com wrote:
From: Peng Zhang <peng1x.zh...@intel.com>
The origin design is whether testpmd is primary or not, if state of
receive queue is stop, then packets will not be dumped for show.
While to secondary process, receive queue will not be set up, and
state will still be stop even if testpmd is started. So packets of
stated secondary process cannot be dumped for show.
The current design is to secondary process state of queue will be
set to start after testpmd is started. Then packets of started
secondary process can be dumped for show.
Fixes: a550baf24af9 ("app/testpmd: support multi-process")
Cc: sta...@dpdk.org
Signed-off-by: Peng Zhang <peng1x.zh...@intel.com>
---
app/test-pmd/testpmd.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
205d98ee3d..93ba7e7c9b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3007,6 +3007,18 @@ start_port(portid_t pid)
if (setup_hairpin_queues(pi, p_pi, cnt_pi) != 0)
return -1;
}
+
+ if (port->need_reconfig_queues > 0 && !is_proc_primary())
+{
+ struct rte_eth_rxconf *rx_conf;
+ for (qi = 0; qi < nb_rxq; qi++) {
+ rx_conf = &(port->rxq[qi].conf);
+ ports[pi].rxq[qi].state =
+ rx_conf->rx_deferred_start ?
+ RTE_ETH_QUEUE_STATE_STOPPED :
+ RTE_ETH_QUEUE_STATE_STARTED;
I'm not sure why it is correct to assume that deferred queue is not
yet started.
+1.
We should also consider whether the queue state can be changed in secondary.
The 'rx_conf->rx_deferred_start' is the data in secondary.
Why not use 'dev->data->rx_queue_state[]'.
In fact, the issue you memtioned was introduced the following patch:
Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues")
The root cause of this issue is that the default value of Rx/Tx queue
state maintained by testpmd is 'RTE_ETH_QUEUE_STATE_STOPPED'. As a
result, secondary doesn't start polling thread to receive packets
when start packet forwarding. And now, secondary cannot receive and
send any packets.
Could you fix it together?
As a simple fix which should work in assumption that everything is
started before secondary process start-up, I'd just set state to
RTE_ETH_QUEUE_STATE_STARTED.
+ }
+ }
+
configure_rxtx_dump_callbacks(verbose_level);
if (clear_ptypes) {
diag = rte_eth_dev_set_ptypes(pi, RTE_PTYPE_UNKNOWN,
.
