This series add two test bug fixes and a print style. Hongbo Zheng (1): app/testpmd: use of Rx/Tx in testpmd Huisong Li (2): app/testpmd: fix forwarding configuration when DCB test app/testpmd: remove forwarding config from parsing Rx and Tx app/test-pmd/cmdline.c | 106 ++++++++++++++++---------------- app/test-pmd/config.c | 147 +++++++++++++++++++++++++-------------------- app/test-pmd/csumonly.c | 22 +++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++--- app/test-pmd/parameters.c | 50 +++++++-------- app/test-pmd/testpmd.c | 132 ++++++++++++++++++++-------------------- app/test-pmd/testpmd.h | 28 ++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 263 insertions(+), 244 deletions(-) -- 2.7.4
From: Huisong Li <lihuisong@huawei.com> Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is lack for dcb_fwd_config_setup, which may lead to a bad events where multiple polling threads operate on the same queue. As a result, various possible exceptions will occur. The procedure is as follows: startup testpmd app as follows: testpmd> port stop all testpmd> set nbcore 8 testpmd> port config 0 dcb vt off 4 pfc on testpmd> port config all rxq 16 testpmd> port config all txq 16 testpmd> port start all testpmd> set fwd mac Logical Core 1 (socket 0) forwards packets on 4 streams: RX P=0/Q=0 (socket 0) -> TX P=0/Q=0 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=1 (socket 0) -> TX P=0/Q=1 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=2 (socket 0) -> TX P=0/Q=2 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=3 (socket 0) -> TX P=0/Q=3 (socket 0) peer=02:00:00:00:00:00 Logical Core 2 (socket 0) forwards packets on 4 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00 Logical Core 3 (socket 0) forwards packets on 4 streams: RX P=0/Q=8 (socket 0) -> TX P=0/Q=8 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=9 (socket 0) -> TX P=0/Q=9 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=10 (socket 0) -> TX P=0/Q=10 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=11 (socket 0) -> TX P=0/Q=11 (socket 0) peer=02:00:00:00:00:00 Logical Core 4 (socket 0) forwards packets on 4 streams: RX P=0/Q=12 (socket 0) -> TX P=0/Q=12 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=13 (socket 0) -> TX P=0/Q=13 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=14 (socket 0) -> TX P=0/Q=14 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=15 (socket 0) -> TX P=0/Q=15 (socket 0) peer=02:00:00:00:00:00 Logical Core 5 (socket 0) forwards packets on 1 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 Logical Core 6 (socket 0) forwards packets on 1 streams: RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 Logical Core 7 (socket 0) forwards packets on 1 streams: RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 Logical Core 8 (socket 0) forwards packets on 1 streams: RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00 Notice: number of rxq and txq in running cmdline are greater than used nb_tc. For the DCB forwarding test, each core is assigned on each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value can be adjusted based on 'total_tc_num' in all forwarding ports. In addition, after operation of port stop/port start, forwarding configuration is changed by rss_fwd_config_setup. In "start_port" function, dcb_test is set to 1 based on dcb_config. So it also should be cleared when dcb_config is 0. Fixes: 900550de04a7 ("app/testpmd: add dcb support") 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 <oulijun@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ app/test-pmd/testpmd.c | 12 +++++++----- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 576d5ac..c89f8cd 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2864,6 +2864,21 @@ rss_fwd_config_setup(void) } } +static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +} + /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2883,12 +2898,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num = 0; uint8_t tc = 0; cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num; /* reinitialize forwarding streams */ init_fwd_streams(); diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 1a57324..7eb810b 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi; - if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return; + /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; + printf("Stopping ports...\n"); RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
From: Huisong Li <lihuisong@huawei.com>
The commit message is too long and redundant. Please simplify it.
Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is lack for dcb_fwd_config_setup, which may lead to a bad events where multiple polling threads operate on the same queue. As a result, various possible exceptions will occur. The procedure is as follows:
startup testpmd app as follows: testpmd> port stop all testpmd> set nbcore 8 testpmd> port config 0 dcb vt off 4 pfc on port config all rxq 16 port testpmd> config all txq 16 port start all set fwd mac Logical Core 1 (socket 0) forwards packets on 4 streams: RX P=0/Q=0 (socket 0) -> TX P=0/Q=0 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=1 (socket 0) -> TX P=0/Q=1 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=2 (socket 0) -> TX P=0/Q=2 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=3 (socket 0) -> TX P=0/Q=3 (socket 0) peer=02:00:00:00:00:00 Logical Core 2 (socket 0) forwards packets on 4 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00 Logical Core 3 (socket 0) forwards packets on 4 streams: RX P=0/Q=8 (socket 0) -> TX P=0/Q=8 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=9 (socket 0) -> TX P=0/Q=9 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=10 (socket 0) -> TX P=0/Q=10 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=11 (socket 0) -> TX P=0/Q=11 (socket 0) peer=02:00:00:00:00:00 Logical Core 4 (socket 0) forwards packets on 4 streams: RX P=0/Q=12 (socket 0) -> TX P=0/Q=12 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=13 (socket 0) -> TX P=0/Q=13 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=14 (socket 0) -> TX P=0/Q=14 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=15 (socket 0) -> TX P=0/Q=15 (socket 0) peer=02:00:00:00:00:00 Logical Core 5 (socket 0) forwards packets on 1 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 Logical Core 6 (socket 0) forwards packets on 1 streams: RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 Logical Core 7 (socket 0) forwards packets on 1 streams: RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 Logical Core 8 (socket 0) forwards packets on 1 streams: RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00
Notice: number of rxq and txq in running cmdline are greater than used nb_tc.
For the DCB forwarding test, each core is assigned on each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value can be adjusted based on 'total_tc_num' in all forwarding ports.
In addition, after operation of port stop/port start, forwarding configuration is changed by rss_fwd_config_setup. In "start_port" function, dcb_test is set to 1 based on dcb_config. So it also should be cleared when dcb_config is 0.
Fixes: 900550de04a7 ("app/testpmd: add dcb support") 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 <oulijun@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ app/test-pmd/testpmd.c | 12 +++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 576d5ac..c89f8cd 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2864,6 +2864,21 @@ rss_fwd_config_setup(void) } }
+static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +}
It's only 3 lines. Is it really necessary to make it into a function which is not called by anyone else? A comment and the loop are enough.
+ /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2883,12 +2898,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num = 0; uint8_t tc = 0;
cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num;
/* reinitialize forwarding streams */ init_fwd_streams(); diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 1a57324..7eb810b 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared. So what's the problem of original codes? Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
在 2021/3/23 16:55, Li, Xiaoyun 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
From: Huisong Li <lihuisong@huawei.com>
The commit message is too long and redundant. Please simplify it.
OK, I will fix it.
Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is lack for dcb_fwd_config_setup, which may lead to a bad events where multiple polling threads operate on the same queue. As a result, various possible exceptions will occur. The procedure is as follows:
startup testpmd app as follows: testpmd> port stop all testpmd> set nbcore 8 testpmd> port config 0 dcb vt off 4 pfc on port config all rxq 16 port testpmd> config all txq 16 port start all set fwd mac Logical Core 1 (socket 0) forwards packets on 4 streams: RX P=0/Q=0 (socket 0) -> TX P=0/Q=0 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=1 (socket 0) -> TX P=0/Q=1 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=2 (socket 0) -> TX P=0/Q=2 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=3 (socket 0) -> TX P=0/Q=3 (socket 0) peer=02:00:00:00:00:00 Logical Core 2 (socket 0) forwards packets on 4 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00 Logical Core 3 (socket 0) forwards packets on 4 streams: RX P=0/Q=8 (socket 0) -> TX P=0/Q=8 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=9 (socket 0) -> TX P=0/Q=9 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=10 (socket 0) -> TX P=0/Q=10 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=11 (socket 0) -> TX P=0/Q=11 (socket 0) peer=02:00:00:00:00:00 Logical Core 4 (socket 0) forwards packets on 4 streams: RX P=0/Q=12 (socket 0) -> TX P=0/Q=12 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=13 (socket 0) -> TX P=0/Q=13 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=14 (socket 0) -> TX P=0/Q=14 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=15 (socket 0) -> TX P=0/Q=15 (socket 0) peer=02:00:00:00:00:00 Logical Core 5 (socket 0) forwards packets on 1 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 Logical Core 6 (socket 0) forwards packets on 1 streams: RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 Logical Core 7 (socket 0) forwards packets on 1 streams: RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 Logical Core 8 (socket 0) forwards packets on 1 streams: RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00
Notice: number of rxq and txq in running cmdline are greater than used nb_tc.
For the DCB forwarding test, each core is assigned on each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value can be adjusted based on 'total_tc_num' in all forwarding ports.
In addition, after operation of port stop/port start, forwarding configuration is changed by rss_fwd_config_setup. In "start_port" function, dcb_test is set to 1 based on dcb_config. So it also should be cleared when dcb_config is 0.
Fixes: 900550de04a7 ("app/testpmd: add dcb support") 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 <oulijun@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ app/test-pmd/testpmd.c | 12 +++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 576d5ac..c89f8cd 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2864,6 +2864,21 @@ rss_fwd_config_setup(void) } }
+static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +}
It's only 3 lines. Is it really necessary to make it into a function which is not called by anyone else? A comment and the loop are enough.
+ /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2883,12 +2898,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num = 0; uint8_t tc = 0;
cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num;
/* reinitialize forwarding streams */ init_fwd_streams(); diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 1a57324..7eb810b 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
在 2021/3/23 16:55, Li, Xiaoyun 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
From: Huisong Li <lihuisong@huawei.com>
The commit message is too long and redundant. Please simplify it.
Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is lack for dcb_fwd_config_setup, which may lead to a bad events where multiple polling threads operate on the same queue. As a result, various possible exceptions will occur. The procedure is as follows:
startup testpmd app as follows: testpmd> port stop all testpmd> set nbcore 8 testpmd> port config 0 dcb vt off 4 pfc on port config all rxq 16 port testpmd> config all txq 16 port start all set fwd mac Logical Core 1 (socket 0) forwards packets on 4 streams: RX P=0/Q=0 (socket 0) -> TX P=0/Q=0 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=1 (socket 0) -> TX P=0/Q=1 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=2 (socket 0) -> TX P=0/Q=2 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=3 (socket 0) -> TX P=0/Q=3 (socket 0) peer=02:00:00:00:00:00 Logical Core 2 (socket 0) forwards packets on 4 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00 Logical Core 3 (socket 0) forwards packets on 4 streams: RX P=0/Q=8 (socket 0) -> TX P=0/Q=8 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=9 (socket 0) -> TX P=0/Q=9 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=10 (socket 0) -> TX P=0/Q=10 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=11 (socket 0) -> TX P=0/Q=11 (socket 0) peer=02:00:00:00:00:00 Logical Core 4 (socket 0) forwards packets on 4 streams: RX P=0/Q=12 (socket 0) -> TX P=0/Q=12 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=13 (socket 0) -> TX P=0/Q=13 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=14 (socket 0) -> TX P=0/Q=14 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=15 (socket 0) -> TX P=0/Q=15 (socket 0) peer=02:00:00:00:00:00 Logical Core 5 (socket 0) forwards packets on 1 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 Logical Core 6 (socket 0) forwards packets on 1 streams: RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 Logical Core 7 (socket 0) forwards packets on 1 streams: RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 Logical Core 8 (socket 0) forwards packets on 1 streams: RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00
Notice: number of rxq and txq in running cmdline are greater than used nb_tc.
For the DCB forwarding test, each core is assigned on each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value can be adjusted based on 'total_tc_num' in all forwarding ports.
In addition, after operation of port stop/port start, forwarding configuration is changed by rss_fwd_config_setup. In "start_port" function, dcb_test is set to 1 based on dcb_config. So it also should be cleared when dcb_config is 0.
Fixes: 900550de04a7 ("app/testpmd: add dcb support") 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 <oulijun@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ app/test-pmd/testpmd.c | 12 +++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 576d5ac..c89f8cd 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2864,6 +2864,21 @@ rss_fwd_config_setup(void) } }
+static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +}
It's only 3 lines. Is it really necessary to make it into a function which is not called by anyone else? A comment and the loop are enough.
To calculate the total number of TCs on all forwarding ports, we have to call rte_eth_dev_get_dcb_info(). However, rte_eth_dev_get_dcb_info() has been called twice by dcb_fwd_config_setup() to setup dcb forwarding configuration. The dcb_fwd_config_setup() might be more readable if encapsulated, I think. In addition, variables in "cur_fwd_config" are initialized more centrally. What do you think?
+ /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2883,12 +2898,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num = 0; uint8_t tc = 0;
cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num;
/* reinitialize forwarding streams */ init_fwd_streams(); diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 1a57324..7eb810b 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 22:18 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
在 2021/3/23 16:55, Li, Xiaoyun 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
From: Huisong Li <lihuisong@huawei.com>
The commit message is too long and redundant. Please simplify it.
<snip>
+static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +}
It's only 3 lines. Is it really necessary to make it into a function which is not called by anyone else? A comment and the loop are enough.
To calculate the total number of TCs on all forwarding ports, we have to call rte_eth_dev_get_dcb_info(). However, rte_eth_dev_get_dcb_info() has been called twice by dcb_fwd_config_setup() to setup dcb forwarding configuration. The dcb_fwd_config_setup() might be more readable if encapsulated, I think. In addition, variables in "cur_fwd_config" are initialized more centrally. What do you think?
I'm not familiar with dcb. Why does the rxp_dcb_info get from fwd_ports_ids[0] and txp_dcb_info get from fwd_ports_ids[1]? Is this a have-to-be? Can it be both from port 0 or last port? If it can be port 0 or last port, you can just store the last dcb_info after your loop (loop from 0, i++ or loop from last i--).
+ <snip> -- 2.7.4
.
On 3/23/2021 2:17 PM, oulijun wrote:
在 2021/3/23 16:55, Li, Xiaoyun 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
From: Huisong Li <lihuisong@huawei.com>
The commit message is too long and redundant. Please simplify it.
Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is lack for dcb_fwd_config_setup, which may lead to a bad events where multiple polling threads operate on the same queue. As a result, various possible exceptions will occur. The procedure is as follows:
startup testpmd app as follows: testpmd> port stop all testpmd> set nbcore 8 testpmd> port config 0 dcb vt off 4 pfc on port config all rxq 16 port testpmd> config all txq 16 port start all set fwd mac Logical Core 1 (socket 0) forwards packets on 4 streams: RX P=0/Q=0 (socket 0) -> TX P=0/Q=0 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=1 (socket 0) -> TX P=0/Q=1 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=2 (socket 0) -> TX P=0/Q=2 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=3 (socket 0) -> TX P=0/Q=3 (socket 0) peer=02:00:00:00:00:00 Logical Core 2 (socket 0) forwards packets on 4 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00 Logical Core 3 (socket 0) forwards packets on 4 streams: RX P=0/Q=8 (socket 0) -> TX P=0/Q=8 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=9 (socket 0) -> TX P=0/Q=9 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=10 (socket 0) -> TX P=0/Q=10 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=11 (socket 0) -> TX P=0/Q=11 (socket 0) peer=02:00:00:00:00:00 Logical Core 4 (socket 0) forwards packets on 4 streams: RX P=0/Q=12 (socket 0) -> TX P=0/Q=12 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=13 (socket 0) -> TX P=0/Q=13 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=14 (socket 0) -> TX P=0/Q=14 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=15 (socket 0) -> TX P=0/Q=15 (socket 0) peer=02:00:00:00:00:00 Logical Core 5 (socket 0) forwards packets on 1 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 Logical Core 6 (socket 0) forwards packets on 1 streams: RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 Logical Core 7 (socket 0) forwards packets on 1 streams: RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 Logical Core 8 (socket 0) forwards packets on 1 streams: RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00
Notice: number of rxq and txq in running cmdline are greater than used nb_tc.
For the DCB forwarding test, each core is assigned on each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value can be adjusted based on 'total_tc_num' in all forwarding ports.
In addition, after operation of port stop/port start, forwarding configuration is changed by rss_fwd_config_setup. In "start_port" function, dcb_test is set to 1 based on dcb_config. So it also should be cleared when dcb_config is 0.
Fixes: 900550de04a7 ("app/testpmd: add dcb support") 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 <oulijun@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ app/test-pmd/testpmd.c | 12 +++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 576d5ac..c89f8cd 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2864,6 +2864,21 @@ rss_fwd_config_setup(void) } }
+static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +}
It's only 3 lines. Is it really necessary to make it into a function which is not called by anyone else? A comment and the loop are enough.
To calculate the total number of TCs on all forwarding ports, we have to call rte_eth_dev_get_dcb_info(). However, rte_eth_dev_get_dcb_info() has been called twice by dcb_fwd_config_setup() to setup dcb forwarding configuration. The dcb_fwd_config_setup() might be more readable if encapsulated, I think. In addition, variables in "cur_fwd_config" are initialized more centrally. What do you think?
+1 to have the function, agree it makes more readable. But for the PMDs that doesn't support '.get_dcb_info' it will return 0, causing 'nb_fwd_lcores' to be zero, I think this should be prevented. Either can add a zero check here, or do the check during "port config # dcb ..." and prevent setting 'dcb_config' at first place for those pmds. I think second option is better, what do you think. Overall DCB support in the testpmd seems missing some corner cases and requires more update/fix.
+ /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2883,12 +2898,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num = 0; uint8_t tc = 0;
cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num;
/* reinitialize forwarding streams */ init_fwd_streams(); diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 1a57324..7eb810b 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
在 2021/4/10 9:12, Ferruh Yigit 写道:
On 3/23/2021 2:17 PM, oulijun wrote:
在 2021/3/23 16:55, Li, Xiaoyun 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
From: Huisong Li <lihuisong@huawei.com>
The commit message is too long and redundant. Please simplify it.
Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is lack for dcb_fwd_config_setup, which may lead to a bad events where multiple polling threads operate on the same queue. As a result, various possible exceptions will occur. The procedure is as follows:
startup testpmd app as follows: testpmd> port stop all testpmd> set nbcore 8 testpmd> port config 0 dcb vt off 4 pfc on port config all rxq 16 port testpmd> config all txq 16 port start all set fwd mac Logical Core 1 (socket 0) forwards packets on 4 streams: RX P=0/Q=0 (socket 0) -> TX P=0/Q=0 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=1 (socket 0) -> TX P=0/Q=1 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=2 (socket 0) -> TX P=0/Q=2 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=3 (socket 0) -> TX P=0/Q=3 (socket 0) peer=02:00:00:00:00:00 Logical Core 2 (socket 0) forwards packets on 4 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00 Logical Core 3 (socket 0) forwards packets on 4 streams: RX P=0/Q=8 (socket 0) -> TX P=0/Q=8 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=9 (socket 0) -> TX P=0/Q=9 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=10 (socket 0) -> TX P=0/Q=10 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=11 (socket 0) -> TX P=0/Q=11 (socket 0) peer=02:00:00:00:00:00 Logical Core 4 (socket 0) forwards packets on 4 streams: RX P=0/Q=12 (socket 0) -> TX P=0/Q=12 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=13 (socket 0) -> TX P=0/Q=13 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=14 (socket 0) -> TX P=0/Q=14 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=15 (socket 0) -> TX P=0/Q=15 (socket 0) peer=02:00:00:00:00:00 Logical Core 5 (socket 0) forwards packets on 1 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 Logical Core 6 (socket 0) forwards packets on 1 streams: RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 Logical Core 7 (socket 0) forwards packets on 1 streams: RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 Logical Core 8 (socket 0) forwards packets on 1 streams: RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00
Notice: number of rxq and txq in running cmdline are greater than used nb_tc.
For the DCB forwarding test, each core is assigned on each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value can be adjusted based on 'total_tc_num' in all forwarding ports.
In addition, after operation of port stop/port start, forwarding configuration is changed by rss_fwd_config_setup. In "start_port" function, dcb_test is set to 1 based on dcb_config. So it also should be cleared when dcb_config is 0.
Fixes: 900550de04a7 ("app/testpmd: add dcb support") 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 <oulijun@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ app/test-pmd/testpmd.c | 12 +++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 576d5ac..c89f8cd 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2864,6 +2864,21 @@ rss_fwd_config_setup(void) } }
+static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +}
It's only 3 lines. Is it really necessary to make it into a function which is not called by anyone else? A comment and the loop are enough.
To calculate the total number of TCs on all forwarding ports, we have to call rte_eth_dev_get_dcb_info(). However, rte_eth_dev_get_dcb_info() has been called twice by dcb_fwd_config_setup() to setup dcb forwarding configuration. The dcb_fwd_config_setup() might be more readable if encapsulated, I think. In addition, variables in "cur_fwd_config" are initialized more centrally. What do you think?
+1 to have the function, agree it makes more readable.
But for the PMDs that doesn't support '.get_dcb_info' it will return 0, causing 'nb_fwd_lcores' to be zero, I think this should be prevented. Either can add a zero check here, or do the check during "port config # dcb ..." and prevent setting 'dcb_config' at first place for those pmds. I think second option is better, what do you think.
Yes, the second one is the source.
Overall DCB support in the testpmd seems missing some corner cases and requires more update/fix.
In testpmd, it seems that the code has not been updated for a long time. I don't know how other dcb-enabled drivers are tested and how they should be used. Do they not apply this method to test the DCB?
+ /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2883,12 +2898,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num = 0; uint8_t tc = 0;
cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num;
/* reinitialize forwarding streams */ init_fwd_streams(); diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 1a57324..7eb810b 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
.
在 2021/3/23 16:55, Li, Xiaoyun 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
From: Huisong Li <lihuisong@huawei.com>
The commit message is too long and redundant. Please simplify it.
Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is lack for dcb_fwd_config_setup, which may lead to a bad events where multiple polling threads operate on the same queue. As a result, various possible exceptions will occur. The procedure is as follows:
startup testpmd app as follows: testpmd> port stop all testpmd> set nbcore 8 testpmd> port config 0 dcb vt off 4 pfc on port config all rxq 16 port testpmd> config all txq 16 port start all set fwd mac Logical Core 1 (socket 0) forwards packets on 4 streams: RX P=0/Q=0 (socket 0) -> TX P=0/Q=0 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=1 (socket 0) -> TX P=0/Q=1 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=2 (socket 0) -> TX P=0/Q=2 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=3 (socket 0) -> TX P=0/Q=3 (socket 0) peer=02:00:00:00:00:00 Logical Core 2 (socket 0) forwards packets on 4 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00 Logical Core 3 (socket 0) forwards packets on 4 streams: RX P=0/Q=8 (socket 0) -> TX P=0/Q=8 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=9 (socket 0) -> TX P=0/Q=9 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=10 (socket 0) -> TX P=0/Q=10 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=11 (socket 0) -> TX P=0/Q=11 (socket 0) peer=02:00:00:00:00:00 Logical Core 4 (socket 0) forwards packets on 4 streams: RX P=0/Q=12 (socket 0) -> TX P=0/Q=12 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=13 (socket 0) -> TX P=0/Q=13 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=14 (socket 0) -> TX P=0/Q=14 (socket 0) peer=02:00:00:00:00:00 RX P=0/Q=15 (socket 0) -> TX P=0/Q=15 (socket 0) peer=02:00:00:00:00:00 Logical Core 5 (socket 0) forwards packets on 1 streams: RX P=0/Q=4 (socket 0) -> TX P=0/Q=4 (socket 0) peer=02:00:00:00:00:00 Logical Core 6 (socket 0) forwards packets on 1 streams: RX P=0/Q=5 (socket 0) -> TX P=0/Q=5 (socket 0) peer=02:00:00:00:00:00 Logical Core 7 (socket 0) forwards packets on 1 streams: RX P=0/Q=6 (socket 0) -> TX P=0/Q=6 (socket 0) peer=02:00:00:00:00:00 Logical Core 8 (socket 0) forwards packets on 1 streams: RX P=0/Q=7 (socket 0) -> TX P=0/Q=7 (socket 0) peer=02:00:00:00:00:00
Notice: number of rxq and txq in running cmdline are greater than used nb_tc.
For the DCB forwarding test, each core is assigned on each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value can be adjusted based on 'total_tc_num' in all forwarding ports.
In addition, after operation of port stop/port start, forwarding configuration is changed by rss_fwd_config_setup. In "start_port" function, dcb_test is set to 1 based on dcb_config. So it also should be cleared when dcb_config is 0.
Fixes: 900550de04a7 ("app/testpmd: add dcb support") 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 <oulijun@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ app/test-pmd/testpmd.c | 12 +++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 576d5ac..c89f8cd 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2864,6 +2864,21 @@ rss_fwd_config_setup(void) } }
+static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +}
It's only 3 lines. Is it really necessary to make it into a function which is not called by anyone else? A comment and the loop are enough.
+ /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2883,12 +2898,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num = 0; uint8_t tc = 0;
cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num;
/* reinitialize forwarding streams */ init_fwd_streams(); diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 1a57324..7eb810b 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
Yes, I think. The forwarding streams should not be changed from "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is configured. But, now, the logical codes do it when stopping ports and then starting ports.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
As far as I know, the current testpmd only supports switching from the normal mode to the dcb mode, but does not support the reverse operation. And " dcb_config" is set to 1, and then "dcb_test" is set to 1 after config.
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 22:19 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
<snip>
@@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
Yes, I think. The forwarding streams should not be changed from "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is configured. But, now, the logical codes do it when stopping ports and then starting ports.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
As far as I know, the current testpmd only supports switching from the normal mode to the dcb mode, but does not support the reverse operation. And " dcb_config" is set to 1, and then "dcb_test" is set to 1 after config.
You're not answering my questions. Why are you changing the behavior of testpmd? Your change will make testpmd stay dcb mode once set dcb mode and can't go back to normal mode. If users want to go back to normal mode, he/she has to restart the whole testpmd. It worked as you can set dcb mode and start port. After stopping port, if you still want dcb mode, you need to set dcb mode command again. But at least the old way won't break anything. @Yigit, Ferruh Not sure which behavior is better, what do you think? And @oulijun can you just answer all comments in one thread?
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
在 2021/3/24 10:03, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 22:19 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
<snip>
@@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
Yes, I think. The forwarding streams should not be changed from "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is configured. But, now, the logical codes do it when stopping ports and then starting ports.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
As far as I know, the current testpmd only supports switching from the normal mode to the dcb mode, but does not support the reverse operation. And " dcb_config" is set to 1, and then "dcb_test" is set to 1 after config.
You're not answering my questions. Why are you changing the behavior of testpmd? Your change will make testpmd stay dcb mode once set dcb mode and can't go back to normal mode. If users want to go back to normal mode, he/she has to restart the whole testpmd.
Yes. Testpmd and PMD driver are both in dcb mode after dcb info is configured. In my opinion, the 'dcb_test' flag can't be clear to go back to normal mode after stopping port and then starting port. Because PMD driver is still dcb mode. If users want to go back it, users should disable dcb mode and enable RSS or other mode.
It worked as you can set dcb mode and start port. After stopping port, if you still want dcb mode, you need to set dcb mode command again. But at least the old way won't break anything. @Yigit, Ferruh Not sure which behavior is better, what do you think?
And @oulijun can you just answer all comments in one thread?
After stopping port, the 'dcb_test' flag is clear. At this moment, the dcb configuration in testpmd has not be changed. users may not understand why the DCB mode needs to be reconfigured.
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
Linuxarm mailing list -- linuxarm@openeuler.org To unsubscribe send an email to linuxarm-leave@openeuler.org
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 21:40 To: linuxarm@openeuler.org; Li, Xiaoyun <xiaoyun.li@intel.com>; dev <dev@dpdk.org> Subject: Re: [Linuxarm] Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
在 2021/3/24 10:03, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 22:19 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
<snip>
@@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
Yes, I think. The forwarding streams should not be changed from "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is
configured.
But, now, the logical codes do it when stopping ports and then starting ports.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
As far as I know, the current testpmd only supports switching from the normal mode to the dcb mode, but does not support the reverse operation. And " dcb_config" is set to 1, and then "dcb_test" is set to 1 after config.
You're not answering my questions. Why are you changing the behavior of testpmd? Your change will make testpmd stay dcb mode once set dcb mode and can't go back to normal mode. If users want to go back to normal mode, he/she has to restart the whole testpmd.
Yes. Testpmd and PMD driver are both in dcb mode after dcb info is configured. In my opinion, the 'dcb_test' flag can't be clear to go back to normal mode after stopping port and then starting port. Because PMD driver is still dcb mode. If users want to go back it, users should disable dcb mode and enable RSS or other mode.
It worked as you can set dcb mode and start port. After stopping port, if you still want dcb mode, you need to set dcb mode command again. But at least the old way won't break anything. @Yigit, Ferruh Not sure which behavior is better, what do you think?
And @oulijun can you just answer all comments in one thread?
After stopping port, the 'dcb_test' flag is clear. At this moment, the dcb configuration in testpmd has not be changed. users may not understand why the DCB mode needs to be reconfigured.
OK. You're right. There's no place writing port->dcb_flag back to 0.
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
Linuxarm mailing list -- linuxarm@openeuler.org To unsubscribe send an email to linuxarm-leave@openeuler.org
在 2021/3/25 10:21, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 21:40 To: linuxarm@openeuler.org; Li, Xiaoyun <xiaoyun.li@intel.com>; dev <dev@dpdk.org> Subject: Re: [Linuxarm] Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
在 2021/3/24 10:03, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 22:19 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
<snip>
@@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
Yes, I think. The forwarding streams should not be changed from "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is
configured.
But, now, the logical codes do it when stopping ports and then starting ports.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
As far as I know, the current testpmd only supports switching from the normal mode to the dcb mode, but does not support the reverse operation. And " dcb_config" is set to 1, and then "dcb_test" is set to 1 after config.
You're not answering my questions. Why are you changing the behavior of testpmd? Your change will make testpmd stay dcb mode once set dcb mode and can't go back to normal mode. If users want to go back to normal mode, he/she has to restart the whole testpmd.
Yes. Testpmd and PMD driver are both in dcb mode after dcb info is configured. In my opinion, the 'dcb_test' flag can't be clear to go back to normal mode after stopping port and then starting port. Because PMD driver is still dcb mode. If users want to go back it, users should disable dcb mode and enable RSS or other mode.
It worked as you can set dcb mode and start port. After stopping port, if you still want dcb mode, you need to set dcb mode command again. But at least the old way won't break anything. @Yigit, Ferruh Not sure which behavior is better, what do you think?
And @oulijun can you just answer all comments in one thread?
After stopping port, the 'dcb_test' flag is clear. At this moment, the dcb configuration in testpmd has not be changed. users may not understand why the DCB mode needs to be reconfigured.
OK. You're right. There's no place writing port->dcb_flag back to 0.
Yes. Unless testpmd supports switching from DCB mode to other modes. Do you have any other suggestions about this patch set, xiaoyun?
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
Linuxarm mailing list -- linuxarm@openeuler.org To unsubscribe send an email to linuxarm-leave@openeuler.org
On 3/25/2021 2:21 AM, Li, Xiaoyun wrote:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 21:40 To: linuxarm@openeuler.org; Li, Xiaoyun <xiaoyun.li@intel.com>; dev <dev@dpdk.org> Subject: Re: [Linuxarm] Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
在 2021/3/24 10:03, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 22:19 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
<snip>
@@ -2707,14 +2707,16 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi;
- if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return;
+ /* + * In "start_port" function, dcb_test is set to 1 based on dcb_config. + * So it should be cleared when dcb_config is 0. + */ + if (dcb_config == 0) + dcb_test = 0; +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
Yes, I think. The forwarding streams should not be changed from "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is
configured.
But, now, the logical codes do it when stopping ports and then starting ports.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
As far as I know, the current testpmd only supports switching from the normal mode to the dcb mode, but does not support the reverse operation. And " dcb_config" is set to 1, and then "dcb_test" is set to 1 after config.
You're not answering my questions. Why are you changing the behavior of testpmd? Your change will make testpmd stay dcb mode once set dcb mode and can't go back to normal mode. If users want to go back to normal mode, he/she has to restart the whole testpmd.
Yes. Testpmd and PMD driver are both in dcb mode after dcb info is configured. In my opinion, the 'dcb_test' flag can't be clear to go back to normal mode after stopping port and then starting port. Because PMD driver is still dcb mode. If users want to go back it, users should disable dcb mode and enable RSS or other mode.
It worked as you can set dcb mode and start port. After stopping port, if you still want dcb mode, you need to set dcb mode command again. But at least the old way won't break anything. @Yigit, Ferruh Not sure which behavior is better, what do you think?
And @oulijun can you just answer all comments in one thread?
After stopping port, the 'dcb_test' flag is clear. At this moment, the dcb configuration in testpmd has not be changed. users may not understand why the DCB mode needs to be reconfigured.
OK. You're right. There's no place writing port->dcb_flag back to 0.
If there is no way to turn off the DCB, isn't this code logically dead? This code is run when "dcb_config == 0" but in that case 'dcb_test' is already 0. If so what is the point of the code, can it be removed? btw, do you know what 'dcb_test' does at all? It seems it is set when 'dcb_config' set, and reset when 'dcb_config' reset.
printf("Stopping ports...\n");
RTE_ETH_FOREACH_DEV(pi) { -- 2.7.4
.
Linuxarm mailing list -- linuxarm@openeuler.org To unsubscribe send an email to linuxarm-leave@openeuler.org
在 2021/4/10 8:56, Ferruh Yigit 写道:
On 3/25/2021 2:21 AM, Li, Xiaoyun wrote:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 21:40 To: linuxarm@openeuler.org; Li, Xiaoyun <xiaoyun.li@intel.com>; dev <dev@dpdk.org> Subject: Re: [Linuxarm] Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
在 2021/3/24 10:03, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 22:19 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
<snip>
> @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) > portid_t peer_pl[RTE_MAX_ETHPORTS]; > int peer_pi; > > - if (dcb_test) { > - dcb_test = 0; > - dcb_config = 0; > - } > - > if (port_id_is_invalid(pid, ENABLED_WARN)) > return; > > + /* > + * In "start_port" function, dcb_test is set to 1 based on > dcb_config. > + * So it should be cleared when dcb_config is 0. > + */ > + if (dcb_config == 0) > + dcb_test = 0; > +
I don't understand why are you changing this. dcb_test will only be set when dcb_config is 1 when starting ports. And both dcb_test and dcb_config will be cleared when stopping ports. So dcb will only affect when you set port dcb and then start port and when stop port dcb will be cleared.
Yes, I think. The forwarding streams should not be changed from "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is
configured.
But, now, the logical codes do it when stopping ports and then starting ports.
So what's the problem of original codes?
Your change will cause issues that there's no place to set dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd.
As far as I know, the current testpmd only supports switching from the normal mode to the dcb mode, but does not support the reverse operation. And " dcb_config" is set to 1, and then "dcb_test" is set to 1 after config.
You're not answering my questions. Why are you changing the behavior of testpmd? Your change will make testpmd stay dcb mode once set dcb mode and can't go back to normal mode. If users want to go back to normal mode, he/she has to restart the whole testpmd.
Yes. Testpmd and PMD driver are both in dcb mode after dcb info is configured. In my opinion, the 'dcb_test' flag can't be clear to go back to normal mode after stopping port and then starting port. Because PMD driver is still dcb mode. If users want to go back it, users should disable dcb mode and enable RSS or other mode.
It worked as you can set dcb mode and start port. After stopping port, if you still want dcb mode, you need to set dcb mode command again. But at least the old way won't break anything. @Yigit, Ferruh Not sure which behavior is better, what do you think?
And @oulijun can you just answer all comments in one thread?
After stopping port, the 'dcb_test' flag is clear. At this moment, the dcb configuration in testpmd has not be changed. users may not understand why the DCB mode needs to be reconfigured.
OK. You're right. There's no place writing port->dcb_flag back to 0.
If there is no way to turn off the DCB, isn't this code logically dead? This code is run when "dcb_config == 0" but in that case 'dcb_test' is already 0. If so what is the point of the code, can it be removed?
yes. Currently, it is a dead logic unless testpmd adds support for switching from DCB mode to other modes. I think dcb_test is a flag at the packet forwarding layer in testpmd. The original purpose is to indicate that the current forwarding test is being performed in DCB mode.
btw, do you know what 'dcb_test' does at all? It seems it is set when 'dcb_config' set, and reset when 'dcb_config' reset.
In my opinion, it is only a global flag. During a DCB test, DCB must be configured on all forwarding ports.
> printf("Stopping ports...\n"); > > RTE_ETH_FOREACH_DEV(pi) { > -- > 2.7.4
.
Linuxarm mailing list -- linuxarm@openeuler.org To unsubscribe send an email to linuxarm-leave@openeuler.org
.
On 4/10/2021 3:58 AM, oulijun wrote:
在 2021/4/10 8:56, Ferruh Yigit 写道:
On 3/25/2021 2:21 AM, Li, Xiaoyun wrote:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 21:40 To: linuxarm@openeuler.org; Li, Xiaoyun <xiaoyun.li@intel.com>; dev <dev@dpdk.org> Subject: Re: [Linuxarm] Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
在 2021/3/24 10:03, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 22:19 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
<snip>
>> @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) >> portid_t peer_pl[RTE_MAX_ETHPORTS]; >> int peer_pi; >> >> - if (dcb_test) { >> - dcb_test = 0; >> - dcb_config = 0; >> - } >> - >> if (port_id_is_invalid(pid, ENABLED_WARN)) >> return; >> >> + /* >> + * In "start_port" function, dcb_test is set to 1 based on dcb_config. >> + * So it should be cleared when dcb_config is 0. >> + */ >> + if (dcb_config == 0) >> + dcb_test = 0; >> + > > I don't understand why are you changing this. > dcb_test will only be set when dcb_config is 1 when starting ports. > And both dcb_test and dcb_config will be cleared when stopping ports. > So dcb will only affect when you set port dcb and then start port > and when stop port dcb will be cleared. > Yes, I think. The forwarding streams should not be changed from "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is
configured.
But, now, the logical codes do it when stopping ports and then starting ports. > So what's the problem of original codes? > > Your change will cause issues that there's no place to set > dcb_config as 0. If you config dcb, then it'll be always dcb mode unless restart the whole testpmd. > As far as I know, the current testpmd only supports switching from the normal mode to the dcb mode, but does not support the reverse operation. And " dcb_config" is set to 1, and then "dcb_test" is set to 1 after config.
You're not answering my questions. Why are you changing the behavior of testpmd? Your change will make testpmd stay dcb mode once set dcb mode and can't go back to normal mode. If users want to go back to normal mode, he/she has to restart the whole testpmd.
Yes. Testpmd and PMD driver are both in dcb mode after dcb info is configured. In my opinion, the 'dcb_test' flag can't be clear to go back to normal mode after stopping port and then starting port. Because PMD driver is still dcb mode. If users want to go back it, users should disable dcb mode and enable RSS or other mode.
It worked as you can set dcb mode and start port. After stopping port, if you still want dcb mode, you need to set dcb mode command again. But at least the old way won't break anything. @Yigit, Ferruh Not sure which behavior is better, what do you think?
And @oulijun can you just answer all comments in one thread?
After stopping port, the 'dcb_test' flag is clear. At this moment, the dcb configuration in testpmd has not be changed. users may not understand why the DCB mode needs to be reconfigured.
OK. You're right. There's no place writing port->dcb_flag back to 0.
If there is no way to turn off the DCB, isn't this code logically dead? This code is run when "dcb_config == 0" but in that case 'dcb_test' is already 0. If so what is the point of the code, can it be removed?
yes. Currently, it is a dead logic unless testpmd adds support for switching from DCB mode to other modes. I think dcb_test is a flag at the packet forwarding layer in testpmd. The original purpose is to indicate that the current forwarding test is being performed in DCB mode.
can 'dcb_config' be sufficient on its own for this? so that we can remove 'dcb_test' and simplify the logic.
btw, do you know what 'dcb_test' does at all? It seems it is set when 'dcb_config' set, and reset when 'dcb_config' reset.
In my opinion, it is only a global flag. During a DCB test, DCB must be configured on all forwarding ports.
>> printf("Stopping ports...\n"); >> >> RTE_ETH_FOREACH_DEV(pi) { >> -- >> 2.7.4 > > . >
Linuxarm mailing list -- linuxarm@openeuler.org To unsubscribe send an email to linuxarm-leave@openeuler.org
.
在 2021/4/12 15:52, Ferruh Yigit 写道:
On 4/10/2021 3:58 AM, oulijun wrote:
在 2021/4/10 8:56, Ferruh Yigit 写道:
On 3/25/2021 2:21 AM, Li, Xiaoyun wrote:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 21:40 To: linuxarm@openeuler.org; Li, Xiaoyun <xiaoyun.li@intel.com>; dev <dev@dpdk.org> Subject: Re: [Linuxarm] Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test
在 2021/3/24 10:03, Li, Xiaoyun 写道:
> -----Original Message----- > From: oulijun <oulijun@huawei.com> > Sent: Tuesday, March 23, 2021 22:19 > To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh > <ferruh.yigit@intel.com> > Cc: dev@dpdk.org; linuxarm@openeuler.org > Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration > when DCB test > <snip> >>> @@ -2707,14 +2707,16 @@ stop_port(portid_t pid) >>> portid_t peer_pl[RTE_MAX_ETHPORTS]; >>> int peer_pi; >>> >>> - if (dcb_test) { >>> - dcb_test = 0; >>> - dcb_config = 0; >>> - } >>> - >>> if (port_id_is_invalid(pid, ENABLED_WARN)) >>> return; >>> >>> + /* >>> + * In "start_port" function, dcb_test is set to 1 based >>> on dcb_config. >>> + * So it should be cleared when dcb_config is 0. >>> + */ >>> + if (dcb_config == 0) >>> + dcb_test = 0; >>> + >> >> I don't understand why are you changing this. >> dcb_test will only be set when dcb_config is 1 when starting >> ports. >> And both > dcb_test and dcb_config will be cleared when stopping ports. >> So dcb will only affect when you set port dcb and then start port >> and when > stop port dcb will be cleared. >> > Yes, I think. The forwarding streams should not be changed from > "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is
configured.
> But, now, the logical codes do it when stopping ports and then > starting ports. >> So what's the problem of original codes? >> >> Your change will cause issues that there's no place to set >> dcb_config as 0. If > you config dcb, then it'll be always dcb mode unless restart the > whole testpmd. >> > As far as I know, the current testpmd only supports switching from > the normal mode to the dcb mode, but does not support the reverse operation. > And " dcb_config" is set to 1, and then "dcb_test" is set to 1 > after > config.
You're not answering my questions. Why are you changing the behavior of testpmd? Your change will make testpmd stay dcb mode once set dcb mode and can't go back to normal mode. If users want to go back to normal mode, he/she has to restart the whole testpmd.
Yes. Testpmd and PMD driver are both in dcb mode after dcb info is configured. In my opinion, the 'dcb_test' flag can't be clear to go back to normal mode after stopping port and then starting port. Because PMD driver is still dcb mode. If users want to go back it, users should disable dcb mode and enable RSS or other mode.
It worked as you can set dcb mode and start port. After stopping port, if you still want dcb mode, you need to set dcb mode command again. But at least the old way won't break anything. @Yigit, Ferruh Not sure which behavior is better, what do you think?
And @oulijun can you just answer all comments in one thread?
After stopping port, the 'dcb_test' flag is clear. At this moment, the dcb configuration in testpmd has not be changed. users may not understand why the DCB mode needs to be reconfigured.
OK. You're right. There's no place writing port->dcb_flag back to 0.
If there is no way to turn off the DCB, isn't this code logically dead? This code is run when "dcb_config == 0" but in that case 'dcb_test' is already 0. If so what is the point of the code, can it be removed?
yes. Currently, it is a dead logic unless testpmd adds support for switching from DCB mode to other modes. I think dcb_test is a flag at the packet forwarding layer in testpmd. The original purpose is to indicate that the current forwarding test is being performed in DCB mode.
can 'dcb_config' be sufficient on its own for this? so that we can remove 'dcb_test' and simplify the logic.
I agree with your proposal. Because the 'dcb_flag' field in struct rte_port indicates whether the current port is DCB, It is sufficient to have 'dcb_config' as a global variable to control the DCB test status. ok，I will send V2 to update all of the previous discussions.
btw, do you know what 'dcb_test' does at all? It seems it is set when 'dcb_config' set, and reset when 'dcb_config' reset.
In my opinion, it is only a global flag. During a DCB test, DCB must be configured on all forwarding ports.
>>> printf("Stopping ports...\n"); >>> >>> RTE_ETH_FOREACH_DEV(pi) { >>> -- >>> 2.7.4 >> >> . >> _______________________________________________ Linuxarm mailing list -- linuxarm@openeuler.org To unsubscribe send an email to linuxarm-leave@openeuler.org
.
.
From: Huisong Li <lihuisong@huawei.com> 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. 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. 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 <oulijun@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
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; 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.
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.
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.
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 <oulijun@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
在 2021/3/23 15:50, Li, Xiaoyun 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; 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.
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?
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 <oulijun@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
.
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 09:01 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> 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 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; 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.
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. 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?
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 <oulijun@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
.
在 2021/3/24 9:44, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 09:01 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> 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 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; 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.
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.
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 <oulijun@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
.
.
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Thursday, March 25, 2021 11:04 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@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 <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 09:01 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> 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 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; 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.
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.
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 <oulijun@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
.
.
在 2021/3/29 9:53, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Thursday, March 25, 2021 11:04 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@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 <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 09:01 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> 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 写道:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; 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.
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: stable@dpdk.org
Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Lijun Ou <oulijun@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
.
.
.
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Friday, April 2, 2021 09:45 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [PATCH 2/3] app/testpmd: remove forwarding config from parsing Rx and Tx
在 2021/3/29 9:53, Li, Xiaoyun 写道:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Thursday, March 25, 2021 11:04 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@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 <oulijun@huawei.com> Sent: Wednesday, March 24, 2021 09:01 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> 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 写道:
Hi
> -----Original Message----- > From: Lijun Ou <oulijun@huawei.com> > Sent: Friday, March 5, 2021 18:22 > To: Yigit, Ferruh <ferruh.yigit@intel.com> > Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; 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.
> 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.
NO. I40e and ixgbe DON'T have the issue. I40e tc mapping is done in dev_configure. ixgbe tc mapping is always fixed due to hw design. It only needs dev_configure to get dcb_conf info. No matter you configure dcb or not, the mapping is always the same. You can check ixgbe_dev_get_dcb_info(). So for i40e and ixgbe, you only need to add dev_configure in dcb_fwd_config_setup().
> > 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 <oulijun@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
.
.
.
On 3/5/2021 10:22 AM, Lijun Ou wrote:
From: Huisong Li <lihuisong@huawei.com>
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.
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.
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.
Hi Lijun, Xiaoyun, I read the thread and checked the code, but still I am not completely clear with the dcp implementation in testpmd. But the "dcb_config = 0" in the 'stop_port()' that has been removed in the patch 1/3 seems preventing these kind of config combination, with implicitly resetting the dcb feature with port stop. What do you think just have the number of forwarding cores adjustment change in the patch 1/3, and keep the 'stop_port()' as it is, will it solve this problem, and how acceptable it will be do you think?
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 <oulijun@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);
在 2021/4/12 18:35, Ferruh Yigit 写道:
On 3/5/2021 10:22 AM, Lijun Ou wrote:
From: Huisong Li <lihuisong@huawei.com>
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.
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.
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.
Hi Lijun, Xiaoyun,
I read the thread and checked the code, but still I am not completely clear with the dcp implementation in testpmd.
But the "dcb_config = 0" in the 'stop_port()' that has been removed in the patch 1/3 seems preventing these kind of config combination, with implicitly resetting the dcb feature with port stop.
What do you think just have the number of forwarding cores adjustment change in the patch 1/3, and keep the 'stop_port()' as it is, will it solve this problem, and how acceptable it will be do you think?
I think it might be more appropriate to follow the discussion with Xiao Yun, since it does have problems. In addition, I found another problem. Before calling dcb_fwd_config_setup(), testpmd must ensure that all ports have been configured with dcb. Let's look at the next V3.
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 <oulijun@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);
.
From: Hongbo Zheng <zhenghongbo3@huawei.com> In testpmd, when we input "show config rxtx", we can see like this: 1: testpmd> show config rxtx 2: io packet forwarding packets/burst=32 3: nb forwarding cores=1 - nb forwarding ports=1 4: port 0: RX queue number: 1 Tx queue number: 1 5: Rx offloads=0x0 Tx offloads=0x10000 6: RX queue: 0 7: RX desc=1024 - RX free threshold=32 8: RX threshold registers: pthresh=0 hthresh=0 wthresh=0 9: RX Offloads=0x0 10: TX queue: 0 11: TX desc=1024 - TX free threshold=928 12: TX threshold registers: pthresh=0 hthresh=0 wthresh=0 13: TX offloads=0x10000 - TX RS bit threshold=32 We can see RX/Rx/TX/Tx is mixed used. Also in other places in testpmd, RX/Rx/TX/Tx is mixed used. This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to unified use Rx/Tx. Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: - fix all RX/TX - rename patch title --- app/test-pmd/cmdline.c | 104 ++++++++++++++++++------------------ app/test-pmd/config.c | 128 ++++++++++++++++++++++----------------------- app/test-pmd/csumonly.c | 22 ++++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++---- app/test-pmd/parameters.c | 50 +++++++++--------- app/test-pmd/testpmd.c | 120 +++++++++++++++++++++--------------------- app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index e316f5c..f33fa2f 100644 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c @@ -182,16 +182,16 @@ static void cmd_help_long_parsed(void *parsed_result, " Clear information for port_id, or all.\n\n" "show (rxq|txq) info (port_id) (queue_id)\n" - " Display information for configured RX/TX queue.\n\n" + " Display information for configured Rx/Tx queue.\n\n" "show config (rxtx|cores|fwd|rxoffs|rxpkts|txpkts)\n" " Display the given configuration.\n\n" "read rxd (port_id) (queue_id) (rxd_id)\n" - " Display an RX descriptor of a port RX queue.\n\n" + " Display an Rx descriptor of a port Rx queue.\n\n" "read txd (port_id) (queue_id) (txd_id)\n" - " Display a TX descriptor of a port TX queue.\n\n" + " Display a Tx descriptor of a port Tx queue.\n\n" "ddp get list (port_id)\n" " Get ddp profile info list\n\n" @@ -245,7 +245,7 @@ static void cmd_help_long_parsed(void *parsed_result, " Show general information about devices probed.\n\n" "show port (port_id) rxq|txq (queue_id) desc (desc_id) status" - " Show status of rx|tx descriptor.\n\n" + " Show status of Rx|Tx descriptor.\n\n" "show port (port_id) rxq (queue_id) desc used count\n" " Show current number of filled receive" @@ -316,7 +316,7 @@ static void cmd_help_long_parsed(void *parsed_result, " and optionally CSUM packets.\n\n" "set txsplit (off|on|rand)\n" - " Set the split policy for the TX packets." + " Set the split policy for the Tx packets." " Right now only applicable for CSUM and TXONLY" " modes\n\n" @@ -334,7 +334,7 @@ static void cmd_help_long_parsed(void *parsed_result, " Select how attached port is retrieved for setup.\n\n" "set tx loopback (port_id) (on|off)\n" - " Enable or disable tx loopback.\n\n" + " Enable or disable Tx loopback.\n\n" "set all queues drop (port_id) (on|off)\n" " Set drop enable bit for all queues.\n\n" @@ -424,7 +424,7 @@ static void cmd_help_long_parsed(void *parsed_result, "depends on the number of VLAN IDs) in packets sent on a port.\n\n" "tx_vlan set pvid port_id vlan_id (on|off)\n" - " Set port based TX VLAN insertion.\n\n" + " Set port based Tx VLAN insertion.\n\n" "tx_vlan reset (port_id)\n" " Disable hardware insertion of a VLAN header in" @@ -444,11 +444,11 @@ static void cmd_help_long_parsed(void *parsed_result, "csum parse-tunnel (on|off) (tx_port_id)\n" " If disabled, treat tunnel packets as non-tunneled" " packets (treat inner headers as payload). The port\n" - " argument is the port used for TX in csum forward" + " argument is the port used for Tx in csum forward" " engine.\n\n" "csum show (port_id)\n" - " Display tx checksum offload configuration\n\n" + " Display Tx checksum offload configuration\n\n" "tso set (segsize) (portid)\n" " Enable TCP Segmentation Offload in csum forward" @@ -536,7 +536,7 @@ static void cmd_help_long_parsed(void *parsed_result, " port.\n\n" "set stat_qmap (tx|rx) (port_id) (queue_id) (qmapping)\n" - " Set statistics mapping (qmapping 0..15) for RX/TX" + " Set statistics mapping (qmapping 0..15) for Rx/Tx" " queue on port.\n" " e.g., 'set stat_qmap rx 0 2 5' sets rx queue 2" " on port 0 to mapping 5.\n\n" @@ -549,7 +549,7 @@ static void cmd_help_long_parsed(void *parsed_result, " Set the option to enable measurement of CPU cycles.\n" "set record-burst-stats on|off\n" - " Set the option to enable display of RX and TX bursts.\n" + " Set the option to enable display of Rx and Tx bursts.\n" "set port (port_id) vf (vf_id) rx|tx on|off\n" " Enable/Disable a VF receive/tranmit from a port\n\n" @@ -588,7 +588,7 @@ static void cmd_help_long_parsed(void *parsed_result, " Reset a mirror rule.\n\n" "set flush_rx (on|off)\n" - " Flush (default) or don't flush RX streams before" + " Flush (default) or don't flush Rx streams before" " forwarding. Mainly used with PCAP drivers.\n\n" "set bypass mode (normal|bypass|isolate) (port_id)\n" @@ -792,7 +792,7 @@ static void cmd_help_long_parsed(void *parsed_result, " Set the max LRO aggregated packet size.\n\n" "port config all drop-en (on|off)\n" - " Enable or disable packet drop on all RX queues of all ports when no " + " Enable or disable packet drop on all Rx queues of all ports when no " "receive buffers available.\n\n" "port config all rss (all|default|ip|tcp|udp|sctp|" @@ -813,29 +813,29 @@ static void cmd_help_long_parsed(void *parsed_result, "port config all (txpt|txht|txwt|rxpt|rxht|rxwt)" " (value)\n" " Set the ring prefetch/host/writeback threshold" - " for tx/rx queue.\n\n" + " for Tx/Rx queue.\n\n" "port config all (txfreet|txrst|rxfreet) (value)\n" - " Set free threshold for rx/tx, or set" - " tx rs bit threshold.\n\n" + " Set free threshold for Rx/Tx, or set" + " Tx rs bit threshold.\n\n" "port config mtu X value\n" " Set the MTU of port X to a given value\n\n" "port config (port_id) (rxq|txq) (queue_id) ring_size (value)\n" - " Set a rx/tx queue's ring size configuration, the new" + " Set a Rx/Tx queue's ring size configuration, the new" " value will take effect after command that (re-)start the port" " or command that setup the specific queue\n\n" "port (port_id) (rxq|txq) (queue_id) (start|stop)\n" - " Start/stop a rx/tx queue of port X. Only take effect" + " Start/stop a Rx/Tx queue of port X. Only take effect" " when port X is started\n\n" "port (port_id) (rxq|txq) (queue_id) deferred_start (on|off)\n" - " Switch on/off a deferred start of port X rx/tx queue. Only" + " Switch on/off a deferred start of port X Rx/Tx queue. Only" " take effect when port X is stopped.\n\n" "port (port_id) (rxq|txq) (queue_id) setup\n" - " Setup a rx/tx queue of port X.\n\n" + " Setup a Rx/Tx queue of port X.\n\n" "port config (port_id) pctype mapping reset\n" " Reset flow type to pctype mapping on a port\n\n" @@ -892,11 +892,11 @@ static void cmd_help_long_parsed(void *parsed_result, "bpf-load rx|tx (port) (queue) (J|M|B) (file_name)\n" " Load an eBPF program as a callback" - " for particular RX/TX queue\n\n" + " for particular Rx/Tx queue\n\n" "bpf-unload rx|tx (port) (queue)\n" " Unload previously loaded eBPF program" - " for particular RX/TX queue\n\n" + " for particular Rx/Tx queue\n\n" "port config (port_id) tx_metadata (value)\n" " Set Tx metadata value per port. Testpmd will add this value" @@ -1807,7 +1807,7 @@ cmd_config_rx_tx_parsed(void *parsed_result, } if (!strcmp(res->name, "rxq")) { if (!res->value && !nb_txq) { - printf("Warning: Either rx or tx queues should be non zero\n"); + printf("Warning: Either Rx or Tx queues should be non zero\n"); return; } if (check_nb_rxq(res->value) != 0) @@ -1816,7 +1816,7 @@ cmd_config_rx_tx_parsed(void *parsed_result, } else if (!strcmp(res->name, "txq")) { if (!res->value && !nb_rxq) { - printf("Warning: Either rx or tx queues should be non zero\n"); + printf("Warning: Either Rx or Tx queues should be non zero\n"); return; } if (check_nb_txq(res->value) != 0) @@ -2082,7 +2082,7 @@ cmdline_parse_inst_t cmd_config_mtu = { }, }; -/* *** configure rx mode *** */ +/* *** configure Rx mode *** */ struct cmd_config_rx_mode_flag { cmdline_fixed_string_t port; cmdline_fixed_string_t keyword; @@ -2496,7 +2496,7 @@ cmd_config_rxtx_ring_size_parsed(void *parsed_result, return; if (isrx && res->size != 0 && res->size <= rx_free_thresh) { - printf("Invalid rx ring_size, must > rx_free_thresh: %d\n", + printf("Invalid Rx ring_size, must > rx_free_thresh: %d\n", rx_free_thresh); return; } @@ -2779,10 +2779,10 @@ cmd_setup_rxtx_queue_parsed( } if (isrx && rx_queue_id_is_invalid(res->qid)) { - printf("Invalid rx queue\n"); + printf("Invalid Rx queue\n"); return; } else if (!isrx && tx_queue_id_is_invalid(res->qid)) { - printf("Invalid tx queue\n"); + printf("Invalid Tx queue\n"); return; } @@ -2794,7 +2794,7 @@ cmd_setup_rxtx_queue_parsed( mp = mbuf_pool_find(socket_id, 0); if (mp == NULL) { - printf("Failed to setup RX queue: " + printf("Failed to setup Rx queue: " "No mempool allocation" " on the socket %d\n", rxring_numa[res->portid]); @@ -2807,7 +2807,7 @@ cmd_setup_rxtx_queue_parsed( &port->rx_conf[res->qid], mp); if (ret) - printf("Failed to setup RX queue\n"); + printf("Failed to setup Rx queue\n"); } else { socket_id = txring_numa[res->portid]; if (!numa_support || socket_id == NUMA_NO_CONFIG) @@ -2819,7 +2819,7 @@ cmd_setup_rxtx_queue_parsed( socket_id, &port->tx_conf[res->qid]); if (ret) - printf("Failed to setup TX queue\n"); + printf("Failed to setup Tx queue\n"); } } @@ -3343,7 +3343,7 @@ cmdline_parse_inst_t cmd_config_burst = { }, }; -/* *** configure rx/tx queues *** */ +/* *** configure Rx/Tx queues *** */ struct cmd_config_thresh { cmdline_fixed_string_t port; cmdline_fixed_string_t keyword; @@ -4144,7 +4144,7 @@ cmdline_parse_inst_t cmd_vlan_offload = { .data = NULL, .help_str = "vlan set strip|filter|qinq_strip|extend|stripq on|off " "<port_id[,queue_id]>: " - "Strip/Filter/QinQ for rx side Extend for both rx/tx sides", + "Strip/Filter/QinQ for Rx side Extend for both Rx/Tx sides", .tokens = { (void *)&cmd_vlan_offload_vlan, (void *)&cmd_vlan_offload_set, @@ -4568,7 +4568,7 @@ cmd_config_queue_tx_offloads(struct rte_port *port) { int k; - /* Apply queue tx offloads configuration */ + /* Apply queue Tx offloads configuration */ for (k = 0; k < port->dev_info.max_rx_queues; k++) port->tx_conf[k].offloads = port->dev_conf.txmode.offloads; @@ -5369,7 +5369,7 @@ cmdline_parse_token_string_t cmd_setflushrx_mode = cmdline_parse_inst_t cmd_set_flush_rx = { .f = cmd_set_flush_rx_parsed, - .help_str = "set flush_rx on|off: Enable/Disable flush on rx streams", + .help_str = "set flush_rx on|off: Enable/Disable flush on Rx streams", .data = NULL, .tokens = { (void *)&cmd_setflushrx_set, @@ -6978,7 +6978,7 @@ cmdline_parse_inst_t cmd_link_flow_control_set_rx = { .f = cmd_link_flow_ctrl_set_parsed, .data = (void *)&cmd_link_flow_control_set_rx, .help_str = "set flow_ctrl rx on|off <port_id>: " - "Change rx flow control parameter", + "Change Rx flow control parameter", .tokens = { (void *)&cmd_lfc_set_set, (void *)&cmd_lfc_set_flow_ctrl, @@ -6993,7 +6993,7 @@ cmdline_parse_inst_t cmd_link_flow_control_set_tx = { .f = cmd_link_flow_ctrl_set_parsed, .data = (void *)&cmd_link_flow_control_set_tx, .help_str = "set flow_ctrl tx on|off <port_id>: " - "Change tx flow control parameter", + "Change Tx flow control parameter", .tokens = { (void *)&cmd_lfc_set_set, (void *)&cmd_lfc_set_flow_ctrl, @@ -7107,9 +7107,9 @@ cmd_link_flow_ctrl_set_parsed(void *parsed_result, int ret; /* - * Rx on/off, flow control is enabled/disabled on RX side. This can indicate + * Rx on/off, flow control is enabled/disabled on Rx side. This can indicate * the RTE_FC_TX_PAUSE, Transmit pause frame at the Rx side. - * Tx on/off, flow control is enabled/disabled on TX side. This can indicate + * Tx on/off, flow control is enabled/disabled on Tx side. This can indicate * the RTE_FC_RX_PAUSE, Respond to the pause frame at the Tx side. */ static enum rte_eth_fc_mode rx_tx_onoff_2_lfc_mode[2][2] = { @@ -7194,9 +7194,9 @@ cmd_priority_flow_ctrl_set_parsed(void *parsed_result, int ret; /* - * Rx on/off, flow control is enabled/disabled on RX side. This can indicate + * Rx on/off, flow control is enabled/disabled on Rx side. This can indicate * the RTE_FC_TX_PAUSE, Transmit pause frame at the Rx side. - * Tx on/off, flow control is enabled/disabled on TX side. This can indicate + * Tx on/off, flow control is enabled/disabled on Tx side. This can indicate * the RTE_FC_RX_PAUSE, Respond to the pause frame at the Tx side. */ static enum rte_eth_fc_mode rx_tx_onoff_2_pfc_mode[2][2] = { @@ -8366,7 +8366,7 @@ cmdline_parse_inst_t cmd_set_qmap = { .f = cmd_set_qmap_parsed, .data = NULL, .help_str = "set stat_qmap rx|tx <port_id> <queue_id> <map_value>: " - "Set statistics mapping value on tx|rx queue_id of port_id", + "Set statistics mapping value on Tx|Rx queue_id of port_id", .tokens = { (void *)&cmd_setqmap_set, (void *)&cmd_setqmap_qmap, @@ -11109,9 +11109,9 @@ cmdline_parse_inst_t cmd_set_vf_vlan_insert = { }, }; -/* tx loopback configuration */ +/* Tx loopback configuration */ -/* Common result structure for tx loopback */ +/* Common result structure for Tx loopback */ struct cmd_tx_loopback_result { cmdline_fixed_string_t set; cmdline_fixed_string_t tx; @@ -11120,7 +11120,7 @@ struct cmd_tx_loopback_result { cmdline_fixed_string_t on_off; }; -/* Common CLI fields for tx loopback enable disable */ +/* Common CLI fields for Tx loopback enable disable */ cmdline_parse_token_string_t cmd_tx_loopback_set = TOKEN_STRING_INITIALIZER (struct cmd_tx_loopback_result, @@ -11216,7 +11216,7 @@ struct cmd_all_queues_drop_en_result { cmdline_fixed_string_t on_off; }; -/* Common CLI fields for tx loopback enable disable */ +/* Common CLI fields for Tx loopback enable disable */ cmdline_parse_token_string_t cmd_all_queues_drop_en_set = TOKEN_STRING_INITIALIZER (struct cmd_all_queues_drop_en_result, @@ -12281,7 +12281,7 @@ cmdline_parse_inst_t cmd_set_vf_vlan_tag = { }, }; -/* Common definition of VF and TC TX bandwidth configuration */ +/* Common definition of VF and TC Tx bandwidth configuration */ struct cmd_vf_tc_bw_result { cmdline_fixed_string_t set; cmdline_fixed_string_t vf; @@ -14439,13 +14439,13 @@ cmd_show_vf_stats_parsed( printf("\n %s NIC statistics for port %-2d vf %-2d %s\n", nic_stats_border, res->port_id, res->vf_id, nic_stats_border); - printf(" RX-packets: %-10"PRIu64" RX-missed: %-10"PRIu64" RX-bytes: " + printf(" Rx-packets: %-10"PRIu64" Rx-missed: %-10"PRIu64" Rx-bytes: " "%-"PRIu64"\n", stats.ipackets, stats.imissed, stats.ibytes); - printf(" RX-errors: %-"PRIu64"\n", stats.ierrors); - printf(" RX-nombuf: %-10"PRIu64"\n", + printf(" Rx-errors: %-"PRIu64"\n", stats.ierrors); + printf(" Rx-nombuf: %-10"PRIu64"\n", stats.rx_nombuf); - printf(" TX-packets: %-10"PRIu64" TX-errors: %-10"PRIu64" TX-bytes: " + printf(" Tx-packets: %-10"PRIu64" Tx-errors: %-10"PRIu64" Tx-bytes: " "%-"PRIu64"\n", stats.opackets, stats.oerrors, stats.obytes); @@ -16602,7 +16602,7 @@ cmdline_parse_inst_t cmd_show_port_supported_ptypes = { }, }; -/* *** display rx/tx descriptor status *** */ +/* *** display Rx/Tx descriptor status *** */ struct cmd_show_rx_tx_desc_status_result { cmdline_fixed_string_t cmd_show; cmdline_fixed_string_t cmd_port; diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index c89f8cd..bc5c989 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -194,11 +194,11 @@ nic_stats_display(portid_t port_id) printf("\n %s NIC statistics for port %-2d %s\n", nic_stats_border, port_id, nic_stats_border); - printf(" RX-packets: %-10"PRIu64" RX-missed: %-10"PRIu64" RX-bytes: " + printf(" Rx-packets: %-10"PRIu64" Rx-missed: %-10"PRIu64" Rx-bytes: " "%-"PRIu64"\n", stats.ipackets, stats.imissed, stats.ibytes); - printf(" RX-errors: %-"PRIu64"\n", stats.ierrors); - printf(" RX-nombuf: %-10"PRIu64"\n", stats.rx_nombuf); - printf(" TX-packets: %-10"PRIu64" TX-errors: %-10"PRIu64" TX-bytes: " + printf(" Rx-errors: %-"PRIu64"\n", stats.ierrors); + printf(" Rx-nombuf: %-10"PRIu64"\n", stats.rx_nombuf); + printf(" Tx-packets: %-10"PRIu64" Tx-errors: %-10"PRIu64" Tx-bytes: " "%-"PRIu64"\n", stats.opackets, stats.oerrors, stats.obytes); diff_ns = 0; @@ -373,27 +373,27 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id) rc = rte_eth_rx_queue_info_get(port_id, queue_id, &qinfo); if (rc != 0) { printf("Failed to retrieve information for port: %u, " - "RX queue: %hu\nerror desc: %s(%d)\n", + "Rx queue: %hu\nerror desc: %s(%d)\n", port_id, queue_id, strerror(-rc), rc); return; } - printf("\n%s Infos for port %-2u, RX queue %-2u %s", + printf("\n%s Infos for port %-2u, Rx queue %-2u %s", info_border, port_id, queue_id, info_border); printf("\nMempool: %s", (qinfo.mp == NULL) ? "NULL" : qinfo.mp->name); - printf("\nRX prefetch threshold: %hhu", qinfo.conf.rx_thresh.pthresh); - printf("\nRX host threshold: %hhu", qinfo.conf.rx_thresh.hthresh); - printf("\nRX writeback threshold: %hhu", qinfo.conf.rx_thresh.wthresh); - printf("\nRX free threshold: %hu", qinfo.conf.rx_free_thresh); - printf("\nRX drop packets: %s", + printf("\nRx prefetch threshold: %hhu", qinfo.conf.rx_thresh.pthresh); + printf("\nRx host threshold: %hhu", qinfo.conf.rx_thresh.hthresh); + printf("\nRx writeback threshold: %hhu", qinfo.conf.rx_thresh.wthresh); + printf("\nRx free threshold: %hu", qinfo.conf.rx_free_thresh); + printf("\nRx drop packets: %s", (qinfo.conf.rx_drop_en != 0) ? "on" : "off"); - printf("\nRX deferred start: %s", + printf("\nRx deferred start: %s", (qinfo.conf.rx_deferred_start != 0) ? "on" : "off"); - printf("\nRX scattered packets: %s", + printf("\nRx scattered packets: %s", (qinfo.scattered_rx != 0) ? "on" : "off"); if (qinfo.rx_buf_size != 0) - printf("\nRX buffer size: %hu", qinfo.rx_buf_size); + printf("\nRx buffer size: %hu", qinfo.rx_buf_size); printf("\nNumber of RXDs: %hu", qinfo.nb_desc); if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0) @@ -416,20 +416,20 @@ tx_queue_infos_display(portid_t port_id, uint16_t queue_id) rc = rte_eth_tx_queue_info_get(port_id, queue_id, &qinfo); if (rc != 0) { printf("Failed to retrieve information for port: %u, " - "TX queue: %hu\nerror desc: %s(%d)\n", + "Tx queue: %hu\nerror desc: %s(%d)\n", port_id, queue_id, strerror(-rc), rc); return; } - printf("\n%s Infos for port %-2u, TX queue %-2u %s", + printf("\n%s Infos for port %-2u, Tx queue %-2u %s", info_border, port_id, queue_id, info_border); - printf("\nTX prefetch threshold: %hhu", qinfo.conf.tx_thresh.pthresh); - printf("\nTX host threshold: %hhu", qinfo.conf.tx_thresh.hthresh); - printf("\nTX writeback threshold: %hhu", qinfo.conf.tx_thresh.wthresh); - printf("\nTX RS threshold: %hu", qinfo.conf.tx_rs_thresh); - printf("\nTX free threshold: %hu", qinfo.conf.tx_free_thresh); - printf("\nTX deferred start: %s", + printf("\nTx prefetch threshold: %hhu", qinfo.conf.tx_thresh.pthresh); + printf("\nTx host threshold: %hhu", qinfo.conf.tx_thresh.hthresh); + printf("\nTx writeback threshold: %hhu", qinfo.conf.tx_thresh.wthresh); + printf("\nTx RS threshold: %hu", qinfo.conf.tx_rs_thresh); + printf("\nTx free threshold: %hu", qinfo.conf.tx_free_thresh); + printf("\nTx deferred start: %s", (qinfo.conf.tx_deferred_start != 0) ? "on" : "off"); printf("\nNumber of TXDs: %hu", qinfo.nb_desc); @@ -678,8 +678,8 @@ port_infos_display(portid_t port_id) } } - printf("Minimum size of RX buffer: %u\n", dev_info.min_rx_bufsize); - printf("Maximum configurable length of RX packet: %u\n", + printf("Minimum size of Rx buffer: %u\n", dev_info.min_rx_bufsize); + printf("Maximum configurable length of Rx packet: %u\n", dev_info.max_rx_pktlen); printf("Maximum configurable size of LRO aggregated packet: %u\n", dev_info.max_lro_pkt_size); @@ -689,16 +689,16 @@ port_infos_display(portid_t port_id) printf("Maximum number of VMDq pools: %u\n", dev_info.max_vmdq_pools); - printf("Current number of RX queues: %u\n", dev_info.nb_rx_queues); - printf("Max possible RX queues: %u\n", dev_info.max_rx_queues); + printf("Current number of Rx queues: %u\n", dev_info.nb_rx_queues); + printf("Max possible Rx queues: %u\n", dev_info.max_rx_queues); printf("Max possible number of RXDs per queue: %hu\n", dev_info.rx_desc_lim.nb_max); printf("Min possible number of RXDs per queue: %hu\n", dev_info.rx_desc_lim.nb_min); printf("RXDs number alignment: %hu\n", dev_info.rx_desc_lim.nb_align); - printf("Current number of TX queues: %u\n", dev_info.nb_tx_queues); - printf("Max possible TX queues: %u\n", dev_info.max_tx_queues); + printf("Current number of Tx queues: %u\n", dev_info.nb_tx_queues); + printf("Max possible Tx queues: %u\n", dev_info.max_tx_queues); printf("Max possible number of TXDs per queue: %hu\n", dev_info.tx_desc_lim.nb_max); printf("Min possible number of TXDs per queue: %hu\n", @@ -2200,14 +2200,14 @@ port_flow_isolate(portid_t port_id, int set) } /* - * RX/TX ring descriptors display functions. + * Rx/Tx ring descriptors display functions. */ int rx_queue_id_is_invalid(queueid_t rxq_id) { if (rxq_id < nb_rxq) return 0; - printf("Invalid RX queue %d (must be < nb_rxq=%d)\n", rxq_id, nb_rxq); + printf("Invalid Rx queue %d (must be < nb_rxq=%d)\n", rxq_id, nb_rxq); return 1; } @@ -2216,7 +2216,7 @@ tx_queue_id_is_invalid(queueid_t txq_id) { if (txq_id < nb_txq) return 0; - printf("Invalid TX queue %d (must be < nb_txq=%d)\n", txq_id, nb_txq); + printf("Invalid Tx queue %d (must be < nb_txq=%d)\n", txq_id, nb_txq); return 1; } @@ -2297,7 +2297,7 @@ rx_desc_id_is_invalid(portid_t port_id, queueid_t rxq_id, uint16_t rxdesc_id) if (rxdesc_id < ring_size) return 0; - printf("Invalid RX descriptor %u (must be < ring_size=%u)\n", + printf("Invalid Rx descriptor %u (must be < ring_size=%u)\n", rxdesc_id, ring_size); return 1; } @@ -2315,7 +2315,7 @@ tx_desc_id_is_invalid(portid_t port_id, queueid_t txq_id, uint16_t txdesc_id) if (txdesc_id < ring_size) return 0; - printf("Invalid TX descriptor %u (must be < ring_size=%u)\n", + printf("Invalid Tx descriptor %u (must be < ring_size=%u)\n", txdesc_id, ring_size); return 1; } @@ -2388,7 +2388,7 @@ ring_rx_descriptor_display(const struct rte_memzone *ring_mz, return; if (strstr(dev_info.driver_name, "i40e") != NULL) { - /* 32 bytes RX descriptor, i40e only */ + /* 32 bytes Rx descriptor, i40e only */ struct igb_ring_desc_32_bytes *ring = (struct igb_ring_desc_32_bytes *)ring_mz->addr; ring[desc_id].lo_dword.dword = @@ -2407,7 +2407,7 @@ ring_rx_descriptor_display(const struct rte_memzone *ring_mz, return; } #endif - /* 16 bytes RX descriptor */ + /* 16 bytes Rx descriptor */ ring[desc_id].lo_dword.dword = rte_le_to_cpu_64(ring[desc_id].lo_dword.dword); ring_rxd_display_dword(ring[desc_id].lo_dword); @@ -2505,14 +2505,14 @@ rxtx_config_display(void) int32_t rc; /* per port config */ - printf(" port %d: RX queue number: %d Tx queue number: %d\n", + printf(" port %d: Rx queue number: %d Tx queue number: %d\n", (unsigned int)pid, nb_rxq, nb_txq); printf(" Rx offloads=0x%"PRIx64" Tx offloads=0x%"PRIx64"\n", ports[pid].dev_conf.rxmode.offloads, ports[pid].dev_conf.txmode.offloads); - /* per rx queue config only for first queue to be less verbose */ + /* per Rx queue config only for first queue to be less verbose */ for (qid = 0; qid < 1; qid++) { rc = rte_eth_rx_queue_info_get(pid, qid, &rx_qinfo); if (rc) { @@ -2533,16 +2533,16 @@ rxtx_config_display(void) offloads_tmp = rx_qinfo.conf.offloads; } - printf(" RX queue: %d\n", qid); - printf(" RX desc=%d - RX free threshold=%d\n", + printf(" Rx queue: %d\n", qid); + printf(" Rx desc=%d - Rx free threshold=%d\n", nb_rx_desc_tmp, rx_free_thresh_tmp); - printf(" RX threshold registers: pthresh=%d hthresh=%d " + printf(" Rx threshold registers: pthresh=%d hthresh=%d " " wthresh=%d\n", pthresh_tmp, hthresh_tmp, wthresh_tmp); - printf(" RX Offloads=0x%"PRIx64"\n", offloads_tmp); + printf(" Rx Offloads=0x%"PRIx64"\n", offloads_tmp); } - /* per tx queue config only for first queue to be less verbose */ + /* per Tx queue config only for first queue to be less verbose */ for (qid = 0; qid < 1; qid++) { rc = rte_eth_tx_queue_info_get(pid, qid, &tx_qinfo); if (rc) { @@ -2565,13 +2565,13 @@ rxtx_config_display(void) tx_rs_thresh_tmp = tx_qinfo.conf.tx_rs_thresh; } - printf(" TX queue: %d\n", qid); - printf(" TX desc=%d - TX free threshold=%d\n", + printf(" Tx queue: %d\n", qid); + printf(" Tx desc=%d - Tx free threshold=%d\n", nb_tx_desc_tmp, tx_free_thresh_tmp); - printf(" TX threshold registers: pthresh=%d hthresh=%d " + printf(" Tx threshold registers: pthresh=%d hthresh=%d " " wthresh=%d\n", pthresh_tmp, hthresh_tmp, wthresh_tmp); - printf(" TX offloads=0x%"PRIx64" - TX RS bit threshold=%d\n", + printf(" Tx offloads=0x%"PRIx64" - Tx RS bit threshold=%d\n", offloads_tmp, tx_rs_thresh_tmp); } } @@ -2816,8 +2816,8 @@ simple_fwd_config_setup(void) /** * For the RSS forwarding test all streams distributed over lcores. Each stream - * being composed of a RX queue to poll on a RX port for input messages, - * associated with a TX queue of a TX port where to send forwarded packets. + * being composed of a Rx queue to poll on a Rx port for input messages, + * associated with a Tx queue of a Tx port where to send forwarded packets. */ static void rss_fwd_config_setup(void) @@ -2883,9 +2883,9 @@ get_fwd_port_total_tc_num(void) * For the DCB forwarding test, each core is assigned on each traffic class. * * Each core is assigned a multi-stream, each stream being composed of - * a RX queue to poll on a RX port for input messages, associated with - * a TX queue of a TX port where to send forwarded packets. All RX and - * TX queues are mapping to the same traffic class. + * a Rx queue to poll on a Rx port for input messages, associated with + * a Tx queue of a Tx port where to send forwarded packets. All Rx and + * Tx queues are mapping to the same traffic class. * If VMDQ and DCB co-exist, each traffic class on different POOLs share * the same core */ @@ -2913,7 +2913,7 @@ dcb_fwd_config_setup(void) init_fwd_streams(); sm_id = 0; txp = 1; - /* get the dcb info on the first RX and TX ports */ + /* get the dcb info on the first Rx and Tx ports */ (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[rxp], &rxp_dcb_info); (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[txp], &txp_dcb_info); @@ -2951,7 +2951,7 @@ dcb_fwd_config_setup(void) tc++; if (tc < rxp_dcb_info.nb_tcs) continue; - /* Restart from TC 0 on next RX port */ + /* Restart from TC 0 on next Rx port */ tc = 0; if (numa_support && (nb_fwd_ports <= (nb_ports >> 1))) rxp = (portid_t) @@ -2960,7 +2960,7 @@ dcb_fwd_config_setup(void) rxp++; if (rxp >= nb_fwd_ports) return; - /* get the dcb information on next RX and TX ports */ + /* get the dcb information on next Rx and Tx ports */ if ((rxp & 0x1) == 0) txp = (portid_t) (rxp + 1); else @@ -3080,7 +3080,7 @@ pkt_fwd_config_display(struct fwd_config *cfg) mp_alloc_to_str(mp_alloc_type)); if (retry_enabled) - printf("TX retry num: %u, delay between TX retries: %uus\n", + printf("Tx retry num: %u, delay between Tx retries: %uus\n", burst_tx_retry_num, burst_tx_delay_time); for (lc_id = 0; lc_id < cfg->nb_fwd_lcores; lc_id++) { printf("Logical Core %u (socket %u) forwards packets on " @@ -3090,7 +3090,7 @@ pkt_fwd_config_display(struct fwd_config *cfg) fwd_lcores[lc_id]->stream_nb); for (sm_id = 0; sm_id < fwd_lcores[lc_id]->stream_nb; sm_id++) { fs = fwd_streams[fwd_lcores[lc_id]->stream_idx + sm_id]; - printf("\n RX P=%d/Q=%d (socket %u) -> TX " + printf("\n Rx P=%d/Q=%d (socket %u) -> Tx " "P=%d/Q=%d (socket %u) ", fs->rx_port, fs->rx_queue, ports[fs->rx_port].socket_id, @@ -3477,7 +3477,7 @@ set_rx_pkt_offsets(unsigned int *seg_offsets, unsigned int nb_offs) unsigned int i; if (nb_offs >= MAX_SEGS_BUFFER_SPLIT) { - printf("nb segments per RX packets=%u >= " + printf("nb segments per Rx packets=%u >= " "MAX_SEGS_BUFFER_SPLIT - ignored\n", nb_offs); return; } @@ -3521,7 +3521,7 @@ set_rx_pkt_segments(unsigned int *seg_lengths, unsigned int nb_segs) unsigned int i; if (nb_segs >= MAX_SEGS_BUFFER_SPLIT) { - printf("nb segments per RX packets=%u >= " + printf("nb segments per Rx packets=%u >= " "MAX_SEGS_BUFFER_SPLIT - ignored\n", nb_segs); return; } @@ -3577,8 +3577,8 @@ nb_segs_is_invalid(unsigned int nb_segs) return true; if (ring_size < nb_segs) { - printf("nb segments per TX packets=%u >= " - "TX queue(%u) ring_size=%u - ignored\n", + printf("nb segments per Tx packets=%u >= " + "Tx queue(%u) ring_size=%u - ignored\n", nb_segs, queue_id, ring_size); return true; } @@ -4204,18 +4204,18 @@ set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_value) return; } - if (!is_rx) { /* tx */ + if (!is_rx) { /* Tx */ ret = rte_eth_dev_set_tx_queue_stats_mapping(port_id, queue_id, map_value); if (ret) { - printf("failed to set tx queue stats mapping.\n"); + printf("failed to set Tx queue stats mapping.\n"); return; } - } else { /* rx */ + } else { /* Rx */ ret = rte_eth_dev_set_rx_queue_stats_mapping(port_id, queue_id, map_value); if (ret) { - printf("failed to set rx queue stats mapping.\n"); + printf("failed to set Rx queue stats mapping.\n"); return; } } diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index 6b4df33..eadfcd9 100644 --- a/app/test-pmd/csumonly.c +++ b/app/test-pmd/csumonly.c @@ -258,7 +258,7 @@ parse_vxlan(struct rte_udp_hdr *udp_hdr, struct rte_ether_hdr *eth_hdr; /* check udp destination port, RTE_VXLAN_DEFAULT_PORT (4789) is the - * default vxlan port (rfc7348) or that the rx offload flag is set + * default vxlan port (rfc7348) or that the Rx offload flag is set * (i40e only currently) */ if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) && @@ -1008,26 +1008,26 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) printf("-----------------\n"); printf("port=%u, mbuf=%p, pkt_len=%u, nb_segs=%u:\n", fs->rx_port, m, m->pkt_len, m->nb_segs); - /* dump rx parsed packet info */ + /* dump Rx parsed packet info */ rte_get_rx_ol_flag_list(rx_ol_flags, buf, sizeof(buf)); - printf("rx: l2_len=%d ethertype=%x l3_len=%d " + printf("Rx: l2_len=%d ethertype=%x l3_len=%d " "l4_proto=%d l4_len=%d flags=%s\n", info.l2_len, rte_be_to_cpu_16(info.ethertype), info.l3_len, info.l4_proto, info.l4_len, buf); if (rx_ol_flags & PKT_RX_LRO) - printf("rx: m->lro_segsz=%u\n", m->tso_segsz); + printf("Rx: m->lro_segsz=%u\n", m->tso_segsz); if (info.is_tunnel == 1) - printf("rx: outer_l2_len=%d outer_ethertype=%x " + printf("Rx: outer_l2_len=%d outer_ethertype=%x " "outer_l3_len=%d\n", info.outer_l2_len, rte_be_to_cpu_16(info.outer_ethertype), info.outer_l3_len); - /* dump tx packet info */ + /* dump Tx packet info */ if ((tx_offloads & (DEV_TX_OFFLOAD_IPV4_CKSUM | DEV_TX_OFFLOAD_UDP_CKSUM | DEV_TX_OFFLOAD_TCP_CKSUM | DEV_TX_OFFLOAD_SCTP_CKSUM)) || info.tso_segsz != 0) - printf("tx: m->l2_len=%d m->l3_len=%d " + printf("Tx: m->l2_len=%d m->l3_len=%d " "m->l4_len=%d\n", m->l2_len, m->l3_len, m->l4_len); if (info.is_tunnel == 1) { @@ -1036,19 +1036,19 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) (tx_offloads & DEV_TX_OFFLOAD_OUTER_UDP_CKSUM) || (tx_ol_flags & PKT_TX_OUTER_IPV6)) - printf("tx: m->outer_l2_len=%d " + printf("Tx: m->outer_l2_len=%d " "m->outer_l3_len=%d\n", m->outer_l2_len, m->outer_l3_len); if (info.tunnel_tso_segsz != 0 && (m->ol_flags & PKT_TX_TCP_SEG)) - printf("tx: m->tso_segsz=%d\n", + printf("Tx: m->tso_segsz=%d\n", m->tso_segsz); } else if (info.tso_segsz != 0 && (m->ol_flags & PKT_TX_TCP_SEG)) - printf("tx: m->tso_segsz=%d\n", m->tso_segsz); + printf("Tx: m->tso_segsz=%d\n", m->tso_segsz); rte_get_tx_ol_flag_list(m->ol_flags, buf, sizeof(buf)); - printf("tx: flags=%s", buf); + printf("Tx: flags=%s", buf); printf("\n"); } } diff --git a/app/test-pmd/icmpecho.c b/app/test-pmd/icmpecho.c index af6f7e7..ddcb0b5 100644 --- a/app/test-pmd/icmpecho.c +++ b/app/test-pmd/icmpecho.c @@ -386,7 +386,7 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs) /* Use source MAC address as destination MAC address. */ rte_ether_addr_copy(ð_h->s_addr, ð_h->d_addr); - /* Set source MAC address with MAC address of TX port */ + /* Set source MAC address with MAC address of Tx port */ rte_ether_addr_copy(&ports[fs->tx_port].eth_addr, ð_h->s_addr); diff --git a/app/test-pmd/ieee1588fwd.c b/app/test-pmd/ieee1588fwd.c index e3b98e3..2fe5a5b 100644 --- a/app/test-pmd/ieee1588fwd.c +++ b/app/test-pmd/ieee1588fwd.c @@ -42,12 +42,12 @@ struct ptpv2_msg { * Check that each received packet is a IEEE1588 PTP V2 packet of type * PTP_SYNC_MESSAGE, and that it has been identified and timestamped * by the hardware. - * Check that the value of the last RX timestamp recorded by the controller + * Check that the value of the last Rx timestamp recorded by the controller * is greater than the previous one. * * If everything is OK, send the received packet back on the same port, * requesting for it to be timestamped by the hardware. - * Check that the value of the last TX timestamp recorded by the controller + * Check that the value of the last Tx timestamp recorded by the controller * is greater than the previous one. */ @@ -57,10 +57,10 @@ port_ieee1588_rx_timestamp_check(portid_t pi, uint32_t index) struct timespec timestamp = {0, 0}; if (rte_eth_timesync_read_rx_timestamp(pi, ×tamp, index) < 0) { - printf("Port %u RX timestamp registers not valid\n", pi); + printf("Port %u Rx timestamp registers not valid\n", pi); return; } - printf("Port %u RX timestamp value %lu s %lu ns\n", + printf("Port %u Rx timestamp value %lu s %lu ns\n", pi, timestamp.tv_sec, timestamp.tv_nsec); } @@ -78,12 +78,12 @@ port_ieee1588_tx_timestamp_check(portid_t pi) wait_us++; } if (wait_us >= MAX_TX_TMST_WAIT_MICROSECS) { - printf("Port %u TX timestamp registers not valid after " + printf("Port %u Tx timestamp registers not valid after " "%u micro-seconds\n", pi, MAX_TX_TMST_WAIT_MICROSECS); return; } - printf("Port %u TX timestamp value %lu s %lu ns validated after " + printf("Port %u Tx timestamp value %lu s %lu ns validated after " "%u micro-second%s\n", pi, timestamp.tv_sec, timestamp.tv_nsec, wait_us, (wait_us == 1) ? "" : "s"); @@ -174,7 +174,7 @@ ieee1588_packet_fwd(struct fwd_stream *fs) /* For i40e we need the timesync register index. It is ignored for the * other PMDs. */ timesync_index = mb->timesync & 0x3; - /* Read and check the RX timestamp. */ + /* Read and check the Rx timestamp. */ port_ieee1588_rx_timestamp_check(fs->rx_port, timesync_index); /* Swap dest and src mac addresses. */ @@ -182,7 +182,7 @@ ieee1588_packet_fwd(struct fwd_stream *fs) rte_ether_addr_copy(ð_hdr->s_addr, ð_hdr->d_addr); rte_ether_addr_copy(&addr, ð_hdr->s_addr); - /* Forward PTP packet with hardware TX timestamp */ + /* Forward PTP packet with hardware Tx timestamp */ mb->ol_flags |= PKT_TX_IEEE1588_TMST; fs->tx_packets += 1; if (rte_eth_tx_burst(fs->rx_port, fs->tx_queue, &mb, 1) == 0) { @@ -193,7 +193,7 @@ ieee1588_packet_fwd(struct fwd_stream *fs) } /* - * Check the TX timestamp. + * Check the Tx timestamp. */ port_ieee1588_tx_timestamp_check(fs->rx_port); } diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index c8acd5d..53c0717 100644 --- a/app/test-pmd/parameters.c +++ b/app/test-pmd/parameters.c @@ -95,15 +95,15 @@ usage(char* progname) printf(" --portmask=PORTMASK: hexadecimal bitmask of ports used " "by the packet forwarding test.\n"); printf(" --portlist=PORTLIST: list of forwarding ports\n"); - printf(" --numa: enable NUMA-aware allocation of RX/TX rings and of " - "RX memory buffers (mbufs).\n"); + printf(" --numa: enable NUMA-aware allocation of Rx/Tx rings and of " + "Rx memory buffers (mbufs).\n"); printf(" --port-numa-config=(port,socket)[,(port,socket)]: " "specify the socket on which the memory pool " "used by the port will be allocated.\n"); printf(" --ring-numa-config=(port,flag,socket)[,(port,flag,socket)]: " - "specify the socket on which the TX/RX rings for " + "specify the socket on which the Tx/Rx rings for " "the port will be allocated " - "(flag: 1 for RX; 2 for TX; 3 for RX and TX).\n"); + "(flag: 1 for Rx; 2 for Tx; 3 for Rx and Tx).\n"); printf(" --socket-num=N: set socket from which all memory is allocated " "in NUMA mode.\n"); printf(" --mbuf-size=N,[N1[,..Nn]: set the data size of mbuf to " @@ -156,32 +156,32 @@ usage(char* progname) printf(" --rss-udp: set RSS functions to IPv4/IPv6 + UDP.\n"); printf(" --rss-level-inner: set RSS hash level to innermost\n"); printf(" --rss-level-outer: set RSS hash level to outermost\n"); - printf(" --rxq=N: set the number of RX queues per port to N.\n"); - printf(" --rxd=N: set the number of descriptors in RX rings to N.\n"); - printf(" --txq=N: set the number of TX queues per port to N.\n"); - printf(" --txd=N: set the number of descriptors in TX rings to N.\n"); + printf(" --rxq=N: set the number of Rx queues per port to N.\n"); + printf(" --rxd=N: set the number of descriptors in Rx rings to N.\n"); + printf(" --txq=N: set the number of Tx queues per port to N.\n"); + printf(" --txd=N: set the number of descriptors in Tx rings to N.\n"); printf(" --hairpinq=N: set the number of hairpin queues per port to " "N.\n"); printf(" --burst=N: set the number of packets per burst to N.\n"); printf(" --flowgen-clones=N: set the number of single packet clones to send in flowgen mode. Should be less than burst value.\n"); printf(" --mbcache=N: set the cache of mbuf memory pool to N.\n"); - printf(" --rxpt=N: set prefetch threshold register of RX rings to N.\n"); - printf(" --rxht=N: set the host threshold register of RX rings to N.\n"); - printf(" --rxfreet=N: set the free threshold of RX descriptors to N " + printf(" --rxpt=N: set prefetch threshold register of Rx rings to N.\n"); + printf(" --rxht=N: set the host threshold register of Rx rings to N.\n"); + printf(" --rxfreet=N: set the free threshold of Rx descriptors to N " "(0 <= N < value of rxd).\n"); - printf(" --rxwt=N: set the write-back threshold register of RX rings to N.\n"); - printf(" --txpt=N: set the prefetch threshold register of TX rings to N.\n"); - printf(" --txht=N: set the nhost threshold register of TX rings to N.\n"); - printf(" --txwt=N: set the write-back threshold register of TX rings to N.\n"); - printf(" --txfreet=N: set the transmit free threshold of TX rings to N " + printf(" --rxwt=N: set the write-back threshold register of Rx rings to N.\n"); + printf(" --txpt=N: set the prefetch threshold register of Tx rings to N.\n"); + printf(" --txht=N: set the nhost threshold register of Tx rings to N.\n"); + printf(" --txwt=N: set the write-back threshold register of Tx rings to N.\n"); + printf(" --txfreet=N: set the transmit free threshold of Tx rings to N " "(0 <= N <= value of txd).\n"); - printf(" --txrst=N: set the transmit RS bit threshold of TX rings to N " + printf(" --txrst=N: set the transmit RS bit threshold of Tx rings to N " "(0 <= N <= value of txd).\n"); - printf(" --no-flush-rx: Don't flush RX streams before forwarding." + printf(" --no-flush-rx: Don't flush Rx streams before forwarding." " Used mainly with PCAP drivers.\n"); - printf(" --rxoffs=X[,Y]*: set RX segment offsets for split.\n"); - printf(" --rxpkts=X[,Y]*: set RX segment sizes to split.\n"); - printf(" --txpkts=X[,Y]*: set TX segment sizes" + printf(" --rxoffs=X[,Y]*: set Rx segment offsets for split.\n"); + printf(" --rxpkts=X[,Y]*: set Rx segment sizes to split.\n"); + printf(" --txpkts=X[,Y]*: set Tx segment sizes" " or total packet length.\n"); printf(" --txonly-multi-flow: generate multiple flows in txonly mode\n"); printf(" --disable-link-check: disable check on link status when " @@ -197,8 +197,8 @@ usage(char* progname) "disable print of designated event or all of them.\n"); printf(" --flow-isolate-all: " "requests flow API isolated mode on all ports at initialization time.\n"); - printf(" --tx-offloads=0xXXXXXXXX: hexadecimal bitmask of TX queue offloads\n"); - printf(" --rx-offloads=0xXXXXXXXX: hexadecimal bitmask of RX queue offloads\n"); + printf(" --tx-offloads=0xXXXXXXXX: hexadecimal bitmask of Tx queue offloads\n"); + printf(" --rx-offloads=0xXXXXXXXX: hexadecimal bitmask of Rx queue offloads\n"); printf(" --hot-plug: enable hot plug for device.\n"); printf(" --vxlan-gpe-port=N: UPD port of tunnel VXLAN-GPE\n"); printf(" --geneve-parsed-port=N: UPD port to parse GENEVE tunnel protocol\n"); @@ -217,10 +217,10 @@ usage(char* progname) printf(" --noisy-lkup-num-writes=N: do N random reads and writes per packet\n"); printf(" --no-iova-contig: mempool memory can be IOVA non contiguous. " "valid only with --mp-alloc=anon\n"); - printf(" --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be " + printf(" --rx-mq-mode=0xX: hexadecimal bitmask of Rx mq mode can be " "enabled\n"); printf(" --record-core-cycles: enable measurement of CPU cycles.\n"); - printf(" --record-burst-stats: enable display of RX and TX bursts.\n"); + printf(" --record-burst-stats: enable display of Rx and Tx bursts.\n"); printf(" --hairpin-mode=0xXX: bitmask set the hairpin port mode.\n " " 0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n" " 0x01 - hairpin ports loop, 0x00 - hairpin port self\n"); diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 7eb810b..d83eb8c 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -92,7 +92,7 @@ char cmdline_filename[PATH_MAX] = {0}; /* * NUMA support configuration. * When set, the NUMA support attempts to dispatch the allocation of the - * RX and TX memory rings, and of the DMA memory buffers (mbufs) for the + * Rx and Tx memory rings, and of the DMA memory buffers (mbufs) for the * probed ports among the CPU sockets 0 and 1. * Otherwise, all memory is allocated from CPU socket 0. */ @@ -120,7 +120,7 @@ uint8_t mp_alloc_type = MP_ALLOC_NATIVE; uint8_t port_numa[RTE_MAX_ETHPORTS]; /* - * Store specified sockets on which RX ring to be used by ports + * Store specified sockets on which Rx ring to be used by ports * is allocated. */ uint8_t rxring_numa[RTE_MAX_ETHPORTS]; @@ -163,7 +163,7 @@ portid_t nb_fwd_ports; /**< Number of forwarding ports. */ unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */ portid_t fwd_ports_ids[RTE_MAX_ETHPORTS]; /**< Port ids configuration. */ -struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */ +struct fwd_stream **fwd_streams; /**< For each Rx queue of each port. */ streamid_t nb_fwd_streams; /**< Is equal to (nb_ports * nb_rxq). */ /* @@ -228,7 +228,7 @@ uint16_t tx_pkt_seg_lengths[RTE_MAX_SEGS_PER_PKT] = { uint8_t tx_pkt_nb_segs = 1; /**< Number of segments in TXONLY packets */ enum tx_pkt_split tx_pkt_split = TX_PKT_SPLIT_OFF; -/**< Split policy for packets to TX. */ +/**< Split policy for packets to Tx. */ uint8_t txonly_multi_flow; /**< Whether multiple flows are generated in TXONLY mode. */ @@ -250,24 +250,24 @@ uint8_t dcb_config = 0; uint8_t dcb_test = 0; /* - * Configurable number of RX/TX queues. + * Configurable number of Rx/Tx queues. */ queueid_t nb_hairpinq; /**< Number of hairpin queues per port. */ -queueid_t nb_rxq = 1; /**< Number of RX queues per port. */ -queueid_t nb_txq = 1; /**< Number of TX queues per port. */ +queueid_t nb_rxq = 1; /**< Number of Rx queues per port. */ +queueid_t nb_txq = 1; /**< Number of Tx queues per port. */ /* - * Configurable number of RX/TX ring descriptors. + * Configurable number of Rx/Tx ring descriptors. * Defaults are supplied by drivers via ethdev. */ #define RTE_TEST_RX_DESC_DEFAULT 0 #define RTE_TEST_TX_DESC_DEFAULT 0 -uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT; /**< Number of RX descriptors. */ -uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT; /**< Number of TX descriptors. */ +uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT; /**< Number of Rx descriptors. */ +uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT; /**< Number of Tx descriptors. */ #define RTE_PMD_PARAM_UNSET -1 /* - * Configurable values of RX and TX ring threshold registers. + * Configurable values of Rx and Tx ring threshold registers. */ int8_t rx_pthresh = RTE_PMD_PARAM_UNSET; @@ -279,22 +279,22 @@ int8_t tx_hthresh = RTE_PMD_PARAM_UNSET; int8_t tx_wthresh = RTE_PMD_PARAM_UNSET; /* - * Configurable value of RX free threshold. + * Configurable value of Rx free threshold. */ int16_t rx_free_thresh = RTE_PMD_PARAM_UNSET; /* - * Configurable value of RX drop enable. + * Configurable value of Rx drop enable. */ int8_t rx_drop_en = RTE_PMD_PARAM_UNSET; /* - * Configurable value of TX free threshold. + * Configurable value of Tx free threshold. */ int16_t tx_free_thresh = RTE_PMD_PARAM_UNSET; /* - * Configurable value of TX RS bit threshold. + * Configurable value of Tx RS bit threshold. */ int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET; @@ -343,7 +343,7 @@ uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */ uint16_t port_topology = PORT_TOPOLOGY_PAIRED; /* Ports are paired by default */ /* - * Avoids to flush all the RX streams before starts forwarding. + * Avoids to flush all the Rx streams before starts forwarding. */ uint8_t no_flush_rx = 0; /* flush by default */ @@ -491,7 +491,7 @@ uint8_t xstats_hide_zero; uint8_t record_core_cycles; /* - * Display of RX and TX bursts disabled by default + * Display of Rx and Tx bursts disabled by default */ uint8_t record_burst_stats; @@ -509,7 +509,7 @@ struct gro_status gro_ports[RTE_MAX_ETHPORTS]; uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES; /* - * hexadecimal bitmask of RX mq mode can be enabled. + * hexadecimal bitmask of Rx mq mode can be enabled. */ enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS; @@ -1085,7 +1085,7 @@ check_socket_id(const unsigned int socket_id) } /* - * Get the allowed maximum number of RX queues. + * Get the allowed maximum number of Rx queues. * *pid return the port id which has minimal value of * max_rx_queues in all ports. */ @@ -1113,7 +1113,7 @@ get_allowed_max_nb_rxq(portid_t *pid) /* * Check input rxq is valid or not. * If input rxq is not greater than any of maximum number - * of RX queues of all ports, it is valid. + * of Rx queues of all ports, it is valid. * if valid, return 0, else return -1 */ int @@ -1135,7 +1135,7 @@ check_nb_rxq(queueid_t rxq) } /* - * Get the allowed maximum number of TX queues. + * Get the allowed maximum number of Tx queues. * *pid return the port id which has minimal value of * max_tx_queues in all ports. */ @@ -1163,7 +1163,7 @@ get_allowed_max_nb_txq(portid_t *pid) /* * Check input txq is valid or not. * If input txq is not greater than any of maximum number - * of TX queues of all ports, it is valid. + * of Tx queues of all ports, it is valid. * if valid, return 0, else return -1 */ int @@ -1185,7 +1185,7 @@ check_nb_txq(queueid_t txq) } /* - * Get the allowed maximum number of RXDs of every rx queue. + * Get the allowed maximum number of RXDs of every Rx queue. * *pid return the port id which has minimal value of * max_rxd in all queues of all ports. */ @@ -1209,7 +1209,7 @@ get_allowed_max_nb_rxd(portid_t *pid) } /* - * Get the allowed minimal number of RXDs of every rx queue. + * Get the allowed minimal number of RXDs of every Rx queue. * *pid return the port id which has minimal value of * min_rxd in all queues of all ports. */ @@ -1271,9 +1271,9 @@ check_nb_rxd(queueid_t rxd) } /* - * Get the allowed maximum number of TXDs of every rx queues. + * Get the allowed maximum number of TXDs of every Rx queues. * *pid return the port id which has minimal value of - * max_txd in every tx queue. + * max_txd in every Tx queue. */ static uint16_t get_allowed_max_nb_txd(portid_t *pid) @@ -1295,9 +1295,9 @@ get_allowed_max_nb_txd(portid_t *pid) } /* - * Get the allowed maximum number of TXDs of every tx queues. + * Get the allowed maximum number of TXDs of every Tx queues. * *pid return the port id which has minimal value of - * min_txd in every tx queue. + * min_txd in every Tx queue. */ static uint16_t get_allowed_min_nb_txd(portid_t *pid) @@ -1781,30 +1781,30 @@ fwd_stream_stats_display(streamid_t stream_id) if ((fs->rx_packets == 0) && (fs->tx_packets == 0) && (fs->fwd_dropped == 0)) return; - printf("\n %s Forward Stats for RX Port=%2d/Queue=%2d -> " - "TX Port=%2d/Queue=%2d %s\n", + printf("\n %s Forward Stats for Rx Port=%2d/Queue=%2d -> " + "Tx Port=%2d/Queue=%2d %s\n", fwd_top_stats_border, fs->rx_port, fs->rx_queue, fs->tx_port, fs->tx_queue, fwd_top_stats_border); - printf(" RX-packets: %-14"PRIu64" TX-packets: %-14"PRIu64 - " TX-dropped: %-14"PRIu64, + printf(" Rx-packets: %-14"PRIu64" Tx-packets: %-14"PRIu64 + " Tx-dropped: %-14"PRIu64, fs->rx_packets, fs->tx_packets, fs->fwd_dropped); /* if checksum mode */ if (cur_fwd_eng == &csum_fwd_engine) { - printf(" RX- bad IP checksum: %-14"PRIu64 + printf(" Rx- bad IP checksum: %-14"PRIu64 " Rx- bad L4 checksum: %-14"PRIu64 " Rx- bad outer L4 checksum: %-14"PRIu64"\n", fs->rx_bad_ip_csum, fs->rx_bad_l4_csum, fs->rx_bad_outer_l4_csum); - printf(" RX- bad outer IP checksum: %-14"PRIu64"\n", + printf(" Rx- bad outer IP checksum: %-14"PRIu64"\n", fs->rx_bad_outer_ip_csum); } else { printf("\n"); } if (record_burst_stats) { - pkt_burst_stats_display("RX", &fs->rx_burst_stats); - pkt_burst_stats_display("TX", &fs->tx_burst_stats); + pkt_burst_stats_display("Rx", &fs->rx_burst_stats); + pkt_burst_stats_display("Tx", &fs->tx_burst_stats); } } @@ -1882,8 +1882,8 @@ fwd_stats_display(void) printf("\n %s Forward statistics for port %-2d %s\n", fwd_stats_border, pt_id, fwd_stats_border); - printf(" RX-packets: %-14"PRIu64" RX-dropped: %-14"PRIu64 - "RX-total: %-"PRIu64"\n", stats.ipackets, stats.imissed, + printf(" Rx-packets: %-14"PRIu64" Rx-dropped: %-14"PRIu64 + "Rx-total: %-"PRIu64"\n", stats.ipackets, stats.imissed, stats.ipackets + stats.imissed); if (cur_fwd_eng == &csum_fwd_engine) { @@ -1897,21 +1897,21 @@ fwd_stats_display(void) ports_stats[pt_id].rx_bad_outer_ip_csum); } if (stats.ierrors + stats.rx_nombuf > 0) { - printf(" RX-error: %-"PRIu64"\n", stats.ierrors); - printf(" RX-nombufs: %-14"PRIu64"\n", stats.rx_nombuf); + printf(" Rx-error: %-"PRIu64"\n", stats.ierrors); + printf(" Rx-nombufs: %-14"PRIu64"\n", stats.rx_nombuf); } - printf(" TX-packets: %-14"PRIu64" TX-dropped: %-14"PRIu64 - "TX-total: %-"PRIu64"\n", + printf(" Tx-packets: %-14"PRIu64" Tx-dropped: %-14"PRIu64 + "Tx-total: %-"PRIu64"\n", stats.opackets, ports_stats[pt_id].tx_dropped, stats.opackets + ports_stats[pt_id].tx_dropped); if (record_burst_stats) { if (ports_stats[pt_id].rx_stream) - pkt_burst_stats_display("RX", + pkt_burst_stats_display("Rx", &ports_stats[pt_id].rx_stream->rx_burst_stats); if (ports_stats[pt_id].tx_stream) - pkt_burst_stats_display("TX", + pkt_burst_stats_display("Tx", &ports_stats[pt_id].tx_stream->tx_burst_stats); } @@ -1922,14 +1922,14 @@ fwd_stats_display(void) printf("\n %s Accumulated forward statistics for all ports" "%s\n", acc_stats_border, acc_stats_border); - printf(" RX-packets: %-14"PRIu64" RX-dropped: %-14"PRIu64"RX-total: " + printf(" Rx-packets: %-14"PRIu64" Rx-dropped: %-14"PRIu64"Rx-total: " "%-"PRIu64"\n" - " TX-packets: %-14"PRIu64" TX-dropped: %-14"PRIu64"TX-total: " + " Tx-packets: %-14"PRIu64" Tx-dropped: %-14"PRIu64"Tx-total: " "%-"PRIu64"\n", total_recv, total_rx_dropped, total_recv + total_rx_dropped, total_xmit, total_tx_dropped, total_xmit + total_tx_dropped); if (total_rx_nombuf > 0) - printf(" RX-nombufs: %-14"PRIu64"\n", total_rx_nombuf); + printf(" Rx-nombufs: %-14"PRIu64"\n", total_rx_nombuf); printf(" %s++++++++++++++++++++++++++++++++++++++++++++++" "%s\n", acc_stats_border, acc_stats_border); @@ -2337,7 +2337,7 @@ setup_hairpin_queues(portid_t pi, portid_t p_pi, uint16_t cnt_pi) } else { uint16_t next_pi; - /* Last port will be the peer RX port of the first. */ + /* Last port will be the peer Rx port of the first. */ RTE_ETH_FOREACH_DEV(next_pi) peer_rx_port = next_pi; } @@ -2366,7 +2366,7 @@ setup_hairpin_queues(portid_t pi, portid_t p_pi, uint16_t cnt_pi) if (diag == 0) continue; - /* Fail to setup rx queue, return */ + /* Fail to setup Rx queue, return */ if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0) @@ -2389,7 +2389,7 @@ setup_hairpin_queues(portid_t pi, portid_t p_pi, uint16_t cnt_pi) if (diag == 0) continue; - /* Fail to setup rx queue, return */ + /* Fail to setup Rx queue, return */ if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0) @@ -2536,27 +2536,27 @@ start_port(portid_t pid) if (diag == 0) continue; - /* Fail to setup tx queue, return */ + /* Fail to setup Tx queue, return */ if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0) printf("Port %d can not be set back " "to stopped\n", pi); - printf("Fail to configure port %d tx queues\n", + printf("Fail to configure port %d Tx queues\n", pi); /* try to reconfigure queues next time */ port->need_reconfig_queues = 1; return -1; } for (qi = 0; qi < nb_rxq; qi++) { - /* setup rx queues */ + /* setup Rx queues */ if ((numa_support) && (rxring_numa[pi] != NUMA_NO_CONFIG)) { struct rte_mempool * mp = mbuf_pool_find (rxring_numa[pi], 0); if (mp == NULL) { - printf("Failed to setup RX queue:" + printf("Failed to setup Rx queue:" "No mempool allocation" " on the socket %d\n", rxring_numa[pi]); @@ -2573,7 +2573,7 @@ start_port(portid_t pid) mbuf_pool_find (port->socket_id, 0); if (mp == NULL) { - printf("Failed to setup RX queue:" + printf("Failed to setup Rx queue:" "No mempool allocation" " on the socket %d\n", port->socket_id); @@ -2588,13 +2588,13 @@ start_port(portid_t pid) if (diag == 0) continue; - /* Fail to setup rx queue, return */ + /* Fail to setup Rx queue, return */ if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0) printf("Port %d can not be set back " "to stopped\n", pi); - printf("Fail to configure port %d rx queues\n", + printf("Fail to configure port %d Rx queues\n", pi); /* try to reconfigure queues next time */ port->need_reconfig_queues = 1; @@ -3542,7 +3542,7 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf, struct rte_eth_vmdq_dcb_tx_conf *vmdq_tx_conf = ð_conf->tx_adv_conf.vmdq_dcb_tx_conf; - /* VMDQ+DCB RX and TX configurations */ + /* VMDQ+DCB Rx and Tx configurations */ vmdq_rx_conf->enable_default_pool = 0; vmdq_rx_conf->default_pool = 0; vmdq_rx_conf->nb_queue_pools = @@ -3561,7 +3561,7 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf, vmdq_tx_conf->dcb_tc[i] = i % num_tcs; } - /* set DCB mode of RX and TX of multiple queues */ + /* set DCB mode of Rx and Tx of multiple queues */ eth_conf->rxmode.mq_mode = (enum rte_eth_rx_mq_mode) (rx_mq_mode & ETH_MQ_RX_VMDQ_DCB); @@ -3844,7 +3844,7 @@ main(int argc, char** argv) } if (!nb_rxq && !nb_txq) - printf("Warning: Either rx or tx queues should be non-zero\n"); + printf("Warning: Either Rx or Tx queues should be non-zero\n"); if (nb_rxq > 1 && nb_rxq > nb_txq) printf("Warning: nb_rxq=%d enables RSS configuration, " diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index ce83f31..e5e7392 100644 --- a/app/test-pmd/testpmd.h +++ b/app/test-pmd/testpmd.h @@ -88,7 +88,7 @@ enum { }; /** - * The data structure associated with RX and TX packet burst statistics + * The data structure associated with Rx and Tx packet burst statistics * that are recorded for each forwarding stream. */ struct pkt_burst_stats { @@ -122,9 +122,9 @@ extern char dynf_names[64][RTE_MBUF_DYN_NAMESIZE]; struct fwd_stream { /* "read-only" data */ portid_t rx_port; /**< port to poll for received packets */ - queueid_t rx_queue; /**< RX queue to poll on "rx_port" */ + queueid_t rx_queue; /**< Rx queue to poll on "rx_port" */ portid_t tx_port; /**< forwarding port of received packets */ - queueid_t tx_queue; /**< TX queue to send forwarded packets */ + queueid_t tx_queue; /**< Tx queue to send forwarded packets */ streamid_t peer_addr; /**< index of peer ethernet address of packets */ unsigned int retry_enabled; @@ -140,7 +140,7 @@ struct fwd_stream { uint64_t rx_bad_outer_ip_csum; /**< received packets having bad outer ip checksum */ unsigned int gro_times; /**< GRO operation times */ - uint64_t core_cycles; /**< used for RX and TX processing */ + uint64_t core_cycles; /**< used for Rx and Tx processing */ struct pkt_burst_stats rx_burst_stats; struct pkt_burst_stats tx_burst_stats; }; @@ -214,10 +214,10 @@ struct rte_port { uint8_t need_reconfig_queues; /**< need reconfiguring queues or not */ uint8_t rss_flag; /**< enable rss or not */ uint8_t dcb_flag; /**< enable dcb */ - uint16_t nb_rx_desc[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue rx desc number */ - uint16_t nb_tx_desc[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue tx desc number */ - struct rte_eth_rxconf rx_conf[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue rx configuration */ - struct rte_eth_txconf tx_conf[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue tx configuration */ + uint16_t nb_rx_desc[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue Rx desc number */ + uint16_t nb_tx_desc[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue Tx desc number */ + struct rte_eth_rxconf rx_conf[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue Rx configuration */ + struct rte_eth_txconf tx_conf[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue Tx configuration */ struct rte_ether_addr *mc_addr_pool; /**< pool of multicast addrs */ uint32_t mc_addr_nb; /**< nb. of addr. in mc_addr_pool */ uint8_t slave_flag; /**< bonding slave port */ @@ -329,7 +329,7 @@ extern uint8_t xstats_hide_zero; /**< Hide zero values for xstats display */ /* globals used for configuration */ extern uint8_t record_core_cycles; /**< Enables measurement of CPU cycles */ -extern uint8_t record_burst_stats; /**< Enables display of RX and TX bursts */ +extern uint8_t record_burst_stats; /**< Enables display of Rx and Tx bursts */ extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */ extern int testpmd_logtype; /**< Log type for testpmd logs */ extern uint8_t interactive; @@ -365,13 +365,13 @@ extern uint32_t bypass_timeout; /**< Store the NIC bypass watchdog timeout */ extern uint8_t port_numa[RTE_MAX_ETHPORTS]; /* - * Store specified sockets on which RX ring to be used by ports + * Store specified sockets on which Rx ring to be used by ports * is allocated. */ extern uint8_t rxring_numa[RTE_MAX_ETHPORTS]; /* - * Store specified sockets on which TX ring to be used by ports + * Store specified sockets on which Tx ring to be used by ports * is allocated. */ extern uint8_t txring_numa[RTE_MAX_ETHPORTS]; @@ -462,7 +462,7 @@ extern uint8_t rx_pkt_nb_offs; /**< Number of specified offsets */ #define TXONLY_DEF_PACKET_LEN 64 extern uint16_t tx_pkt_length; /**< Length of TXONLY packet */ extern uint16_t tx_pkt_seg_lengths[RTE_MAX_SEGS_PER_PKT]; /**< Seg. lengths */ -extern uint8_t tx_pkt_nb_segs; /**< Number of segments in TX packets */ +extern uint8_t tx_pkt_nb_segs; /**< Number of segments in Tx packets */ extern uint32_t tx_pkt_times_intra; extern uint32_t tx_pkt_times_inter; @@ -504,8 +504,8 @@ extern uint16_t geneve_udp_port; /**< UDP port of tunnel GENEVE. */ extern portid_t nb_peer_eth_addrs; /**< Number of peer ethernet addresses. */ extern struct rte_ether_addr peer_eth_addrs[RTE_MAX_ETHPORTS]; -extern uint32_t burst_tx_delay_time; /**< Burst tx delay time(us) for mac-retry. */ -extern uint32_t burst_tx_retry_num; /**< Burst tx retry number for mac-retry. */ +extern uint32_t burst_tx_delay_time; /**< Burst Tx delay time(us) for mac-retry. */ +extern uint32_t burst_tx_retry_num; /**< Burst Tx retry number for mac-retry. */ #define GRO_DEFAULT_ITEM_NUM_PER_FLOW 32 #define GRO_DEFAULT_FLOW_NUM (RTE_GRO_MAX_BURST_ITEM_NUM / \ diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c index d55ee7c..f41823f 100644 --- a/app/test-pmd/txonly.c +++ b/app/test-pmd/txonly.c @@ -52,7 +52,7 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << 16) | (0 << 8) | 2; static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of transmitted packets. */ RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */ -static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets. */ +static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of Tx packets. */ RTE_DEFINE_PER_LCORE(uint64_t, timestamp_qskew); /**< Timestamp offset per queue */ RTE_DEFINE_PER_LCORE(uint32_t, timestamp_idone); /**< Timestamp init done. */ -- 2.7.4
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
From: Hongbo Zheng <zhenghongbo3@huawei.com>
In testpmd, when we input "show config rxtx", we can see like this:
1: testpmd> show config rxtx 2: io packet forwarding packets/burst=32 3: nb forwarding cores=1 - nb forwarding ports=1 4: port 0: RX queue number: 1 Tx queue number: 1 5: Rx offloads=0x0 Tx offloads=0x10000 6: RX queue: 0 7: RX desc=1024 - RX free threshold=32 8: RX threshold registers: pthresh=0 hthresh=0 wthresh=0 9: RX Offloads=0x0 10: TX queue: 0 11: TX desc=1024 - TX free threshold=928 12: TX threshold registers: pthresh=0 hthresh=0 wthresh=0 13: TX offloads=0x10000 - TX RS bit threshold=32
We can see RX/Rx/TX/Tx is mixed used. Also in other places in testpmd, RX/Rx/TX/Tx is mixed used.
This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to unified use Rx/Tx.
The commit log is too redundant. The following is enough to explain what this patch does: RX/TX and Rx/Tx are mixed used in testpmd print and comments. This patch unifies them as Rx/Tx. Except this, the patch looks good to me. But one big concern, this patch will break all of the CI tests because the DTS scripts check if the results are the same as expected and this patch change a lot of the print. So I think the DTS maintainer needs to be aware of this. +Lijuan DTS maintainer
Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: - fix all RX/TX - rename patch title --- app/test-pmd/cmdline.c | 104 ++++++++++++++++++------------------ app/test-pmd/config.c | 128 ++++++++++++++++++++++----------------------- app/test-pmd/csumonly.c | 22 ++++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++---- app/test-pmd/parameters.c | 50 +++++++++--------- app/test-pmd/testpmd.c | 120 +++++++++++++++++++++--------------------- app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) -- 2.7.4
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
From: Hongbo Zheng <zhenghongbo3@huawei.com>
In testpmd, when we input "show config rxtx", we can see like this:
1: testpmd> show config rxtx 2: io packet forwarding packets/burst=32 3: nb forwarding cores=1 - nb forwarding ports=1 4: port 0: RX queue number: 1 Tx queue number: 1 5: Rx offloads=0x0 Tx offloads=0x10000 6: RX queue: 0 7: RX desc=1024 - RX free threshold=32 8: RX threshold registers: pthresh=0 hthresh=0 wthresh=0 9: RX Offloads=0x0 10: TX queue: 0 11: TX desc=1024 - TX free threshold=928 12: TX threshold registers: pthresh=0 hthresh=0 wthresh=0 13: TX offloads=0x10000 - TX RS bit threshold=32
We can see RX/Rx/TX/Tx is mixed used. Also in other places in testpmd, RX/Rx/TX/Tx is mixed used.
This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to unified use Rx/Tx.
The commit log is too redundant. The following is enough to explain what this patch does: RX/TX and Rx/Tx are mixed used in testpmd print and comments. This patch unifies them as Rx/Tx. Except this, the patch looks good to me. But one big concern, this patch will break all of the CI tests because the DTS scripts check if the results are the same as expected and this patch change a lot of the print. So I think the DTS maintainer needs to be aware of this. +Lijuan DTS maintainer And please separate this patch with the other twos. Don't mix them in one patchset. It's not for the same purpose.
Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: - fix all RX/TX - rename patch title --- app/test-pmd/cmdline.c | 104 ++++++++++++++++++------------------ app/test-pmd/config.c | 128 ++++++++++++++++++++++----------------------- app/test-pmd/csumonly.c | 22 ++++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++---- app/test-pmd/parameters.c | 50 +++++++++--------- app/test-pmd/testpmd.c | 120 +++++++++++++++++++++--------------------- app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) -- 2.7.4
On 3/23/21 6:17 AM, Li, Xiaoyun wrote:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
From: Hongbo Zheng <zhenghongbo3@huawei.com>
In testpmd, when we input "show config rxtx", we can see like this:
1: testpmd> show config rxtx 2: io packet forwarding packets/burst=32 3: nb forwarding cores=1 - nb forwarding ports=1 4: port 0: RX queue number: 1 Tx queue number: 1 5: Rx offloads=0x0 Tx offloads=0x10000 6: RX queue: 0 7: RX desc=1024 - RX free threshold=32 8: RX threshold registers: pthresh=0 hthresh=0 wthresh=0 9: RX Offloads=0x0 10: TX queue: 0 11: TX desc=1024 - TX free threshold=928 12: TX threshold registers: pthresh=0 hthresh=0 wthresh=0 13: TX offloads=0x10000 - TX RS bit threshold=32
We can see RX/Rx/TX/Tx is mixed used. Also in other places in testpmd, RX/Rx/TX/Tx is mixed used.
This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to unified use Rx/Tx.
The commit log is too redundant. The following is enough to explain what this patch does: RX/TX and Rx/Tx are mixed used in testpmd print and comments. This patch unifies them as Rx/Tx.
Except this, the patch looks good to me.
But one big concern, this patch will break all of the CI tests because the DTS scripts check if the results are the same as expected and this patch change a lot of the print. So I think the DTS maintainer needs to be aware of this.
I think test dpdk-testpmd output is a part of API. Of course, it is not an API, but such cosmetic changes in output will be much more painful than API changes. Output parsers will simply stop to work. I think such changes should go through the deprecation process. Of course, it would be useful to change comments and may be even error log messages right now, but not commands output.
+Lijuan DTS maintainer
And please separate this patch with the other twos. Don't mix them in one patchset. It's not for the same purpose.
Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: - fix all RX/TX - rename patch title --- app/test-pmd/cmdline.c | 104 ++++++++++++++++++------------------ app/test-pmd/config.c | 128 ++++++++++++++++++++++----------------------- app/test-pmd/csumonly.c | 22 ++++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++---- app/test-pmd/parameters.c | 50 +++++++++--------- app/test-pmd/testpmd.c | 120 +++++++++++++++++++++--------------------- app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) -- 2.7.4
-----Original Message----- From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Sent: 2021年3月23日 15:25 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Lijun Ou <oulijun@huawei.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Tu, Lijuan <lijuan.tu@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
On 3/23/21 6:17 AM, Li, Xiaoyun wrote:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
From: Hongbo Zheng <zhenghongbo3@huawei.com>
In testpmd, when we input "show config rxtx", we can see like this:
1: testpmd> show config rxtx 2: io packet forwarding packets/burst=32 3: nb forwarding cores=1 - nb forwarding ports=1 4: port 0: RX queue number: 1 Tx queue number: 1 5: Rx offloads=0x0 Tx offloads=0x10000 6: RX queue: 0 7: RX desc=1024 - RX free threshold=32 8: RX threshold registers: pthresh=0 hthresh=0 wthresh=0 9: RX Offloads=0x0 10: TX queue: 0 11: TX desc=1024 - TX free threshold=928 12: TX threshold registers: pthresh=0 hthresh=0 wthresh=0 13: TX offloads=0x10000 - TX RS bit threshold=32
We can see RX/Rx/TX/Tx is mixed used. Also in other places in testpmd, RX/Rx/TX/Tx is mixed used.
This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to unified use Rx/Tx.
The commit log is too redundant. The following is enough to explain what this patch does: RX/TX and Rx/Tx are mixed used in testpmd print and comments. This patch unifies them as Rx/Tx.
Except this, the patch looks good to me.
But one big concern, this patch will break all of the CI tests because the DTS scripts check if the results are the same as expected and this patch change a lot of the print. So I think the DTS maintainer needs to be aware of this.
I think test dpdk-testpmd output is a part of API. Of course, it is not an API, but such cosmetic changes in output will be much more painful than API changes. Output parsers will simply stop to work. I think such changes should go through the deprecation process.
Of course, it would be useful to change comments and may be even error log messages right now, but not commands output.
+Lijuan DTS maintainer
It has big impact with CI system, most of test cases depended on the testpmd output, if changed, our CI system will be broken, Is it really worth it ?
And please separate this patch with the other twos. Don't mix them in one
patchset. It's not for the same purpose.
Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: - fix all RX/TX - rename patch title --- app/test-pmd/cmdline.c | 104 ++++++++++++++++++------------------ app/test-pmd/config.c | 128 ++++++++++++++++++++++---------------------
--
app/test-pmd/csumonly.c | 22 ++++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++---- app/test-pmd/parameters.c | 50 +++++++++--------- app/test-pmd/testpmd.c | 120 +++++++++++++++++++++--------------------- app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) -- 2.7.4
在 2021/3/23 15:46, Tu, Lijuan 写道:
-----Original Message----- From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Sent: 2021年3月23日 15:25 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Lijun Ou <oulijun@huawei.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Tu, Lijuan <lijuan.tu@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
On 3/23/21 6:17 AM, Li, Xiaoyun wrote:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
From: Hongbo Zheng <zhenghongbo3@huawei.com>
In testpmd, when we input "show config rxtx", we can see like this:
1: testpmd> show config rxtx 2: io packet forwarding packets/burst=32 3: nb forwarding cores=1 - nb forwarding ports=1 4: port 0: RX queue number: 1 Tx queue number: 1 5: Rx offloads=0x0 Tx offloads=0x10000 6: RX queue: 0 7: RX desc=1024 - RX free threshold=32 8: RX threshold registers: pthresh=0 hthresh=0 wthresh=0 9: RX Offloads=0x0 10: TX queue: 0 11: TX desc=1024 - TX free threshold=928 12: TX threshold registers: pthresh=0 hthresh=0 wthresh=0 13: TX offloads=0x10000 - TX RS bit threshold=32
We can see RX/Rx/TX/Tx is mixed used. Also in other places in testpmd, RX/Rx/TX/Tx is mixed used.
This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to unified use Rx/Tx.
The commit log is too redundant. The following is enough to explain what this patch does: RX/TX and Rx/Tx are mixed used in testpmd print and comments. This patch unifies them as Rx/Tx.
Except this, the patch looks good to me.
But one big concern, this patch will break all of the CI tests because the DTS scripts check if the results are the same as expected and this patch change a lot of the print. So I think the DTS maintainer needs to be aware of this.
I think test dpdk-testpmd output is a part of API. Of course, it is not an API, but such cosmetic changes in output will be much more painful than API changes. Output parsers will simply stop to work. I think such changes should go through the deprecation process.
Of course, it would be useful to change comments and may be even error log messages right now, but not commands output.
+Lijuan DTS maintainer
It has big impact with CI system, most of test cases depended on the testpmd output, if changed, our CI system will be broken, Is it really worth it ?
I think it's necessary to keep the style consistent and to constrain everyone to do so in the future.However, if the DTS changes greatly, I think it is possible to require the new print to be consistent and the historical part to remain unchanged.Because the uniform change is also Xiaoyun's suggestion, we think the opinion is reasonable, but we are not sure how much it will affect DTS.
And please separate this patch with the other twos. Don't mix them in one
patchset. It's not for the same purpose.
Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: - fix all RX/TX - rename patch title --- app/test-pmd/cmdline.c | 104 ++++++++++++++++++------------------ app/test-pmd/config.c | 128 ++++++++++++++++++++++---------------------
--
app/test-pmd/csumonly.c | 22 ++++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++---- app/test-pmd/parameters.c | 50 +++++++++--------- app/test-pmd/testpmd.c | 120 +++++++++++++++++++++--------------------- app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) -- 2.7.4
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 15:52 To: Tu, Lijuan <lijuan.tu@intel.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
在 2021/3/23 15:46, Tu, Lijuan 写道:
-----Original Message----- From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Sent: 2021年3月23日 15:25 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Lijun Ou <oulijun@huawei.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Tu, Lijuan <lijuan.tu@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
On 3/23/21 6:17 AM, Li, Xiaoyun wrote:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
From: Hongbo Zheng <zhenghongbo3@huawei.com>
In testpmd, when we input "show config rxtx", we can see like this:
1: testpmd> show config rxtx 2: io packet forwarding packets/burst=32 3: nb forwarding cores=1 - nb forwarding ports=1 4: port 0: RX queue number: 1 Tx queue number: 1 5: Rx offloads=0x0 Tx offloads=0x10000 6: RX queue: 0 7: RX desc=1024 - RX free threshold=32 8: RX threshold registers: pthresh=0 hthresh=0 wthresh=0 9: RX Offloads=0x0 10: TX queue: 0 11: TX desc=1024 - TX free threshold=928 12: TX threshold registers: pthresh=0 hthresh=0 wthresh=0 13: TX offloads=0x10000 - TX RS bit threshold=32
We can see RX/Rx/TX/Tx is mixed used. Also in other places in testpmd, RX/Rx/TX/Tx is mixed used.
This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to unified use Rx/Tx.
The commit log is too redundant. The following is enough to explain what this patch does: RX/TX and Rx/Tx are mixed used in testpmd print and comments. This patch unifies them as Rx/Tx.
Except this, the patch looks good to me.
But one big concern, this patch will break all of the CI tests because the DTS scripts check if the results are the same as expected and this patch change a lot of the print. So I think the DTS maintainer needs to be aware of this.
I think test dpdk-testpmd output is a part of API. Of course, it is not an API, but such cosmetic changes in output will be much more painful
than API changes.
Output parsers will simply stop to work. I think such changes should go through the deprecation process.
Of course, it would be useful to change comments and may be even error log messages right now, but not commands output.
+Lijuan DTS maintainer
It has big impact with CI system, most of test cases depended on the testpmd output, if changed, our CI system will be broken, Is it really worth it ?
I think it's necessary to keep the style consistent and to constrain everyone to do so in the future.However, if the DTS changes greatly, I think it is possible to require the new print to be consistent and the historical part to remain unchanged.Because the uniform change is also Xiaoyun's suggestion, we think the opinion is reasonable, but we are not sure how much it will affect DTS.
I think we should drop this patch. But we'll keep consistent for future print when reviewing. What do you think?
And please separate this patch with the other twos. Don't mix them in one
patchset. It's not for the same purpose.
Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: - fix all RX/TX - rename patch title --- app/test-pmd/cmdline.c | 104 ++++++++++++++++++------------------ app/test-pmd/config.c | 128 ++++++++++++++++++++++------------------
--
app/test-pmd/csumonly.c | 22 ++++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++---- app/test-pmd/parameters.c | 50 +++++++++--------- app/test-pmd/testpmd.c | 120 +++++++++++++++++++++------------------
app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) -- 2.7.4
On 3/23/2021 7:58 AM, Li, Xiaoyun wrote:
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: Tuesday, March 23, 2021 15:52 To: Tu, Lijuan <lijuan.tu@intel.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
在 2021/3/23 15:46, Tu, Lijuan 写道:
-----Original Message----- From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Sent: 2021年3月23日 15:25 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Lijun Ou <oulijun@huawei.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Tu, Lijuan <lijuan.tu@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
On 3/23/21 6:17 AM, Li, Xiaoyun wrote:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
From: Hongbo Zheng <zhenghongbo3@huawei.com>
In testpmd, when we input "show config rxtx", we can see like this:
1: testpmd> show config rxtx 2: io packet forwarding packets/burst=32 3: nb forwarding cores=1 - nb forwarding ports=1 4: port 0: RX queue number: 1 Tx queue number: 1 5: Rx offloads=0x0 Tx offloads=0x10000 6: RX queue: 0 7: RX desc=1024 - RX free threshold=32 8: RX threshold registers: pthresh=0 hthresh=0 wthresh=0 9: RX Offloads=0x0 10: TX queue: 0 11: TX desc=1024 - TX free threshold=928 12: TX threshold registers: pthresh=0 hthresh=0 wthresh=0 13: TX offloads=0x10000 - TX RS bit threshold=32
We can see RX/Rx/TX/Tx is mixed used. Also in other places in testpmd, RX/Rx/TX/Tx is mixed used.
This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to unified use Rx/Tx.
The commit log is too redundant. The following is enough to explain what this patch does: RX/TX and Rx/Tx are mixed used in testpmd print and comments. This patch unifies them as Rx/Tx.
Except this, the patch looks good to me.
But one big concern, this patch will break all of the CI tests because the DTS scripts check if the results are the same as expected and this patch change a lot of the print. So I think the DTS maintainer needs to be aware of this.
I think test dpdk-testpmd output is a part of API. Of course, it is not an API, but such cosmetic changes in output will be much more painful
than API changes.
Output parsers will simply stop to work. I think such changes should go through the deprecation process.
Of course, it would be useful to change comments and may be even error log messages right now, but not commands output.
+Lijuan DTS maintainer
It has big impact with CI system, most of test cases depended on the testpmd output, if changed, our CI system will be broken, Is it really worth it ?
I think it's necessary to keep the style consistent and to constrain everyone to do so in the future.However, if the DTS changes greatly, I think it is possible to require the new print to be consistent and the historical part to remain unchanged.Because the uniform change is also Xiaoyun's suggestion, we think the opinion is reasonable, but we are not sure how much it will affect DTS.
I think we should drop this patch. But we'll keep consistent for future print when reviewing. What do you think?
+1 to have consistency on the new code, and as the old code changed by time. Instead of big syntax cleanup. As well as output parsing problem, this also can cause trouble on backporting patches for stable trees because of conflicts it will create.
And please separate this patch with the other twos. Don't mix them in one
patchset. It's not for the same purpose.
Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: - fix all RX/TX - rename patch title --- app/test-pmd/cmdline.c | 104 ++++++++++++++++++------------------ app/test-pmd/config.c | 128 ++++++++++++++++++++++------------------
--
app/test-pmd/csumonly.c | 22 ++++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++---- app/test-pmd/parameters.c | 50 +++++++++--------- app/test-pmd/testpmd.c | 120 +++++++++++++++++++++------------------
app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) -- 2.7.4
-----Original Message----- From: oulijun <oulijun@huawei.com> Sent: 2021年3月23日 15:52 To: Tu, Lijuan <lijuan.tu@intel.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
在 2021/3/23 15:46, Tu, Lijuan 写道:
-----Original Message----- From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Sent: 2021年3月23日 15:25 To: Li, Xiaoyun <xiaoyun.li@intel.com>; Lijun Ou <oulijun@huawei.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Tu, Lijuan <lijuan.tu@intel.com> Cc: dev@dpdk.org; linuxarm@openeuler.org Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
On 3/23/21 6:17 AM, Li, Xiaoyun wrote:
Hi
-----Original Message----- From: Lijun Ou <oulijun@huawei.com> Sent: Friday, March 5, 2021 18:22 To: Yigit, Ferruh <ferruh.yigit@intel.com> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org; linuxarm@openeuler.org Subject: [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
From: Hongbo Zheng <zhenghongbo3@huawei.com>
In testpmd, when we input "show config rxtx", we can see like this:
1: testpmd> show config rxtx 2: io packet forwarding packets/burst=32 3: nb forwarding cores=1 - nb forwarding ports=1 4: port 0: RX queue number: 1 Tx queue number: 1 5: Rx offloads=0x0 Tx offloads=0x10000 6: RX queue: 0 7: RX desc=1024 - RX free threshold=32 8: RX threshold registers: pthresh=0 hthresh=0 wthresh=0 9: RX Offloads=0x0 10: TX queue: 0 11: TX desc=1024 - TX free threshold=928 12: TX threshold registers: pthresh=0 hthresh=0 wthresh=0 13: TX offloads=0x10000 - TX RS bit threshold=32
We can see RX/Rx/TX/Tx is mixed used. Also in other places in testpmd, RX/Rx/TX/Tx is mixed used.
This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to unified use Rx/Tx.
The commit log is too redundant. The following is enough to explain what this patch does: RX/TX and Rx/Tx are mixed used in testpmd print and comments. This patch unifies them as Rx/Tx.
Except this, the patch looks good to me.
But one big concern, this patch will break all of the CI tests because the DTS scripts check if the results are the same as expected and this patch change a lot of the print. So I think the DTS maintainer needs to be aware of this.
I think test dpdk-testpmd output is a part of API. Of course, it is not an API, but such cosmetic changes in output will be much more painful
than API changes.
Output parsers will simply stop to work. I think such changes should go through the deprecation process.
Of course, it would be useful to change comments and may be even error log messages right now, but not commands output.
+Lijuan DTS maintainer
It has big impact with CI system, most of test cases depended on the testpmd output, if changed, our CI system will be broken, Is it really worth it ?
I think it's necessary to keep the style consistent and to constrain everyone to do so in the future.However, if the DTS changes greatly, I think it is possible to require the new print to be consistent and the historical part to remain unchanged.Because the uniform change is also Xiaoyun's suggestion, we think the opinion is reasonable, but we are not sure how much it will affect DTS.
If that, I think it should go to DPDK tech board, shall we only unify testpmd or consider libraries and other applications.
And please separate this patch with the other twos. Don't mix them in one
patchset. It's not for the same purpose.
Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: - fix all RX/TX - rename patch title --- app/test-pmd/cmdline.c | 104 ++++++++++++++++++------------------ app/test-pmd/config.c | 128 ++++++++++++++++++++++------------------
--
app/test-pmd/csumonly.c | 22 ++++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++---- app/test-pmd/parameters.c | 50 +++++++++--------- app/test-pmd/testpmd.c | 120 +++++++++++++++++++++------------------
app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) -- 2.7.4
Hi, Ferruh and all, have any comments about this set of patches? thanks. 在 2021/3/5 18:22, Lijun Ou 写道:
This series add two test bug fixes and a print style.
Hongbo Zheng (1): app/testpmd: use of Rx/Tx in testpmd
Huisong Li (2): app/testpmd: fix forwarding configuration when DCB test app/testpmd: remove forwarding config from parsing Rx and Tx
app/test-pmd/cmdline.c | 106 ++++++++++++++++---------------- app/test-pmd/config.c | 147 +++++++++++++++++++++++++-------------------- app/test-pmd/csumonly.c | 22 +++---- app/test-pmd/icmpecho.c | 2 +- app/test-pmd/ieee1588fwd.c | 18 +++--- app/test-pmd/parameters.c | 50 +++++++-------- app/test-pmd/testpmd.c | 132 ++++++++++++++++++++-------------------- app/test-pmd/testpmd.h | 28 ++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 263 insertions(+), 244 deletions(-)
On 3/20/2021 1:19 AM, Min Hu (Connor) wrote:
Hi, Ferruh and all, have any comments about this set of patches? thanks.
在 2021/3/5 18:22, Lijun Ou 写道:
This series add two test bug fixes and a print style.
Hongbo Zheng (1): app/testpmd: use of Rx/Tx in testpmd
Huisong Li (2): app/testpmd: fix forwarding configuration when DCB test app/testpmd: remove forwarding config from parsing Rx and Tx
Off the list. Hi Xiaoyun, do you have any time to review the set please? Thanks, ferruh
participants (7)
-
Andrew Rybchenko -
Ferruh Yigit -
Li, Xiaoyun -
Lijun Ou -
Min Hu (Connor) -
oulijun
-
Tu, Lijuan