-----Original Message-----
From: oulijun <redacted>
Sent: Wednesday, March 24, 2021 09:01
To: Li, Xiaoyun <redacted>; Yigit, Ferruh <redacted>
Cc: dev@dpdk.org; linuxarm@openeuler.org
Subject: Re: [PATCH 2/3] app/testpmd: remove forwarding config from parsing
Rx and Tx



在 2021/3/23 15:50, Li, Xiaoyun 写道:

quoted

Hi

quoted

-----Original Message-----
From: Lijun Ou <redacted>
Sent: Friday, March 5, 2021 18:22
To: Yigit, Ferruh <redacted>
Cc: Li, Xiaoyun <redacted>; dev@dpdk.org;
linuxarm@openeuler.org
Subject: [PATCH 2/3] app/testpmd: remove forwarding config from
parsing Rx and Tx

From: Huisong Li <lihuisong@huawei.com>

The commit message should be more simple and avoids grammar mistakes.

All right, I will simply it.

quoted

quoted

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"

quoted

quoted

function.

It's not redundant. This command may configure number of rxq/txq. So the

fwd streams map may change.

quoted

Then it's common to check the fwd streams after this command using "show

config fwd".

quoted

If you remove this fwd stream update, users can't get the correct new fwd

streams until they start the traffic.

quoted

But they may change a lot of things and want to check if the setting is correct

before they start the traffic.

quoted

Yes, you are right. It's really unfriendly.

quoted

quoted

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().

quoted

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.

quoted

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.

quoted

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.
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?

quoted

quoted

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: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Lijun Ou <redacted>
---
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

.