在 2021/3/29 9:53, Li, Xiaoyun 写道:
-----Original Message-----
From: oulijun <ouli...@huawei.com>
Sent: Thursday, March 25, 2021 11:04
To: Li, Xiaoyun <xiaoyun...@intel.com>; Yigit, Ferruh <ferruh.yi...@intel.com>
Cc: dev@dpdk.org; linux...@openeuler.org
Subject: Re: [PATCH 2/3] app/testpmd: remove forwarding config from parsing
Rx and Tx
在 2021/3/24 9:44, Li, Xiaoyun 写道:
-----Original Message-----
From: oulijun <ouli...@huawei.com>
Sent: Wednesday, March 24, 2021 09:01
To: Li, Xiaoyun <xiaoyun...@intel.com>; Yigit, Ferruh
<ferruh.yi...@intel.com>
Cc: dev@dpdk.org; linux...@openeuler.org
Subject: Re: [PATCH 2/3] app/testpmd: remove forwarding config from
parsing Rx and Tx
在 2021/3/23 15:50, Li, Xiaoyun 写道:
Hi
-----Original Message-----
From: Lijun Ou <ouli...@huawei.com>
Sent: Friday, March 5, 2021 18:22
To: Yigit, Ferruh <ferruh.yi...@intel.com>
Cc: Li, Xiaoyun <xiaoyun...@intel.com>; dev@dpdk.org;
linux...@openeuler.org
Subject: [PATCH 2/3] app/testpmd: remove forwarding config from
parsing Rx and Tx
From: Huisong Li <lihuis...@huawei.com>
The commit message should be more simple and avoids grammar mistakes.
All right, I will simply it.
The "fwd_config_setup()" function does release and apply for memory
of forwarding flows, and re-establish these streams when rxq/txq or
rxd/txd is changed. The function is also called by
"start_packet_forwarding()" when user executes "start" cmd.
All changes for rxq/txq or rxd/txd can be updated uniformly when
this command is executed. Therefore, it is a little redundant in
the
"cmd_config_rx_tx_parsed"
function.
It's not redundant. This command may configure number of rxq/txq. So
the
fwd streams map may change.
Then it's common to check the fwd streams after this command using
"show
config fwd".
If you remove this fwd stream update, users can't get the correct
new fwd
streams until they start the traffic.
But they may change a lot of things and want to check if the setting
is correct
before they start the traffic.
Yes, you are right. It's really unfriendly.
In addition, the forwarding stream under one TC is configured based
on number of queues allocated to TC. And number of queues allocated
to TC is updated after calling "rte_eth_dev_configure"
again. If the number of queues is reduced after configuring the
DCB, and then, release and apply for stream memory, and
reinitialize the forwarding stream under the DCB mode based on the old TC
information.
As a result, null pointer may be accessed.
I think you should add "rte_eth_dev_configure " into
dcb_fwd_config_setup()
before rte_eth_dev_get_dcb_info().
And the commit message should be similar like the following:
Segment fault might happen after configuring queue number to less
because
dcb_fwd_config_setup setup dcb based on old dcb info.
And dcb info can only update after rte_eth_dev_configure().
So this patch adds rte_eth_dev_configure() before
rte_eth_dev_get_dcb_info()
to get updated dcb info to fix this issue.
Thank you for your advice. But the above adjustments may still not
work for some drivers. The mapping between queues and TCs in these
drivers is updated in the dev_start stage.
I have an idea. We can move fwd_config_setup() to start_port(), which
is called by main() and after starting ports This not only solves the
segment fault, but also does not have the problem you mentioned above. I
test it and it is ok.
What do you think, xiaoyun?
How can you fix the issue I mentioned?
You still need to start port first to see the updated fwd config.
Yes. But it can make sure that users get the correct new fwd streams before
executing the 'start' command.
And for those drivers, why does the mapping has to be updated in dev_start
stage?
What does it need? Can't it be moved to dev_configure?
The framework does not require that the configuration parameters transferred
by the dev_configure API must be updated in the interface of driver. The driver
can verify the correctness of these parameters in the interface, and then
complete the update in the dev_start stage. It depends on the design and need
of the driver. Maybe it's more appropriate to put it in dev_configure, but now
we can't ask all these drivers to modify the operation.
I still think it's driver's responsibility to configure DCB in dev_configure.
Calling fwd_config_setup here is because this command changes queue number.
Users just change queue number so want to check fwd setup. It's a normal and
reasonable behavior.
Starting port will be called in many situations. I don't think it's appropriate
to call fwd_config_setup every time calling port_start. And you can't expect
users know the fwd setup only change after port_start.
These are only my thoughts. If anyone else agrees you, they can give you ack.
@Ferruh, what do you think?
Currently, both ixgbe and i40e also have this problem. Even if testpmd
can be modified according to Xiaoyun's suggestion,
and the driver is not modified, this problem still exists in some
drivers, such as ixgbe and hns3.
Like:
set nbcore 4
port stop all
port config 0 dcb vt off 4 pfc on
port start all
port stop all
port config all rxq 8
port config all txq 8
At the moment, a segmentation fault occurs.
Fixes: ce8d561418d4 ("app/testpmd: add port configuration
settings")
Cc: sta...@dpdk.org
Signed-off-by: Huisong Li <lihuis...@huawei.com>
Signed-off-by: Lijun Ou <ouli...@huawei.com>
---
V1->V2:
- use stream instead of flow
---
app/test-pmd/cmdline.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
4df0c32..e316f5c 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1837,8 +1837,6 @@ cmd_config_rx_tx_parsed(void *parsed_result,
return;
}
- fwd_config_setup();
-
init_port_config();
cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
--
2.7.4
.
.
.
